PSR2.Classes.ClassDeclaration: thorough review of the sniff code

[jrfnl]

As per the note left on PR #424 :

at a glance, this sniff could probably do with a more thorough review and more defensive token walking/more precise checking

And from a reply to a PR review:

For the record: the above note was triggered by seeing code like line 451-453 which checks that the code after a name is not a \ and not a comma and then presumes it is whitespace, but doesn't actually check if the token is whitespace (and replaces the token anyhow, potentially destroying valid code).

This will need further investigation (and proof of the bug via a code sample, not just my experience telling me there is a likely to be bug). It might be that the code earlier on in the sniff already checks for \, comma and whitespace, so that if the next token is not \ or comma, the sniff can be sure that it will be whitespace (but then again, why not check for that in line 451-453 ?). Considering the bugs now fixed and seeing that code, I have a feeling there will be more, potentially incorrect, presumptions build into the sniff.

[jrfnl]

Another comment from a PR review which should probably be looked into:

... it should be verified that the sniff is actually in line with PSR-2 (including the errata).

What I mean by that is as follows:

Given the following code sample (multiple blank lines between interface names, no comments):

class ClassName extends ParentClass implements
    \Foo\Bar\Countable,

    \Serializable
{}

The sniff would currently throw a Only one interface may be specified per line in a multi-line implements declaration (PSR2.Classes.ClassDeclaration.InterfaceSameLine) error. And the fixer would remove the blank lines.

However, PSR-2 states "Lists of implements 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 interface per line.".

Given the above, there are two things to question about the sniff:

1. If each interface name is already on its own line, should the sniff enforce "no blank lines between interface name lines" ? I don't see anything in PSR-2 which says it should, so this looks like something where the sniffs goes beyond its remit.
2. If it would be decided that it should (possibly because this has been clarified on the PSR mailinglist or in the PSR-2 errata, or even because not doing so would change the expected behaviour of the sniff), is it correct that this is handled by the InterfaceSameLine error code ? Or should this be a separate error code, something like NoBlankLinesBetweenInterfaceNames ?

If I would have ever come across a multi-line implements like the above and would then have seen the Only one interface may be specified per line in a multi-line implements declaration error, I would have reported this as a bug in the sniff as the sniff is reporting something which is just not true (as each interface is already on its own line).