Skip to content

Call super from JUnit 3 setUp and tearDown overrides - #1257

Merged
Jammy-Louie merged 5 commits into
mainfrom
add-rspec-s2188-testcase-super-call
Nov 2, 2026
Merged

Jammy-Louie merged 5 commits into
mainfrom
add-rspec-s2188-testcase-super-call

Conversation

@Jammy-Louie

Copy link
Copy Markdown
Contributor

Sonar rule S2188 reports JUnit 3 setUp and tearDown overrides that do not call the inherited implementation, skipping the setup or cleanup an ancestor performs, and no recipe covered it yet. The new AddMissingSuperCallInTestCase recipe adds super.setUp() first and super.tearDown() last, so an ancestor initializes first and cleans up last.

An override whose throws is missing or narrower than the inherited one gains the inherited clause, so the call compiles. It is a ScanningRecipe because a wider clause breaks code relying on the old signature, so the override is left alone when the repository extends its class, calls it, or has it implement an interface method.

Only Java sources are changed. Also left alone: a class extending TestCase directly, whose implementations are empty, an empty override, which turns the ancestor's off on purpose, an abstract ancestor declaration, and a tearDown not ending in a simple statement, where the appended call could be unreachable. A super call inside a nested class or as Fixture.super.setUp() does not count.

The call survives MigrateJUnitTestCase, which only strips calls to TestCase's own methods, and JUnit 5 still needs it, as an overriding @BeforeEach hides the ancestor's.

Over the 71 Eclipse Foundation repositories with an LST it changed one file, glassfish-grizzly's ByteBufferStreamsTest, which skipped the thread-cache tracking its GrizzlyTestCase parent sets.

It is not in cleanup.BestPractices, since restoring an ancestor's setup can turn a passing test red.

Not covered: an unresolved parent method, callers in another repository, and a tearDown ending in try or if/else.

Sonar S2188: an override that does not call the inherited implementation
silently skips the shared setup or cleanup an ancestor performs.

`super.setUp()` is inserted first and `super.tearDown()` last. An override
declaring no `throws` gains the clause the inherited method declares, which
an override may always repeat, and without which the inserted call would not
compile. An override whose `throws` is narrower is left alone, since widening
it breaks a subclass that narrows it again and calls `super`.

The bail-out check matches `super` by name rather than with a `MethodMatcher`,
because the inserted call has no method type yet and a typed matcher would
miss it and add a second one on the next cycle.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Oct 30, 2026
@Jammy-Louie
Jammy-Louie marked this pull request as ready for review October 30, 2026 20:49
@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Nov 2, 2026
@Jammy-Louie
Jammy-Louie merged commit cf7be61 into main Nov 2, 2026
1 check passed
@Jammy-Louie
Jammy-Louie deleted the add-rspec-s2188-testcase-super-call branch November 2, 2026 15:36
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants