Generic/IncrementDecrementSpacing: handle more situations

[jrfnl]

The Generic.WhiteSpace.IncrementDecrementSpacing sniff, so far, only handled incrementors/decrementors when they were directly before/after the variable they apply to.

This commit enhances the sniff to also allow for finding superfluous whitespace when incrementing/decrementing a property or an array item.

Includes unit tests.

Note: I've only made this change for PHP files as JS support will be dropped anyway, so it didn't feel like a good use of my time to work on that.

[DannyvdSluijs]

Seems all to be working okay, just had a small comment about adding test cases for the pre increment instead of only testing post increment.

[DannyvdSluijs]

Should we expand the test case with pre-increments of the array and object properties?

++$i['key'];
++ $i['key'];
++$i['key']['id'];
++ $i['key']['id'];

++$obj->prop;
++ $obj->prop;
++ $obj?->prop;

++$obj->obj->prop;
++ $obj->obj->prop;
++ $obj?->obj->prop;

++$obj->prop['key'];
++ $obj->prop['key'];

[jrfnl]

@DannyvdSluijs Good question. In my opinion: no, those tests are not needed as they wouldn't be testing anything (new).

The difference in this case for pre vs post in/decrement is that to identify whether something is a "pre" vs "post" increment (i.e. to identify whether there should be no whitespace after or no whitespace before, the sniff needs to look at different tokens. These extra tests cover the changes I made to that logic.

Adding the same tests for pre-increment wouldn't actually test anything new as it was already handled correctly (as whether something is a plain variable or a non-static property access or array key access on a variable doesn't make a difference in the identification of pre vs post).

For pre-increment, I can think of a further/future iteration for the sniff - checking whether a pre-increment is used on a static property with a fully qualified classname or namespace relative classname, but that is something I choose not to handle (yet) when I made this change last year. When that change would be added, then, yes, extra tests would be needed for pre-in/decrement.

++\ClassName::$prop;
++Relative\ClassName::$prop;
--namespace\Relative\ClassName::$prop;

The reason I did not make that change (yet) is that this would need a different patch for PHPCS 3.x vs PHPCS 4.x, which would make the merge more complex. Also see #3041.

[DannyvdSluijs]

That all makes sense to me. Which makes this PR ready to merge as far as I'm concerned.

[jrfnl]

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#46

[jrfnl]

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).