refactor: declare org.testng null-marked

CHANGES.txt

@@ -15,10 +15,47 @@ Changed: org.testng.internal.ClonedMethod.getConstructorOrMethod() returns the w
 Fixed: org.testng.internal.TestNGMethod.clone() no longer throws a NullPointerException when the method has not been bound to a test class yet. It wrapped getTestClass() in a NoOpTestClass, which dereferences it on the spot; the absence is now propagated, which is what ConfigurationMethod.clone() already did (Julien Herr)
 Fixed: A configuration method that is not a @BeforeGroups or @AfterGroups method now reports an empty array from getBeforeGroups() and getAfterGroups() instead of null. TestNGMethodFinder wrote null into fields whose declaration says {}, and MethodGroupsHelper iterates them without testing (Julien Herr)
 Fixed: Sorting test methods by index no longer throws a NullPointerException when a <test> tag carries no name (Julien Herr)
+Changed: org.testng is now declared @NullMarked, so every member of the published API states whether it can answer null. Thirty-seven members widen to @Nullable because their implementations already answered null, and the rest promise not to. This is binary compatible and source compatible for Java; a Kotlin caller that dereferences one of the thirty-seven without testing it stops compiling. They are listed under Possible backward incompatible changes below (Julien Herr)
+Fixed: org.testng.internal.MethodSorting.INSTANCES orders two invocations of the same method on different @Factory instances instead of leaving the decision to a hash code comparison. Its identity branch asked IInstanceIdentity.isIdentityAware about the ids it had just resolved rather than about the methods, which could never hold, so the branch had never run (Julien Herr)
+Fixed: org.testng.IAnnotationTransformer.transform(IFactoryAnnotation, Method) is now declared to accept a null method, which is what TestNG has always passed for a @Factory annotation found on a constructor (Julien Herr)
+Changed: org.testng.internal.MethodInstance.SORT_BY_INDEX no longer throws a NullPointerException when a method a @Factory produced belongs to no <test> tag. It answers that the two methods cannot be compared, which is what the neighbouring branch already answers for a missing <class> (Julien Herr)
+Changed: org.testng.internal.IInstanceIdentity.getInstanceId(Object) answers the new NO_INSTANCE token instead of null for a method that carries no instance, so the value can be used as a map key without every caller deciding what an absent key means. The grouping is unchanged: every method without an instance still lands in one bucket (Julien Herr)
+Fixed: In memory friendly mode (testng.memory.friendly), ITestNGMethod.getDataProviderMethod() answers null for a method that has no data provider, instead of a stand-in whose getName() answered an empty string and whose getMethod() threw UnsupportedOperationException. The interface has always documented null for that case, and the three call sites in TestNG already tested for it (Julien Herr)
 Fixed: A <package> tag that carries no name attribute is now reported the way an unreadable package already was, instead of raising a NullPointerException from inside PackageUtils.findClassesInPackage (Julien Herr)
 
 Possible backward incompatible changes:
 
