Fix DeprecatedFunctions override and PHP 8.5 tokenizer support - #71
Merged
Merged
Conversation
Generic.PHP.DeprecatedFunctions builds its forbiddenFunctions list in its constructor from the Reflection API, covering every internal function the running PHP version marks deprecated. The ruleset set that same property, which replaces the list rather than extending it, so the standard reported fewer deprecations than plain Generic did. utf8_encode(), for instance, went unreported. The property is gone; create_function() and each() move to Generic.PHP.ForbiddenFunctions, which is where removed-in-8.0 functions belong since the reflection-driven list can no longer see them. PHP 8.5 tokenizes both features these sniffs target as single tokens: the void cast becomes T_VOID_CAST, carrying any inner whitespace in its content, and the pipe operator becomes T_PIPE. Both sniffs registered only the pre-8.5 multi-token shapes, so on PHP 8.5 they matched nothing at all and silently passed everything. They now register both shapes, and the tests confirm identical findings on 8.4 and 8.5. Two fixers had to move which token they grow. With the operator collapsed into one token, the before-fix and after-fix both landed on that same index and the second was dropped. They now extend the neighboring token instead. PHP 8.5 is added to the CI matrix so this class of breakage is visible. That also required tightening getFileDocBlockLines() to a non-nullable int parameter, which PHPStan only flags when analyzing on 8.5; both call sites already pass a non-null value.
There was a problem hiding this comment.
Pull request overview
This PR fixes two cases where the PhpCollective standard was unintentionally checking less than intended: (1) Generic.PHP.DeprecatedFunctions was effectively disabled by an overriding ruleset property, and (2) new PHP 8.5 tokenizer behavior prevented the VoidCast and PipeOperatorSpacing sniffs from ever receiving the relevant tokens. It also extends CI to include PHP 8.5 and updates tests/fixtures accordingly.
Changes:
- Remove the
Generic.PHP.DeprecatedFunctionsproperty override that replaced the sniff’s reflection-built deprecated-function list, and move removed-function checks intoGeneric.PHP.ForbiddenFunctions. - Update
VoidCastSniffandPipeOperatorSpacingSniffto support PHP 8.5’s single-token forms (T_VOID_CAST,T_PIPE) while preserving pre-8.5 behavior. - Add PHP 8.5 to CI and extend pipe operator fixtures/tests to cover the two-token (
| >) branch on 8.5.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
PhpCollective/Sniffs/WhiteSpace/PipeOperatorSpacingSniff.php |
Registers and handles T_PIPE (PHP 8.5) and adjusts fix targeting to avoid fixer collisions. |
PhpCollective/Sniffs/PHP/VoidCastSniff.php |
Registers and handles T_VOID_CAST (PHP 8.5) and validates inner whitespace via token content. |
PhpCollective/Sniffs/Commenting/FileDocBlockSniff.php |
Tightens getFileDocBlockLines() parameter type to int based on non-null call sites. |
PhpCollective/ruleset.xml |
Stops overriding DeprecatedFunctions list; configures ForbiddenFunctions with required replacements and removed functions. |
tests/PhpCollective/Sniffs/WhiteSpace/PipeOperatorSpacingSniffTest.php |
Updates expected error counts to reflect the added fixture case. |
tests/_data/PipeOperatorSpacing/before.php |
Adds a ` |
tests/_data/PipeOperatorSpacing/after.php |
Adds the expected fixed output for the new ` |
.github/workflows/ci.yml |
Adds PHP 8.5 to the validation matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+110
to
+112
| // Grow the preceding token rather than the operator itself: on PHP 8.5 the | ||
| // whole `|>` is one token, so the "after" fix would target the same index and | ||
| // one of the two changes would be dropped. |
This was referenced Aug 6, 2026
Merged
dereuromark
added a commit
to php-fig-rectified/psr2r-sniffer
that referenced
this pull request
Aug 6, 2026
Generic.PHP.DeprecatedFunctions builds its forbiddenFunctions list in its constructor from the Reflection API, covering every internal function the running PHP version marks deprecated. The ruleset set that same property, which replaces the list rather than extending it, so the standard reported fewer deprecations than plain Generic did. utf8_encode(), for instance, went unreported. The property is gone; create_function() and each() move to Generic.PHP.ForbiddenFunctions, which is where removed-in-8.0 functions belong since the reflection-driven list can no longer see them. That sniff's own defaults, sizeof and delete, are repeated there because setting the property replaces them too. Same fix as php-collective/code-sniffer#71.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent ways the standard was checking less than it looked like it was checking.
Generic.PHP.DeprecatedFunctionswas disabled by its own configurationThe sniff has no static list. Its constructor walks
get_defined_functions()and asksReflectionFunction::isDeprecated(), so it tracks whatever the running PHP marks deprecated - automatically, per version. The ruleset setforbiddenFunctionsto a three-entry array, and a ruleset property assignment replaces the value rather than extending it. The reflection-built list was discarded on every run.Before:
The three entries were also a poor trade:
eachandcreate_functionwere removed in PHP 8.0, so reflection cannot see them either way, anddeletewas never a PHP function. They now sit onGeneric.PHP.ForbiddenFunctions, whose list is a plain property and is the right home for removed-in-8.0 names. Its two defaults are repeated there because that override replaces too.After, all five report:
VoidCastandPipeOperatorSpacingmatched nothing on PHP 8.5Both sniffs target PHP 8.5 syntax, and both broke on the version that introduced it. PHP 8.5 collapses each construct into one token:
(void)T_OPEN_PARENTHESIST_STRINGT_CLOSE_PARENTHESIST_VOID_CAST( void )T_VOID_CAST, content'( void )'|>T_BITWISE_ORT_GREATER_THANT_PIPE| >T_BITWISE_ORT_GREATER_THANregister()returned only the pre-8.5 shapes, so on 8.5 neither sniff was ever handed a token and every violation passed. Both now register both shapes. Inner cast whitespace moves into the token content on 8.5, so it is read from there instead of from surrounding tokens.Two fixers needed a target change. Once the operator is a single token,
MissingBeforeandMissingAfterboth mutated that same index and PHPCS dropped the second, yielding|>trim(...). They now grow the neighboring token, which also keeps the pre-8.5 path correct.The pipe fixture gains a
| >case: on 8.5 that is the only input still reaching the two-token branch, so without it that branch would go uncovered on the newest version.CI
8.5joins the validation matrix, which is what would have caught this. That surfaced one PHPStan finding only visible when analyzing on 8.5:getFileDocBlockLines()accepted?intand used it as an array key. Both call sites already pass a non-null value, so the parameter is nowint.