build(errorprone): promote the ten checks that have no sites left

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

@@ -12,8 +12,6 @@ dependencies {
 }
 
 tasks.withType<JavaCompile>().configureEach {
-    val testCompile = name.contains("Test")
-
     options.errorprone {
         disableWarningsInGeneratedCode.set(true)
 
@@ -24,7 +22,7 @@ tasks.withType<JavaCompile>().configureEach {
         // its jar is on the processor path, even for a task that disables the check below, and
         // NullAway refuses to start unless one of OnlyNullMarked or AnnotatedPackages is set.
         // Moving them into an else branch fails every test compile.
-        check("NullAway", CheckSeverity.ERROR)
+        error("NullAway")
         option("NullAway:OnlyNullMarked", true)
 
         // Without JSpecifyMode, NullAway reads declarations only and never looks inside a generic
@@ -33,22 +31,49 @@ tasks.withType<JavaCompile>().configureEach {
         // what makes @NullMarked mean what JSpecify says it means rather than roughly half of it.
         option("NullAway:JSpecifyMode", true)
 
-        if (testCompile) {
-            // SelfAssertion only fires on TestNG's own sample/fixture classes, where trivial
-            // assertions such as assertThat("abc").isEqualTo("abc") exist solely to give the
-            // runner a passing method. Production code keeps the check enabled.
-            disable("SelfAssertion")
-
-            // NullAway stays on here: @NullMarked is per package, not per source set, so twelve
-            // test packages are already marked by the main package-info.class on their compile
-            // classpath.
-            //
-            // HandleTestAssertionLibraries teaches NullAway that assertThat(x).isNotNull() refines
-            // x. It is keyed on the task name, so testng-test-kit -- test code that lives in a main
-            // source set -- does not get it, even though its org.testng.xml half is marked and
-            // checked today. That is inert only because the AssertJ use in that module sits in the
-            // unmarked test package, so nothing there refines a nullable value yet.
-            option("NullAway:HandleTestAssertionLibraries", true)
-        }
+        // Checks measured at zero unsuppressed sites, promoted from warning so the next
+        // violation stops the build instead of joining an output nobody reads. The sites that
+        // remain carry a @SuppressWarnings saying why. Finalize is here on new violations alone --
+        // both of its sites are suppressed, because the finalizers are what the leak test watches.
+        //
+        // Measure before adding one: javac caps at 100 warnings per compile task and several
+        // tasks here are past that, so an ordinary build undercounts. Nothing raises the cap, so
+        // count from a throwaway init script that adds -Xmaxwarns rather than from a plain build.
+        //
+        // Promoting also takes a check out of disableWarningsInGeneratedCode above: Error Prone
+        // only honours that exemption while the check is below ERROR. Nothing here generates Java
+        // today, so the list costs nothing; a module that adds a processor pays for it.
+        error(
+            "BadImport",
+            "BooleanLiteral",
+            "Finalize",
+            "InconsistentCapitalization",
+            "MissingOverride",
+            "NotJavadoc",
+            "StringCaseLocaleUsage",
+            "TypeParameterUnusedInFormals",
+            "UnnecessaryParentheses",
+            "UnusedVariable",
+        )
+
+        // Which compiles carry test code. The Error Prone plugin derives compilingTestOnlyCode
+        // from the *source set* name and lets a module override it, so a module whose main source
+        // set holds test fixtures can declare that rather than be classified by whether its task
+        // name happens to contain "Test". testng-test-kit is exactly that module.
+        //
+        // orElse: a JavaCompile task that belongs to no source set gets no convention, and feeding
+        // an absent provider to the options below would make them unresolvable.
+        val testCode = compilingTestOnlyCode.orElse(false)
+
+        // SelfAssertion only fires on TestNG's own sample/fixture classes, where trivial
+        // assertions such as assertThat("abc").isEqualTo("abc") exist solely to give the runner a
+        // passing method. Production code keeps the check enabled.
+        check("SelfAssertion", testCode.map { if (it) CheckSeverity.OFF else CheckSeverity.DEFAULT })
+
+        // NullAway stays on for test code: @NullMarked is per package, not per source set, so the
+        // test half of every marked main package is already marked by the package-info.class on
+        // its compile classpath. HandleTestAssertionLibraries is what teaches NullAway that
+        // assertThat(x).isNotNull() refines x.
+        option("NullAway:HandleTestAssertionLibraries", testCode.map { it.toString() })
     }
 }

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

@@ -11,6 +11,12 @@ default <T> T newInstance(Class<T> cls, Object... parameters) {
     return InstanceCreator.newInstance(cls, parameters);
   }
 
+  // The sibling overloads take Class<T> or Constructor<T>, so T is inferred from the argument.
+  // This one identifies the class by name, so nothing in the formals carries T and the caller
+  // picks it by assignment -- newInstance("com.acme.Bar") will happily fill a Foo variable and
+  // fail with a ClassCastException at the call site. That is what the check flags, and it is
+  // published API, so it is suppressed rather than obeyed.
+  @SuppressWarnings("TypeParameterUnusedInFormals")
   default <T> T newInstance(String clsName, Object... parameters) {
     return InstanceCreator.newInstance(clsName, parameters);
   }

testng-core-api/src/main/java/org/testng/annotations/ITestAnnotation.java

@@ -65,21 +65,27 @@ public interface ITestAnnotation extends ITestOrConfiguration, IDataProvidable {
 
   void setSingleThreaded(boolean f);
 
+  @Override
   String getDataProvider();
 
+  @Override
   void setDataProvider(String v);
 
   /**
    * @return The class holding the data provider, or {@code null} when neither the method nor
    *     anything it inherits from names one.
    */
+  @Override
   @Nullable
   Class<?> getDataProviderClass();
 
+  @Override
   void setDataProviderClass(@Nullable Class<?> v);
 
+  @Override
   String getDataProviderDynamicClass();
 
+  @Override
   void setDataProviderDynamicClass(String v);
 
   void setRetryAnalyzer(Class<? extends IRetryAnalyzer> c);

testng-core-api/src/main/java/org/testng/internal/PackageUtils.java

@@ -10,6 +10,7 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.Spliterator;
 import java.util.Spliterators;
@@ -108,8 +109,8 @@ public static String[] findClassesInPackage(
 
     for (int i = 0; i < classpathFragments.length; i++) {
       String path;
-      if (classpathFragments[i].toLowerCase().endsWith(".jar")
-          || classpathFragments[i].toLowerCase().endsWith(".zip")) {
+      String fragment = classpathFragments[i].toLowerCase(Locale.ROOT);
+      if (fragment.endsWith(".jar") || fragment.endsWith(".zip")) {
         path = classpathFragments[i] + "!/";
       } else {
         if (classpathFragments[i].endsWith(File.separator)) {

testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java

@@ -19,6 +19,10 @@ private InstanceCreator() {
     // Hide Constructor
   }
 
+  // Named by String rather than by Class<T>, so T appears only in the return type and the cast
+  // below is unchecked -- which is the check's complaint. It implements the ITestObjectFactory
+  // overload of the same shape, so the signature is not ours to change.
+  @SuppressWarnings("TypeParameterUnusedInFormals")
   public static <T> T newInstance(String className, Object... parameters) {
     Class<?> clazz = ClassHelper.forName(className);
     Objects.requireNonNull(clazz, "Could not find a valid class");

testng-core-api/src/main/java/org/testng/internal/protocols/Processor.java

@@ -4,6 +4,7 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.regex.Pattern;
 import org.testng.internal.Utils;
 
@@ -13,7 +14,7 @@ public abstract class Processor {
 
   public static Processor newInstance(String protocol) {
     Processor instance;
-    switch (protocol.toLowerCase()) {
+    switch (protocol.toLowerCase(Locale.ROOT)) {
       case "file":
         instance = new FileProcessor();
         break;
@@ -47,8 +48,8 @@ protected static List<String> findClassesInDirPackage(
         dir.listFiles(
             file ->
                 (recursive && file.isDirectory())
-                    || (file.getName().endsWith(".class"))
-                    || (file.getName().endsWith(".groovy")));
+                    || file.getName().endsWith(".class")
+                    || file.getName().endsWith(".groovy"));
 
     Utils.log(CLS_NAME, 4, "Looking for test classes in the directory: " + dir);
     if (dirfiles == null) {

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

@@ -9,6 +9,7 @@
 import java.util.Enumeration;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
@@ -94,7 +95,7 @@ private boolean testngXmlExistsInJar(File jarFile, List<String> classes) throws
       while (entries.hasMoreElements()) {
         JarEntry je = entries.nextElement();
         String jeName = je.getName();
-        if (Parser.canParse(jeName.toLowerCase())) {
+        if (Parser.canParse(jeName.toLowerCase(Locale.ROOT))) {
           InputStream inputStream = jf.getInputStream(je);
           File copyFile = new File(file, jeName);
           if (!copyFile.toPath().normalize().startsWith(file.toPath().normalize())) {

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

@@ -12,7 +12,6 @@
 import org.testng.internal.*;
 import org.testng.internal.annotations.IAnnotationFinder;
 import org.testng.internal.invokers.ConfigMethodArguments;
-import org.testng.internal.invokers.ConfigMethodArguments.Builder;
 import org.testng.internal.invokers.IInvocationStatus;
 import org.testng.internal.invokers.IInvoker;
 import org.testng.internal.invokers.InvokedMethod;
@@ -156,6 +155,7 @@ public <T> T newInstance(Class<T> cls, Object... parameters) {
             }
 
             @Override
+            @SuppressWarnings("TypeParameterUnusedInFormals") // signature fixed by the interface
             public <T> T newInstance(String clsName, Object... parameters) {
               try {
                 return suiteObjectFactory.newInstance(clsName, parameters);
@@ -241,6 +241,7 @@ public void setReportResults(boolean reportResults) {
     useDefaultListeners = reportResults;
   }
 
+  @Override
   public ITestListener getExitCodeListener() {
     return exitCodeListener;
   }
@@ -262,12 +263,9 @@ private void invokeListeners(boolean start) {
     }
   }
 
-  private void setOutputDir(String outputdir) {
-    if (isStringBlank(outputdir) && useDefaultListeners) {
-      outputdir = DEFAULT_OUTPUT_DIR;
-    }
-
-    outputDir = null != outputdir ? new File(outputdir).getAbsolutePath() : null;
+  private void setOutputDir(String dir) {
+    String resolved = isStringBlank(dir) && useDefaultListeners ? DEFAULT_OUTPUT_DIR : dir;
+    outputDir = null != resolved ? new File(resolved).getAbsolutePath() : null;
   }
 
   private ITestRunnerFactory buildRunnerFactory(Comparator<ITestNGMethod> comparator) {
@@ -361,7 +359,7 @@ private void privateRun() {
     if (invoker != null) {
       if (!beforeSuiteMethods.values().isEmpty()) {
         ConfigMethodArguments arguments =
-            new Builder()
+            new ConfigMethodArguments.Builder()
                 .usingConfigMethodsAs(beforeSuiteMethods.values())
                 .forSuite(xmlSuite)
                 .usingParameters(xmlSuite.getParameters())
@@ -389,7 +387,7 @@ private void privateRun() {
       //
       if (!afterSuiteMethods.values().isEmpty()) {
         ConfigMethodArguments arguments =
-            new Builder()
+            new ConfigMethodArguments.Builder()
                 .usingConfigMethodsAs(afterSuiteMethods.values())
                 .forSuite(xmlSuite)
                 .usingParameters(xmlSuite.getAllParameters())

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

@@ -175,7 +175,7 @@
 
   private @Nullable String m_jarPath;
   /** The path of the testng.xml file inside the jar file */
   private String m_xmlPathInJar = CommandLineArgs.XML_PATH_IN_JAR_DEFAULT;
 
   private List<String> m_stringSuites = new ArrayList<>();
   private final List<Class<? extends ITestNGListener>> m_listenerClasses = new ArrayList<>();
@@ -876,11 +876,11 @@
 
   private boolean m_ignoreMissedTestNames;
 
   private Integer m_suiteThreadPoolSize = CommandLineArgs.SUITE_THREAD_POOL_SIZE_DEFAULT;
 
-  private boolean m_randomizeSuites = Boolean.FALSE;
+  private boolean m_randomizeSuites = false;
 
-  private boolean m_alwaysRun = Boolean.TRUE;
+  private boolean m_alwaysRun = true;
 
   private Boolean m_preserveOrder = XmlSuite.DEFAULT_PRESERVE_ORDER;
   private @Nullable Boolean m_groupByInstances;
@@ -1807,7 +1807,7 @@
         (String)
             cmdLineArgs.getOrDefault(
                 CommandLineArgs.XML_PATH_IN_JAR, CommandLineArgs.XML_PATH_IN_JAR_DEFAULT);
-    result.mixed = (Boolean) cmdLineArgs.getOrDefault(CommandLineArgs.MIXED, Boolean.FALSE);
+    result.mixed = (Boolean) cmdLineArgs.getOrDefault(CommandLineArgs.MIXED, false);
     Object tmpValue = cmdLineArgs.get(CommandLineArgs.INCLUDE_ALL_DATA_DRIVEN_TESTS_WHEN_SKIPPING);
     if (tmpValue != null) {
       result.includeAllDataDrivenTestsWhenSkipping = Boolean.parseBoolean(tmpValue.toString());
@@ -1816,9 +1816,7 @@
         (Boolean) cmdLineArgs.get(CommandLineArgs.SKIP_FAILED_INVOCATION_COUNTS);
     result.failIfAllTestsSkipped =
         Boolean.parseBoolean(
-            cmdLineArgs
-                .getOrDefault(CommandLineArgs.FAIL_IF_ALL_TESTS_SKIPPED, Boolean.FALSE)
-                .toString());
+            cmdLineArgs.getOrDefault(CommandLineArgs.FAIL_IF_ALL_TESTS_SKIPPED, false).toString());
     result.spiListenersToSkip =
         (String) cmdLineArgs.getOrDefault(CommandLineArgs.LISTENERS_TO_SKIP_VIA_SPI, "");
     String parallelMode = (String) cmdLineArgs.get(CommandLineArgs.PARALLEL);

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

@@ -53,7 +53,6 @@
 import org.testng.internal.annotations.IAnnotationFinder;
 import org.testng.internal.invokers.AbstractParallelWorker;
 import org.testng.internal.invokers.ConfigMethodArguments;
-import org.testng.internal.invokers.ConfigMethodArguments.Builder;
 import org.testng.internal.invokers.IInvoker;
 import org.testng.internal.invokers.Invoker;
 import org.testng.internal.objects.IObjectDispenser;
@@ -668,7 +667,7 @@ private void beforeRun() {
   private void invokeTestConfigurations(ITestNGMethod[] testConfigurationMethods) {
     if (null != testConfigurationMethods && testConfigurationMethods.length > 0) {
       ConfigMethodArguments arguments =
-          new Builder()
+          new ConfigMethodArguments.Builder()
               .usingConfigMethodsAs(testConfigurationMethods)
               .forSuite(m_xmlTest.getSuite())
               .usingParameters(m_xmlTest.getAllParameters())

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

@@ -777,11 +777,11 @@ public void setRetryAnalyzerClass(Class<? extends IRetryAnalyzer> clazz) {
     m_retryAnalyzerClass = clazz == null ? DisabledRetryAnalyzer.class : clazz;
   }
 
-  @Override
   /**
    * @return the retry analyzer class, never null: it is {@link DisabledRetryAnalyzer} until a retry
    *     analyzer is set, and the setter normalises null back to it.
    */
+  @Override
   public Class<? extends IRetryAnalyzer> getRetryAnalyzerClass() {
     return m_retryAnalyzerClass;
   }

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

@@ -2,12 +2,8 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import org.testng.log4testng.Logger;
 
 public class DataProviderLoader extends ClassLoader {
-  private static final int BUFFER_SIZE = 1 << 20;
-  private static final Logger log = Logger.getLogger(DataProviderLoader.class);
-
   public Class loadClazz(String path) throws ClassNotFoundException {
     Class clazz = findLoadedClass(path);
     if (clazz == null) {