+- org.testng is declared @NullMarked, and thirty-seven members of it answer @Nullable because that
+  is what their implementations already answered. ITestNGMethod.getTestClass, getInstance, getId,
+  getDescription, getMissingGroup, getXmlTest, getRetryAnalyzer, getDataProviderMethod and the
+  deprecated getFactoryMethodParamsInfo; ITestResult.getMethod, getName, getTestName, getInstance,
+  getInstanceName, getHost, getThrowable and getTestContext; IClass.getXmlTest, getXmlClass,
+  getTestName and getInstanceHashCodes; IAttributes.getAttribute and removeAttribute;
+  IDataProviderMethod.getInstance and getMethod; IMethodInstance.getInstance;
+  ITestClassFinder.getIClass; ITestNGListenerFactory.createListener;
+  ITestObjectFactory.newInstance(Constructor, Object...); ITestContext.getName, getEndDate,
+  getHost and getInjectorFactory; ISuite.getHost, getParameter, getParentInjector and
+  getObjectFactory. A Java caller is unaffected. A Kotlin caller that dereferences one of them
+  without testing it stops compiling, and must add a test or a !!.
+- The same mark widens the parameters of IAnnotationTransformer.transform for ITestAnnotation and
+  IConfigurationAnnotation -- testClass, testConstructor and testMethod, of which the javadoc has
+  always said only one is non-null -- of the four IConfigurationListener callbacks that take an
+  ITestNGMethod, of the three IDataProviderListener callbacks and IDataProviderInterceptor.intercept
+  that take an ITestContext, of IModuleFactory.createModule, of IMethodSelector.includeMethod, of IClass.getInstances'
+  error message prefix, and of Reporter.setCurrentTestResult, which TestNG calls with null to clear
+  the current result. A Java
+  implementation is unaffected. A Kotlin implementation whose override declares the parameter
+  non-null stops overriding and must add the question mark.
+- org.testng.internal.MethodSorting.INSTANCES, the default method order, produces a different order
+  for two invocations of the same method on different @Factory instances. Both the old and the new
+  order are arbitrary -- instance ids are random UUIDs -- so no run that did not already depend on
+  an arbitrary order is affected, but a run that pinned the old one will see it change.
+- org.testng.internal.IInstanceIdentity.getInstanceId(Object) answers IInstanceIdentity.NO_INSTANCE
+  rather than null for a method that carries no instance. Code testing the result for null must test
+  for the token instead. The package is internal and OSGi exported.
+- testng-core and testng-core-api no longer declare a compileOnly dependency on
+  com.github.spotbugs:spotbugs. Nothing in TestNG uses javax.annotation any more.
+
 - testng-failed.xml no longer records the index of a @Factory produced instance as an invocation-number. That attribute selects rows of a method's own data provider, which is the only thing TestNG ever reads it back as, so a factory powered failure produced a file that looked filtered and re-ran everything -- and, for a method that had a data provider of its own, re-ran the wrong rows because the factory index had overwritten the row index. The instance index now goes to the new factory-instances attribute of <include>, which is honoured on re-run. Tooling that parses testng-failed.xml to learn which factory instance failed must read factory-instances rather than invocation-numbers; a method with its own data provider now re-runs the rows that actually failed. A file generated by 7.13 and re-run by an older TestNG ignores the unknown attribute and re-runs every instance, which is what those versions already did. (GITHUB-3111, GITHUB-2517, GITHUB-2521)
 - The constructors of org.testng.internal.ParameterInfo and org.testng.internal.LazyParameterInfo now take an org.testng.internal.FactoryInstance instead of a loose index and parameter array. Both are implementation classes of an internal package; only code constructing them directly is affected. (GITHUB-3111)
 - org.testng.internal.ClonedMethod.getConstructorOrMethod() answers the wrapped method rather than null. Code that tested the result for null now takes the other branch; no caller in TestNG did, and the method's own toString() could never run before. ITestNGMethod.getConstructorOrMethod() therefore stays non-null when org.testng is marked in turn.

testng-cli/src/main/java/org/testng/cli/CliConfigurer.java

