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
880 stars 54 forks source link

Tokenizer/PHP: arrow function tokenization broken when true/false used in return type #453

Closed jrfnl closed 4 months ago

jrfnl commented 4 months ago

Description

BackfillFnTokenTest: use data providers when appropriate

These two tests were testing multiple test cases, but not using a data provider, which means that if one of the test cases would fail, the ones after the failing test case wouldn't even be tested.

By using a data provider, that issue is avoided.

It also allows for simplifying some of the test code for the testKeywordReturnTypes() test which duplicated the logic for the scopePositionTestHelper() just to allow for custom error messages mentioning the test marker. This is no longer needed when each marker is tested individually via the data provider.

Tokenizer/PHP: arrow function tokenization broken when true/false used in return type

Since PHP 8.0, false and null can be included in a union return type. As of PHP 8.2, both true, false and null can be used as a stand-alone return type.

The tokenizer layer handling arrow functions did not take this into account correctly. While null was handled correctly, true and false was not and would result in the arrow function fn keyword being tokenized as T_STRING across all PHP versions. As a result of that, the other typical tokenizer changes related to arrow functions (=> as T_FN_ARROW, scope/parenthesis owners etc) would also not be executed correctly.

In practice, I suspect few people will have run into this bug as, after all, what's the point of declaring an arrow function which will only ever return true or false ? so in practice, it is likely to only have come into play for people using true or false as part of an arrow function union type. All the same, PHPCS should handle this correctly.

Includes unit tests proving the bug and safeguarding the fix.

Suggested changelog entry

Fixed broken arrow function tokenization when the return type contained true or false

Types of changes

jrfnl commented 4 months ago

Rebased without changes. Merging once the build passes.