SONARJAVA-5480 S2699 recognizes SpringBoot AssertableApplicationContext (#5125)

Author: leonardo-pilastri-sonarsource

its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S2699",
   "hasTruePositives": true,
-  "falseNegatives": 152,
+  "falseNegatives": 154,
   "falsePositives": 1
 }

java-checks-test-sources/default/src/test/java/checks/tests/AssertionsInTestsCheck/ForeignNonAssertingContextConsumer.java

@@ -0,0 +1,11 @@
+package checks.tests.AssertionsInTestsCheck;
+
+import org.springframework.boot.test.context.assertj.AssertableApplicationContext;
+import org.springframework.boot.test.context.runner.ContextConsumer;
+
+public class ForeignNonAssertingContextConsumer implements ContextConsumer<AssertableApplicationContext> {
+  @Override
+  public void accept(AssertableApplicationContext context) throws Throwable {
+    // do something but no assertions
+  }
+}

java-checks-test-sources/default/src/test/java/checks/tests/AssertionsInTestsCheck/SpringBootAppContextRunnerSample.java

@@ -0,0 +1,72 @@
+package checks.tests.AssertionsInTestsCheck;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.test.context.assertj.ApplicationContextAssertProvider;
+import org.springframework.boot.test.context.assertj.AssertableApplicationContext;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.boot.test.context.runner.ContextConsumer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class SpringBootAppContextRunnerSampleTest {
+
+  ApplicationContextRunner contextRunner = new ApplicationContextRunner();
+
+  @Test
+  void test_with_assertable_app_context_different_file() { // FN, contextConsumer has no assertions, but we cannot know
+    var contextConsumer = new ForeignNonAssertingContextConsumer();
+    contextRunner.run(contextConsumer);
+  }
+
+  @Test
+  void test_with_assertable_app_context() { // Compliant, contextConsumer has param type AssertableApplicationContext
+    contextRunner.run(getAssertingContextConsumer());
+  }
+
+  @Test
+  void test_with_assertable_app_context2() { // Compliant, contextConsumer has param type AssertableApplicationContext
+    var contextConsumer = new LocalAssertingContextConsumer();
+    contextRunner.run(contextConsumer);
+  }
+
+  @Test
+  void test_without_assertable_app_context() { // Noncompliant
+    var nonAssertingContextConsumer = new NonAssertingContextConsumer();
+    contextRunner.run(nonAssertingContextConsumer);
+  }
+
+  @Test
+  void test_with_lambda_assertable_app_ctx() {
+    contextRunner.run(context -> {
+      assertThat(context).getBean("myBean").isNotNull();
+    });
+  }
+
+  @Test
+  void test_without_assertable_app_ctx() { // Noncompliant
+    contextRunner.run((ContextConsumer<ApplicationContextAssertProvider>) _ -> {
+      // do some unrelated stuff ...
+    });
+  }
+
+  private LocalAssertingContextConsumer getAssertingContextConsumer(){
+    return new LocalAssertingContextConsumer();
+  }
+
+  class LocalAssertingContextConsumer implements ContextConsumer<AssertableApplicationContext> {
+    @Override
+    public void accept(AssertableApplicationContext context) throws Throwable {
+      assertThat(context).getBean("myBean").isNotNull();
+    }
+  }
+
+  class NonAssertingContextConsumer implements ContextConsumer<AssertableApplicationContext> {
+    private int count = 0;
+
+    @Override
+    public void accept(AssertableApplicationContext context) throws Throwable {
+      count++;
+    }
+  }
+
+}

java-checks/src/main/java/org/sonar/java/checks/tests/AssertionsInTestsCheck.java

@@ -27,13 +27,17 @@
 import org.sonar.check.RuleProperty;
 import org.sonar.java.checks.helpers.AbstractAssertionVisitor;
 import org.sonar.java.model.ModifiersUtils;
+import org.sonar.java.model.declaration.ClassTreeImpl;
+import org.sonar.java.model.expression.MethodInvocationTreeImpl;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.semantic.MethodMatchers;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata;
+import org.sonar.plugins.java.api.semantic.Type;
 import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
 import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Modifier;
 import org.sonar.plugins.java.api.tree.Tree;
@@ -54,6 +58,13 @@ public class AssertionsInTestsCheck extends BaseTreeVisitor implements JavaFileS
   public String customAssertionMethods = "";
   private MethodMatchers customAssertionMethodsMatcher = null;
 
+  private static final MethodMatchers SPRING_BOOT_APP_CTX_RUNNER_RUN_MATCHER = MethodMatchers.create()
+    .ofTypes("org.springframework.boot.test.context.runner.ApplicationContextRunner")
+    .names("run")
+    .addParametersMatcher("org.springframework.boot.test.context.runner.ContextConsumer")
+    .build();
+  private static final MethodInvocationMatcherVisitor SPRING_BOOT_APP_CTX_RUNNER_VISITOR = new MethodInvocationMatcherVisitor(SPRING_BOOT_APP_CTX_RUNNER_RUN_MATCHER);
+
   private final Map<Symbol, Boolean> assertionInMethod = new HashMap<>();
   private JavaFileScannerContext context;
 