@@ -129,7 +129,9 @@ public static void configure(TestNG testng, CliOptions cli) {
         .map(it -> it.asSubclass(IExecutorServiceFactory.class))
         .ifPresent(testng::setExecutorServiceFactoryClass);
 
-    testng.setOutputDirectory(cli.outputDirectory);
+    if (cli.outputDirectory != null) {
+      testng.setOutputDirectory(cli.outputDirectory);
+    }
 
     String testClasses = cli.testClass;
     if (null != testClasses) {

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

@@ -2,6 +2,7 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
+import org.jspecify.annotations.Nullable;
 import org.testng.annotations.IConfigurationAnnotation;
 import org.testng.annotations.IDataProviderAnnotation;
 import org.testng.annotations.IFactoryAnnotation;
@@ -27,7 +28,10 @@ public interface IAnnotationTransformer extends ITestNGListener {
    *     method (null otherwise).
    */
   default void transform(
-      ITestAnnotation annotation, Class testClass, Constructor testConstructor, Method testMethod) {
+      ITestAnnotation annotation,
+      @Nullable Class testClass,
+      @Nullable Constructor testConstructor,
+      @Nullable Method testMethod) {
     // not implemented
   }
 
@@ -47,9 +51,9 @@ default void transform(
    */
   default void transform(
       IConfigurationAnnotation annotation,
-      Class testClass,
-      Constructor testConstructor,
-      Method testMethod) {
+      @Nullable Class testClass,
+      @Nullable Constructor testConstructor,
+      @Nullable Method testMethod) {
     // not implemented
   }
 
@@ -67,9 +71,10 @@ default void transform(IDataProviderAnnotation annotation, Method method) {
    * Transform an IFactory annotation.
    *
    * @param annotation The annotation factory
-   * @param method The method annotated with the IFactory annotation.
+   * @param method The method annotated with the IFactory annotation, or {@code null} when the
+   *     annotation was found on a constructor.
    */
-  default void transform(IFactoryAnnotation annotation, Method method) {
+  default void transform(IFactoryAnnotation annotation, @Nullable Method method) {
     // not implemented
   }
 

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

@@ -1,13 +1,15 @@
 package org.testng;
 
 import java.util.Set;
+import org.jspecify.annotations.Nullable;
 
 /** A trait that is used by all interfaces that lets the user add or remove their own attributes. */
 public interface IAttributes {
   /**
    * @param name The name of the attribute to return
-   * @return The attribute
+   * @return The attribute, or {@code null} when no attribute is registered under that name.
    */
+  @Nullable
   Object getAttribute(String name);
 
   /**
@@ -27,5 +29,6 @@ public interface IAttributes {
    * @param name The attribute name
    * @return the attribute value if found, null otherwise
    */
+  @Nullable
   Object removeAttribute(String name);
 }

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

@@ -1,5 +1,6 @@
 package org.testng;
 
+import org.jspecify.annotations.Nullable;
 import org.testng.xml.XmlClass;
 import org.testng.xml.XmlTest;
 
@@ -9,13 +10,22 @@ public interface IClass {
   /** @return this test class name. This is the name of the corresponding Java class. */
   String getName();
 
-  /** @return the <test> tag this class was found in. */
+  /**
+   * @return the <test> tag this class was found in, or {@code null} when it was not found in
+   *     one.
+   */
+  @Nullable
   XmlTest getXmlTest();
 
-  /** @return the *lt;class> tag this class was found in. */
+  /**
+   * @return the *lt;class> tag this class was found in, or {@code null} when it was not found in
+   *     one.
+   */
+  @Nullable
   XmlClass getXmlClass();
 
   /** @return its test name if this class implements org.testng.ITest, null otherwise. */
+  @Nullable
   String getTestName();
 
   /** @return the Java class corresponding to this IClass. */
@@ -43,13 +53,13 @@ public interface IClass {
    * @deprecated - As of TestNG <code>v7.10.0</code>
    */
   @Deprecated
-  default Object[] getInstances(boolean create, String errorMsgPrefix) {
+  default Object[] getInstances(boolean create, @Nullable String errorMsgPrefix) {
     return getInstances(create);
   }
 
   /** @deprecated - As of TestNG <code>v7.10.0</code> */
   @Deprecated
-  long[] getInstanceHashCodes();
+  long @Nullable [] getInstanceHashCodes();
 
   /**
    * @param instance - The instance to be added.

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

@@ -1,5 +1,7 @@
 package org.testng;
 
+import org.jspecify.annotations.Nullable;
+
 /** Listener interface for events related to configuration methods. */
 public interface IConfigurationListener extends ITestNGListener {
 
@@ -16,9 +18,9 @@ default void onConfigurationSuccess(ITestResult tr) {
    * Invoked whenever a configuration method succeeded.
    *
    * @param tr The test result
-   * @param tm The test method
+   * @param tm The test method, or {@code null} when the configuration method is not bound to one
    */
-  default void onConfigurationSuccess(ITestResult tr, ITestNGMethod tm) {
+  default void onConfigurationSuccess(ITestResult tr, @Nullable ITestNGMethod tm) {
     // not implemented
   }
 
@@ -35,9 +37,9 @@ default void onConfigurationFailure(ITestResult tr) {
    * Invoked whenever a configuration method failed.
    *
    * @param tr The test result
-   * @param tm The test method
+   * @param tm The test method, or {@code null} when the configuration method is not bound to one
    */
-  default void onConfigurationFailure(ITestResult tr, ITestNGMethod tm) {
+  default void onConfigurationFailure(ITestResult tr, @Nullable ITestNGMethod tm) {
     // not implemented
   }
 
@@ -54,9 +56,9 @@ default void onConfigurationSkip(ITestResult tr) {
    * Invoked whenever a configuration method was skipped.
    *
    * @param tr The test result
-   * @param tm The test method
+   * @param tm The test method, or {@code null} when the configuration method is not bound to one
    */
-  default void onConfigurationSkip(ITestResult tr, ITestNGMethod tm) {
+  default void onConfigurationSkip(ITestResult tr, @Nullable ITestNGMethod tm) {
     // not implemented
   }
 
@@ -73,9 +75,9 @@ default void beforeConfiguration(ITestResult tr) {
    * Invoked before a configuration method is invoked.
    *
    * @param tr The test result
-   * @param tm The test method
+   * @param tm The test method, or {@code null} when the configuration method is not bound to one
    */
-  default void beforeConfiguration(ITestResult tr, ITestNGMethod tm) {
+  default void beforeConfiguration(ITestResult tr, @Nullable ITestNGMethod tm) {
     // not implemented
   }
 }

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

@@ -1,6 +1,7 @@
 package org.testng;
 
 import java.util.Iterator;
+import org.jspecify.annotations.Nullable;
 
 /**
  * This interface helps define an interceptor for data providers. Implementations of this TestNG
@@ -25,5 +26,5 @@ Iterator<Object[]> intercept(
       Iterator<Object[]> original,
       IDataProviderMethod dataProviderMethod,
       ITestNGMethod method,
-      ITestContext iTestContext);
+      @Nullable ITestContext iTestContext);
 }

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

@@ -1,5 +1,7 @@
 package org.testng;
 
+import org.jspecify.annotations.Nullable;
+
 /** A listener that gets invoked before and after a data provider is invoked by TestNG. */
 public interface IDataProviderListener extends ITestNGListener {
 
@@ -12,7 +14,9 @@ public interface IDataProviderListener extends ITestNGListener {
    * @param iTestContext - The current test context
    */
   default void beforeDataProviderExecution(
-      IDataProviderMethod dataProviderMethod, ITestNGMethod method, ITestContext iTestContext) {
+      IDataProviderMethod dataProviderMethod,
+      ITestNGMethod method,
+      @Nullable ITestContext iTestContext) {
     // not implemented
   }
 
@@ -25,7 +29,9 @@ default void beforeDataProviderExecution(
    * @param iTestContext - The current test context
    */
   default void afterDataProviderExecution(
-      IDataProviderMethod dataProviderMethod, ITestNGMethod method, ITestContext iTestContext) {
+      IDataProviderMethod dataProviderMethod,
+      ITestNGMethod method,
+      @Nullable ITestContext iTestContext) {
     // not implemented
   }
 
@@ -39,7 +45,8 @@ default void afterDataProviderExecution(
    * @param t - The {@link RuntimeException} that embeds the actual exception. Use {@link
    *     RuntimeException#getCause()} to get to the actual exception.
    */
-  default void onDataProviderFailure(ITestNGMethod method, ITestContext ctx, RuntimeException t) {
+  default void onDataProviderFailure(
+      ITestNGMethod method, @Nullable ITestContext ctx, RuntimeException t) {
     // not implemented
   }
 }

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

@@ -2,19 +2,24 @@
 
 import java.lang.reflect.Method;
 import java.util.List;
+import org.jspecify.annotations.Nullable;
 
 /** Represents the attributes of a {@link org.testng.annotations.DataProvider} annotated method. */
 public interface IDataProviderMethod {
   /**
    * @return - The instance to which the data provider belongs to. <code>null</code> if the data
    *     provider is a static one.
    */
+  @Nullable
   Object getInstance();
 
   /**
    * @return - A {@link Method} object that represents the actual {@literal @}{@link
-   *     org.testng.annotations.DataProvider} method.
+   *     org.testng.annotations.DataProvider} method, or {@code null} once TestNG has released it --
+   *     which it does as soon as the data provider has yielded its rows, so that the method and its
+   *     instance do not outlive the run.
    */
+  @Nullable
   Method getMethod();
 
   /** @return The name of this DataProvider. */

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

@@ -3,7 +3,7 @@
 import com.google.inject.Injector;
 import com.google.inject.Module;
 import com.google.inject.Stage;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.Nullable;
 
 /** Allows customization of the {@link Injector} creation when working with dependency injection. */
 public interface IInjectorFactory {

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

@@ -1,9 +1,16 @@
 package org.testng;
 
+import org.jspecify.annotations.Nullable;
+
 /** This interface captures a test method along with all the instances it should be run on. */
 public interface IMethodInstance {
 
   ITestNGMethod getMethod();
 
+  /**
+   * @return The instance the method will be invoked on, or {@code null} when the method carries no
+   *     instance.
+   */
+  @Nullable
   Object getInstance();
 }

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

@@ -1,6 +1,7 @@
 package org.testng;
 
 import com.google.inject.Module;
+import org.jspecify.annotations.Nullable;
 
 /**
  * This interface is used by the moduleFactory attribute of the @Guice annotation. It allows users
@@ -13,5 +14,5 @@ public interface IModuleFactory {
    * @param testClass The test class
    * @return The Guice module that should be used to get an instance of this test class.
    */
-  Module createModule(ITestContext context, Class<?> testClass);
+  Module createModule(@Nullable ITestContext context, Class<?> testClass);
 }

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

@@ -4,6 +4,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import org.jspecify.annotations.Nullable;
 import org.testng.internal.annotations.IAnnotationFinder;
 import org.testng.xml.XmlSuite;
 
@@ -21,6 +22,7 @@ public interface ISuite extends IAttributes {
   Map<String, ISuiteResult> getResults();
 
   /** @return The object factory used to create all test instances. */
+  @Nullable
   ITestObjectFactory getObjectFactory();
 
   /** @return The output directory used for the reports. */
@@ -37,6 +39,7 @@ public interface ISuite extends IAttributes {
    * @param parameterName The name of the parameter
    * @return The value of this parameter, or null if none was specified.
    */
+  @Nullable
   String getParameter(String parameterName);
 
   /**
@@ -59,6 +62,7 @@ public interface ISuite extends IAttributes {
    * @return The host where this suite was run, or null if it was run locally. The returned string
    *     has the form: host:port
    */
+  @Nullable
   String getHost();
 
   /**
@@ -76,6 +80,7 @@ public interface ISuite extends IAttributes {
 
   void addListener(ITestNGListener listener);
 
+  @Nullable
   Injector getParentInjector();
 
   void setParentInjector(Injector injector);