From 012fa90ecd40730bee2f1023afad314111f5b951 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 15 Jul 2024 18:13:17 +0200 Subject: [PATCH 1/6] Ignore non-explicit NEVER in purity check --- src/Analyser/NodeScopeResolver.php | 8 +++- .../PHPStan/Rules/Pure/PureMethodRuleTest.php | 22 +++++++++++ tests/PHPStan/Rules/Pure/data/bug-11207.php | 38 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Pure/data/bug-11207.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 26a056877a..70f01a2c6b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4623,7 +4623,13 @@ private function processArgs( $hasYield = $hasYield || $exprResult->hasYield(); if ($exprType->isCallable()->yes()) { - $acceptors = $exprType->getCallableParametersAcceptors($scope); + $acceptors = []; + if ( + (!$exprType instanceof NeverType || $exprType->isExplicit()) + ) { + $acceptors = $exprType->getCallableParametersAcceptors($scope); + } + if (count($acceptors) === 1) { $scope = $this->processImmediatelyCalledCallable($scope, $acceptors[0]->getInvalidateExpressions(), $acceptors[0]->getUsedVariables()); if ($callCallbackImmediately) { diff --git a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php index a3b7495691..1be2f550f8 100644 --- a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php @@ -12,13 +12,21 @@ class PureMethodRuleTest extends RuleTestCase { + private bool $treatPhpDocTypesAsCertain; + public function getRule(): Rule { return new PureMethodRule(new FunctionPurityCheck()); } + protected function shouldTreatPhpDocTypesAsCertain(): bool + { + return $this->treatPhpDocTypesAsCertain; + } + public function testRule(): void { + $this->treatPhpDocTypesAsCertain = true; $this->analyse([__DIR__ . '/data/pure-method.php'], [ [ 'Method PureMethod\Foo::doFoo() is marked as pure but parameter $p is passed by reference.', @@ -141,6 +149,7 @@ public function testPureConstructor(): void $this->markTestSkipped('Test requires PHP 8.0.'); } + $this->treatPhpDocTypesAsCertain = true; $this->analyse([__DIR__ . '/data/pure-constructor.php'], [ [ 'Impure property assignment in pure method PureConstructor\Foo::__construct().', @@ -159,6 +168,7 @@ public function testPureConstructor(): void public function testImpureAssignRef(): void { + $this->treatPhpDocTypesAsCertain = true; $this->analyse([__DIR__ . '/data/impure-assign-ref.php'], [ [ 'Possibly impure property assignment by reference in pure method ImpureAssignRef\HelloWorld::bar6().', @@ -167,4 +177,16 @@ public function testImpureAssignRef(): void ]); } + public function testBug11207a(): void + { + $this->treatPhpDocTypesAsCertain = false; + $this->analyse([__DIR__ . '/data/bug-11207.php'], []); + } + + public function testBug11207b(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-11207.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Pure/data/bug-11207.php b/tests/PHPStan/Rules/Pure/data/bug-11207.php new file mode 100644 index 0000000000..81559bef19 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/bug-11207.php @@ -0,0 +1,38 @@ + Date: Mon, 15 Jul 2024 18:40:37 +0200 Subject: [PATCH 2/6] simplify --- src/Analyser/NodeScopeResolver.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 70f01a2c6b..1b128272a4 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4622,14 +4622,11 @@ private function processArgs( $scope = $exprResult->getScope(); $hasYield = $hasYield || $exprResult->hasYield(); - if ($exprType->isCallable()->yes()) { - $acceptors = []; - if ( - (!$exprType instanceof NeverType || $exprType->isExplicit()) - ) { - $acceptors = $exprType->getCallableParametersAcceptors($scope); - } - + if ( + $exprType->isCallable()->yes() + && (!$exprType instanceof NeverType || $exprType->isExplicit()) + ) { + $acceptors = $exprType->getCallableParametersAcceptors($scope); if (count($acceptors) === 1) { $scope = $this->processImmediatelyCalledCallable($scope, $acceptors[0]->getInvalidateExpressions(), $acceptors[0]->getUsedVariables()); if ($callCallbackImmediately) { From d361fd2a978342831a148d3eb10404fc857a6172 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 15 Jul 2024 18:44:25 +0200 Subject: [PATCH 3/6] Update bug-11207.php --- tests/PHPStan/Rules/Pure/data/bug-11207.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Pure/data/bug-11207.php b/tests/PHPStan/Rules/Pure/data/bug-11207.php index 81559bef19..e69bdb5573 100644 --- a/tests/PHPStan/Rules/Pure/data/bug-11207.php +++ b/tests/PHPStan/Rules/Pure/data/bug-11207.php @@ -1,4 +1,4 @@ -= 8.0 namespace Bug11207; From 18e7e7f02aafc70fd39b0202bb99bdb9608e5025 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 15 Jul 2024 22:52:00 +0200 Subject: [PATCH 4/6] NeverType is not callable --- src/Analyser/NodeScopeResolver.php | 5 +---- src/Type/NeverType.php | 5 ++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 1b128272a4..26a056877a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4622,10 +4622,7 @@ private function processArgs( $scope = $exprResult->getScope(); $hasYield = $hasYield || $exprResult->hasYield(); - if ( - $exprType->isCallable()->yes() - && (!$exprType instanceof NeverType || $exprType->isExplicit()) - ) { + if ($exprType->isCallable()->yes()) { $acceptors = $exprType->getCallableParametersAcceptors($scope); if (count($acceptors) === 1) { $scope = $this->processImmediatelyCalledCallable($scope, $acceptors[0]->getInvalidateExpressions(), $acceptors[0]->getUsedVariables()); diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php index da6da83793..7cb90522b8 100644 --- a/src/Type/NeverType.php +++ b/src/Type/NeverType.php @@ -9,7 +9,6 @@ use PHPStan\Reflection\ConstantReflection; use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Reflection\PropertyReflection; -use PHPStan\Reflection\TrivialParametersAcceptor; use PHPStan\Reflection\Type\UnresolvedMethodPrototypeReflection; use PHPStan\Reflection\Type\UnresolvedPropertyPrototypeReflection; use PHPStan\ShouldNotHappenException; @@ -334,12 +333,12 @@ public function shuffleArray(): Type public function isCallable(): TrinaryLogic { - return TrinaryLogic::createYes(); + return TrinaryLogic::createNo(); } public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array { - return [new TrivialParametersAcceptor()]; + return []; } public function isCloneable(): TrinaryLogic From f9205d7ab7bbe25d079642804286dd7b5ed33999 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 15 Jul 2024 23:13:11 +0200 Subject: [PATCH 5/6] use dataprovider --- tests/PHPStan/Rules/Pure/PureMethodRuleTest.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php index 1be2f550f8..4ec5028172 100644 --- a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php @@ -177,16 +177,21 @@ public function testImpureAssignRef(): void ]); } - public function testBug11207a(): void + /** + * @dataProvider dataBug11207 + */ + public function testBug11207(bool $treatPhpDocTypesAsCertain): void { - $this->treatPhpDocTypesAsCertain = false; + $this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain; $this->analyse([__DIR__ . '/data/bug-11207.php'], []); } - public function testBug11207b(): void + public function dataBug11207(): array { - $this->treatPhpDocTypesAsCertain = true; - $this->analyse([__DIR__ . '/data/bug-11207.php'], []); + return [ + [true], + [false], + ]; } } From cdeaba0e70591741a722b3a72d640b7a9d988b87 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 15 Jul 2024 23:19:28 +0200 Subject: [PATCH 6/6] fix --- src/Analyser/NodeScopeResolver.php | 12 +++++++++++- src/Type/NeverType.php | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 26a056877a..226bedd4f5 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4330,7 +4330,13 @@ public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array { $callableParameters = null; if ($args !== null) { - $acceptors = $scope->getType($closureExpr)->getCallableParametersAcceptors($scope); + $closureType = $scope->getType($closureExpr); + + if ($closureType->isCallable()->no()) { + return null; + } + + $acceptors = $closureType->getCallableParametersAcceptors($scope); if (count($acceptors) === 1) { $callableParameters = $acceptors[0]->getParameters(); @@ -4356,6 +4362,10 @@ public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array $passedToType->getTypes(), static fn (Type $type) => $type->isCallable()->yes(), )); + + if ($passedToType->isCallable()->no()) { + return null; + } } $acceptors = $passedToType->getCallableParametersAcceptors($scope); diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php index 7cb90522b8..b6e378a330 100644 --- a/src/Type/NeverType.php +++ b/src/Type/NeverType.php @@ -338,7 +338,7 @@ public function isCallable(): TrinaryLogic public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array { - return []; + throw new ShouldNotHappenException(); } public function isCloneable(): TrinaryLogic