SONARJAVA-4424 add methods annotated with @test to the list of testNg test methods (#4953)

Author: erwan-serandour

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

@@ -2,5 +2,5 @@
   "ruleKey": "S2187",
   "hasTruePositives": true,
   "falseNegatives": 13,
-  "falsePositives": 0
+  "falsePositives": 1
 }

java-checks-test-sources/default/src/main/files/non-compiling/checks/NoTestInTestClassCheckSample.java

@@ -121,6 +121,33 @@ public class TestNGClassTest { // Noncompliant
   private void test1() { }
   public static void foo() {}
 }
+
+@org.testng.annotations.Test
+class TestNGClassTestWithMethodAnnotated { // compliant, with testng when the class is annotated with @Test all public methods are considered as tests
+                                          // non public methods can also be added to tests with the @Test annotation
+  @org.testng.annotations.Test
+  void myMethod(){
+
+  }
+}
+
+@org.testng.annotations.Test
+class TestNGClassWithUnkownAnnotation {
+  @Unkown
+  void myMethod(){
+
+  }
+}
+
+@org.testng.annotations.Test
+class TestNGClassWithWrongAnnotation { // Noncompliant
+  @Override
+  void myMethod(){
+
+  }
+}
+
+
 @org.testng.annotations.Test(groups ="integration")
 public abstract class AbstractIntegrationTest2{
 }

java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckTestNgTest.java

@@ -0,0 +1,38 @@
+package checks.tests;
+
+import org.testng.annotations.Test;
+
+class TestNGTest {
+  @Test
+  void foo() {
+  }
+}
+
+@Test
+class TestNgFooTest {
+  public void test1() {
+  }
+
+  public void test2() {
+  }
+}
+
+@Test
+class TestNGClassTest { // Noncompliant
+  public int field;
+  private void test1() { }
+  public static void foo() {}
+}
+
+@Test
+class TestNGClassTestUseAnnotation {
+
+  @Test
+  void myMethod(){
+
+  }
+}
+
+@Test(groups ="integration")
+abstract class AbstractIntegrationTest2{
+}

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

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import org.apache.commons.lang3.StringUtils;
@@ -44,6 +45,7 @@ public class NoTestInTestClassCheck extends IssuableSubscriptionVisitor {
   public static final String ARCH_UNIT_RUNNER = "ArchUnitRunner";
   public static final String ARCH_UNIT_ANALYZE_CLASSES = "com.tngtech.archunit.junit.AnalyzeClasses";
   public static final String ARCH_UNIT_TEST = "com.tngtech.archunit.junit.ArchTest";
+  private static final String TEST_NG_TEST = "org.testng.annotations.Test";
 
   private static final List<String> PACT_UNIT_TEST = Arrays.asList("au.com.dius.pact.provider.junit.State", "au.com.dius.pact.provider.junitsupport.State");
 
@@ -75,7 +77,7 @@ public void visitNode(Tree tree) {
 
   private void resetAnnotationCache() {
     Arrays.asList(testFieldAnnotations, testMethodAnnotations, seenAnnotations).forEach(Set::clear);
-    testMethodAnnotations.addAll(Arrays.asList("org.junit.Test", "org.testng.annotations.Test", "org.junit.jupiter.api.Test"));
+    testMethodAnnotations.addAll(Arrays.asList("org.junit.Test", TEST_NG_TEST, "org.junit.jupiter.api.Test"));
   }
 
   private void checkClass(ClassTree classTree) {
@@ -93,7 +95,7 @@ private void checkClass(ClassTree classTree) {
       Symbol.TypeSymbol classSymbol = classTree.symbol();
       Stream<Symbol> members = getAllMembers(classSymbol, checkRunWith(classSymbol, "Enclosed"));
       IdentifierTree simpleName = classTree.simpleName();
-      if (classSymbol.metadata().isAnnotatedWith("org.testng.annotations.Test")) {
+      if (classSymbol.metadata().isAnnotatedWith(TEST_NG_TEST)) {
         checkTestNGmembers(simpleName, members);
       } else {
         boolean isJunit3TestClass = classSymbol.type().isSubtypeOf("junit.framework.TestCase");
@@ -124,7 +126,22 @@ private static boolean isArchUnitTestClass(Symbol.TypeSymbol classSymbol) {
   }
 
   private void checkTestNGmembers(IdentifierTree className, Stream<Symbol> members) {
-    if (members.noneMatch(member -> member.isMethodSymbol() && member.isPublic() && !member.isStatic() && !"<init>".equals(member.name()))) {
+    Predicate<SymbolMetadata.AnnotationInstance> isTestNgAnnotation = ann -> {
+      Type type = ann.symbol().type();
+      return type.isUnknown() || type.is(TEST_NG_TEST);
+    };
+    Predicate<Symbol> isTestMethod = member -> {
+      if(!member.isMethodSymbol()){
+        return false;
+      }
+
+      // we know member is a method.
+      boolean annotatedWithTest = member.metadata().annotations().stream().anyMatch(isTestNgAnnotation);
+      boolean publicMethod = member.isPublic() && !member.isStatic() && !"<init>".equals(member.name());
+      return annotatedWithTest || publicMethod;
+    };
+
+    if (members.noneMatch(isTestMethod)) {
       reportClass(className);
     }
   }

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

@@ -85,4 +85,12 @@ void pactUnit() {
       .withCheck(new NoTestInTestClassCheck())
       .verifyIssues();
   }
+
+  @Test
+  void testNg() {
+    CheckVerifier.newVerifier()
+      .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckTestNgTest.java"))
+      .withCheck(new NoTestInTestClassCheck())
+      .verifyIssues();
+  }
 }