SONARJAVA-6942: Implemented rule S9389 - Data provider names should be unique within a class by romainbrenguier · Pull Request #6125 · SonarSource/sonar-java

romainbrenguier · 2026-09-11T15:00:57Z

This PR implements rule S9389 which detects duplicate @dataProvider names within a single TestNG test class.

Summary

When multiple methods are annotated with using the same name, TestNG cannot reliably determine which provider to use, leading to unpredictable test behavior. This rule identifies such duplicates and reports them as issues.

Implementation Details

Rule Class: extends
Scope: Checks methods within each class independently (nested classes have separate scopes)
Detection: Uses semantic model to identify annotations and extracts the attribute value
Reporting: Reports on the duplicate (second occurrence) with a secondary location pointing to the first occurrence

Test Coverage

Duplicate names within the same class (non-compliant)
Three or more methods with the same name (reports each duplicate)
Unique names (compliant)
Methods without explicit name (compliant - uses method name as default)
Same name in different classes (compliant - different scopes)
Nested classes with same names (compliant - different scopes)
No false positives when semantic is unavailable

Files Changed

- Rule implementation
- Test class
- Test sample
- Rule description
- Rule metadata

@dataProvider

…e unique within a class This rule detects duplicate @dataProvider names within a single TestNG test class. When multiple methods are annotated with @dataProvider(name = "...") using the same name, TestNG cannot reliably determine which provider to use, leading to unpredictable test behavior. The implementation follows the pattern established by TestsStabilityCheck for annotation detection and uses a map to track first occurrences of each provider name. Files added: - DataProviderNameUniquenessCheck.java: Rule implementation - DataProviderNameUniquenessCheckTest.java: Test class - DataProviderNameUniquenessCheckSample.java: Test sample with compliant/non-compliant examples

hashicorp-vault-sonar-prod · 2026-09-11T15:01:10Z

SONARJAVA-6942

@dataProvider

- Fall back to method name when @dataProvider has no explicit name attribute - Report issues on method identifier instead of whole method tree - Use LiteralUtils.trimQuotes for consistent string comparison - Remove dead branches (null check, MEMBER_SELECT) to improve coverage - Add Tree.Kind.RECORD to nodesToVisit - Add test cases for implicit/explicit name collision and records - Delete scratch design note from repo root Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Make isDataProviderAnnotation and extractNameAttribute static (S2325), add test cases for non-string-literal name attributes and non-name annotation attributes to increase coverage above 90%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@dataProvider

Use ExpressionTree.asConstant() to resolve compile-time constants in @dataProvider name attributes instead of only handling string literals. When a name attribute is present but cannot be resolved, skip the method rather than falling back to the method name. This prevents false positives and false negatives with constant references. Also makes extractDataProviderName static to fix S2325. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add ^ underlines to all Noncompliant lines in the test sample to verify exact issue positions on method identifiers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

nathsou

The implementation looks good overall. I left one non-blocking edge case to address.

nathsou · 2026-09-15T10:00:22Z

+        if (nameExpression == null) {
+          return method.simpleName().name();
+        }
+        return nameExpression.asConstant(String.class).orElse(null);


@DataProvider(name = "") needs the same fallback as an omitted name. TestNG replaces an empty name with the method name, whereas this currently returns "". This creates both a false positive for two differently named methods that each specify name = "", and a false negative when foo() uses name = "" while another provider explicitly uses name = "foo". Could we normalize an empty resolved constant to method.simpleName().name() and cover both cases?

@dataProvider

…S9389 TestNG treats @dataProvider(name = "") the same as omitting the name attribute, falling back to the method name. This change normalizes empty resolved names to the method name, preventing false positives when two methods specify name="" and false negatives when a method with name="" collides with an explicit name matching that method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gitar-bot · 2026-09-15T10:23:35Z

Code Review ✅ Approved 9 resolved / 9 findings

