Repository navigation
Conversation
…ILTER_THROW_ON_FAILURE hasThrowOnFailureFlag() only returned no when none of the analysed PHP versions has FILTER_THROW_ON_FAILURE. For a range like 8.4 - 8.5 it checked the flag bit as if every version had it. Before PHP 8.5 that bit (0x10000000) is FILTER_FLAG_GLOBAL_RANGE, so on PHP 8.4 a call with FILTER_FLAG_GLOBAL_RANGE lost false from its return type and comparing the result with false was reported as always true. The flag result is now combined with the PHP version check, so it is only yes when every analysed version throws instead of returning the failure value. Closes phpstan/phpstan#15481
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.
Closes phpstan/phpstan#15481
Thanks @gregorpilz for the detailed report.
Since 2.3.0, when PHPStan runs on PHP 8.4 and the analysed PHP version range reaches 8.5 (a plain
"php": "^8.4"is enough),filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_GLOBAL_RANGE)is inferred asnon-falsy-stringinstead ofnon-falsy-string|false, and!== falseon the result is reported as always true.FilterFunctionReturnTypeHelper::hasThrowOnFailureFlag()used to checkFILTER_THROW_ON_FAILUREonly when the configured PHP version had it. Since #6551 it reads the version from the scope and skips the check only when no analysed version has the constant. For a range like 8.4 - 8.5 it then checks the flag bit as if every version had it. Before PHP 8.5 that bit (0x10000000) isFILTER_FLAG_GLOBAL_RANGE, which PHP 8.5 moved to0x20000000. On a PHP 8.4 runtime,FILTER_FLAG_GLOBAL_RANGEmatches the flag andfalseis removed from the return type.This PR combines the flag result with
hasFilterThrowOnFailureConstant(). The result isyesonly when every analysed version has the constant and the flag is set, so the failure value is removed only in that case. Exact versions behave as before:nobelow 8.5, and the flag check alone from 8.5. Code that checksPHP_VERSION_ID >= 80500first still gets the narrowed type (scope-php-version-return-type-extensions.phpcovers this and still passes).Bug15481Testanalysesdata/bug-15481.phpwithphpVersion: {min: 80400, max: 80599}. The first function is the issue's code, withdumpType()replaced byassertType(). On PHP 8.4 those assertions fail without the fix. On PHP 8.5,FILTER_FLAG_GLOBAL_RANGEhas its own bit and they pass either way. So I added a second function that passesFILTER_THROW_ON_FAILUREdirectly under the same range. It expectsint|falseand fails without the fix on any runtime.I left
FilterFunctionsThrowTypeExtensionunchanged. With the same range on PHP 8.4, it still says that a call withFILTER_FLAG_GLOBAL_RANGEmay throwFilter\FilterFailedException. That is a possible throw point, not a type change, and it reports nothing with the default configuration. I can handle it separately if you want.Verified:
non-falsy-string|falseand report no error. 2.3.x reportsnotIdentical.alwaysTrueandreturn.tooWideBool.Bug15481Testfails without the fix on PHP 8.4 (3 assertions) and on PHP 8.5 (theFILTER_THROW_ON_FAILUREassertion), and passes with it on both.