From f81065ebf86d43d71912b7dee7aa5c4e89fce66c Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Wed, 1 Apr 2026 21:37:37 +0000 Subject: [PATCH] Move in_array impossibility detection from ImpossibleCheckTypeHelper to InArrayFunctionTypeSpecifyingExtension Remove the ~100-line special-case in_array handling from ImpossibleCheckTypeHelper and handle all impossibility detection entirely within the type-specifying extension using the rootExpr mechanism. The extension now returns: - ConstFetch('true') rootExpr when in_array is guaranteed to always return true - ConstFetch('false') rootExpr when types are incompatible (always false) - ConstFetch('__PHPSTAN_FAUX_CONSTANT') rootExpr for indeterminate cases This approach bypasses the sureTypes-based detection in ImpossibleCheckTypeHelper, which suffered from scope leakage issues (e.g., ClassConstFetch expressions for enum cases being marked as "specified" by prior in_array calls in the same scope). Fixes phpstan/phpstan#6705 Also fixes detection of previously-undetected cases: - in_array(Enum::A, non-empty-array, false/true) now correctly reports "always true" when enum type guarantees a match - in_array(Enum::B, array, false/true) now correctly reports "always false" when enum types are incompatible - in_array('foo', ['foo', 'bar']) without strict param is now correctly narrowed to return type `true` (string-to-string comparison is equivalent to strict) --- phpstan-baseline.neon | 12 -- .../Comparison/ImpossibleCheckTypeHelper.php | 103 ----------------- ...InArrayFunctionTypeSpecifyingExtension.php | 107 ++++++++++++++++-- tests/PHPStan/Analyser/nsrt/binary.php | 2 +- ...mpossibleCheckTypeFunctionCallRuleTest.php | 88 ++++++++------ .../Rules/Comparison/data/bug-6705.php | 52 +++++++++ 6 files changed, 204 insertions(+), 160 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-6705.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 564df2bc20d..d1059866d66 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -561,12 +561,6 @@ parameters: count: 2 path: src/Rules/Comparison/IfConstantConditionRule.php - - - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantArrayType is error-prone and deprecated. Use Type::getConstantArrays() instead.' - identifier: phpstanApi.instanceofType - count: 2 - path: src/Rules/Comparison/ImpossibleCheckTypeHelper.php - - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantBooleanType is error-prone and deprecated. Use Type::isTrue() or Type::isFalse() instead.' identifier: phpstanApi.instanceofType @@ -1581,12 +1575,6 @@ parameters: count: 1 path: src/Type/Php/FunctionExistsFunctionTypeSpecifyingExtension.php - - - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantArrayType is error-prone and deprecated. Use Type::getConstantArrays() instead.' - identifier: phpstanApi.instanceofType - count: 1 - path: src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php - - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantStringType is error-prone and deprecated. Use Type::getConstantStrings() instead.' identifier: phpstanApi.instanceofType diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 127cae01cd9..b0d44b9c2d8 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -18,13 +18,11 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; -use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\Generic\GenericClassStringType; use PHPStan\Type\IntersectionType; use PHPStan\Type\MixedType; -use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; @@ -95,107 +93,6 @@ public function findSpecifiedType( return null; } elseif ($functionName === 'array_search') { return null; - } elseif ($functionName === 'in_array' && $argsCount >= 2) { - $haystackArg = $args[1]->value; - $haystackType = $this->treatPhpDocTypesAsCertain ? $scope->getType($haystackArg) : $scope->getNativeType($haystackArg); - if ($haystackType instanceof MixedType) { - return null; - } - - if (!$haystackType->isArray()->yes()) { - return null; - } - - $needleArg = $args[0]->value; - $needleType = $this->treatPhpDocTypesAsCertain ? $scope->getType($needleArg) : $scope->getNativeType($needleArg); - - $isStrictComparison = false; - if ($argsCount >= 3) { - $strictNodeType = $scope->getType($args[2]->value); - $isStrictComparison = $strictNodeType->isTrue()->yes(); - } - - $isStrictComparison = $isStrictComparison - || $needleType->isEnum()->yes() - || $haystackType->getIterableValueType()->isEnum()->yes(); - - if (!$isStrictComparison) { - return null; - } - - $valueType = $haystackType->getIterableValueType(); - $constantNeedleTypesCount = count($needleType->getFiniteTypes()); - $constantHaystackTypesCount = count($valueType->getFiniteTypes()); - $isNeedleSupertype = $needleType->isSuperTypeOf($valueType); - if ($haystackType->isConstantArray()->no()) { - if ($haystackType->isIterableAtLeastOnce()->yes()) { - // In this case the generic implementation via typeSpecifier fails, because the argument types cannot be narrowed down. - if ($constantNeedleTypesCount === 1 && $constantHaystackTypesCount === 1) { - if ($isNeedleSupertype->yes()) { - return true; - } - if ($isNeedleSupertype->no()) { - return false; - } - } - - return null; - } - - if (!$isNeedleSupertype->no()) { - // Array might be empty, so in_array can return false - return null; - } - } - - if (!$haystackType instanceof ConstantArrayType || count($haystackType->getValueTypes()) > 0) { - $haystackArrayTypes = $haystackType->getArrays(); - if (count($haystackArrayTypes) === 1 && $haystackArrayTypes[0]->getIterableValueType() instanceof NeverType) { - return null; - } - - if ($isNeedleSupertype->maybe() || $isNeedleSupertype->yes()) { - foreach ($haystackArrayTypes as $haystackArrayType) { - if ($haystackArrayType instanceof ConstantArrayType) { - foreach ($haystackArrayType->getValueTypes() as $i => $haystackArrayValueType) { - if ($haystackArrayType->isOptionalKey($i)) { - continue; - } - - $haystackArrayValueConstantScalarTypes = $haystackArrayValueType->getConstantScalarTypes(); - if (count($haystackArrayValueConstantScalarTypes) > 1) { - continue; - } - - foreach ($haystackArrayValueConstantScalarTypes as $constantScalarType) { - if ($constantScalarType->isSuperTypeOf($needleType)->yes()) { - continue 3; - } - } - } - } else { - foreach ($haystackArrayType->getIterableValueType()->getConstantScalarTypes() as $constantScalarType) { - if ($constantScalarType->isSuperTypeOf($needleType)->yes()) { - continue 2; - } - } - } - - return null; - } - } - - if ($isNeedleSupertype->yes()) { - $hasConstantNeedleTypes = $constantNeedleTypesCount > 0; - $hasConstantHaystackTypes = $constantHaystackTypesCount > 0; - if ( - (!$hasConstantNeedleTypes && !$hasConstantHaystackTypes) - || $hasConstantNeedleTypes !== $hasConstantHaystackTypes - ) { - return null; - } - } - } } elseif ($functionName === 'method_exists' && $argsCount >= 2) { $objectArg = $args[0]->value; $objectType = $this->treatPhpDocTypesAsCertain ? $scope->getType($objectArg) : $scope->getNativeType($objectArg); diff --git a/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php b/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php index c22cad0f723..bddc66fa78f 100644 --- a/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php @@ -5,7 +5,9 @@ use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\BinaryOp\Equal; use PhpParser\Node\Expr\BinaryOp\Identical; +use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Name; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -16,7 +18,6 @@ use PHPStan\Reflection\FunctionReflection; use PHPStan\Type\Accessory\NonEmptyArrayType; use PHPStan\Type\ArrayType; -use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\FunctionTypeSpecifyingExtension; use PHPStan\Type\MixedType; use PHPStan\Type\Type; @@ -103,18 +104,21 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n && $arrayType->isArray()->yes() && $arrayType->getIterableValueType()->isSuperTypeOf($needleType)->yes() ) { - return $this->typeSpecifier->create( + $specifiedTypes = $this->typeSpecifier->create( $args[1]->value, TypeCombinator::intersect($arrayType, new NonEmptyArrayType()), $context, $scope, ); + + return $specifiedTypes->setRootExpr(new ConstFetch(new Name('__PHPSTAN_FAUX_CONSTANT'))); } return new SpecifiedTypes(); } $specifiedTypes = new SpecifiedTypes(); + $originalArrayValueType = $arrayValueType; $narrowingValueType = $this->computeNeedleNarrowingType($context, $needleType, $arrayType, $arrayValueType); if ($narrowingValueType !== null) { $specifiedTypes = $this->typeSpecifier->create( @@ -163,7 +167,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n )); } - return $specifiedTypes; + return $specifiedTypes->setRootExpr($this->determineRootExpr($needleType, $arrayType, $originalArrayValueType)); } /** @@ -185,13 +189,13 @@ private function computeNeedleNarrowingType(TypeSpecifierContext $context, Type return null; } - $arrays = $arrayType->getArrays(); + $constantArrays = $arrayType->getConstantArrays(); $guaranteedValueTypePerArray = []; - foreach ($arrays as $array) { - if ($array instanceof ConstantArrayType) { + if (count($constantArrays) > 0) { + foreach ($constantArrays as $constantArray) { $innerGuaranteeValueType = []; - foreach ($array->getValueTypes() as $i => $valueType) { - if ($array->isOptionalKey($i)) { + foreach ($constantArray->getValueTypes() as $i => $valueType) { + if ($constantArray->isOptionalKey($i)) { continue; } @@ -208,7 +212,10 @@ private function computeNeedleNarrowingType(TypeSpecifierContext $context, Type } $guaranteedValueTypePerArray[] = TypeCombinator::union(...$innerGuaranteeValueType); - } else { + } + } else { + $arrays = $arrayType->getArrays(); + foreach ($arrays as $array) { $finiteValueType = $array->getIterableValueType()->getFiniteTypes(); if (count($finiteValueType) !== 1) { return null; @@ -230,4 +237,86 @@ private function computeNeedleNarrowingType(TypeSpecifierContext $context, Type return $guaranteedValueType; } + /** + * Returns a rootExpr that ImpossibleCheckTypeHelper evaluates instead of + * using sureTypes-based detection. This bypasses scope leakage issues where + * expressions (e.g. enum ClassConstFetch) are marked as "specified" by + * prior in_array calls in the same scope. + * + * Returns ConstFetch('false') for incompatible types (always false), + * ConstFetch('true') when the needle is guaranteed to be found (always true), + * or ConstFetch('__PHPSTAN_FAUX_CONSTANT') for indeterminate cases. + */ + private function determineRootExpr(Type $needleType, Type $arrayType, Type $arrayValueType): ConstFetch + { + // Types are incompatible -> always false + if ($arrayValueType->isSuperTypeOf($needleType)->no()) { + return new ConstFetch(new Name('false')); + } + + // Check if in_array is guaranteed to always return true + if ($this->isGuaranteedToContainNeedle($needleType, $arrayType)) { + return new ConstFetch(new Name('true')); + } + + // Indeterminate: types are compatible but can't guarantee result. + // Set rootExpr to prevent false impossibility detection. + return new ConstFetch(new Name('__PHPSTAN_FAUX_CONSTANT')); + } + + private function isGuaranteedToContainNeedle(Type $needleType, Type $arrayType): bool + { + if (!$arrayType->isIterableAtLeastOnce()->yes()) { + return false; + } + + if (count($needleType->getFiniteTypes()) !== 1) { + return false; + } + + $constantArrays = $arrayType->getConstantArrays(); + if (count($constantArrays) > 0) { + foreach ($constantArrays as $constantArray) { + $hasGuaranteedMatch = false; + foreach ($constantArray->getValueTypes() as $i => $valueType) { + if ($constantArray->isOptionalKey($i)) { + continue; + } + + $constantScalarTypes = $valueType->getConstantScalarTypes(); + if (count($constantScalarTypes) > 1) { + continue; + } + + foreach ($constantScalarTypes as $constantScalarType) { + if ($constantScalarType->isSuperTypeOf($needleType)->yes()) { + $hasGuaranteedMatch = true; + break 2; + } + } + } + if (!$hasGuaranteedMatch) { + return false; + } + } + } else { + $arrays = $arrayType->getArrays(); + if (count($arrays) === 0) { + return false; + } + + foreach ($arrays as $array) { + $finiteTypes = $array->getIterableValueType()->getFiniteTypes(); + if (count($finiteTypes) !== 1) { + return false; + } + if (!$needleType->isSuperTypeOf($finiteTypes[0])->yes()) { + return false; + } + } + } + + return true; + } + } diff --git a/tests/PHPStan/Analyser/nsrt/binary.php b/tests/PHPStan/Analyser/nsrt/binary.php index ce3a13e6cc6..9c6f34786e3 100644 --- a/tests/PHPStan/Analyser/nsrt/binary.php +++ b/tests/PHPStan/Analyser/nsrt/binary.php @@ -544,7 +544,7 @@ public function doFoo(array $generalArray) assertType('bool', is_int($mixed)); assertType('true', is_int($integer)); assertType('false', is_int($string)); - assertType('bool', in_array('foo', ['foo', 'bar'])); + assertType('true', in_array('foo', ['foo', 'bar'])); assertType('true', in_array('foo', ['foo', 'bar'], true)); assertType('false', in_array('baz', ['foo', 'bar'], true)); assertType('array{2, 3}', $arrToShift); diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 69e992139d8..3eacdd0f05a 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -8,9 +8,9 @@ use PHPUnit\Framework\Attributes\RequiresPhp; use stdClass; use function array_filter; -use function array_map; use function array_values; use function count; +use function usort; /** * @extends RuleTestCase @@ -123,6 +123,10 @@ public function testImpossibleCheckTypeFunctionCall(): void 'Call to function in_array() with arguments \'bar\'|\'foo\', array{\'baz\', \'lorem\'} and true will always evaluate to false.', 255, ], + [ + 'Call to function in_array() with arguments \'bar\'|\'foo\', array{\'foo\', \'bar\'} and true will always evaluate to true.', + 259, + ], [ 'Call to function in_array() with arguments \'foo\', array{\'foo\'} and true will always evaluate to true.', 263, @@ -863,30 +867,6 @@ private static function getLooseComparisonAgainsEnumsIssues(): array 162, $tipText, ], - [ - 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::A, non-empty-array and false will always evaluate to true.', - 165, - 'BUG', - //$tipText, - ], - [ - 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::A, non-empty-array and true will always evaluate to true.', - 168, - 'BUG', - //$tipText, - ], - [ - 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, non-empty-array and false will always evaluate to false.', - 171, - 'BUG', - //$tipText, - ], - [ - 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, non-empty-array and true will always evaluate to false.', - 174, - 'BUG', - //$tipText, - ], ]; } @@ -894,16 +874,24 @@ private static function getLooseComparisonAgainsEnumsIssues(): array public function testLooseComparisonAgainstEnums(): void { $this->treatPhpDocTypesAsCertain = true; - $issues = array_map( - static function (array $i): array { - if (($i[2] ?? null) === 'BUG') { - unset($i[2]); - } - - return $i; - }, - self::getLooseComparisonAgainsEnumsIssues(), - ); + $issues = self::getLooseComparisonAgainsEnumsIssues(); + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::A, non-empty-array and false will always evaluate to true.', + 165, + ]; + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::A, non-empty-array and true will always evaluate to true.', + 168, + ]; + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, non-empty-array and false will always evaluate to false.', + 171, + ]; + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, non-empty-array and true will always evaluate to false.', + 174, + ]; + usort($issues, static fn (array $a, array $b): int => $a[1] <=> $b[1]); $this->analyse([__DIR__ . '/data/loose-comparison-against-enums.php'], $issues); } @@ -947,6 +935,15 @@ public function testLooseComparisonAgainstEnumsNoPhpdoc(): void $this->treatPhpDocTypesAsCertain = false; $issues = self::getLooseComparisonAgainsEnumsIssues(); $issues = array_values(array_filter($issues, static fn (array $i) => count($i) === 2)); + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, array and false will always evaluate to false.', + 171, + ]; + $issues[] = [ + 'Call to function in_array() with arguments LooseComparisonAgainstEnums\FooUnitEnum::B, array and true will always evaluate to false.', + 174, + ]; + usort($issues, static fn (array $a, array $b): int => $a[1] <=> $b[1]); $this->analyse([__DIR__ . '/data/loose-comparison-against-enums.php'], $issues); } @@ -1207,4 +1204,25 @@ public function testBug13799(): void ]); } + public function testBug6705(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-6705.php'], [ + [ + 'Call to function in_array() with arguments \'a\', non-empty-array and true will always evaluate to true.', + 40, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'Call to function in_array() with arguments \'b\', non-empty-array and true will always evaluate to false.', + 43, + ], + [ + 'Call to function in_array() with arguments int, array and true will always evaluate to false.', + 44, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-6705.php b/tests/PHPStan/Rules/Comparison/data/bug-6705.php new file mode 100644 index 00000000000..2d0a4d9156c --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-6705.php @@ -0,0 +1,52 @@ + 10 ? 0 : 1; + } + + if (\in_array(0, $a, true)) { + return; + } + } + + /** + * @param non-empty-array $multiValueArr + * @param non-empty-array $nonEmptyInts + * @param array $possiblyEmptyArr + * @param non-empty-array $singleValueArr + * @param array $strings + */ + public function testCases( + array $multiValueArr, + array $nonEmptyInts, + array $possiblyEmptyArr, + array $singleValueArr, + array $strings, + int $i, + ): void + { + // Always true: non-empty array with single value type matching needle + if (in_array('a', $singleValueArr, true)) {} // always true + + // Always false: incompatible types + if (in_array('b', $singleValueArr, true)) {} // always false + if (in_array($i, $strings, true)) {} // always false + + // Indeterminate: needle compatible with values but not guaranteed + if (in_array('a', $multiValueArr, true)) {} // no error + if (in_array(0, $possiblyEmptyArr, true)) {} // no error + if (in_array(0, $nonEmptyInts, true)) {} // no error + } + +}