Implements rule S9389 to detect duplicate @DataProvider names within TestNG test classes, with comprehensive test coverage for nested classes and multiple duplicates. Multiple issues were resolved including normalizing default provider names, narrowing issue locations, handling record declarations, normalizing string literal comparisons, fixing dead code branches, and correcting static method declarations. No issues remain.

✅ 9 resolved

✅ Edge Case: Providers relying on the default (method) name are never compared

📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:67-74 📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:80-94 📄 new_rule_proposal_S9389.txt:39 📄 new_rule_proposal_S9389.txt:81-83 📄 java-checks-test-sources/default/src/test/java/checks/tests/DataProviderNameUniquenessCheckSample.java:27-30
extractDataProviderName returns null whenever @DataProvider has no name attribute, so those methods are dropped entirely. In TestNG a @DataProvider without name registers under the method name, so @DataProvider public Object[][] testData() collides with @DataProvider(name = "testData") public Object[][] other() — the exact duplication this rule exists to catch — and the check stays silent. The PR's own design note (new_rule_proposal_S9389.txt:39) states these cases are "handled separately by using the method name as the key", and the accepted-FN section (lines 81-83) does not list this gap, so the implementation contradicts its specification. Fall back to method.simpleName().name() when the name attribute is absent, and add a sample case mixing an implicit and an explicit name.

✅ Quality: Issue is reported on the whole method instead of a narrow location

📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:58-60 📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:114-120
reportDuplicate passes the MethodTree as the primary location, so the highlighted range spans the annotation, signature and full method body; for a realistic data provider that is dozens of lines of highlighted code for a one-word problem. Sibling checks in the same package report on a narrow node (NoTestInTestClassCheck.java:308 uses the class name, TestsStabilityCheck.java:59 uses the annotation argument, CallSuperInTestCaseCheck.java:52 uses method.simpleName()). Report on the duplicated name argument or on method.simpleName(), and use the same narrow node for the secondary location.

✅ Quality: Dead branches in extractNameAttribute and getAttributeName

📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:80-94
AnnotationTree.arguments() returns an Arguments list tree and is never null, and the left-hand side of an annotation element assignment is always a simple identifier — every other check in this package casts assignment.variable() straight to IdentifierTree (TestsStabilityCheck.java:57, TestAnnotationWithExpectedExceptionCheck.java:69). The arguments == null test and the MEMBER_SELECT branch of getAttributeName are therefore unreachable, which adds complexity and leaves uncoverable lines in the new check. Drop both and inline the identifier cast.

✅ Edge Case: Records are not visited, so providers declared in a record are missed

📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:44-46
nodesToVisit registers only CLASS, INTERFACE and ENUM. Tree.Kind.RECORD is also backed by ClassTreeImpl/ClassTree (see ClassTreeImpl handling Kind.RECORD), and other class-scoped checks include it (CallOuterPrivateMethodCheck, TooManyMethodsCheck, UselessExtendsCheck). A record holding two @DataProvider(name = "x") methods is therefore never analysed. Add Tree.Kind.RECORD to the visited kinds and a record case to the sample.

✅ Quality: String literal values are compared with quotes and escapes unnormalized

📄 java-checks/src/main/java/org/sonar/java/checks/tests/DataProviderNameUniquenessCheck.java:107-112
extractStringValue returns LiteralTree.value(), which for a STRING_LITERAL is the raw source text including the surrounding double quotes and un-decoded escape sequences; the repo's helper for this is LiteralUtils.trimQuotes (used at LiteralUtils.java:110,143,169). Two providers whose names are the same string but spelled differently in source (e.g. name = "ab" and name = "a\142") are treated as distinct, and TEXT_BLOCK literals are ignored altogether. Use LiteralUtils.trimQuotes (or ExpressionsHelper constant resolution) so keys reflect the actual provider name.

...and 4 more resolved 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-6942 - 1 of 1 objectives covered

This PR implements rule S9389 to ensure data provider names are unique within a class.

✅ 1 covered here

✅ Implement rule S9389 to ensure data provider names are unique within a class

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 · 2026-09-15T10:30:21Z

Quality Gate passed