Merge pull request #2958 from testng-team/feature/errorprone

Add errorprone

Author: juherr

build-logic/build-parameters/build.gradle.kts

@@ -49,4 +49,8 @@ buildParameters {
         defaultValue.set(true)
         description.set("Fail build on javadoc warnings")
     }
+    bool("skipErrorProne") {
+        defaultValue.set(false)
+        description.set("Skip Error Prone verifications")
+    }
 }

build-logic/code-quality/build.gradle.kts

@@ -7,4 +7,5 @@ dependencies {
     api(projects.basics)
     api("org.sonarqube:org.sonarqube.gradle.plugin:4.4.1.3373")
     api("com.github.autostyle:autostyle-plugin-gradle:4.0")
+    api("net.ltgt.gradle:gradle-errorprone-plugin:3.1.0")
 }

build-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.kts

@@ -0,0 +1,14 @@
+import net.ltgt.gradle.errorprone.errorprone
+
+plugins {
+    id("net.ltgt.errorprone")
+    id("build-logic.build-params")
+}
+
+dependencies {
+    errorprone("com.google.errorprone:error_prone_core:2.20.0")
+}
+
+tasks.withType<JavaCompile>().configureEach {
+    options.errorprone.disableWarningsInGeneratedCode.set(true)
+}

build-logic/jvm/src/main/kotlin/testng.java.gradle.kts

@@ -42,3 +42,7 @@ tasks.configureEach<Test> {
     inputs.property("java.vm.version", System.getProperty("java.vm.version"))
     inputs.property("java.vm.vendor", System.getProperty("java.vm.vendor"))
 }
+
+if (!buildParameters.skipErrorProne) {
+    apply(plugin = "testng.errorprone")
+}

testng-asserts/src/test/java/org/testng/AssertTest.java

@@ -305,22 +305,10 @@ public void compareUnEqualDoubleArraysWithDelta() {
     Assert.assertEquals(actual, expected, 0.1d);
   }
 
-  @SuppressWarnings("serial")
   @Test(expectedExceptions = AssertionError.class)
   public void assertEqualsMapShouldFail() {
-    Map<String, String> mapActual =
-        new HashMap<String, String>() {
-          {
-            put("a", "1");
-          }
-        };
-    Map<String, String> mapExpected =
-        new HashMap<String, String>() {
-          {
-            put("a", "1");
-            put("b", "2");
-          }
-        };
+    Map<String, String> mapActual = Map.of("a", "1");
+    Map<String, String> mapExpected = Map.of("a", "1", "b", "2");
 
     Assert.assertEquals(mapActual, mapExpected);
   }
@@ -654,12 +642,22 @@ static class BrokenEqualsTrue {
     public boolean equals(Object o) {
       return true; // broken implementation
     }
+
+    @Override
+    public int hashCode() {
+      return 0;
+    }
   }
 
   static class BrokenEqualsFalse {
     @Override
     public boolean equals(Object o) {
       return false; // broken implementation
     }
+
+    @Override
+    public int hashCode() {
+      return 0;
+    }
   }
 }

testng-core/src/main/java/org/testng/SuiteResult.java

@@ -6,7 +6,7 @@
 import org.testng.xml.XmlSuite;
 
 /** This class logs the result of an entire Test Suite (defined by a property file). */
-class SuiteResult implements ISuiteResult, Comparable<ISuiteResult> {
+class SuiteResult implements ISuiteResult, Comparable<SuiteResult> {
   private final XmlSuite m_suite;
   private final ITestContext m_testContext;
 
@@ -26,7 +26,7 @@ public XmlSuite getSuite() {
   }
 
   @Override
-  public int compareTo(@Nonnull ISuiteResult other) {
+  public int compareTo(@Nonnull SuiteResult other) {
     int result = 0;
     try {
       String n1 = getTestContext().getName();

testng-core/src/main/java/org/testng/internal/ClonedMethod.java

@@ -138,7 +138,7 @@ public boolean hasMoreInvocation() {
 
   @Override
   public Class<?> getRealClass() {
-    return m_javaMethod.getClass();
+    return m_javaMethod.getDeclaringClass();
   }
 
   @Override

testng-core/src/main/java/org/testng/internal/Graph.java

@@ -1,9 +1,11 @@
 package org.testng.internal;
 
+import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.Deque;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -190,9 +192,9 @@ public List<T> findPredecessors(T o) {
     // already encountered.  "queue" is the queue of items whose
     // predecessors we haven't yet explored.
 
-    LinkedList<T> result = new LinkedList<>();
+    Deque<T> result = new ArrayDeque<>();
     Set<T> visited = new HashSet<>();
-    LinkedList<T> queue = new LinkedList<>();
+    Deque<T> queue = new ArrayDeque<>();
     visited.add(o);
     queue.addLast(o);
 
@@ -206,7 +208,7 @@ public List<T> findPredecessors(T o) {
       }
     }
 
-    return result;
+    return new ArrayList<>(result);
   }
 
   @Override

testng-core/src/main/java/org/testng/internal/Tarjan.java

@@ -1,9 +1,9 @@
 package org.testng.internal;
 
+import java.util.ArrayDeque;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Stack;
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
 
@@ -14,13 +14,13 @@
  */
 public class Tarjan<T> {
   int m_index = 0;
-  private final Stack<T> stack;
+  private final ArrayDeque<T> stack;
   Map<T, Integer> visitedNodes = Maps.newHashMap();
   Map<T, Integer> m_lowlinks = Maps.newHashMap();
   private List<T> m_cycle;
 
   public Tarjan(Graph<T> graph, T start) {
-    stack = new Stack<>();
+    stack = new ArrayDeque<>();
     run(graph, start);
   }
 

testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java

@@ -644,7 +644,7 @@ private Object invokeMethod(Annotation test, String methodName) {
     Object result = null;
     try {
       // Note:  we should cache methods already looked up
-      Method m = test.getClass().getMethod(methodName);
+      Method m = test.annotationType().getMethod(methodName);
       result = m.invoke(test);
     } catch (Exception e) {
       Logger.getLogger(JDK15TagFactory.class).error(e.getMessage(), e);