Skip to content

Commit 2e1cd29

Browse files
authored
[PHPUnit60] Fix false positive in AddDoesNotPerformAssertionToNonAssertingTestRector (#687)
The nesting-depth counter in AssertCallAnalyzer was incremented on every containsAssertCall() but never decremented, so it accumulated across the whole depth-first walk of a single test method instead of tracking real recursion depth. When a non-asserting call (e.g. a production static call) was visited before an assert-performing helper, it exhausted the level-5 budget; the later helper was then short-circuited to "no assertions" and the test wrongly got @doesNotPerformAssertions. Wrap the body in try/finally so the level is restored on unwind. Add a keep fixture with a deep non-asserting chain preceding an assert helper.
1 parent 191ed98 commit 2e1cd29

2 files changed

Lines changed: 70 additions & 18 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\PHPUnit60\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
class KeepDeepCallsBeforeAssertHelper extends TestCase
8+
{
9+
public function testSomething(): void
10+
{
11+
// a deep, non-asserting call chain visited before the assert helper;
12+
// it must not exhaust the nested-call budget and hide the assertion below
13+
$this->prepare();
14+
15+
$this->checkResult(true);
16+
}
17+
18+
private function prepare(): void
19+
{
20+
$this->stepTwo();
21+
}
22+
23+
private function stepTwo(): void
24+
{
25+
$this->stepThree();
26+
}
27+
28+
private function stepThree(): void
29+
{
30+
$this->stepFour();
31+
}
32+
33+
private function stepFour(): void
34+
{
35+
$this->stepFive();
36+
}
37+
38+
private function stepFive(): void
39+
{
40+
// no assertion here
41+
}
42+
43+
private function checkResult(bool $bool): void
44+
{
45+
self::assertTrue($bool);
46+
}
47+
}

src/NodeAnalyzer/AssertCallAnalyzer.php

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,34 @@ public function containsAssertCall(ClassMethod $classMethod): bool
6161
{
6262
++$this->classMethodNestingLevel;
6363

64-
// probably no assert method in the end
65-
if ($this->classMethodNestingLevel > self::MAX_NESTED_METHOD_CALL_LEVEL) {
66-
return false;
67-
}
64+
try {
65+
// probably no assert method in the end
66+
if ($this->classMethodNestingLevel > self::MAX_NESTED_METHOD_CALL_LEVEL) {
67+
return false;
68+
}
6869

69-
$cacheHash = md5($this->betterStandardPrinter->prettyPrint([$classMethod]));
70+
$cacheHash = md5($this->betterStandardPrinter->prettyPrint([$classMethod]));
7071

71-
if (isset($this->containsAssertCallByClassMethod[$cacheHash])) {
72-
return $this->containsAssertCallByClassMethod[$cacheHash];
73-
}
72+
if (isset($this->containsAssertCallByClassMethod[$cacheHash])) {
73+
return $this->containsAssertCallByClassMethod[$cacheHash];
74+
}
7475

75-
// A. try "->assert" shallow search first for performance
76-
$hasDirectAssertOrMockCall = $this->hasDirectAssertOrMockCall($classMethod);
77-
if ($hasDirectAssertOrMockCall) {
78-
$this->containsAssertCallByClassMethod[$cacheHash] = $hasDirectAssertOrMockCall;
79-
return true;
80-
}
76+
// A. try "->assert" shallow search first for performance
77+
$hasDirectAssertOrMockCall = $this->hasDirectAssertOrMockCall($classMethod);
78+
if ($hasDirectAssertOrMockCall) {
79+
$this->containsAssertCallByClassMethod[$cacheHash] = $hasDirectAssertOrMockCall;
80+
return true;
81+
}
8182

82-
// B. look for nested calls
83-
$hasNestedAssertOrMockCall = $this->hasNestedAssertCall($classMethod);
84-
$this->containsAssertCallByClassMethod[$cacheHash] = $hasNestedAssertOrMockCall;
83+
// B. look for nested calls
84+
$hasNestedAssertOrMockCall = $this->hasNestedAssertCall($classMethod);
85+
$this->containsAssertCallByClassMethod[$cacheHash] = $hasNestedAssertOrMockCall;
8586

86-
return $hasNestedAssertOrMockCall;
87+
return $hasNestedAssertOrMockCall;
88+
} finally {
89+
// restore depth so sibling calls in the same DFS keep their full budget
90+
--$this->classMethodNestingLevel;
91+
}
8792
}
8893

8994
public function isAssertMethodCall(MethodCall|StaticCall $call): bool

0 commit comments

Comments
 (0)