Skip to content

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
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-vn0k5ps
Open

Un-prefix Filter\ and PDO\ class name strings in the phar, detect FILTER_THROW_ON_FAILURE in all filter_*()#6112
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-vn0k5ps

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

Since 2.2.6 PHPStan detects that filter_var() throws Filter\FilterFailedException when FILTER_THROW_ON_FAILURE is 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 @throws tag PHPStan asks for cannot be written.

The class name is a string literal in FilterVarThrowTypeExtension and php-scoper prefixes every string that looks like a fully qualified class name. compiler/build/scoper.inc.php already un-prefixes such literals again, but only for BcMath\, Dom\, FFI\ and Ds\.

While fixing that, the surrounding FILTER_THROW_ON_FAILURE support turned out to have several sibling bugs, which are fixed too.

Changes

  • compiler/build/scoper-namespaces.php (new) holds both php-scoper's exclude-namespaces and the namespaces whose class name strings in src/ must be un-prefixed again; compiler/build/scoper.inc.php consumes both.
    • Filter added - the reported bug.
    • PDO added - the same bug in src/Type/Php/PDOConnectReturnTypeExtension.php, where PDO::connect() returns PDO\Mysql, PDO\Pgsql, PDO\Sqlite, PDO\Firebird, PDO\Dblib and PDO\Odbc.
    • The one-off patcher for src/Testing/ErrorFormatterTestCase.php is dropped, Foobar is handled by the generic one instead.
  • tests/PHPStan/Build/ScoperClassNameStringsTest.php (new) guards the whole family: it scans every class name string literal in src/ and fails when its root namespace is neither excluded from prefixing nor un-prefixed by the patcher.
  • resources/constantToFunctionParameterMap.php - FILTER_THROW_ON_FAILURE added to the $options bitmask of filter_var() and filter_input(); passing it was reported as argument.invalidConstant.
  • src/Type/Php/FilterFunctionFlagsHelper.php (new) resolves the values that may carry filter flags in a filter_*() call. It normalizes the arguments through ArgumentsNormalizer and knows that filter_var_array()/filter_input_array() take one filter specification per input key (and that an integer $options is the filter id there, so it carries no flags).
  • src/Type/Php/FilterVarThrowTypeExtension.php renamed to FilterFunctionsThrowTypeExtension.php and extended to filter_input(), filter_var_array() and filter_input_array().
  • src/Rules/Functions/FilterVarRule.php uses 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 makes filter_input() throw when FILTER_THROW_ON_FAILURE is set, so it no longer contributes null/false to the return type.

Probed and found correct

  • FilterVarDynamicReturnTypeExtension, FilterInputDynamicReturnTypeExtension and FilterVarArrayDynamicReturnTypeExtension read their arguments positionally too, but dynamic return type extensions receive an already normalized FuncCall from FuncCallHandler, so named arguments work there. The same applies to the throw type extension; it is the Rule that gets the raw AST and needed the normalization.
  • No other class name string literal in src/ is affected - the new build test enumerates them all.

Root cause

Two independent patterns:

  1. php-scoper prefixes class name string literals in src/. src/ refers to classes of the analysed code through strings such as new ObjectType('BcMath\Number'). php-scoper cannot tell those apart from references to bundled vendor classes and prefixes them, and Filter\FilterFailedException is additionally declared by the bundled symfony/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\FilterFailedException in the throw type extension and all six PDO\* names in PDOConnectReturnTypeExtension. The list is now shared with a test that fails whenever a new namespace shows up.

  2. FILTER_THROW_ON_FAILURE support was written for filter_var() only. The flag works with every validation filter, and php_filter_call() in php-src - shared by filter_var(), filter_input(), filter_var_array() and filter_input_array() - throws Filter\FilterFailedException and raises the "cannot use both FILTER_NULL_ON_FAILURE and FILTER_THROW_ON_FAILURE" ValueError. Only filter_var() was covered, so the other three produced "Dead catch" false positives and no error for the conflicting flags, and the $options allowed-constants map was missing the constant for filter_var() and filter_input() alike.

Test

  • tests/PHPStan/Build/ScoperClassNameStringsTest.php - fails on the unhandled Filter\FilterFailedException and PDO\Mysql literals before the fix.
  • tests/PHPStan/Rules/Functions/data/constant-parameter-check.php - FILTER_THROW_ON_FAILURE passed to filter_var() (alone and combined with another flag) and to filter_input(); reports three argument.invalidConstant errors before the fix.
  • tests/PHPStan/Rules/Exceptions/data/filter-var-throw-on-failure.php - filter_var() with named arguments plus filter_input(), filter_var_array() and filter_input_array(); each catch (\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 $options for the array variants.
  • tests/PHPStan/Analyser/nsrt/filter-var-php85.php - filter_input() returns int instead of int|false|null with FILTER_THROW_ON_FAILURE, variable certainty inside the catch block, and named-argument calls of filter_var()/filter_input().

These fixtures require PHP 8.5, so they are skipped on lower runtimes; they were additionally verified with bin/phpstan analyse under phpVersion: 80500.

Fixes phpstan/phpstan#15007

… `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.
@ondrejmirtes

Copy link
Copy Markdown
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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid/redundant missingType.checkedException since 2.2.6 on FILTER_THROW_ON_FAILURE flag.

2 participants