Skip to content

Commit f5e4a5b

Browse files
staabmstaabm
authored
Fix false positives in ShouldCallParentMethodsRule (#355)
* Fix false positives in ShouldCallParentMethodsRule * address review feedback
1 parent 4041c82 commit f5e4a5b

4 files changed

Lines changed: 175 additions & 22 deletions

File tree

‎src/Rules/PHPUnit/ShouldCallParentMethodsRule.php‎

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
namespace PHPStan\Rules\PHPUnit;
44

55
use PhpParser\Node;
6+
use PhpParser\NodeTraverser;
7+
use PhpParser\NodeVisitor;
8+
use PhpParser\NodeVisitorAbstract;
69
use PHPStan\Analyser\Scope;
710
use PHPStan\Node\InClassMethodNode;
811
use PHPStan\Rules\Rule;
@@ -66,43 +69,52 @@ public function processNode(Node $node, Scope $scope): array
6669

6770
/**
6871
* @param Node\Stmt[]|null $stmts
69-
*
7072
*/
7173
private function hasParentClassCall(?array $stmts, string $methodName): bool
7274
{
7375
if ($stmts === null) {
7476
return false;
7577
}
7678

77-
foreach ($stmts as $stmt) {
78-
if (! $stmt instanceof Node\Stmt\Expression) {
79-
continue;
80-
}
81-
82-
if (! $stmt->expr instanceof Node\Expr\StaticCall) {
83-
continue;
84-
}
79+
$visitor = new class ($methodName) extends NodeVisitorAbstract {
8580

86-
if (! $stmt->expr->class instanceof Node\Name) {
87-
continue;
88-
}
81+
private string $methodName;
8982

90-
$class = (string) $stmt->expr->class;
83+
public bool $found = false;
9184

92-
if (strtolower($class) !== 'parent') {
93-
continue;
85+
public function __construct(string $methodName)
86+
{
87+
$this->methodName = $methodName;
9488
}
9589

96-
if (! $stmt->expr->name instanceof Node\Identifier) {
97-
continue;
90+
public function enterNode(Node $node): ?int
91+
{
92+
if ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike) {
93+
return NodeVisitor::DONT_TRAVERSE_CHILDREN;
94+
}
95+
96+
if (
97+
$node instanceof Node\Expr\StaticCall
98+
&& !$node->isFirstClassCallable()
99+
&& $node->class instanceof Node\Name
100+
&& $node->class->toLowerString() === 'parent'
101+
&& $node->name instanceof Node\Identifier
102+
&& $node->name->toLowerString() === $this->methodName
103+
) {
104+
$this->found = true;
105+
106+
return NodeVisitor::STOP_TRAVERSAL;
107+
}
108+
109+
return null;
98110
}
99111

100-
if ($stmt->expr->name->toLowerString() === $methodName) {
101-
return true;
102-
}
103-
}
112+
};
113+
114+
$traverser = new NodeTraverser($visitor);
115+
$traverser->traverse($stmts);
104116

105-
return false;
117+
return $visitor->found;
106118
}
107119

108120
}

‎tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
78

89
/**
910
* @extends RuleTestCase<ShouldCallParentMethodsRule>
@@ -31,6 +32,36 @@ public function testRule(): void
3132
'Missing call to parent::tearDown() method.',
3233
63,
3334
],
35+
[
36+
'Missing call to parent::setUp() method.',
37+
140,
38+
],
39+
[
40+
'Missing call to parent::tearDown() method.',
41+
147,
42+
],
43+
[
44+
'Missing call to parent::setUp() method.',
45+
155,
46+
],
47+
]);
48+
}
49+
50+
public function testFirstClassCallable(): void
51+
{
52+
if (PHP_VERSION_ID < 80100) {
53+
self::markTestSkipped('First-class callable syntax requires PHP 8.1.');
54+
}
55+
56+
$this->analyse([__DIR__ . '/data/missing-parent-method-calls-first-class-callable.php'], [
57+
[
58+
'Missing call to parent::setUp() method.',
59+
20,
60+
],
61+
[
62+
'Missing call to parent::tearDown() method.',
63+
25,
64+
],
3465
]);
3566
}
3667

‎tests/Rules/PHPUnit/data/missing-parent-method-calls-first-class-callable.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php // lint >= 8.1
2+
3+
namespace MissingParentMethodCallsFirstClassCallable;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
class BaseTestCase extends TestCase
8+
{
9+
public function setUp(): void
10+
{
11+
}
12+
13+
public function tearDown(): void
14+
{
15+
}
16+
}
17+
18+
class FirstClassCallableTest extends BaseTestCase
19+
{
20+
public function setUp(): void
21+
{
22+
$callback = parent::setUp(...);
23+
}
24+
25+
public function tearDown(): void
26+
{
27+
$callback = parent::tearDown(...);
28+
}
29+
}
30+
31+
class FirstClassCallableInvokedTest extends BaseTestCase
32+
{
33+
public function setUp(): void
34+
{
35+
$callback = parent::setUp(...);
36+
parent::setUp();
37+
}
38+
}

‎tests/Rules/PHPUnit/data/missing-parent-method-calls.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,75 @@ protected function setUp(): void
9090
}
9191

9292
}
93+
94+
class ParentCallInIfTest extends BaseTestCase
95+
{
96+
private bool $enabled = true;
97+
98+
public function setUp(): void
99+
{
100+
if ($this->enabled) {
101+
parent::setUp();
102+
}
103+
}
104+
105+
public function tearDown(): void
106+
{
107+
if ($this->enabled) {
108+
parent::tearDown();
109+
} else {
110+
parent::tearDown();
111+
}
112+
}
113+
}
114+
115+
class ParentCallInTryFinallyTest extends BaseTestCase
116+
{
117+
private ?object $resource = null;
118+
119+
public function setUp(): void
120+
{
121+
try {
122+
parent::setUp();
123+
} catch (\Throwable $e) {
124+
throw $e;
125+
}
126+
}
127+
128+
public function tearDown(): void
129+
{
130+
try {
131+
$this->resource = null;
132+
} finally {
133+
parent::tearDown();
134+
}
135+
}
136+
}
137+
138+
class ParentCallInClosureTest extends BaseTestCase
139+
{
140+
public function setUp(): void
141+
{
142+
$fn = function (): void {
143+
parent::setUp();
144+
};
145+
}
146+
147+
public function tearDown(): void
148+
{
149+
$fn = fn () => parent::tearDown();
150+
}
151+
}
152+
153+
class ParentCallInAnonymousClassTest extends BaseTestCase
154+
{
155+
public function setUp(): void
156+
{
157+
new class extends BaseTestCase {
158+
public function setUp(): void
159+
{
160+
parent::setUp();
161+
}
162+
};
163+
}
164+
}

0 commit comments

Comments
 (0)