Fix phpstan/phpstan#13133: Conditionally defined classes will be evaluated regardless of the current version#5170
Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Closed
Conversation
- Added RemoveUnusedCodeByPhpVersionIdVisitor to RichParser so that PHP_VERSION_ID-guarded dead code is stripped during analysis, not just during reflection discovery - Previously only the CleaningParser (used for non-analysed files) applied this visitor; the RichParser (used for analysed files) did not - Updated AnalyserTest to pass the new PhpVersion parameter to RichParser - Updated CallMethodsRuleTest to account for dead branch stripping - New regression test for conditionally defined classes with typed constants guarded by PHP_VERSION_ID checks Closes phpstan/phpstan#13133
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
3 participants
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.
Summary
Conditionally defined classes using
if (PHP_VERSION_ID >= 80300)were still being analysed in the dead branch, causing non-ignorable errors likeClass constants with native types are supported only on PHP 8.3 and later.even when running on PHP 8.2.The fix ensures that
RemoveUnusedCodeByPhpVersionIdVisitoris applied inRichParser(used for analysed files), the same way it was already applied inCleaningParser(used for non-analysed/reflection files).Changes
PhpVersiondependency toRichParserconstructor (src/Parser/RichParser.php)RemoveUnusedCodeByPhpVersionIdVisitoras the first AST traversal step inRichParser::parseString()tests/PHPStan/Analyser/AnalyserTest.phpto pass the newPhpVersionparametertests/PHPStan/Rules/Methods/CallMethodsRuleTest.phpto conditionally expect errors based on PHP version (dead branch is now correctly stripped)tests/PHPStan/Rules/Constants/NativeTypedClassConstantRuleBug13133Test.phpwith test data and configRoot cause
The
RemoveUnusedCodeByPhpVersionIdVisitorstrips dead code branches guarded byPHP_VERSION_IDcomparisons from the AST at parse time. This visitor was only applied inCleaningParser(used viaPathRoutingParserfor non-analysed files/reflection discovery). When a file was in the "analysed files" set,PathRoutingParserrouted it throughRichParserinstead, which did not apply this visitor. As a result, classes defined in deadPHP_VERSION_IDbranches were still analysed and rules likeNativeTypedClassConstantRulereported non-ignorable errors.Test
Added
NativeTypedClassConstantRuleBug13133Testwhich setsphpVersion: 80200via a config file and analyses a file containing a class with typed constants insideif (PHP_VERSION_ID >= 80300) { ... } else { ... }. The test verifies no errors are reported since the typed-constant branch should be stripped as dead code on PHP 8.2.Fixes phpstan/phpstan#13133