SONARJAVA-6944: Implement rule S9387 TestNG Javadoc tags should be converted to annotations

[romainbrenguier]

Summary

• Implement new rule S9387 that detects TestNG-specific Javadoc tags (@test, @beforeMethod, @afterMethod, @beforeClass, @afterClass, @dataProvider, etc.) used as configuration markers instead of proper TestNG annotations
• Case-insensitive matching for all 15 TestNG tags with secondary locations when multiple tags appear on a single method
• Includes test sample with compliant and noncompliant examples covering edge cases (case variants, mixed standard/TestNG tags, non-Javadoc comments)

Test plan

• Unit tests pass (with and without semantic analysis)
• CI build passes
• Ruling tests updated if needed (auto-PR)

🤖 Generated with Claude Code

Agent workflow

Tool link: Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
Addressed review comments in pr_report_6124.md using uv run address_reviews.py pr_report_6124.md

[hashicorp-vault-sonar-prod]

SONARJAVA-6944

[nathsou]

The rule is well structured. I left four non-blocking coverage and false-positive concerns.

[nathsou]

This is a TestNG test-configuration rule, but All also runs it on production methods where tags such as @factory may be ordinary documentation. Sibling checks under checks/tests use the Tests scope. Could we switch this to Tests and regenerate the derived metadata/profile files?

[nathsou]

Lowercasing makes annotation names indistinguishable from the legacy case-sensitive tags. For example, documentation containing @Test is the annotation used by TestNG is treated as obsolete @test. Could we match the documented legacy spellings exactly (@test, @beforeMethod, etc.) and keep uppercase annotation names compliant?

[nathsou]

This pattern requires a space or tab after {@code, so a valid multiline block such as `{@code

• @test
• void example() {}
• }is not masked; the inner@Test` can then be reported as a block tag. Could we support line breaks here (or use structured Javadoc parsing) and add a multiline regression test?

[nathsou]

Java 23 Markdown documentation comments are represented as SyntaxTrivia.CommentKind.MARKDOWN, so /// @test is currently skipped even though it is still a documentation-based declaration. If the rule is intended to cover all Java documentation comments, could we inspect MARKDOWN trivia too, as the existing documentation checks do?

✅ All code review findings resolved.

_{Configure merge blocking}

[gitar-bot]

Code Review ✅ Approved 10 closed / 10 findings

Implements rule S9387 to detect TestNG-specific Javadoc tags (@test, @beforeMethod, @afterMethod, etc.) that should be converted to annotations, with case-insensitive matching and secondary locations for multiple tags. All previously identified issues have been resolved, including fixes for single-line Javadoc detection, annotation example filtering, location spanning, scope configuration, and test placement.

✅ 10 closed

✅ Bug: Single-line Javadoc /* @test / is never detected

📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:36 📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:63-70 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9387.html:19 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9387.html:36-40 📄 java-checks-test-sources/default/src/main/java/checks/tests/TestNGJavadocTagsCheckSample.java:5-9
BLOCK_TAG_PATTERN anchors every match to ^ (MULTILINE), so a tag is only found when it starts a line, optionally after a leading *. For the single-line form /** @test */ the only ^ position is offset 0, where the next character is /, so the match fails and no issue is raised — yet that is exactly the form the rule description presents as noncompliant (Javadoc-style tags such as /** @test */, and the whole "How to fix it" list). Allow the tag to follow the /** opener, e.g. Pattern.compile("(?:^|/\*\*)\s*\*?\s*@(\w+)", Pattern.MULTILINE), and add a /** @test */ case to the sample file. (The same filter also ignores CommentKind.MARKDOWN, so /// @test doc comments are silently skipped.)

✅ Edge Case: Annotation examples in <pre>/{@code} Javadoc blocks are flagged

📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:36 📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:76-90 📄 java-checks-test-sources/default/src/main/java/checks/tests/TestNGJavadocTagsCheckSample.java:97-107
Matching is case-insensitive, so a Javadoc that documents usage by embedding a code sample raises a false positive: in `/**

• @test
• public void t() {}

/the line @testsatisfies^\s**?\s*@(\w+)and is reported as a legacy TestNG tag, even though it is illustrating the correct annotation. Skip regions inside

/{@code ...}` (or require the tag line to contain nothing but the tag) before reporting.

✅ Quality: Issue and secondary locations both span the whole method

📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:84-98
reportIssue(tree, ...) on a METHOD/CONSTRUCTOR tree highlights the entire declaration including the body, while every comparable check in this repo reports on ((MethodTree) tree).simpleName(); worse, each secondary location is built from the same tree, so "Also replace "@beforeMethod"..." points at the identical range as the primary message instead of at the second tag (matcher offsets relative to trivia.range().start() are available for that, see MarkdownJavadocSyntaxCheck). The if (secondaryLocations.isEmpty()) branch is also redundant since reportIssue accepts an empty list, and line 86 repeats the map lookup already held in annotation.

✅ Edge Case: Class-level tags unreachable while @listeners is method-only advice

📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:53 📄 java-checks/src/main/java/org/sonar/java/checks/tests/TestNGJavadocTagsCheck.java:57-59 📄 java-checks-test-sources/default/src/main/java/checks/tests/TestNGJavadocTagsCheckSample.java:91-95
nodesToVisit() only registers METHOD and CONSTRUCTOR, but listeners (and parameters) map to TestNG annotations that are declared on a type — org.testng.annotations.Listeners is @Target(TYPE). As a result a class Javadoc carrying @listeners/@beforeClass is never inspected, and when the tag appears on a method the rule advises adding @Listeners there, which does not compile. Either add Tree.Kind.CLASS/INTERFACE/ENUM/RECORD to the visited kinds, or drop the type-only tags from the map.

✅ Quality: Generated rule description S9387.html edited by hand

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9387.html:39-40
This commit adds two <li> entries to S9387.html, but that folder's own README states the files are generated from RSPEC and "do not attempt to manually modify files here, nor submit Pull Request (PR) modifying description or rule metatada. Any PR will be systematically rejected." The next rule-api sync will drop the @beforeClass/@afterClass bullets, leaving the shipped rule description inconsistent with the implementation, which now raises on those two tags. Update RSPEC-9387 upstream and regenerate S9387.html/S9387.json rather than editing the resource in this repo.

...and 5 more closed from earlier reviews

Review coverage

Functional validation 1 of 1 objectives covered

Rules No rules evaluated

Auto-approval Not enabled · Set up

Implementation Status ✅ 1 of 1 objectives covered

✅ SONARJAVA-6944 - 1 of 1 objectives covered

This PR implements the rule S9387 to detect TestNG Javadoc tags and recommend converting them to annotations.

✅ 1 covered here

• ✅ Implement rule S9387 TestNG Javadoc tags should be converted to annotations

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube