magento / marketplace-eqp

Magento 1.x Coding Standard
http://docs.magento.com/marketplace/user_guide/Resources/pdf/Extension_Quality_Program_Overview.pdf
MIT License
224 stars 68 forks source link

Multi-line function parameters not resolvable #46

Closed bobbrodie closed 7 years ago

bobbrodie commented 7 years ago

Let's say you have a multi-line function with long parameter names that would force you to go over 120 characters. The rules say that if you're going to line-break, you need to do one parameter per line.

Since this is a multi-line function, the closing parenthesis must be on its own line, sure. Then, the opening brace needs to be on a new line.

So we run into this:

The closing parenthesis and the opening brace of
a multi-line function declaration must be on the
same line

This is not valid:

    public function functionWithManyParams(
        $parameter1,
        $parameter2 = array(),
        $parameter3 = 'custom',
        $parameter4 = false,
        $parameter5 = '',
        $parameter6 = null,
        $parameter7 = false,
        $parameter8 = false
    )
    {

Nor is this:

    public function functionWithManyParams(
        $parameter1,
        $parameter2 = array(),
        $parameter3 = 'custom',
        $parameter4 = false,
        $parameter5 = '',
        $parameter6 = null,
        $parameter7 = false,
        $parameter8 = false
    ) {

I know there's something to be said for the number of parameters, but the issue at hand is that this is throwing a warning.

Am I completely missing something here?

lenaorobei commented 7 years ago

@bobbrodie thanks for your reporting. This issue is caused by conflicting rules in MEQP1 ruleset. We should mark <rule ref="Generic.Functions.OpeningFunctionBraceBsdAllman"/> with severity 0. So the correct multi-line declaration is:

public function functionWithManyParams(
    $parameter1,
    $parameter2 = array(),
    $parameter3 = 'custom',
    $parameter4 = false,
    $parameter5 = '',
    $parameter6 = null,
    $parameter7 = false,
    $parameter8 = false
) {