Un-prefix Filter\ and PDO\ class name strings in the phar, detect FILTER_THROW_ON_FAILURE in all filter_*() - #6112
Open
phpstan-bot wants to merge 1 commit into
Conversation
… `FILTER_THROW_ON_FAILURE` in all `filter_*()`
* Move php-scoper's excluded namespaces and the list of namespaces whose
class name strings in src/ have to be un-prefixed again into the shared
compiler/build/scoper-namespaces.php, and add `Filter` (the reported bug:
`new ObjectType('Filter\FilterFailedException')` was reported as
`_PHPStan_<sha>\Filter\FilterFailedException`) and `PDO` (the same bug in
`PDOConnectReturnTypeExtension`). Replaces the one-off
`src/Testing/ErrorFormatterTestCase.php` patcher with the generic one.
* Add ScoperClassNameStringsTest, which scans every class name string
literal in src/ and fails when its namespace is neither excluded from
prefixing nor un-prefixed by the patcher.
* Add `FILTER_THROW_ON_FAILURE` to the allowed constants of `filter_var()`
and `filter_input()`'s `$options` parameter - passing it was reported as
`argument.invalidConstant`.
* Extract `FilterFunctionFlagsHelper`, which resolves the flag-carrying
values of a `filter_*()` call through `ArgumentsNormalizer`, so named
arguments work, and which understands the per-input-key filter
specification of `filter_var_array()`/`filter_input_array()`.
* Rename `FilterVarThrowTypeExtension` to `FilterFunctionsThrowTypeExtension`
and let it cover `filter_input()`, `filter_var_array()` and
`filter_input_array()`, which throw `Filter\FilterFailedException` too.
* `FilterVarRule` now checks `filter_input()`, `filter_var_array()` and
`filter_input_array()` as well, per input key for the array variants, and
no longer reads the `$options` argument positionally.
* `filter_input()` with `FILTER_THROW_ON_FAILURE` no longer returns `null`
for a missing input value - that case throws.
Member
|
This PR does too much, should be several PRs. Also historically if PHP-Scoper adds a prefix to a string, it's because it didn't recognize it's a built-in PHP class. Meaning phpstorm-stubs should be updated in compiler/box most likely? |
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.
Summary
Since 2.2.6 PHPStan detects that
filter_var()throwsFilter\FilterFailedExceptionwhenFILTER_THROW_ON_FAILUREis passed. In the distributed phar the reported class name is_PHPStan_<commit>\Filter\FilterFailedException, a name that does not exist in the analysed code, so the@throwstag PHPStan asks for cannot be written.The class name is a string literal in
FilterVarThrowTypeExtensionand php-scoper prefixes every string that looks like a fully qualified class name.compiler/build/scoper.inc.phpalready un-prefixes such literals again, but only forBcMath\,Dom\,FFI\andDs\.While fixing that, the surrounding
FILTER_THROW_ON_FAILUREsupport turned out to have several sibling bugs, which are fixed too.Changes
compiler/build/scoper-namespaces.php(new) holds both php-scoper'sexclude-namespacesand the namespaces whose class name strings insrc/must be un-prefixed again;compiler/build/scoper.inc.phpconsumes both.Filteradded - the reported bug.PDOadded - the same bug insrc/Type/Php/PDOConnectReturnTypeExtension.php, wherePDO::connect()returnsPDO\Mysql,PDO\Pgsql,PDO\Sqlite,PDO\Firebird,PDO\DblibandPDO\Odbc.src/Testing/ErrorFormatterTestCase.phpis dropped,Foobaris handled by the generic one instead.tests/PHPStan/Build/ScoperClassNameStringsTest.php(new) guards the whole family: it scans every class name string literal insrc/and fails when its root namespace is neither excluded from prefixing nor un-prefixed by the patcher.resources/constantToFunctionParameterMap.php-FILTER_THROW_ON_FAILUREadded to the$optionsbitmask offilter_var()andfilter_input(); passing it was reported asargument.invalidConstant.src/Type/Php/FilterFunctionFlagsHelper.php(new) resolves the values that may carry filter flags in afilter_*()call. It normalizes the arguments throughArgumentsNormalizerand knows thatfilter_var_array()/filter_input_array()take one filter specification per input key (and that an integer$optionsis the filter id there, so it carries no flags).src/Type/Php/FilterVarThrowTypeExtension.phprenamed toFilterFunctionsThrowTypeExtension.phpand extended tofilter_input(),filter_var_array()andfilter_input_array().src/Rules/Functions/FilterVarRule.phpuses the helper, so it covers the same four functions, handles named arguments and checks the array variants per input key.src/Type/Php/FilterFunctionReturnTypeHelper.php- a missing input value makesfilter_input()throw whenFILTER_THROW_ON_FAILUREis set, so it no longer contributesnull/falseto the return type.Probed and found correct
FilterVarDynamicReturnTypeExtension,FilterInputDynamicReturnTypeExtensionandFilterVarArrayDynamicReturnTypeExtensionread their arguments positionally too, but dynamic return type extensions receive an already normalizedFuncCallfromFuncCallHandler, so named arguments work there. The same applies to the throw type extension; it is theRulethat gets the raw AST and needed the normalization.src/is affected - the new build test enumerates them all.Root cause
Two independent patterns:
php-scoper prefixes class name string literals in
src/.src/refers to classes of the analysed code through strings such asnew ObjectType('BcMath\Number'). php-scoper cannot tell those apart from references to bundled vendor classes and prefixes them, andFilter\FilterFailedExceptionis additionally declared by the bundledsymfony/polyfill-php85. The patcher that strips the prefix back off was an explicit, hand-maintained list, so every newly referenced namespace silently regressed. Affected:Filter\FilterFailedExceptionin the throw type extension and all sixPDO\*names inPDOConnectReturnTypeExtension. The list is now shared with a test that fails whenever a new namespace shows up.FILTER_THROW_ON_FAILUREsupport was written forfilter_var()only. The flag works with every validation filter, andphp_filter_call()in php-src - shared byfilter_var(),filter_input(),filter_var_array()andfilter_input_array()- throwsFilter\FilterFailedExceptionand raises the "cannot use bothFILTER_NULL_ON_FAILUREandFILTER_THROW_ON_FAILURE"ValueError. Onlyfilter_var()was covered, so the other three produced "Dead catch" false positives and no error for the conflicting flags, and the$optionsallowed-constants map was missing the constant forfilter_var()andfilter_input()alike.Test
tests/PHPStan/Build/ScoperClassNameStringsTest.php- fails on the unhandledFilter\FilterFailedExceptionandPDO\Mysqlliterals before the fix.tests/PHPStan/Rules/Functions/data/constant-parameter-check.php-FILTER_THROW_ON_FAILUREpassed tofilter_var()(alone and combined with another flag) and tofilter_input(); reports threeargument.invalidConstanterrors before the fix.tests/PHPStan/Rules/Exceptions/data/filter-var-throw-on-failure.php-filter_var()with named arguments plusfilter_input(),filter_var_array()andfilter_input_array(); eachcatch (\Filter\FilterFailedException)is reported as a dead catch before the fix.tests/PHPStan/Rules/Functions/data/filter-functions-null-and-throw.php(new) - the conflicting-flags rule for all four functions, with named arguments, plus two cases that must stay silent: flags spread over different per-key specifications, and an integer$optionsfor the array variants.tests/PHPStan/Analyser/nsrt/filter-var-php85.php-filter_input()returnsintinstead ofint|false|nullwithFILTER_THROW_ON_FAILURE, variable certainty inside the catch block, and named-argument calls offilter_var()/filter_input().These fixtures require PHP 8.5, so they are skipped on lower runtimes; they were additionally verified with
bin/phpstan analyseunderphpVersion: 80500.Fixes phpstan/phpstan#15007