From 6af5882398dd217a025a5df83a6ddd86547a77d9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 3 Dec 2022 17:00:14 +0100 Subject: [PATCH 1/3] Fix comparison with get_class() in traits always evaluate to true --- .../GetClassDynamicReturnTypeExtension.php | 6 ++++ ...rictComparisonOfDifferentTypesRuleTest.php | 6 ++++ .../Rules/Comparison/data/bug-3633.php | 30 +++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-3633.php diff --git a/src/Type/Php/GetClassDynamicReturnTypeExtension.php b/src/Type/Php/GetClassDynamicReturnTypeExtension.php index 863871f75b..05b9f3e6e4 100644 --- a/src/Type/Php/GetClassDynamicReturnTypeExtension.php +++ b/src/Type/Php/GetClassDynamicReturnTypeExtension.php @@ -17,6 +17,7 @@ use PHPStan\Type\ObjectType; use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\StaticType; +use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; use PHPStan\Type\TypeWithClassName; @@ -34,6 +35,7 @@ public function isFunctionSupported(FunctionReflection $functionReflection): boo public function getTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope): Type { $args = $functionCall->getArgs(); + if (count($args) === 0) { if ($scope->isInClass()) { return new ConstantStringType($scope->getClassReflection()->getName(), true); @@ -42,6 +44,10 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, return new ConstantBooleanType(false); } + if ($scope->isInTrait()) { + return new StringType(); + } + $argType = $scope->getType($args[0]->value); return TypeTraverser::map( diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 4f8b9cbfc2..284baa7605 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -619,4 +619,10 @@ public function testBug8586(): void $this->analyse([__DIR__ . '/data/bug-8586.php'], []); } + public function testBug3633(): void + { + $this->checkAlwaysTrueStrictComparison = true; + $this->analyse([__DIR__ . '/data/bug-3633.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-3633.php b/tests/PHPStan/Rules/Comparison/data/bug-3633.php new file mode 100644 index 0000000000..8477f6d403 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-3633.php @@ -0,0 +1,30 @@ +test(); + } +} + +class OtherClass { + use Foo; + + public function bar(): void { + $this->test(); + } +} From 5acb4416a19b9bbe06b083e9bf2f9a4dac961b7b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 4 Dec 2022 15:50:35 +0100 Subject: [PATCH 2/3] add more non-trait tests --- ...rictComparisonOfDifferentTypesRuleTest.php | 23 +++++++++++- .../Rules/Comparison/data/bug-3633.php | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 284baa7605..b79534b21e 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -622,7 +622,28 @@ public function testBug8586(): void public function testBug3633(): void { $this->checkAlwaysTrueStrictComparison = true; - $this->analyse([__DIR__ . '/data/bug-3633.php'], []); + $this->analyse([__DIR__ . '/data/bug-3633.php'], [ + [ + 'Strict comparison using === between class-string and \'Bug3633\\\OtherClass\' will always evaluate to false.', + 23, + ], + [ + 'Strict comparison using === between class-string and \'Bug3633\\\HelloWorld\' will always evaluate to false.', + 35, + ], + [ + 'Strict comparison using === between class-string and \'Bug3633\\\HelloWorld\' will always evaluate to false.', + 50, + ], + [ + 'Strict comparison using === between class-string and \'Bug3633\\\OtherClass\' will always evaluate to false.', + 53, + ], + [ + 'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to true.', + 59, + ], + ]); } } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-3633.php b/tests/PHPStan/Rules/Comparison/data/bug-3633.php index 8477f6d403..f4f2ad637f 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-3633.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-3633.php @@ -17,6 +17,13 @@ class HelloWorld { use Foo; public function bar(): void { + if (get_class($this) === HelloWorld::class) { + echo "OK"; + } + if (get_class($this) === OtherClass::class) { + echo "OK"; + } + $this->test(); } } @@ -25,6 +32,34 @@ class OtherClass { use Foo; public function bar(): void { + if (get_class($this) === HelloWorld::class) { + echo "OK"; + } + if (get_class($this) === OtherClass::class) { + echo "OK"; + } + + $this->test(); + } +} + +final class FinalClass { + use Foo; + + public function bar(): void { + if (get_class($this) === HelloWorld::class) { + echo "OK"; + } + if (get_class($this) === OtherClass::class) { + echo "OK"; + } + if (get_class($this) !== FinalClass::class) { + echo "OK"; + } + if (get_class($this) === FinalClass::class) { + echo "OK"; + } + $this->test(); } } From 108df96c21bb8ab9207f83f71e16100bde08d569 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 8 Jan 2023 20:49:45 +0100 Subject: [PATCH 3/3] more tests --- .../GetClassDynamicReturnTypeExtension.php | 14 ++-- ...rictComparisonOfDifferentTypesRuleTest.php | 42 +++++++++-- .../Rules/Comparison/data/bug-3633.php | 71 +++++++++++++++++-- 3 files changed, 113 insertions(+), 14 deletions(-) diff --git a/src/Type/Php/GetClassDynamicReturnTypeExtension.php b/src/Type/Php/GetClassDynamicReturnTypeExtension.php index 05b9f3e6e4..cbbc45f049 100644 --- a/src/Type/Php/GetClassDynamicReturnTypeExtension.php +++ b/src/Type/Php/GetClassDynamicReturnTypeExtension.php @@ -17,7 +17,7 @@ use PHPStan\Type\ObjectType; use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\StaticType; -use PHPStan\Type\StringType; +use PHPStan\Type\ThisType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; use PHPStan\Type\TypeWithClassName; @@ -37,6 +37,10 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, $args = $functionCall->getArgs(); if (count($args) === 0) { + if ($scope->isInTrait()) { + return new ClassStringType(); + } + if ($scope->isInClass()) { return new ConstantStringType($scope->getClassReflection()->getName(), true); } @@ -44,12 +48,12 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, return new ConstantBooleanType(false); } - if ($scope->isInTrait()) { - return new StringType(); - } - $argType = $scope->getType($args[0]->value); + if ($scope->isInTrait() && $argType instanceof ThisType) { + return new ClassStringType(); + } + return TypeTraverser::map( $argType, static function (Type $type, callable $traverse): Type { diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index b79534b21e..6683505ed9 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -625,23 +625,55 @@ public function testBug3633(): void $this->analyse([__DIR__ . '/data/bug-3633.php'], [ [ 'Strict comparison using === between class-string and \'Bug3633\\\OtherClass\' will always evaluate to false.', - 23, + 37, + ], + [ + 'Strict comparison using === between \'Bug3633\\\HelloWorld\' and \'Bug3633\\\HelloWorld\' will always evaluate to true.', + 41, + ], + [ + 'Strict comparison using === between \'Bug3633\\\HelloWorld\' and \'Bug3633\\\OtherClass\' will always evaluate to false.', + 44, ], [ 'Strict comparison using === between class-string and \'Bug3633\\\HelloWorld\' will always evaluate to false.', - 35, + 64, + ], + [ + 'Strict comparison using === between \'Bug3633\\\OtherClass\' and \'Bug3633\\\HelloWorld\' will always evaluate to false.', + 71, + ], + [ + 'Strict comparison using === between \'Bug3633\\\OtherClass\' and \'Bug3633\\\OtherClass\' will always evaluate to true.', + 74, ], [ 'Strict comparison using === between class-string and \'Bug3633\\\HelloWorld\' will always evaluate to false.', - 50, + 93, ], [ 'Strict comparison using === between class-string and \'Bug3633\\\OtherClass\' will always evaluate to false.', - 53, + 96, + ], + [ + 'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to true.', + 102, + ], + [ + 'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\HelloWorld\' will always evaluate to false.', + 106, + ], + [ + 'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\OtherClass\' will always evaluate to false.', + 109, + ], + [ + 'Strict comparison using !== between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to false.', + 112, ], [ 'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to true.', - 59, + 115, ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-3633.php b/tests/PHPStan/Rules/Comparison/data/bug-3633.php index f4f2ad637f..95aedbcf12 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-3633.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-3633.php @@ -3,20 +3,34 @@ namespace Bug3633; trait Foo { - public function test(): void { + public function test($obj): void { if (get_class($this) === HelloWorld::class) { echo "OK"; } if (get_class($this) === OtherClass::class) { echo "OK"; } + + if (get_class() === HelloWorld::class) { + echo "OK"; + } + if (get_class() === OtherClass::class) { + echo "OK"; + } + + if (get_class($obj) === HelloWorld::class) { + echo "OK"; + } + if (get_class($obj) === OtherClass::class) { + echo "OK"; + } } } class HelloWorld { use Foo; - public function bar(): void { + public function bar($obj): void { if (get_class($this) === HelloWorld::class) { echo "OK"; } @@ -24,6 +38,21 @@ public function bar(): void { echo "OK"; } + if (get_class() === HelloWorld::class) { + echo "OK"; + } + if (get_class() === OtherClass::class) { + echo "OK"; + } + + if (get_class($obj) === HelloWorld::class) { + echo "OK"; + } + if (get_class($obj) === OtherClass::class) { + echo "OK"; + } + + $this->test(); } } @@ -31,7 +60,7 @@ public function bar(): void { class OtherClass { use Foo; - public function bar(): void { + public function bar($obj): void { if (get_class($this) === HelloWorld::class) { echo "OK"; } @@ -39,6 +68,20 @@ public function bar(): void { echo "OK"; } + if (get_class() === HelloWorld::class) { + echo "OK"; + } + if (get_class() === OtherClass::class) { + echo "OK"; + } + + if (get_class($obj) === HelloWorld::class) { + echo "OK"; + } + if (get_class($obj) === OtherClass::class) { + echo "OK"; + } + $this->test(); } } @@ -46,7 +89,7 @@ public function bar(): void { final class FinalClass { use Foo; - public function bar(): void { + public function bar($obj): void { if (get_class($this) === HelloWorld::class) { echo "OK"; } @@ -60,6 +103,26 @@ public function bar(): void { echo "OK"; } + if (get_class() === HelloWorld::class) { + echo "OK"; + } + if (get_class() === OtherClass::class) { + echo "OK"; + } + if (get_class() !== FinalClass::class) { + echo "OK"; + } + if (get_class() === FinalClass::class) { + echo "OK"; + } + + if (get_class($obj) === HelloWorld::class) { + echo "OK"; + } + if (get_class($obj) === OtherClass::class) { + echo "OK"; + } + $this->test(); } }