From 6bbb2a6b4c6374e54e1411f180184a1cd3e2295c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 12 May 2022 15:33:09 +0200 Subject: [PATCH 01/12] use conditional parameter for is_a() via stub --- stubs/core.stub | 7 +++++ .../Analyser/NodeScopeResolverTest.php | 1 + tests/PHPStan/Analyser/data/bug-4371.php | 30 +++++++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 tests/PHPStan/Analyser/data/bug-4371.php diff --git a/stubs/core.stub b/stubs/core.stub index 5111e994d1..04e23200a2 100644 --- a/stubs/core.stub +++ b/stubs/core.stub @@ -1,5 +1,12 @@ gatherAssertTypes(__DIR__ . '/data/pr-1244.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7144.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7144-composer-integration.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4371.php'); } /** diff --git a/tests/PHPStan/Analyser/data/bug-4371.php b/tests/PHPStan/Analyser/data/bug-4371.php new file mode 100644 index 0000000000..d566289009 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-4371.php @@ -0,0 +1,30 @@ + Date: Thu, 12 May 2022 15:36:51 +0200 Subject: [PATCH 02/12] fix missing param in stub --- stubs/core.stub | 1 + 1 file changed, 1 insertion(+) diff --git a/stubs/core.stub b/stubs/core.stub index 04e23200a2..d04a95ddd1 100644 --- a/stubs/core.stub +++ b/stubs/core.stub @@ -3,6 +3,7 @@ /** * @param mixed $object_or_class * @param class-string $class + * @param bool $allow_string * @return ($allow_string is false ? ($object_or_class is object ? bool : false) : bool) */ function is_a($object_or_class, string $class, $allow_string = false): bool{} From 49610a6eee16515c1ea3c1da97a76b97ab55aeb7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 09:21:39 +0200 Subject: [PATCH 03/12] added failling test --- ...mpossibleCheckTypeFunctionCallRuleTest.php | 15 +++++++++++++ .../Rules/Comparison/data/bug-4371.php | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-4371.php diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 1d624ad3a9..4b5d01a2b9 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -535,4 +535,19 @@ public function testNonEmptySpecifiedString(): void $this->analyse([__DIR__ . '/data/non-empty-string-impossible-type.php'], []); } + public function testBug4371(): void + { + $this->checkAlwaysTrueCheckTypeFunctionCall = true; + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-4371.php'], [ + [ + "Call to function is_a() with 'Bug4371\\\\Bar' and 'Bug4371\\\\Foo' will always evaluate to false.", + 12, + ], + [ + "Call to function is_a() with 'Bug4371\\\\Bar' and 'Bug4371\\\\Foo' will always evaluate to false.", + 18, + ], + ]); + } } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-4371.php b/tests/PHPStan/Rules/Comparison/data/bug-4371.php new file mode 100644 index 0000000000..4206c26576 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-4371.php @@ -0,0 +1,22 @@ + Date: Fri, 13 May 2022 09:24:08 +0200 Subject: [PATCH 04/12] fix --- .../Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 4b5d01a2b9..a410e11c67 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -545,9 +545,10 @@ public function testBug4371(): void 12, ], [ - "Call to function is_a() with 'Bug4371\\\\Bar' and 'Bug4371\\\\Foo' will always evaluate to false.", + "Call to function is_a() with arguments 'Bug4371\\\\Bar', 'Bug4371\\\\Foo' and false will always evaluate to false.", 18, ], ]); } + } From 60789534e9f84e04bd7a2ddcf3bb3fc26b5d4630 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 09:24:24 +0200 Subject: [PATCH 05/12] impl fix --- src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php | 4 ---- .../Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php index e60656e751..3eb735e57a 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php @@ -7,7 +7,6 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function sprintf; -use function strtolower; /** * @implements Rule @@ -35,9 +34,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/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php index 08eaf84e5b..10f72ab5aa 100644 --- a/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php +++ b/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php @@ -42,6 +42,7 @@ public function isFunctionSupported(FunctionReflection $functionReflection): boo 'in_array', 'is_numeric', 'is_int', + 'is_a', 'is_array', 'is_bool', 'is_callable', From 93b4674905178c2f8cce17dd4ea3439a7d944c61 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 09:26:56 +0200 Subject: [PATCH 06/12] fix tests --- .../ImpossibleCheckTypeFunctionCallRuleTest.php | 8 ++++++++ .../Rules/Comparison/data/check-type-function-call.php | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index a410e11c67..3ae6144efa 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -51,6 +51,10 @@ public function testImpossibleCheckTypeFunctionCall(): void 'Call to function is_int() with string will always evaluate to false.', 31, ], + [ + 'Call to function is_a() with arguments \'Foo\', \'Throwable\' and true will always evaluate to false.', + 35, + ], [ 'Call to function is_callable() with array will always evaluate to false.', 44, @@ -248,6 +252,10 @@ public function testImpossibleCheckTypeFunctionCallWithoutAlwaysTrue(): void 'Call to function is_int() with string will always evaluate to false.', 31, ], + [ + 'Call to function is_a() with arguments \'Foo\', \'Throwable\' and true will always evaluate to false.', + 35, + ], [ 'Call to function is_callable() with array will always evaluate to false.', 44, diff --git a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php index a52b8828f3..40e27fbc86 100644 --- a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php +++ b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php @@ -32,7 +32,7 @@ public function doFoo( } $className = 'Foo'; - if (is_a($className, \Throwable::class, true)) { // should be fine + if (is_a($className, \Throwable::class, true)) { // always false } if (is_array($callable)) { From b3dd3f31782f95a2ed52c3e9c8514527ff49f4db Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 10:53:32 +0200 Subject: [PATCH 07/12] added tests --- .../data/check-type-function-call.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php index 40e27fbc86..eea295a6d7 100644 --- a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php +++ b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php @@ -860,3 +860,21 @@ public function doFoo(int $i, array $is): void } } + +class NonFinalSubclassMightImplementInterface { + /** + * @param class-string $classString + */ + public function doFoo($classString, Foo $obj): void { + if (is_a($classString, \Throwable::class, true)) { // should be fine + + } + + if (is_a($obj, \Throwable::class, true)) { // should be fine + + } + if (is_a($obj, \Throwable::class)) { // should be fine + + } + } +} From e569f1e8efcbf6b39a2db16383164547490b2dd9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 10:53:42 +0200 Subject: [PATCH 08/12] missing use --- tests/PHPStan/Analyser/data/bug-4371.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/PHPStan/Analyser/data/bug-4371.php b/tests/PHPStan/Analyser/data/bug-4371.php index d566289009..62deac5ba0 100644 --- a/tests/PHPStan/Analyser/data/bug-4371.php +++ b/tests/PHPStan/Analyser/data/bug-4371.php @@ -5,6 +5,7 @@ namespace Bug4371; use function PHPStan\Testing\assertType; +use Exception; class HelloWorld { From 3460b044ea14ba1dbe36b29c5d80f9ea8a842a45 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 11:30:45 +0200 Subject: [PATCH 09/12] separate scope for test --- ...mpossibleCheckTypeFunctionCallRuleTest.php | 4 ++-- .../Rules/Comparison/data/bug-4371.php | 24 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 3ae6144efa..aedeb85d3a 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -550,11 +550,11 @@ public function testBug4371(): void $this->analyse([__DIR__ . '/data/bug-4371.php'], [ [ "Call to function is_a() with 'Bug4371\\\\Bar' and 'Bug4371\\\\Foo' will always evaluate to false.", - 12, + 14, ], [ "Call to function is_a() with arguments 'Bug4371\\\\Bar', 'Bug4371\\\\Foo' and false will always evaluate to false.", - 18, + 21, ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-4371.php b/tests/PHPStan/Rules/Comparison/data/bug-4371.php index 4206c26576..4a15f84264 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-4371.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-4371.php @@ -9,14 +9,20 @@ class Bar extends Foo { } -if(is_a(Bar::class, Foo::class)) { // should be reported - echo "This will never be true"; -} else { - echo "NO"; +class HalloWorld { + public function doFoo() { + if(is_a(Bar::class, Foo::class)) { // should be reported + echo "This will never be true"; + } else { + echo "NO"; + } + } + public function doBar() { + if(is_a(Bar::class, Foo::class, false)) { // should be reported + echo "This will never be true"; + } else { + echo "NO"; + } + } } -if(is_a(Bar::class, Foo::class, false)) { // should be reported - echo "This will never be true"; -} else { - echo "NO"; -} From d1155f31200f5cd4d118ff1c87cdb7c6c21c9e8d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 12:40:39 +0200 Subject: [PATCH 10/12] ugly hack arround newly introduced false positives, triggered by removing a early return in ImpossibleCheckTypeFunctionCallRule --- .../Php/IsAFunctionTypeSpecifyingExtension.php | 14 ++++++++++++++ stubs/core.stub | 8 -------- tests/PHPStan/Analyser/data/bug-4371.php | 6 +----- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php index 2e03074b37..1a32df0d84 100644 --- a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php @@ -9,9 +9,13 @@ 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\Generic\GenericClassStringType; +use PHPStan\Type\ObjectType; +use PHPStan\Type\StringType; use function count; use function strtolower; @@ -42,6 +46,16 @@ 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)); + // early exit in some cases, so we don't create false positives via IsAFunctionTypeSpecifyingHelper + if ($classType instanceof ClassStringType && !$classType instanceof GenericClassStringType) { + if ($objectOrClassType->isString()->yes() && $allowString) { + return new SpecifiedTypes([], []); + } + if (!$objectOrClassType->isString()->yes()) { + return new SpecifiedTypes([], []); + } + } + if (!$classType instanceof ConstantStringType && !$context->truthy()) { return new SpecifiedTypes([], []); } diff --git a/stubs/core.stub b/stubs/core.stub index d04a95ddd1..5111e994d1 100644 --- a/stubs/core.stub +++ b/stubs/core.stub @@ -1,13 +1,5 @@ Date: Fri, 13 May 2022 14:00:59 +0200 Subject: [PATCH 11/12] fix regression in TypeSpecifierTest --- src/Type/Php/IsAFunctionTypeSpecifyingExtension.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php index 1a32df0d84..17024b969d 100644 --- a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php @@ -14,6 +14,7 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FunctionTypeSpecifyingExtension; use PHPStan\Type\Generic\GenericClassStringType; +use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\StringType; use function count; @@ -51,7 +52,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n if ($objectOrClassType->isString()->yes() && $allowString) { return new SpecifiedTypes([], []); } - if (!$objectOrClassType->isString()->yes()) { + if (!$objectOrClassType->isString()->yes() && !$objectOrClassType instanceof MixedType) { return new SpecifiedTypes([], []); } } From 7c3047835054b5e21303758288020b51dca9ac50 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 May 2022 14:06:17 +0200 Subject: [PATCH 12/12] cs --- src/Type/Php/IsAFunctionTypeSpecifyingExtension.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php index 17024b969d..752e95bc6f 100644 --- a/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php +++ b/src/Type/Php/IsAFunctionTypeSpecifyingExtension.php @@ -15,8 +15,6 @@ use PHPStan\Type\FunctionTypeSpecifyingExtension; use PHPStan\Type\Generic\GenericClassStringType; use PHPStan\Type\MixedType; -use PHPStan\Type\ObjectType; -use PHPStan\Type\StringType; use function count; use function strtolower;