Squiz/FunctionSpacing: bug fix for attribute handling

[jrfnl]

Description

The Squiz.WhiteSpace.FunctionSpacing sniff is supposed to check the number of blank lines before (and after) a function declaration, while taking docblocks and attributes attached to the function into account. I.e. if there is a docblock and/or attributes, the blank lines above those will be checked.

Additionally, the sniff has a separate error code BeforeFirst for the number of blank lines before a first function in a class and the expected number of blank lines for the first function in a class may also be configured to be different than the "normal" number of blank lines before a function/between methods.

Now, while the sniff does take attributes into account, it does so incorrectly:

1. It doesn't allow for attributes before a docblock, even though this is perfectly valid PHP and the attributes still belong with the function.
2. It doesn't allow for multiple attributes on the same line. Again, this is perfectly valid PHP.

The effect of this bug can be seen by checking the new tests without the fix:

1. The "Correct" functions will also be flagged as incorrect as the sniff gets confused by multiple attributes on the same line.
2. The set of tests with the attributes before the docblock will be flagged with the Before error code instead of BeforeFirst as the functions will not be recognized as the first in class due to the attributes before the docblock.
3. The fixer will make incorrect fixes.

Fixed now by removing the presumption about the order of docblocks vs attributes and stabilizing the "current line" determination.

Includes tests.

Suggested changelog entry

Squiz.WhiteSpace.FunctionSpacing: would break on attributes attached to a function under certain circumstances.

Related issues/external references

Related to #152

Types of changes

• Bug fix (non-breaking change which fixes an issue)

[jrfnl]

Force push was to fix a minor typo in the test case file.

[jrfnl]

I've pushed a new commit to this PR as I came across yet more problems with attributes, so I've now rewritten an even larger part of the sniff to handle this better.

As per the commit message:

Turns out the "silence the "before" error if the thing before was another function" functionality still wasn't working correctly.

And if there is a blank line between the first thing before the first "preamble" for the function and the declaration itself, the sniff would also disregard the "preamble" and insert the blank lines straight above the function declaration.

Fixed now.

Includes tests proving the bug.

Includes one minor change in the existing tests, where the recognition of "the thing before is also a function" no longer works.
This is a specific edge case when there are two functions declared on the same line and, in my opinion, this tiny regression is a valid trade-off for correctly handling functions with attributes, which is much more commonly found in code.

[fredden]

Reviewing this took a lot of brain power. Good work on this one @jrfnl.

[jrfnl]

Reviewing this took a lot of brain power. Good work on this one @jrfnl.

Thanks for taking the time to do so @fredden . It wasn't an easy fix to create either ;-)

I'll rebase the PR (without changes) to get a passing build before merging.