Skip to content

Instantly share code, notes, and snippets.

@philsturgeon
Last active June 7, 2018 09:34
Show Gist options
  • Save philsturgeon/6320152 to your computer and use it in GitHub Desktop.
Save philsturgeon/6320152 to your computer and use it in GitHub Desktop.
PSR-2 v CodeSniffer PSR-2

This is a list of issues or discrepencies between the wording or intention of PSR-2 itself and the CodeSniffer PSR-2 ruleset.

Add suggestions in the comments or tweet me (@philsturgeon) if you have more inconsistencies to report.

1.) Multi-line Arguments

Here is one I call "The Multi-line Argument Conundrum".

Status: Fixed

Example #1

A normal function declaration with some arguments.

somefunction($foo, $bar, $baz, $ban);

PSR-2: Valid
CodeSniffer "PSR-2": Valid

Example #2

As PSR-2 Section 4.6 says, if you'd like to split this argument list onto multiple lines (with the intended meaning being to avoid super-wide declarations), you can chose to drop each argument onto its own line, and indent each one with an extra paragraph.

somefunction(
    $foo,
    $bar,
    $baz,
    $ban
);

PSR-2: Valid
CodeSniffer "PSR-2": Valid

Example #3

What SHOULD be a perfectly valid example of PSR-2 code (and is considered by many PSR-2 members to be perfectly valid), is the option of one of your arguments being a multi-line argument.

somefunction($foo, $bar, [
  // ...
], $ban);

PSR-2: Valid
CodeSniffer "PSR-2": Invalid

Or, the same type of example taken right from the Silex homepage:

$app->get('/hello/{name}', function($name) use($app) { 
    return 'Hello '.$app->escape($name); 
});

PSR-2: Valid
CodeSniffer "PSR-2": Invalid

Now, a multi-line argument is assumed to be treated as a single argument and so this example is technically no different from Example #1, and as such should not automatically trigger 4.6. By triggering 4.6 you are forced against your will to suddenly drop everything onto new lines and indent everything, and then it looks like...:

Example #4

This nasty-ass shit which is forced upon you by using CodeSniffer/CS-Fixer. This is ugly as sin, and not ever what anyone was talking about when PSR-2 was written.

somefunction(
    $foo,
    $bar,
    [
      // ...
    ],
    $ban
);

PSR-2: Valid
CodeSniffer "PSR-2": Valid

$app->get(
    '/hello/{name}',
    function($name) use($app) { 
        return 'Hello '.$app->escape($name); 
    }
);

PSR-2: Valid
CodeSniffer "PSR-2": Valid

Check this one out:

Forced CodeSniffer code

Fuck that noise.

Example #5

Avoid PHP CodeSniffers wrath by assigning multi-line arguments to their own variable, which essentially makes Example #5 into Example #1 or Example #2:

$arr = [
  // ...
];

somefunction(
    $foo,
    $bar,
    $arr,
    $ban
);

PSR-2: Valid
CodeSniffer "PSR-2": Valid

Yay for uneccessary code.

Short-Term Fix

You can avoid all of this with a standards file:

<?xml version="1.0"?>
<ruleset name="PHP_CodeSniffer">

  <description>PHP_CodeSniffer configuration</description>

    <rule ref="PSR2">
        <exclude name="PEAR.Functions.FunctionCallSignature"/>
        <exclude name="PEAR.Functions.FunctionCallSignature.SpaceAfterCloseBracket"/>
    </rule>

</ruleset>

Shove it in Jenkins, or reference it in Sublime Text with:

"phpcs_additional_args": {
    "--standard": "/Users/foo/phpcs.xml",
    "-n": ""
},

Further Reading

2.) Aligning Arguments in Signature

This is another "PSR-2 doesn't care but CodeSniffer says it does" situation.

Status: Fixed

<?php
 
namespace Test;
 
class MyClass
{
    public function someFunction(
        MyType       $param1,
        LongTypeName $param2,
        array        $param3
    ) {
        // code ...
    }
}

CodeSniffer should have no reason to care about anything in this code. The text is here:

4.4. Method Arguments

In the argument list, there MUST NOT be a space before each comma, and there MUST be one space after each comma.

Method arguments with default values MUST go at the end of the argument list.

...some code...

Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line.

When the argument list is split across multiple lines, the closing parenthesis and opening brace MUST be placed together on their own line with one space between them.

Expected Output

Nothing.

Actual Output

--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
  8 | ERROR | Expected 1 space between type hint and argument "$param1"; 7
    |       | found
 10 | ERROR | Expected 1 space between type hint and argument "$param3"; 8
    |       | found
--------------------------------------------------------------------------------

Resolution

Report as CodeSniffer Bug

Thanks

Props to Tom Oram for catching this one.

3.) Comment Alignment

This is another "PSR-2 doesn't care but CodeSniffer says it does" situation.

Status: Waiting on confirmation of being fixed

<?php

// Do something if yes
if ($foo === true) {
    echo "Yep"

// Do something if nope
} elseif ($foo !== true) {
    echo "Nope";

// Fuck knows
} else {
    die("Universe Implosion");
}

Sadly CodeSniffer is not letting comments sit next to their appropriate constrol structure keyword.

Expected Output

Nothing.

Actual Output

--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
  7 | ERROR | Line indented incorrectly; expected at least 4 spaces, found 0
 11 | ERROR | Line indented incorrectly; expected at least 4 spaces, found 0
--------------------------------------------------------------------------------

Resolution

Greg is on the job in the comments.

4.) Class name resolution

When using the new Foo::class CodeSniffer gets all confused.

Status: Fixed

<?php

call_user_func(Foo::class);

Expected Output

Nothing.

Actual Output

---------------------------------------------------------------------------
-----
FOUND 2 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
---------------------------------------------------------------------------
-----
 1 | ERROR   | The closing brace for the class must go on the 
next line after
   |         | the body
 3 | WARNING | Possible parse error: class missing opening or 
closing brace
 3 | WARNING | Possible parse error: class missing opening or 
closing brace
 3 | ERROR   | Each class must be in a namespace of at least one 
level (a
   |         | top-level vendor name)
---------------------------------------------------------------------------
-----

Resolution

Reported as CodeSniffer Bug

5.) Multi-Line Interface Extends

PSR-2 didn't ever mention what to do if you want to extend multiple interfaces in an interface, but CodeSniffer will tell you you're wrong.

Status: Waiting on Errata Pull Request

<?php

namespace Test;

interface MyInterface extends
    Interface1,
    Interface2
{
}

Expected Output

Not entirely sure. We'll work it out on the mailing list.

Actual Output

--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 6 | ERROR | Expected 1 space before "Interface1"; 4 found
 7 | ERROR | Expected 1 space before "Interface2"; 4 found
--------------------------------------------------------------------------------

Resolution

Poll the FIG to see if we can (or need to) make this Errata. If so, make errata. If not... bug?

@designermonkey
Copy link

@philsturgeon is there any movement on the number 3, comment alignment issue?

We totally agree at work, but are stuck with it being broken every time, especially when using cs fixer. Does anyone have a ruleset that would allow this to be overridden in the meantime?

@aalaap
Copy link

aalaap commented Jul 4, 2016

I prefer the following style of comment alignment (using the exact example above):

<?php

if ($foo === true) {
    // Do something if yes
    echo "Yep"
} elseif ($foo !== true) {
    // Do something if nope
    echo "Nope";
} else {
    // Fuck knows
    die("Universe Implosion");
}

PSR-2: Valid (Doesn't care)
CodeSniffer: Valid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment