From a1f4dae3d4f19aa2b8b3724a8062fc18de6c50ad Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 19 Jan 2025 17:02:36 +0100 Subject: [PATCH 1/3] Revert "Revert "Fix ImpossibleCheckTypeFunctionCallRule for `is_subclass_of` and `is_a`"" This reverts commit dce063a18a9dddcff98f2345942216205661b230. --- .../ImpossibleCheckTypeFunctionCallRule.php | 4 - .../IsAFunctionTypeSpecifyingExtension.php | 16 ++- ...classOfFunctionTypeSpecifyingExtension.php | 16 ++- tests/PHPStan/Analyser/TypeSpecifierTest.php | 4 +- ...mpossibleCheckTypeFunctionCallRuleTest.php | 7 + .../Rules/Comparison/data/bug-3979.php | 130 ++++++++++++++++++ 6 files changed, 168 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-3979.php diff --git a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php index 9033aa3865..29b0801ece 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php @@ -8,7 +8,6 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function sprintf; -use function strtolower; /** * @implements Rule @@ -38,9 +37,6 @@ public function processNode(Node $node, Scope $scope): array } $functionName = (string) $node->name; - if (strtolower($functionName) === 'is_a') { - return []; - } $isAlways = $this->impossibleCheckTypeHelper->findSpecifiedType($scope, $node); if ($isAlways === null) { return []; diff --git a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php index c4000b9aff..4d062efd56 100644 --- a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php @@ -9,9 +9,12 @@ use PHPStan\Analyser\TypeSpecifierAwareExtension; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Type\ClassStringType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FunctionTypeSpecifyingExtension; +use PHPStan\Type\ObjectWithoutClassType; +use PHPStan\Type\TypeCombinator; use function count; use function strtolower; @@ -47,9 +50,20 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n $allowStringType = isset($node->getArgs()[2]) ? $scope->getType($node->getArgs()[2]->value) : new ConstantBooleanType(false); $allowString = !$allowStringType->equals(new ConstantBooleanType(false)); + $superType = $allowString + ? TypeCombinator::union(new ObjectWithoutClassType(), new ClassStringType()) + : new ObjectWithoutClassType(); + + $resultType = $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, true); + + // prevent false-positives in IsAFunctionTypeSpecifyingHelper + if ($resultType->equals($superType) && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { + return new SpecifiedTypes([], []); + } + return $this->typeSpecifier->create( $node->getArgs()[0]->value, - $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, true), + $resultType, $context, false, $scope, diff --git a/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php index 2d52ee99e1..0ca6cf4e76 100644 --- a/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php @@ -9,9 +9,12 @@ use PHPStan\Analyser\TypeSpecifierAwareExtension; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Type\ClassStringType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\FunctionTypeSpecifyingExtension; use PHPStan\Type\Generic\GenericClassStringType; +use PHPStan\Type\ObjectWithoutClassType; +use PHPStan\Type\TypeCombinator; use function count; use function strtolower; @@ -48,9 +51,20 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n return new SpecifiedTypes([], []); } + $superType = $allowString + ? TypeCombinator::union(new ObjectWithoutClassType(), new ClassStringType()) + : new ObjectWithoutClassType(); + + $resultType = $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, false); + + // prevent false-positives in IsAFunctionTypeSpecifyingHelper + if ($resultType->equals($superType) && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { + return new SpecifiedTypes([], []); + } + return $this->typeSpecifier->create( $node->getArgs()[0]->value, - $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, false), + $resultType, $context, false, $scope, diff --git a/tests/PHPStan/Analyser/TypeSpecifierTest.php b/tests/PHPStan/Analyser/TypeSpecifierTest.php index 999c7169a3..4ad6a7cec4 100644 --- a/tests/PHPStan/Analyser/TypeSpecifierTest.php +++ b/tests/PHPStan/Analyser/TypeSpecifierTest.php @@ -1133,9 +1133,7 @@ public function dataCondition(): iterable new Arg(new Variable('stringOrNull')), new Arg(new Expr\ConstFetch(new Name('false'))), ]), - [ - '$object' => 'object', - ], + [], [], ], [ diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 16529f3a74..b15cf528b0 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1102,4 +1102,11 @@ public function testAlwaysTruePregMatch(): void $this->analyse([__DIR__ . '/data/always-true-preg-match.php'], []); } + public function testBug3979(): void + { + $this->checkAlwaysTrueCheckTypeFunctionCall = true; + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-3979.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-3979.php b/tests/PHPStan/Rules/Comparison/data/bug-3979.php new file mode 100644 index 0000000000..f0f21220d1 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-3979.php @@ -0,0 +1,130 @@ + Date: Sun, 19 Jan 2025 17:02:49 +0100 Subject: [PATCH 2/3] Revert "Revert "Add non regression tests"" This reverts commit e6cd6515a468ad8e8155ee37af98135d89780fe2. --- ...mpossibleCheckTypeFunctionCallRuleTest.php | 18 ++++++++++++ .../Rules/Comparison/data/bug-8464.php | 18 ++++++++++++ .../Rules/Comparison/data/bug-8954.php | 28 +++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-8464.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-8954.php diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index b15cf528b0..896173a6ee 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1109,4 +1109,22 @@ public function testBug3979(): void $this->analyse([__DIR__ . '/data/bug-3979.php'], []); } + public function testBug8464(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->checkAlwaysTrueCheckTypeFunctionCall = true; + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-8464.php'], []); + } + + public function testBug8954(): void + { + $this->checkAlwaysTrueCheckTypeFunctionCall = true; + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-8954.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-8464.php b/tests/PHPStan/Rules/Comparison/data/bug-8464.php new file mode 100644 index 0000000000..23cd280d7a --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-8464.php @@ -0,0 +1,18 @@ += 8.0 + +namespace Bug8464; + +final class ObjectUtil +{ + /** + * @param class-string $type + */ + public static function instanceOf(mixed $object, string $type): bool + { + return \is_object($object) + && ( + $object::class === $type || + is_subclass_of($object, $type) + ); + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-8954.php b/tests/PHPStan/Rules/Comparison/data/bug-8954.php new file mode 100644 index 0000000000..b89b47ba6d --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-8954.php @@ -0,0 +1,28 @@ + $class + * @param class-string $expected + * + * @return ?class-string + */ +function ensureSubclassOf(?string $class, string $expected): ?string { + if ($class === null) { + return $class; + } + + if (!class_exists($class)) { + throw new \Exception("Class “{$class}” does not exist."); + } + + if (!is_subclass_of($class, $expected)) { + throw new \Exception("Class “{$class}” is not a subclass of “{$expected}”."); + } + + return $class; +} From f4a6d4ebf6e1eb0b371e7d9081cab4e123be305c Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 19 Jan 2025 17:55:49 +0100 Subject: [PATCH 3/3] Better strategy --- .../IsAFunctionTypeSpecifyingExtension.php | 9 +------ ...classOfFunctionTypeSpecifyingExtension.php | 9 +------ ...mpossibleCheckTypeFunctionCallRuleTest.php | 12 ++++++++++ .../Rules/Comparison/data/bug-pr-3404.php | 24 +++++++++++++++++++ 4 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-pr-3404.php diff --git a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php index 4d062efd56..55789676aa 100644 --- a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php @@ -9,12 +9,9 @@ use PHPStan\Analyser\TypeSpecifierAwareExtension; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Reflection\FunctionReflection; -use PHPStan\Type\ClassStringType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FunctionTypeSpecifyingExtension; -use PHPStan\Type\ObjectWithoutClassType; -use PHPStan\Type\TypeCombinator; use function count; use function strtolower; @@ -50,14 +47,10 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n $allowStringType = isset($node->getArgs()[2]) ? $scope->getType($node->getArgs()[2]->value) : new ConstantBooleanType(false); $allowString = !$allowStringType->equals(new ConstantBooleanType(false)); - $superType = $allowString - ? TypeCombinator::union(new ObjectWithoutClassType(), new ClassStringType()) - : new ObjectWithoutClassType(); - $resultType = $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, true); // prevent false-positives in IsAFunctionTypeSpecifyingHelper - if ($resultType->equals($superType) && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { + if ($classType->getConstantStrings() === [] && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { return new SpecifiedTypes([], []); } diff --git a/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php index 0ca6cf4e76..71800c5366 100644 --- a/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsSubclassOfFunctionTypeSpecifyingExtension.php @@ -9,12 +9,9 @@ use PHPStan\Analyser\TypeSpecifierAwareExtension; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Reflection\FunctionReflection; -use PHPStan\Type\ClassStringType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\FunctionTypeSpecifyingExtension; use PHPStan\Type\Generic\GenericClassStringType; -use PHPStan\Type\ObjectWithoutClassType; -use PHPStan\Type\TypeCombinator; use function count; use function strtolower; @@ -51,14 +48,10 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n return new SpecifiedTypes([], []); } - $superType = $allowString - ? TypeCombinator::union(new ObjectWithoutClassType(), new ClassStringType()) - : new ObjectWithoutClassType(); - $resultType = $this->isAFunctionTypeSpecifyingHelper->determineType($objectOrClassType, $classType, $allowString, false); // prevent false-positives in IsAFunctionTypeSpecifyingHelper - if ($resultType->equals($superType) && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { + if ($classType->getConstantStrings() === [] && $resultType->isSuperTypeOf($objectOrClassType)->yes()) { return new SpecifiedTypes([], []); } diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 896173a6ee..3236b7feee 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1127,4 +1127,16 @@ public function testBug8954(): void $this->analyse([__DIR__ . '/data/bug-8954.php'], []); } + public function testBugPR3404(): void + { + $this->checkAlwaysTrueCheckTypeFunctionCall = true; + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-pr-3404.php'], [ + [ + 'Call to function is_a() with arguments BugPR3404\Location, \'BugPR3404\\\\Location\' and true will always evaluate to true.', + 21, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-pr-3404.php b/tests/PHPStan/Rules/Comparison/data/bug-pr-3404.php new file mode 100644 index 0000000000..7dd533ff98 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-pr-3404.php @@ -0,0 +1,24 @@ += 8.0 + +namespace BugPR3404; + +interface Location +{ + +} + +/** @return class-string */ +function aaa(): string +{ + +} + +function (Location $l): void { + if (is_a($l, aaa(), true)) { + // might not always be true. $l might be one subtype of Location, aaa() might return a name of a different subtype of Location + } + + if (is_a($l, Location::class, true)) { + // always true + } +};