Skip to content

SONARJAVA-5797 False Negative with java:S2698 for org.testng #5302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 15, 2025
Merged

SONARJAVA-5797 False Negative with java:S2698 for org.testng #5302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b679e12
USER-1011 False Negative with java:S2698 for org.testng
leonardo-pilastri-sonarsource Sep 25, 2025
b6489e6
Handle FP case
leonardo-pilastri-sonarsource Oct 2, 2025
c2b7487
Review changes
leonardo-pilastri-sonarsource Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...-test-sources/default/src/test/java/checks/tests/AssertionsWithoutMessageCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

class AssertionsWithoutMessageCheckSample {
void foo() {
org.testng.Assert.assertTrue(true, "msg"); // Compliant
org.testng.Assert.assertTrue(true); // Noncompliant
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to org.testng.Assert.assertTrue(true, "message"); // Noncompliant and it still passes. I guess that's because the messages in testng are the last parameter.

org.testng.AssertJUnit.assertTrue(true); // Noncompliant
org.assertj.core.api.Assertions.assertThat("").usingComparator(null).as("a").isEqualTo(222); // Compliant
org.junit.Assert.assertTrue(true); // Noncompliant {{Add a message to this assertion.}}
// ^^^^^^^^^^
Expand Down
14 changes: 12 additions & 2 deletions java-checks/src/main/java/org/sonar/java/checks/tests/AssertionsWithoutMessageCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected MethodMatchers getMethodInvocationMatchers() {
return MethodMatchers.or(
MethodMatchers.create()
.ofTypes("org.junit.jupiter.api.Assertions", "org.junit.Assert", "junit.framework.Assert", "org.fest.assertions.Fail",
"org.assertj.core.api.Fail")
"org.assertj.core.api.Fail", "org.testng.Assert", "org.testng.AssertJUnit")
.name(name -> name.startsWith(ASSERT) || "fail".equals(name)).withAnyParameters().build(),
FEST_LIKE_ABSTRACT_ASSERT
);
Expand All @@ -105,11 +105,21 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {
checkFestLikeAssertion(mit, symbol, reportLocation);
} else if (type.is("org.junit.jupiter.api.Assertions")) {
checkJUnit5(mit, reportLocation);
} else if (mit.arguments().isEmpty() || !isString(mit.arguments().get(0)) || isAssertingOnStringWithNoMessage(mit)) {
} else if (mit.arguments().isEmpty() || !hasMessageArg(mit, type) || isAssertingOnStringWithNoMessage(mit)) {
reportIssue(reportLocation, MESSAGE);
}
}

/**
* True if the call has a message argument. Such an argument is a string
* and it is the first of the last argument (depending on the assertion library).
*/
private static boolean hasMessageArg(MethodInvocationTree mit, Type type) {
int expectedMessageArgIndex = (type.is("org.testng.Assert") || type.is("org.testng.AssertJUnit")) ? 1 : 0;
List<ExpressionTree> args = mit.arguments();
return expectedMessageArgIndex < args.size() && isString(args.get(expectedMessageArgIndex));
}

private void checkFestLikeAssertion(MethodInvocationTree mit, Symbol symbol, IdentifierTree reportLocation) {
if (isConstructor(symbol)) {
return;
Expand Down