Add checkerframework and errorprone

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

@@ -11,6 +11,8 @@ repositories {
 dependencies {
     implementation("org.sonarqube:org.sonarqube.gradle.plugin:2.8")
     implementation("com.github.autostyle:autostyle-plugin-gradle:3.1")
+    implementation("org.checkerframework:checkerframework-gradle-plugin:0.6.29")
+    implementation("net.ltgt.gradle:gradle-errorprone-plugin:3.1.0")
 }
 
 tasks.withType<KotlinCompile>().configureEach {

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

@@ -0,0 +1,27 @@
+import org.checkerframework.gradle.plugin.CheckerFrameworkExtension
+
+plugins {
+    id("org.checkerframework")
+}
+
+dependencies {
+    checkerFramework("org.checkerframework:checker:3.36.0")
+}
+
+plugins.withId("org.checkerframework") {
+    configure<CheckerFrameworkExtension> {
+        checkers = listOf(
+            "org.checkerframework.checker.nullness.NullnessChecker",
+            "org.checkerframework.checker.optional.OptionalChecker"
+        )
+    }
+}
+
+tasks.withType<JavaCompile>().configureEach {
+    // Don't run the checker on generated code.
+    if (name.startsWith("compileMainGenerated")) {
+        checkerFramework {
+            skipCheckerFramework = true
+        }
+    }
+}

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

@@ -0,0 +1,13 @@
+import net.ltgt.gradle.errorprone.errorprone
+
+plugins {
+    id("net.ltgt.errorprone")
+}
+
+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

@@ -2,6 +2,8 @@ plugins {
     `java-base`
     id("testng.versioning")
     id("testng.style")
+    id("testng.errorprone")
+    id("testng.checkerframework")
     id("testng.repositories")
     // Improves Gradle Test logging
     // See https://github.com/vlsi/vlsi-release-plugins/tree/master/plugins/gradle-extensions-plugin

testng-asserts/src/main/java/org/testng/FileAssert.java

@@ -2,6 +2,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Assertion tool for File centric assertions. Conceptually, this is an extension of {@link Assert}.
@@ -77,7 +78,7 @@ public static void assertFile(File tstvalue) {
    * @param expected the expected value
    * @param message the assertion error message
    */
-  public static void assertLength(File tstvalue, long expected, String message) {
+  public static void assertLength(File tstvalue, long expected, @Nullable String message) {
     long actual = -1L;
     try {
       actual = tstvalue.isDirectory() ? tstvalue.list().length : tstvalue.length();
@@ -291,7 +292,7 @@ private static void failFile(File path, String actual, String expected, String m
    * @param message
    */
   private static void failSecurity(
-      Exception e, File path, String actual, String expected, String message) {
+      Exception e, File path, String actual, String expected, @Nullable String message) {
     String formatted = "";
     if (message != null) {
       formatted = message + " ";

testng-asserts/src/main/java/org/testng/asserts/Assertion.java

@@ -3,6 +3,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** An assert class with various hooks allowing its behavior to be modified by subclasses. */
 public class Assertion implements IAssertLifecycle {
@@ -41,9 +42,9 @@ public void onBeforeAssert(IAssert<?> assertCommand) {}
   public void onAfterAssert(IAssert<?> assertCommand) {}
 
   private abstract static class SimpleAssert<T> implements IAssert<T> {
-    private final T actual;
-    private final T expected;
-    private final String m_message;
+    private final @Nullable T actual;
+    private final @Nullable T expected;
+    private final @Nullable String m_message;
 
     public SimpleAssert(String message) {
       this(null, null, message);

testng-asserts/src/main/java/org/testng/asserts/SoftAssert.java

@@ -1,6 +1,7 @@
 package org.testng.asserts;
 
 import java.util.Map;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.testng.collections.Maps;
 
 /**
@@ -30,7 +31,7 @@ public void assertAll() {
     assertAll(null);
   }
 
-  public void assertAll(String message) {
+  public void assertAll(@Nullable String message) {
     if (!m_errors.isEmpty()) {
       StringBuilder sb = new StringBuilder(null == message ? DEFAULT_SOFT_ASSERT_MESSAGE : message);
       boolean first = true;

testng-asserts/testng-asserts-build.gradle.kts

@@ -6,6 +6,7 @@ dependencies {
     implementation(projects.testngCollections) {
         because("Lists.newArrayList")
     }
+    implementation("org.checkerframework:checker-qual:3.36.0")
 
     testImplementation("org.testng:testng:7.3.0") {
         because("core depends on assertions and we need testng to test assertions")

testng-collections/src/main/java/org/testng/collections/Lists.java

@@ -3,6 +3,7 @@
 import java.util.*;
 import java.util.function.BiPredicate;
 import java.util.stream.Collectors;
+import org.checkerframework.checker.nullness.qual.NonNull;
 
 public final class Lists {
 
@@ -51,7 +52,7 @@ public static <K> List<K> newArrayList(int size) {
     return new ArrayList<>(size);
   }
 
-  public static <K> List<K> intersection(List<K> list1, List<K> list2) {
+  public static <K> List<K> intersection(List<@NonNull K> list1, List<@NonNull K> list2) {
     return list1.stream().filter(list2::contains).collect(Collectors.toList());
   }
 

testng-collections/src/main/java/org/testng/collections/Maps.java

@@ -1,5 +1,6 @@
 package org.testng.collections;
 
+import com.google.errorprone.annotations.InlineMe;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Hashtable;
@@ -13,6 +14,8 @@ public static <K, V> Map<K, V> newHashMap() {
     return new HashMap<>();
   }
 
+  @Deprecated
+  @InlineMe(replacement = "new Hashtable<>()", imports = "java.util.Hashtable")
   public static <K, V> Map<K, V> newHashtable() {
     return new Hashtable<>();
   }

testng-collections/src/main/java/org/testng/collections/MultiMap.java

@@ -5,9 +5,12 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import org.checkerframework.checker.nullness.qual.KeyFor;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public abstract class MultiMap<K, V, C extends Collection<V>> {
-  protected final Map<K, C> m_objects;
+  protected final @NonNull Map<@NonNull K, @NonNull C> m_objects;
 
   protected MultiMap(boolean isSorted) {
     if (isSorted) {
@@ -19,7 +22,7 @@ protected MultiMap(boolean isSorted) {
 
   protected abstract C createValue();
 
-  public boolean put(K key, V method) {
+  public boolean put(@NonNull K key, @NonNull V method) {
     AtomicBoolean exists = new AtomicBoolean(true);
     return m_objects
             .computeIfAbsent(
@@ -32,58 +35,57 @@ public boolean put(K key, V method) {
         && exists.get();
   }
 
-  public C get(K key) {
+  public C get(@NonNull K key) {
     return m_objects.computeIfAbsent(key, k -> createValue());
   }
 
-  public Set<K> keySet() {
+  public @NonNull Set<K> keySet() {
     return new HashSet<>(m_objects.keySet());
   }
 
-  public boolean containsKey(K k) {
+  public boolean containsKey(@NonNull K k) {
     return m_objects.containsKey(k);
   }
 
   @Override
   public String toString() {
     StringBuilder result = new StringBuilder();
-    Set<K> indices = keySet();
-    for (K i : indices) {
-      result.append("\n    ").append(i).append(" <-- ");
-      for (Object o : m_objects.get(i)) {
-        result.append(o).append(" ");
+    for (Map.Entry<K, C> entry : m_objects.entrySet()) {
+      result.append("\n    ").append(entry.getKey()).append(" <-- ");
+      for (V v : entry.getValue()) {
+        result.append(v).append(" ");
       }
     }
     return result.toString();
   }
 
   public boolean isEmpty() {
-    return m_objects.size() == 0;
+    return m_objects.isEmpty();
   }
 
   public int size() {
     return m_objects.size();
   }
 
-  public boolean remove(K key, V value) {
+  public boolean remove(@NonNull K key, @NonNull V value) {
     return get(key).remove(value);
   }
 
-  public C removeAll(K key) {
+  public @Nullable C removeAll(@NonNull K key) {
     return m_objects.remove(key);
   }
 
-  public Set<Map.Entry<K, C>> entrySet() {
+  public Set<Map.Entry<@NonNull @KeyFor("this.m_objects") K, @NonNull C>> entrySet() {
     return m_objects.entrySet();
   }
 
   public Collection<C> values() {
     return m_objects.values();
   }
 
-  public boolean putAll(K k, Collection<? extends V> values) {
+  public boolean putAll(@NonNull K k, @NonNull Collection<? extends @NonNull V> values) {
     boolean result = false;
-    for (V v : values) {
+    for (@NonNull V v : values) {
       result = put(k, v) || result;
     }
     return result;

testng-collections/src/main/java/org/testng/util/Strings.java

@@ -2,8 +2,8 @@
 
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.stream.Collectors;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.testng.collections.Maps;
 
 public final class Strings {
@@ -12,7 +12,10 @@ private Strings() {
   }
 
   public static boolean isNullOrEmpty(String string) {
-    return Optional.ofNullable(string).orElse("").trim().isEmpty();
+    if (string == null) {
+      return true;
+    }
+    return string.isEmpty();
   }
 
   public static boolean isNotNullAndNotEmpty(String string) {
@@ -52,7 +55,7 @@ public static String escapeHtml(String text) {
     return result;
   }
 
-  public static String valueOf(Map<?, ?> m) {
+  public static String valueOf(Map<?, @NonNull ?> m) {
     return m.values().stream().map(Object::toString).collect(Collectors.joining(" "));
   }
 

testng-collections/testng-collections-build.gradle.kts

@@ -1,3 +1,8 @@
 plugins {
     id("testng.java-library")
 }
+
+dependencies {
+    implementation("org.checkerframework:checker-qual:3.36.0")
+    implementation("com.google.errorprone:error_prone_annotations:2.20.0")
+}

testng-core-api/src/main/java/org/testng/IAttributes.java

@@ -5,8 +5,9 @@
 /** A trait that is used by all interfaces that lets the user add or remove their own attributes. */
 public interface IAttributes {
   /**
+   * Returns the attribute with the given name.
+   *
    * @param name The name of the attribute to return
-   * @return The attribute
    */
   Object getAttribute(String name);
 
@@ -18,7 +19,7 @@ public interface IAttributes {
    */
   void setAttribute(String name, Object value);
 
-  /** @return all the attributes names. */
+  /** Returns all the attributes names. */
   Set<String> getAttributeNames();
 
   /**

testng-core-api/src/main/java/org/testng/IClass.java

@@ -6,19 +6,19 @@
 /** <code>IClass</code> represents a test class and a collection of its instances. */
 public interface IClass {
 
-  /** @return this test class name. This is the name of the corresponding Java class. */
+  /** Returns this test class name. This is the name of the corresponding Java class. */
   String getName();
 
-  /** @return the &lt;test&gt; tag this class was found in. */
+  /** Returns the &lt;test&gt; tag this class was found in. */
   XmlTest getXmlTest();
 
-  /** @return the *lt;class&gt; tag this class was found in. */
+  /** Returns the *lt;class&gt; tag this class was found in. */
   XmlClass getXmlClass();
 
-  /** @return its test name if this class implements org.testng.ITest, null otherwise. */
+  /** Returns its test name if this class implements org.testng.ITest, null otherwise. */
   String getTestName();
 
-  /** @return the Java class corresponding to this IClass. */
+  /** Returns the Java class corresponding to this IClass. */
   Class<?> getRealClass();
 
   /**

testng-core-api/src/main/java/org/testng/IObjectFactory2.java

@@ -1,5 +1,7 @@
 package org.testng;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+
 /**
  * Factory used to create all test instances. This object factory only receives the class in
  * parameter.
@@ -17,7 +19,7 @@ public interface IObjectFactory2 extends ITestObjectFactory {
    * @return - The newly created object.
    */
   @Deprecated
-  default Object newInstance(Class<?> cls) {
+  default Object newInstance(@NonNull Class<?> cls) {
     return newInstance(cls, new Object[0]);
   }
 }