[staabm]

when adding a new setUp or tearDown method into a test-case base class, you get easily a lot of phpstan-phpunit errors in test-cases like:

Missing call to parent::setUp() method.

my idea is to add a parent::setUp() calls as a first statement into the existing setup method, and in tearDown I think we should append parent::tearDown to the very end of the existing tearDown.

[Copilot]

🟡 Changes recommended

The fixer appends an unreachable parent::tearDown() call after a throw, suppressing subsequent diagnostics without ensuring the parent method executes.

1 open issue

• Avoid appending parent teardown after an unconditional throw · New

What changed in this PR

Adds automatic fixes to ShouldCallParentMethodsRule, allowing the PHPUnit extension to insert missing parent lifecycle calls.

Changes:

• Prepends parent::setUp() and appends parent::tearDown(), placing teardown calls before trailing returns.
• Leaves trait methods and teardowns with early returns without automatic fixes.
• Adds fix fixtures covering insertion order, comments, method-name casing, and nested scopes.

File	Description
tests/​Rules/​PHPUnit/​ShouldCallParentMethodsRuleTest.php	Adds a fixture-based automatic-fix test.
tests/​Rules/​PHPUnit/​data/​missing-parent-method-calls-fixable.php.fixed	Defines expected fixed output and unchanged cases.
tests/​Rules/​PHPUnit/​data/​missing-parent-method-calls-fixable.php	Provides lifecycle-method fix scenarios.
src/​Rules/​PHPUnit/​ShouldCallParentMethodsRule.php	Implements parent-call insertion and shared method-scope traversal.

_{Review effort: Balanced}

[staabm]

//cc @SanderMuller

[Copilot]

🔵 Needs a closer look

The fix can append an unreachable parent::tearDown() after exit/die or known non-returning calls, suppressing the diagnostic without running cleanup.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Avoid appending parent tearDown after non-returning calls

src/​Rules/​PHPUnit/​ShouldCallParentMethodsRule.php:123

A tearDown() ending in exit/die or a known non-returning call such as self::fail() is marked fixable, but the appended parent::tearDown() is unreachable and suppresses the missing-parent diagnostic without running cleanup. The filter only recognizes returns and throws, although extension.neon:7–11 already registers fail, markTestSkipped, and markTestIncomplete as early-terminating. Withhold fixes for Node\Expr\Exit_ and recognized non-returning calls, and add corresponding fix fixtures.

1 resolved since last review

• Avoid appending parent teardown after an unconditional throw

_{Review effort: Balanced}

[SanderMuller]

I ran the fixer on some cases that the fixture does not have.

One gap, the same kind as the throw case: a tearDown() that ends in a call that never returns still gets the call appended. For example:

protected function tearDown(): void
{
	$this->fail('always');
	parent::tearDown(); // added by the fix
}

exit(1); at the end gives the same result. After the fix the rule finds the call and reports nothing, but parent::tearDown() never runs. exit is an Expr\Exit_ node, so it could join the throw check. A call like $this->fail() needs its never return type. MethodReturnStatementsNode has getStatementResult(), which might be an option, but I did not try it.

These are correct:

• A try/finally without a return gets the call after the finally block.
• fn () => throw new \RuntimeException() inside tearDown() does not stop the fix, because the visitor skips the arrow function.
• A setUp() with a docblock, #[\Override] and a trailing comment keeps them. The call goes in before the first statement.

A minor point: the fix splits a one-line body over two lines. protected function tearDown(): void { $this->x = null; } ends with parent::tearDown(); } on a new line. The code is valid.

testFix fails without the change in the rule and passes with it. On 6036e8e the tests (99), PHPStan and phpcs pass, and CI is green. I could not run parallel-lint locally.

Performance: I analysed 1,500 generated test classes at level 0, 6 runs each.

• Every tearDown() without the parent call: 8.23 s on 2.1.x and 9.49 s with this PR, with 54.1 s against 41.6 s user CPU. Without the fixNode() call, the PR takes 8.33 s. So the cost comes from making the error fixable, about 8 ms of CPU for each error.
• Every class with both parent calls: 8.54 s and 8.53 s, so no difference when there is no error.

[staabm]

One gap, the same kind as the throw case: a tearDown() that ends in a call that never returns still gets the call appended.

such a test is a fatal error in PHPUnit:

➜  phpunit git:(main) ✗ cat MyTest.php 
<?php

class MyTest extends \PHPUnit\Framework\TestCase  {
    public function testFoo() {

    }

    public function tearDown(): void
    {
        exit();
    }
}
➜  phpunit git:(main) ✗ php phpunit MyTest.php
PHPUnit 14.0-g9a67d9ff4e by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.12
Configuration: /Users/m.staab/dvl/phpunit/phpunit.xml

Fatal error: Premature end of PHP process when running MyTest::testFoo.

therefore I don't think its worth adding separate logic for it

A minor point: the fix splits a one-line body over two lines. protected function tearDown(): void { $this->x = null; } ends with parent::tearDown(); } on a new line. The code is valid.

I think this is a code-style thing which should php-cs-fixer or similar to should take care of after phpstan-fixing finished.

[ondrejmirtes]

What's the logic with parent setUp being called first, and parent tearDown being called last?

[staabm]

What's the logic with parent setUp being called first, and parent tearDown being called last?

the idea is that the base-class usually will do basic things and the concrete subclass will do more specific things.

therefore we setups the basis first and do the specific thing later (at setUp stage).
and on teardown we do it in reverse order. undo all the specific things first and do the more basic teardown last.