search

PHPCSStandards / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
906 stars 55 forks source link

File::findStartOfStatement(): 3 bug fixes related to `match` expressions #502

Closed jrfnl closed 4 months ago

jrfnl commented 4 months ago

Description

File::findStartOfStatement(): bug fix - don't give nested scopes the "match" treatment

The only "scopes" which can be nested within a match expression are closures, anonymous classes and other match expressions.

The File::findStartOfStatement() method has special handling for match expressions to find the start of a statement, but that special handling would also kick in when the $start token is within another scope nested within the match, while, in that case, the special handling is not needed and ends up resulting in an incorrect "start" pointer being returned, in most cases, even a "start pointer" which is after the token for which the start of statement is requested, which should never be possible.

Fixed now by changing the special handling for match expressions to only kick in when the match expression is the deepest nested scope.

Includes unit tests.

File::findStartOfStatement(): bug fix - don't give tokens within nested parentheses the "match" treatment

The File::findStartOfStatement() method has special handling for match expressions to find the start of a statement, but that special handling would also kick in when the $start token is within a parenthesized expression within the match, while, in that case, the special handling is not needed and ends up returning an incorrect "start" pointer, in most cases, even a "start pointer" which is after the token for which the start of statement is requested, which should never be possible.

Fixed now by changing the special handling for match expressions to only kick in when the $start pointer and the match pointer are at the same parentheses nesting level.

Includes unit tests. Of the tests, the first array item/parameter was not affected by this bug, but subsequent array items/parameters were all affected by this bug.

File::findStartOfStatement(): bug fix - don't confuse comma's in short arrays with comma's belonging to a match expression

The File::findStartOfStatement() method has special handling for match expressions to find the start of a statement, but that special handling did not correctly take the potential of comma's in nested short arrays into account. This would lead to any array item after the first getting the wrong return value.

As there is currently isn't a "nested_brackets" index in the token array (also see the proposal for this in #12), there is no information available within the token array to prevent the special handling for match expressions from kicking in.

So, we need to skip out of the special handling if it is detected that the $start token is within a short array. In practice, this means we cannot break on a T_COMMA, as at that point we don't know if it is a comma from within a nested short array. Instead, we have to walk back to the last T_MATCH_ARROW to be sure and break out off the special handling if we encounter a [/bracket opener with a bracket_closer which is beyond the $start pointer.

With these changes, the $prevMatch token will now always either be the match scope opener or a T_MATCH_ARROW. With this knowledge, we can now simplify the special handling for tokens which do belong to the match expression itself a little.

Includes unit tests. Of the tests, the first array item was not affected by this bug, but subsequent array items were all affected by this bug.

Generic/ScopeIndent: add tests for issues #110 and #437

Both the mentioned issues are fixed by the improvements to the File::findStartOfStatement() method in this same PR.

Suggested changelog entry

Related issues/external references

Fixes #110 Fixes #437 Fixes #475 Fixes squizlabs/PHP_CodeSniffer#3875

Types of changes

jrfnl commented 4 months ago

Rebased without changes. Merging once the build passes.