Report always-true in_array(..., true) when every finite needle value is guaranteed in the haystack#5944
Open
phpstan-bot wants to merge 1 commit into
Open
Conversation
…ue is guaranteed in the haystack
- `ImpossibleCheckTypeHelper`: rewrite the strict `in_array()` guard so it checks that *each* finite needle value is guaranteed present in the haystack, instead of requiring a single haystack value to be a supertype of the whole needle. This makes union needles that are a subset of the haystack values (e.g. `'a'|'b'` in `array{'a','b','c'}`) report as always-true.
- Use `getFiniteTypes()` instead of `getConstantScalarTypes()` when collecting the haystack's guaranteed values, so enum-case needles are handled the same way as string/int/bool needles (mirrors `InArrayFunctionTypeSpecifyingExtension::computeNeedleNarrowingType`).
- `InArrayFunctionTypeSpecifyingExtension`: when combining the per-item comparisons of an array-literal haystack, clear the leftover root expression. Otherwise one item's `$needle === $oneItem` comparison (e.g. the always-false `$s === 'baz'`) was promoted by `mergeRootExpr(null, X)` to stand in for the whole call, making `in_array($subset, ['a','b','c'], true)` wrongly infer `false`.
- `ParameterCastableToStringRule`: drop the now-redundant `elseif (in_array(...))`/dead `else` branch that the stricter analysis correctly flags as always-true.
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
PHPStan did not report
in_array($needle, $haystack, true)as always-true when$needlecan take a finite set of values that is a subset of the haystack's values (e.g.$needleis'a'|'b'and$haystackisarray{'a','b','c'}). For the array-literal form of the same call it was even worse: the call's return type was wrongly inferred asfalse, producing a false "always false" report.This change makes both forms correctly report (and infer) always-true, and extends the same behaviour to integer- and enum-case needles.
Changes
src/Rules/Comparison/ImpossibleCheckTypeHelper.phpin_array()guard loop. Instead of bailing unless a single haystack value is a supertype of the entire needle, it now collects each haystack variant's guaranteed values and requires that every finite needle value is guaranteed present. Union needles that are subsets of the haystack are now provable.getConstantScalarTypes()togetFiniteTypes(), so enum-case needles are treated like scalar needles (matching the symmetric false-context logic inInArrayFunctionTypeSpecifyingExtension::computeNeedleNarrowingType).src/Type/Php/InArrayFunctionTypeSpecifyingExtension.phpArray_haystack, clear the resulting root expression when more than one item was combined. The per-item root expression is meaningless for the whole call.src/Rules/Functions/ParameterCastableToStringRule.phpelseif (in_array($functionName, $checkFirstArgFunctions, true))(plus its unreachableelse { return []; }) with a plainelse— the stricter analysis correctly proves this check always-true.tests/PHPStan/Analyser/nsrt/bug-14873.php(type inference) andtests/PHPStan/Rules/Comparison/data/bug-14873.php+testBug14873()(rule). Added the newly-detected always-true at line 259 of the existingcheck-type-function-call.phpfixture.Root cause
Two independent defects, both surfacing only for a finite needle that is a subset of the haystack:
Helper (variable haystack). The guard loop asked "is there one haystack value that is a supertype of the needle?". For a single needle value
'a'that works, but for a union needle'a'|'b'no single haystack value ('a','b', …) is a supertype of the union, so the helper always bailed tonull. The correct question is per-needle-value: each finite value of the needle must be guaranteed present. Additionally the loop only considered constant scalar haystack values, so enum-case haystacks/needles were never provable.Type-specifying extension (array-literal haystack). The
Array_branch builds the call's narrowing by combining oneSpecifiedTypesper array item, each carrying its own$needle === $itemas its root expression.SpecifiedTypes::mergeRootExpr(null, X)returnsX, so after the first two differing items zeroed the root expression, the last item's comparison ($needle === $lastItem) re-became the combined root expression. The helper then readgetType($lastItem comparison)— which for a subset needle is the constantfalse— and concluded the wholein_array()call was always false. Clearing the root expression once multiple items are combined removes this leak; the call result is then derived from the (correct) per-needle narrowing.Test
tests/PHPStan/Analyser/nsrt/bug-14873.phpasserts the inferred return type ofin_array(..., true)for variable and literal haystacks across string, integer and enum-case needles (subset ⇒true, non-subset ⇒bool, disjoint ⇒false). Fails before the fix (literal subset case inferredfalse, variable/enum subset cases inferredbool).tests/PHPStan/Rules/Comparison/data/bug-14873.php+testBug14873()assert the rule reports always-true for the reported variable-haystack scenario. Fails before the fix (no error reported).ImpossibleCheckTypeFunctionCallRuleTestgained the always-true atcheck-type-function-call.php:259(in_array($fooOrBar, ['foo','bar'], true)with$fooOrBarof type'foo'|'bar'), which was previously a missed report.in_array()(intentionally left inconclusive),array_key_exists()/class_exists()type-specifying extensions (no per-item root-expression leak — they set their root expression explicitly), and integer/enum needle variants (now fixed viagetFiniteTypes()).Fixes phpstan/phpstan#14873