From c28d4347eedd46130b53f14b576277e476722e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Tue, 27 Jan 2026 12:08:06 +0100 Subject: [PATCH 1/3] Fix assertions counting in S5961 for chained method calls --- .../tests/TooManyAssertionsCheckCustom2.java | 14 +++++++++++++ .../tests/TooManyAssertionsCheckCustom25.java | 20 +++++++++++++++++++ .../checks/tests/TooManyAssertionsCheck.java | 16 +++++++-------- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java index aebd7b5ff52..8b6324fafbe 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java @@ -3,6 +3,7 @@ import org.junit.jupiter.api.Test; import rx.Observable; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.junit.Assert.assertEquals; public class TooManyAssertionsCheckCustom2 { @@ -37,6 +38,19 @@ void test3() { // Noncompliant {{Refactor this method to reduce the number of as // ^^^^^^^^^^^^^^< } + @Test + void test4() { // Noncompliant {{Refactor this method to reduce the number of assertions from 3 to less than 2.}} +// ^^^^^ + var myObject = new Object(); + + assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); +// ^^^^^^^^^^^^^^^^^^^^< + assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); +// ^^^^^^^^^^^^^^^^^^^^< + assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); +// ^^^^^^^^^^^^^^^^^^^^< + } + void customAssert() { assertEquals(2, f(2)); assertEquals(3, f(1)); diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java index dc41fac7f85..2eb61d710b7 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java @@ -75,6 +75,26 @@ void test2() { // Noncompliant {{Refactor this method to reduce the number of as assertEquals(1026, g(26)); } + @Test + void test3() { // Compliant, chained assertions only count as a single instance + var myObject = new Object(); + + assertThat(true).isTrue().isTrue().isTrue(); + assertThat(true).isTrue().as("").isTrue().as("").isTrue(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).as("description").isNotNull(); + assertThat(myObject).describedAs("hjksa").isNotNull(); + assertThat(myObject).withFailMessage("hjksa").isNotNull(); + assertThat(myObject).overridingErrorMessage("hjksa").isNotNull(); + } + int g(int x) { return x + 100; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/TooManyAssertionsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/TooManyAssertionsCheck.java index ee8973f04e9..e39fa4ad10a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/TooManyAssertionsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/TooManyAssertionsCheck.java @@ -111,15 +111,15 @@ private class AssertionsCounterVisitor extends BaseTreeVisitor { @Override public void visitMethodInvocation(MethodInvocationTree mit) { super.visitMethodInvocation(mit); - if (isAssertion(methodName(mit), mit.methodSymbol())) { - ExpressionTree methodSelect = mit.methodSelect(); - if (methodSelect.is(Tree.Kind.MEMBER_SELECT)) { - ExpressionTree expression = ((MemberSelectExpressionTree) methodSelect).expression(); - if (assertions.contains(expression) || chainedAssertions.contains(expression)) { - chainedAssertions.add(mit); - return; - } + ExpressionTree methodSelect = mit.methodSelect(); + if (methodSelect.is(Tree.Kind.MEMBER_SELECT)) { + ExpressionTree expression = ((MemberSelectExpressionTree) methodSelect).expression(); + if (assertions.contains(expression) || chainedAssertions.contains(expression)) { + chainedAssertions.add(mit); + return; } + } + if (isAssertion(methodName(mit), mit.methodSymbol())) { assertions.add(mit); } } From 8ad5b0ab9bf517726f135f3b60db53cc4d0d560f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Wed, 28 Jan 2026 12:56:39 +0100 Subject: [PATCH 2/3] Improve the tests --- .../tests/TooManyAssertionsCheckCustom25.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java index 2eb61d710b7..e2467f263bc 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java @@ -80,19 +80,18 @@ void test3() { // Compliant, chained assertions only count as a single instance var myObject = new Object(); assertThat(true).isTrue().isTrue().isTrue(); - assertThat(true).isTrue().as("").isTrue().as("").isTrue(); + assertThat(true).isTrue().as("true").isTrue().describedAs("isTrue").isTrue(); + assertThat(myObject).as("object").isNotNull(); + assertThat(myObject).isNotNull().as("object").isNotNull(); assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).describedAs("hjksa").isNotNull(); - assertThat(myObject).withFailMessage("hjksa").isNotNull(); - assertThat(myObject).overridingErrorMessage("hjksa").isNotNull(); + assertThat(myObject).as("description").isInstanceOf(Object.class); + assertThat(myObject).isNotNull().as("description").isInstanceOf(Object.class); + assertThat(myObject).describedAs("someObject").isNotNull(); + assertThat(myObject).isNotNull().describedAs("someObject").isNotNull(); + assertThat(myObject).withFailMessage("failure").describedAs("someObject").isNotNull(); + assertThat(myObject).isNotNull().withFailMessage("failure").describedAs("someObject").isNotNull(); + assertThat(myObject).overridingErrorMessage("error").isNotNull(); + assertThat(myObject).isNotNull().overridingErrorMessage("error").isNotNull(); } int g(int x) { From 95b1153ea3a76ae6e1332025c5e73859fbbe3d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Wed, 28 Jan 2026 16:47:36 +0100 Subject: [PATCH 3/3] Change the unit tests to pass the autoscan its --- .../tests/TooManyAssertionsCheckCustom2.java | 23 ++++++++++++++++--- .../tests/TooManyAssertionsCheckCustom25.java | 19 --------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java index 8b6324fafbe..41beb0419d9 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom2.java @@ -1,5 +1,6 @@ package checks.tests; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import rx.Observable; @@ -39,15 +40,31 @@ void test3() { // Noncompliant {{Refactor this method to reduce the number of as } @Test - void test4() { // Noncompliant {{Refactor this method to reduce the number of assertions from 3 to less than 2.}} + void test4() { // Compliant, chained assertions only count as a single instance + var trueCondition = true; + var falseCondition = false; + + assertThat(trueCondition).isTrue().as("true").isTrue().describedAs("isTrue").isTrue(); + assertThat(falseCondition).isFalse().as("false").isFalse().describedAs("isFalse").isFalse(); + } + + @Test + void test5() { // Compliant, chained assertions only count as a single instance + var myObject = new Object(); + + Assertions.assertThat(myObject).as("someObject").isNotNull().withFailMessage("fail").isNotNull().overridingErrorMessage("error").isInstanceOf(Object.class); + } + + @Test + void test6() { // Noncompliant {{Refactor this method to reduce the number of assertions from 3 to less than 2.}} // ^^^^^ var myObject = new Object(); assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); // ^^^^^^^^^^^^^^^^^^^^< - assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); + assertThat(myObject).describedAs("description").isNotNull(); // ^^^^^^^^^^^^^^^^^^^^< - assertThat(myObject).as("description").isNotNull().describedAs("description").isNotNull(); + assertThat(myObject).withFailMessage("failure").describedAs("someObject").isNotNull(); // ^^^^^^^^^^^^^^^^^^^^< } diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java index e2467f263bc..dc41fac7f85 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/TooManyAssertionsCheckCustom25.java @@ -75,25 +75,6 @@ void test2() { // Noncompliant {{Refactor this method to reduce the number of as assertEquals(1026, g(26)); } - @Test - void test3() { // Compliant, chained assertions only count as a single instance - var myObject = new Object(); - - assertThat(true).isTrue().isTrue().isTrue(); - assertThat(true).isTrue().as("true").isTrue().describedAs("isTrue").isTrue(); - assertThat(myObject).as("object").isNotNull(); - assertThat(myObject).isNotNull().as("object").isNotNull(); - assertThat(myObject).as("description").isNotNull(); - assertThat(myObject).as("description").isInstanceOf(Object.class); - assertThat(myObject).isNotNull().as("description").isInstanceOf(Object.class); - assertThat(myObject).describedAs("someObject").isNotNull(); - assertThat(myObject).isNotNull().describedAs("someObject").isNotNull(); - assertThat(myObject).withFailMessage("failure").describedAs("someObject").isNotNull(); - assertThat(myObject).isNotNull().withFailMessage("failure").describedAs("someObject").isNotNull(); - assertThat(myObject).overridingErrorMessage("error").isNotNull(); - assertThat(myObject).isNotNull().overridingErrorMessage("error").isNotNull(); - } - int g(int x) { return x + 100; }