From a5e591630de05a49bfd540ca309374b52affbd42 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Nov 2026 11:12:17 +0100 Subject: [PATCH 1/2] Fix false positives in ShouldCallParentMethodsRule --- .../PHPUnit/ShouldCallParentMethodsRule.php | 55 ++++++++------ .../ShouldCallParentMethodsRuleTest.php | 12 ++++ .../data/missing-parent-method-calls.php | 72 +++++++++++++++++++ 3 files changed, 117 insertions(+), 22 deletions(-) diff --git a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php index bfd31690..c72aedd5 100644 --- a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php +++ b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php @@ -3,6 +3,9 @@ namespace PHPStan\Rules\PHPUnit; use PhpParser\Node; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor; +use PhpParser\NodeVisitorAbstract; use PHPStan\Analyser\Scope; use PHPStan\Node\InClassMethodNode; use PHPStan\Rules\Rule; @@ -66,7 +69,6 @@ public function processNode(Node $node, Scope $scope): array /** * @param Node\Stmt[]|null $stmts - * */ private function hasParentClassCall(?array $stmts, string $methodName): bool { @@ -74,35 +76,44 @@ private function hasParentClassCall(?array $stmts, string $methodName): bool return false; } - foreach ($stmts as $stmt) { - if (! $stmt instanceof Node\Stmt\Expression) { - continue; - } - - if (! $stmt->expr instanceof Node\Expr\StaticCall) { - continue; - } + $visitor = new class ($methodName) extends NodeVisitorAbstract { - if (! $stmt->expr->class instanceof Node\Name) { - continue; - } + private string $methodName; - $class = (string) $stmt->expr->class; + public bool $found = false; - if (strtolower($class) !== 'parent') { - continue; + public function __construct(string $methodName) + { + $this->methodName = $methodName; } - if (! $stmt->expr->name instanceof Node\Identifier) { - continue; + public function enterNode(Node $node): ?int + { + if ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike) { + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + + if ( + $node instanceof Node\Expr\StaticCall + && $node->class instanceof Node\Name + && $node->class->toLowerString() === 'parent' + && $node->name instanceof Node\Identifier + && $node->name->toLowerString() === $this->methodName + ) { + $this->found = true; + + return NodeVisitor::STOP_TRAVERSAL; + } + + return null; } - if ($stmt->expr->name->toLowerString() === $methodName) { - return true; - } - } + }; + + $traverser = new NodeTraverser($visitor); + $traverser->traverse($stmts); - return false; + return $visitor->found; } } diff --git a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php index b378c67f..8ca1974d 100644 --- a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php +++ b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php @@ -31,6 +31,18 @@ public function testRule(): void 'Missing call to parent::tearDown() method.', 63, ], + [ + 'Missing call to parent::setUp() method.', + 140, + ], + [ + 'Missing call to parent::tearDown() method.', + 147, + ], + [ + 'Missing call to parent::setUp() method.', + 155, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/missing-parent-method-calls.php b/tests/Rules/PHPUnit/data/missing-parent-method-calls.php index 9aa2525e..12d9ce06 100644 --- a/tests/Rules/PHPUnit/data/missing-parent-method-calls.php +++ b/tests/Rules/PHPUnit/data/missing-parent-method-calls.php @@ -90,3 +90,75 @@ protected function setUp(): void } } + +class ParentCallInIfTest extends BaseTestCase +{ + private bool $enabled = true; + + public function setUp(): void + { + if ($this->enabled) { + parent::setUp(); + } + } + + public function tearDown(): void + { + if ($this->enabled) { + parent::tearDown(); + } else { + parent::tearDown(); + } + } +} + +class ParentCallInTryFinallyTest extends BaseTestCase +{ + private ?object $resource = null; + + public function setUp(): void + { + try { + parent::setUp(); + } catch (\Throwable $e) { + throw $e; + } + } + + public function tearDown(): void + { + try { + $this->resource = null; + } finally { + parent::tearDown(); + } + } +} + +class ParentCallInClosureTest extends BaseTestCase +{ + public function setUp(): void + { + $fn = function (): void { + parent::setUp(); + }; + } + + public function tearDown(): void + { + $fn = fn () => parent::tearDown(); + } +} + +class ParentCallInAnonymousClassTest extends BaseTestCase +{ + public function setUp(): void + { + new class extends BaseTestCase { + public function setUp(): void + { + parent::setUp(); + } + }; + } +} From 3ee4b0a4c794fc968fb55a7f322b270b3a15b550 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Nov 2026 11:24:15 +0100 Subject: [PATCH 2/2] address review feedback --- .../PHPUnit/ShouldCallParentMethodsRule.php | 1 + .../ShouldCallParentMethodsRuleTest.php | 19 ++++++++++ ...rent-method-calls-first-class-callable.php | 38 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 tests/Rules/PHPUnit/data/missing-parent-method-calls-first-class-callable.php diff --git a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php index c72aedd5..f272cc54 100644 --- a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php +++ b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php @@ -95,6 +95,7 @@ public function enterNode(Node $node): ?int if ( $node instanceof Node\Expr\StaticCall + && !$node->isFirstClassCallable() && $node->class instanceof Node\Name && $node->class->toLowerString() === 'parent' && $node->name instanceof Node\Identifier diff --git a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php index 8ca1974d..76705d15 100644 --- a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php +++ b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -46,6 +47,24 @@ public function testRule(): void ]); } + public function testFirstClassCallable(): void + { + if (PHP_VERSION_ID < 80100) { + self::markTestSkipped('First-class callable syntax requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/missing-parent-method-calls-first-class-callable.php'], [ + [ + 'Missing call to parent::setUp() method.', + 20, + ], + [ + 'Missing call to parent::tearDown() method.', + 25, + ], + ]); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/missing-parent-method-calls-first-class-callable.php b/tests/Rules/PHPUnit/data/missing-parent-method-calls-first-class-callable.php new file mode 100644 index 00000000..1e598036 --- /dev/null +++ b/tests/Rules/PHPUnit/data/missing-parent-method-calls-first-class-callable.php @@ -0,0 +1,38 @@ += 8.1 + +namespace MissingParentMethodCallsFirstClassCallable; + +use PHPUnit\Framework\TestCase; + +class BaseTestCase extends TestCase +{ + public function setUp(): void + { + } + + public function tearDown(): void + { + } +} + +class FirstClassCallableTest extends BaseTestCase +{ + public function setUp(): void + { + $callback = parent::setUp(...); + } + + public function tearDown(): void + { + $callback = parent::tearDown(...); + } +} + +class FirstClassCallableInvokedTest extends BaseTestCase +{ + public function setUp(): void + { + $callback = parent::setUp(...); + parent::setUp(); + } +}