From 2b7168e5199d04dc5d107d745b2e94626d826f68 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:59:29 +0000 Subject: [PATCH] Do not remember an equality assertion statement as `true` when its arguments contain an impure call - ExpressionHandler stored the result of a void `@phpstan-assert =T` call statement as `true`, so that a duplicate assertion is reported as always true. It did this with assignExpression(), which skips the purity check that TypeSpecifier::create() applies. A repeated `assertSame($key, $counter->next())` or `assertSame($key, random_int(1, 10))` was then wrongly reported as alreadyNarrowedType. - Add TypeSpecifier::callOperandsContainNonPureCall(). It reuses the existing non-pure-call search on the call's arguments and on what the call is made on, without the call itself. The void assertion call is usually impure itself. - Only store `true` for the statement when that check passes. Function, method and static method assertions are all covered. Repeated assertions with pure arguments are still reported. - Probed the bool-returning `@phpstan-assert-if-true =T` variant in a condition (filterByTruthyValue path). It is already correct because it goes through TypeSpecifier::create(), which rejects impure expressions. --- .../StmtHandler/ExpressionHandler.php | 6 +- src/Analyser/TypeSpecifier.php | 16 +++ ...mpossibleCheckTypeFunctionCallRuleTest.php | 11 ++ .../ImpossibleCheckTypeMethodCallRuleTest.php | 11 ++ ...sibleCheckTypeStaticMethodCallRuleTest.php | 11 ++ .../Rules/Comparison/data/bug-15328.php | 112 ++++++++++++++++++ 6 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-15328.php diff --git a/src/Analyser/StmtHandler/ExpressionHandler.php b/src/Analyser/StmtHandler/ExpressionHandler.php index cff4422bd71..0384f7b999e 100644 --- a/src/Analyser/StmtHandler/ExpressionHandler.php +++ b/src/Analyser/StmtHandler/ExpressionHandler.php @@ -95,7 +95,11 @@ public function processStmt( ); $scope = $scope->applySpecifiedTypes($specifiedTypes); - if ($specifiedTypes->isEquality()) { + if ( + $specifiedTypes->isEquality() + && $stmt->expr instanceof Expr\CallLike + && !$this->typeSpecifier->callOperandsContainNonPureCall($stmt->expr, $scope) + ) { // Statement counterpart of the equality handling in filterByTruthyValue(): // store the call's true result so a duplicate void assertion statement is // reported as always-true. We assign directly because void calls have no diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b6ff5c797bd..a48f4c023f5 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -653,6 +653,22 @@ private function findNonPureCall(Node $node, Scope $scope, bool &$containsCall): } } + return $this->findNonPureCallInSubNodes($node, $scope, $containsCall); + } + + /** + * Whether the arguments of the call, or what it is called on, contain a call + * that isn't known to be pure. The purity of the call itself is not considered. + */ + public function callOperandsContainNonPureCall(Expr\CallLike $call, Scope $scope): bool + { + $containsCall = false; + + return $this->findNonPureCallInSubNodes($call, $scope, $containsCall); + } + + private function findNonPureCallInSubNodes(Node $node, Scope $scope, bool &$containsCall): bool + { foreach ($node->getSubNodeNames() as $subNodeName) { $subNode = $node->$subNodeName; if ($subNode instanceof Node) { diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index d14a0d2093c..a7f5ba3ed86 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -603,6 +603,17 @@ public function testBug14705Php8(): void ]); } + public function testBug15328(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-15328.php'], [ + [ + 'Call to function assertSame() with int and int will always evaluate to true.', + 83, + ], + ]); + } + public function testBug2755(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php index 270b49d18db..d238c398736 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php @@ -325,6 +325,17 @@ public function testBug14705(): void ]); } + public function testBug15328(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-15328.php'], [ + [ + 'Call to method Bug15328\Assert::assertSameMethod() with int and int will always evaluate to true.', + 111, + ], + ]); + } + public function testInTrait(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php index 1a479ec9784..a1671cfad5a 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php @@ -174,6 +174,17 @@ public function testBug13566(): void $this->analyse([__DIR__ . '/data/bug-13566.php'], []); } + public function testBug15328(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-15328.php'], [ + [ + 'Call to static method Bug15328\Assert::assertSameStatic() with int and int will always evaluate to true.', + 97, + ], + ]); + } + public function testInTrait(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/data/bug-15328.php b/tests/PHPStan/Rules/Comparison/data/bug-15328.php new file mode 100644 index 00000000000..d2ef218b14b --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-15328.php @@ -0,0 +1,112 @@ +count; + } +} + +final class Assert +{ + + /** + * @template ExpectedType + * + * @param ExpectedType $expected + * @param mixed $actual + * + * @phpstan-assert =ExpectedType $actual + */ + public static function assertSameStatic($expected, $actual): void + { + } + + /** + * @template ExpectedType + * + * @param ExpectedType $expected + * @param mixed $actual + * + * @phpstan-assert =ExpectedType $actual + */ + public function assertSameMethod($expected, $actual): void + { + } + +} + +function impureMethod(int $key): void +{ + $counter = new Counter; + + assertSame($key, $counter->next()); + assertSame($key, $counter->next()); +} + +function impureFunction(int $key): void +{ + assertSame($key, random_int(1, 10)); + assertSame($key, random_int(1, 10)); +} + +function impureExpected(int $key): void +{ + assertSame(random_int(1, 10), $key); + assertSame(random_int(1, 10), $key); +} + +function pure(int $key, int $value): void +{ + assertSame($key, $value); + assertSame($key, $value); +} + +function staticMethod(int $key, Counter $counter): void +{ + Assert::assertSameStatic($key, $counter->next()); + Assert::assertSameStatic($key, $counter->next()); + Assert::assertSameStatic($key, random_int(1, 10)); + Assert::assertSameStatic($key, random_int(1, 10)); +} + +function staticMethodPure(int $key, int $value): void +{ + Assert::assertSameStatic($key, $value); + Assert::assertSameStatic($key, $value); +} + +function method(int $key, Counter $counter, Assert $assert): void +{ + $assert->assertSameMethod($key, $counter->next()); + $assert->assertSameMethod($key, $counter->next()); + $assert->assertSameMethod($key, random_int(1, 10)); + $assert->assertSameMethod($key, random_int(1, 10)); +} + +function methodPure(int $key, int $value, Assert $assert): void +{ + $assert->assertSameMethod($key, $value); + $assert->assertSameMethod($key, $value); +}