@@ -75,13 +86,86 @@ public void visitMethod(MethodTree methodTree) {
       return;
     }
 
-    if (isUnitTest(methodTree) && !isSpringBootSanityTest(methodTree) && !expectAssertion(methodTree) && !isLocalMethodWithAssertion(methodTree.symbol())) {
-      context.reportIssue(this, methodTree.simpleName(), "Add at least one assertion to this test case.");
+    if (isUnitTest(methodTree)) {
+      if (isSpringBootAssertableContext(methodTree)) {
+        return;
+      }
+      if (!isSpringBootSanityTest(methodTree) && !expectAssertion(methodTree) && !isLocalMethodWithAssertion(methodTree.symbol())) {
+        context.reportIssue(this, methodTree.simpleName(), "Add at least one assertion to this test case.");
+      }
+    }
+  }
+
+  private boolean isSpringBootAssertableContext(MethodTree methodTree) {
+    var runMethodInvocation = SPRING_BOOT_APP_CTX_RUNNER_VISITOR.findMethodInvocation(methodTree);
+    if (runMethodInvocation != null) {
+      var contextConsumerImplSymbol = runMethodInvocation.arguments().get(0).symbolType().symbol();
+      if (contextConsumerImplSymbol.isUnknown()) {
+        // In this case we cannot know if the provided ContextConsumer has the type param <AssertableApplicationContext>, but we want to avoid FPs
+        return true;
+      }
+      Type contextConsumerType;
+      if (contextConsumerImplSymbol.isInterface()) {
+        contextConsumerType = contextConsumerImplSymbol.type();
+      } else {
+        contextConsumerType = contextConsumerImplSymbol.interfaces().get(0);
+      }
+      return isAssertableApplicationContext(contextConsumerType) && hasDeclaredAssertions(contextConsumerImplSymbol);
+    }
+    return false;
+  }
+
+  private static boolean isAssertableApplicationContext(Type contextConsumerType) {
+    return contextConsumerType.typeArguments().get(0).is("org.springframework.boot.test.context.assertj.AssertableApplicationContext");
+  }
+
+  /**
+   * Takes a Symbol as input and checks if it has a declaring class available. If so, it will also check that the class has at least
+   * one method with an assertion.
+   * Used by {@link #isSpringBootAssertableContext(MethodTree)} to check if a ContextConsumer of AssertableApplicationContext
+   *   has at least an assertion in its methods.
+   * @param contextConsumerImplSymbol The symbol for which we want to check the declaration of
+   * @return true if the symbol has no declaration (to avoid FPs), or if its declaration is a class with at least one method with assertions
+   */
+  private boolean hasDeclaredAssertions(Symbol contextConsumerImplSymbol) {
+    Tree declaration = contextConsumerImplSymbol.declaration();
+    if (declaration instanceof ClassTreeImpl contextConsumerImpl) {
+      return contextConsumerImpl.members().stream()
+        .anyMatch(m -> m instanceof MethodTree method && isLocalMethodWithAssertion(method.symbol()));
+    }
+    return true;
+  }
+
+  /**
+   * Finds the first nested method invocation in a tree that matches the MethodMatchers provided in the constructor
+   */
+  private static class MethodInvocationMatcherVisitor extends BaseTreeVisitor {
+
+    private MethodInvocationTreeImpl methodInvocationTree;
+    private final MethodMatchers matcher;
+
+    private MethodInvocationMatcherVisitor(MethodMatchers matcher) {
+      this.matcher = matcher;
+    }
+
+    @Override
+    public void visitMethodInvocation(MethodInvocationTree tree) {
+      if (matcher.matches(tree)) {
+        methodInvocationTree = (MethodInvocationTreeImpl) tree;
+        return;
+      }
+      super.visitMethodInvocation(tree);
+    }
+
+    public MethodInvocationTreeImpl findMethodInvocation(Tree tree) {
+      methodInvocationTree = null;
+      tree.accept(this);
+      return methodInvocationTree;
     }
   }
 
-  private static boolean isSpringBootSanityTest(MethodTree methodTree){
-    if("contextLoads".equals(methodTree.simpleName().name())){
+  private static boolean isSpringBootSanityTest(MethodTree methodTree) {
+    if ("contextLoads".equals(methodTree.simpleName().name())) {
       ClassTree classTree = (ClassTree) methodTree.parent();
       return classTree.symbol().metadata().isAnnotatedWith("org.springframework.boot.test.context.SpringBootTest");
     }

java-checks/src/test/java/org/sonar/java/checks/tests/AssertionsInTestsCheckTest.java

@@ -117,4 +117,13 @@ void testSpringBootSanity(){
       .withCheck(check)
       .verifyIssues();
   }
+
+  @Test
+  void testSpringBootAssertableApplicationContext(){
+    CheckVerifier.newVerifier()
+      .onFile(testCodeSourcesPath("checks/tests/AssertionsInTestsCheck/SpringBootAppContextRunnerSample.java"))
+      .withCheck(check)
+      .verifyIssues();
+  }
+
 }