From 820a627fdf6c4e7ea6da8b3afbd2b4e52b60b9d9 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Sun, 16 Aug 2026 23:02:47 +0200 Subject: [PATCH] refactor(internal.annotations): declare the package null-marked Thirty files in testng-core, three in testng-core-api -- the first package whose minority half is more than a single file, and the first one upstream of packages already marked and merged: org.testng.internal.invokers, org.testng.internal.objects, org.testng.annotations and org.testng.internal.objects.pojo all read it. Marking a leaf could only produce errors in its own files; this one could produce them in code already shipped. The package-info goes in testng-core-api, which testng-core depends on, and the per-module control confirms both halves are covered: a throwaway null-returning method is clean in both modules before the file and fails after it, at DisabledRetryAnalyzer.java:14 and, separately, at BaseAnnotation.java:9. Forty-nine errors -- one in testng-core-api, forty-eight in testng-core. 91 @Nullable, two Objects.requireNonNull and nine restructurings answer them. The shape of the package is a tag hierarchy -- BaseAnnotation, then TestOrConfiguration and the leaves TestAnnotation, FactoryAnnotation, DataProviderAnnotation, ListenersAnnotation -- with one producer, JDK15TagFactory, which constructs a tag and immediately fills every field from the Java annotation it mirrors. That producer decides how the "field not initialized" errors are answered. Where the interface already published a @Nullable getter the field takes the annotation; where it published a non-null one the field takes the default of the annotation member it mirrors, because that is what the tag factory assigns a line later and what every reader already treats as absent: "" for the data provider name and the two dataProvider strings, an empty list for indices, DisableDataProviderRetries for retryUsing, an empty array for @Listeners, and DisabledRetryAnalyzer for retryAnalyzer -- the value BaseTestMethod.setRetryAnalyzerClass already substitutes for null. Annotating those instead would have published nullity on six getters whose every caller dereferences them. IAnnotationFinder is the other half. Its javadoc has said "or null if none found" since it was written, and JDK15AnnotationFinder returns a literal null in two of the six overloads, so the implementation had to be @Nullable -- and NullAway then rejected it against the non-null interface. Those six returns are demanded, not documentation: NullAway is silent on type *arguments*, not on a @Nullable return that happens to be a type variable. AnnotationHelper's five delegates and both findConfiguration overloads follow from the same source. Downstream, ten files in the four marked packages read this package and two needed anything. ClassBasedParallelWorker.isSequential already tested its ITestAnnotation for null; #3380 had to read that guard as residue because findAnnotation was non-null, and it is now demanded. ConfigInvoker:308 passes the result of AnnotationHelper.findConfiguration to handleConfigurationSkip, which #3381 tightened to non-null; requireNonNull there records that a ConfigurationMethod only exists because TestNGMethodFinder:130 found a configuration annotation on that very method, through the same lookup. That settles the second deferred finding of #3381 -- handleConfigurationFailure keeps `null != annotation` on one path and Objects.requireNonNull on the other. Annotating the return makes the asymmetry compiler-visible instead of a reading, and it does not reproduce: that parameter is null only when the throw happened before the assignment, and the statements that precede it can only produce an NPE (requireNonNull on the instance) or a TestNGException (the annotation finder), neither of which passes isSkipExceptionAndSkip -- the sole gate to the requireNonNull path. The remaining guard is unreachable rather than wrong, so no issue is opened. DisabledRetryAnalyzer needs nothing, which is worth saying rather than leaving as a zero: it is public surface despite the package -- Test#retryAnalyzer() names it -- but its one method overrides IRetryAnalyzer.retry(ITestResult), which is unannotated, and NullAway never widens an implementation against an unannotated supertype. Of the other two files in the minority, IDataProvidable needed the getDataProviderClass pair, which its own subinterface ITestAnnotation had already declared @Nullable and therefore contradicted, and IAnnotationFinder needed the six returns plus the one class its own overload passes null for. findOptionalValues stays as it is: its javadoc describes null *elements*, which NullAway does not check. Three of the nine restructurings are not field defaults. JDK15AnnotationFinder returns early when findAnnotationInSuperClasses finds nothing, which is what the private overload it calls did on its first line, and keeps Pair's constructor non-null. JDK15TagFactory asserts the method it was handed when building a data provider tag -- @DataProvider is only ever looked up on a method, never on a class. ListenersAnnotation starts from an empty array instead of null. Every annotation is classified by deletion and recompilation, one at a time: 87 of the 91 bring back a named error. The four that do not stay, because dropping them would leave a contract contradicting itself. Two are interface parameters, which NullAway never checks an implementation against: IAnnotationFinder's @Nullable class, whose matching parameter in JDK15AnnotationFinder is demanded by the literal null its own overload passes, and IDataProvidable.setDataProviderClass, which ITestAnnotation already declares @Nullable and whose two implementations are demanded. The other two are JDK models NullAway reads optimistically: IgnoreListener walks up package names with Package.getPackage, and the private findAnnotation in JDK15AnnotationFinder takes the result of Method.getAnnotation. Both test their parameter on the first line of the body, and both are null on every lookup that finds nothing. One trap worth recording: a plain javac error anywhere in the module -- here a generic array creation -- stops the NullAway pass entirely, so the compiler reports zero NullAway errors on code full of them. A clean reading means nothing until the compile itself is clean. --- .../annotations/IAnnotationFinder.java | 16 ++++--- .../internal/annotations/IDataProvidable.java | 6 ++- .../internal/annotations/package-info.java | 5 ++ .../annotations/AnnotationHelper.java | 43 +++++++++-------- .../internal/annotations/BaseAnnotation.java | 13 +++--- .../internal/annotations/BaseBeforeAfter.java | 8 ++-- .../annotations/DataProviderAnnotation.java | 8 ++-- .../DefaultAnnotationTransformer.java | 7 ++- .../annotations/FactoryAnnotation.java | 17 +++---- .../annotations/IAnnotationTransformer.java | 9 ++-- .../annotations/IBaseBeforeAfter.java | 2 + .../internal/annotations/IgnoreListener.java | 15 +++--- .../annotations/JDK15AnnotationFinder.java | 46 +++++++++++-------- .../internal/annotations/JDK15TagFactory.java | 19 +++++--- .../annotations/ListenersAnnotation.java | 7 ++- .../internal/annotations/TestAnnotation.java | 11 +++-- .../annotations/TestOrConfiguration.java | 7 +-- .../invokers/ClassBasedParallelWorker.java | 3 +- .../internal/invokers/ConfigInvoker.java | 4 +- 19 files changed, 151 insertions(+), 95 deletions(-) create mode 100644 testng-core-api/src/main/java/org/testng/internal/annotations/package-info.java diff --git a/testng-core-api/src/main/java/org/testng/internal/annotations/IAnnotationFinder.java b/testng-core-api/src/main/java/org/testng/internal/annotations/IAnnotationFinder.java index c1754b1ab7..e7113e2e44 100644 --- a/testng-core-api/src/main/java/org/testng/internal/annotations/IAnnotationFinder.java +++ b/testng-core-api/src/main/java/org/testng/internal/annotations/IAnnotationFinder.java @@ -3,6 +3,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.annotations.IAnnotation; import org.testng.internal.ConstructorOrMethod; @@ -19,7 +20,7 @@ public interface IAnnotationFinder { * @param The expected {@link IAnnotation} type * @return The annotation on the class or null if none found. */ - A findAnnotation(Class cls, Class annotationClass); + @Nullable A findAnnotation(Class cls, Class annotationClass); /** * @param m - The corresponding {@link Method} @@ -28,14 +29,15 @@ public interface IAnnotationFinder { * @return The annotation on the method. If not found, return the annotation on the declaring * class. If not found, return null. */ - A findAnnotation(Method m, Class annotationClass); + @Nullable A findAnnotation(Method m, Class annotationClass); - A findAnnotation(ITestNGMethod m, Class annotationClass); + @Nullable A findAnnotation(ITestNGMethod m, Class annotationClass); - A findAnnotation(ConstructorOrMethod com, Class annotationClass); + @Nullable A findAnnotation( + ConstructorOrMethod com, Class annotationClass); - A findAnnotation( - Class clazz, Method m, java.lang.Class annotationClass); + @Nullable A findAnnotation( + @Nullable Class clazz, Method m, java.lang.Class annotationClass); /** * @param cons - The corresponding {@link Constructor} @@ -44,7 +46,7 @@ A findAnnotation( * @return The annotation on the method. If not found, return the annotation on the declaring * class. If not found, return null. */ - A findAnnotation(Constructor cons, Class annotationClass); + @Nullable A findAnnotation(Constructor cons, Class annotationClass); /** * @param cls - The corresponding class. diff --git a/testng-core-api/src/main/java/org/testng/internal/annotations/IDataProvidable.java b/testng-core-api/src/main/java/org/testng/internal/annotations/IDataProvidable.java index 075fd9cf3b..43c6c9c650 100644 --- a/testng-core-api/src/main/java/org/testng/internal/annotations/IDataProvidable.java +++ b/testng-core-api/src/main/java/org/testng/internal/annotations/IDataProvidable.java @@ -1,14 +1,18 @@ package org.testng.internal.annotations; +import org.jspecify.annotations.Nullable; + /** A trait shared by all the annotations that have dataProvider/dataProviderClass attributes. */ public interface IDataProvidable { String getDataProvider(); void setDataProvider(String v); + /** @return The class holding the data provider, or {@code null} when none was named. */ + @Nullable Class getDataProviderClass(); - void setDataProviderClass(Class v); + void setDataProviderClass(@Nullable Class v); String getDataProviderDynamicClass(); diff --git a/testng-core-api/src/main/java/org/testng/internal/annotations/package-info.java b/testng-core-api/src/main/java/org/testng/internal/annotations/package-info.java new file mode 100644 index 0000000000..a24c81736d --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/internal/annotations/package-info.java @@ -0,0 +1,5 @@ +/** Reads TestNG's annotations off classes, methods and constructors, and models what they say. */ +@NullMarked +package org.testng.internal.annotations; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-core/src/main/java/org/testng/internal/annotations/AnnotationHelper.java b/testng-core/src/main/java/org/testng/internal/annotations/AnnotationHelper.java index 1cb7a9cd04..05719c4547 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/AnnotationHelper.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/AnnotationHelper.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Objects; import java.util.function.Predicate; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.ITestObjectFactory; import org.testng.annotations.IAnnotation; @@ -63,27 +64,32 @@ private AnnotationHelper() { // Utility class.defeat instantiation. } - public static ITestAnnotation findTest(IAnnotationFinder finder, Class cls) { + public static @Nullable ITestAnnotation findTest(IAnnotationFinder finder, Class cls) { return finder.findAnnotation(cls, ITestAnnotation.class); } - public static ITestAnnotation findTest(IAnnotationFinder finder, Method m) { + public static @Nullable ITestAnnotation findTest(IAnnotationFinder finder, Method m) { return finder.findAnnotation(m, ITestAnnotation.class); } - public static ITestAnnotation findTest(IAnnotationFinder finder, ITestNGMethod m) { + public static @Nullable ITestAnnotation findTest(IAnnotationFinder finder, ITestNGMethod m) { return finder.findAnnotation(m, ITestAnnotation.class); } - public static IFactoryAnnotation findFactory(IAnnotationFinder finder, Method m) { + public static @Nullable IFactoryAnnotation findFactory(IAnnotationFinder finder, Method m) { return finder.findAnnotation(m, IFactoryAnnotation.class); } - public static IFactoryAnnotation findFactory(IAnnotationFinder finder, Constructor c) { + public static @Nullable IFactoryAnnotation findFactory( + IAnnotationFinder finder, Constructor c) { return finder.findAnnotation(c, IFactoryAnnotation.class); } - public static IConfigurationAnnotation findConfiguration( + /** + * @return The configuration annotation carried by the method, or {@code null} when it carries + * none. + */ + public static @Nullable IConfigurationAnnotation findConfiguration( IAnnotationFinder finder, ConstructorOrMethod m) { IConfigurationAnnotation result = null; boolean ignoreFailure = false; @@ -137,21 +143,22 @@ public static IConfigurationAnnotation findConfiguration( return result; } - public static IConfigurationAnnotation findConfiguration(IAnnotationFinder finder, Method m) { + public static @Nullable IConfigurationAnnotation findConfiguration( + IAnnotationFinder finder, Method m) { return findConfiguration(finder, new ConstructorOrMethod(m)); } private static IConfigurationAnnotation createConfiguration( - IConfigurationAnnotation bs, - IConfigurationAnnotation as, - IConfigurationAnnotation bt, - IConfigurationAnnotation at, - IConfigurationAnnotation bg, - IConfigurationAnnotation ag, - IConfigurationAnnotation bc, - IConfigurationAnnotation ac, - IConfigurationAnnotation bm, - IConfigurationAnnotation am) { + @Nullable IConfigurationAnnotation bs, + @Nullable IConfigurationAnnotation as, + @Nullable IConfigurationAnnotation bt, + @Nullable IConfigurationAnnotation at, + @Nullable IConfigurationAnnotation bg, + @Nullable IConfigurationAnnotation ag, + @Nullable IConfigurationAnnotation bc, + @Nullable IConfigurationAnnotation ac, + @Nullable IConfigurationAnnotation bm, + @Nullable IConfigurationAnnotation am) { ConfigurationAnnotation result = new ConfigurationAnnotation(); if (bs != null) { @@ -306,7 +313,7 @@ && isAnnotationPresent(annotationFinder, cls, ITestAnnotation.class)) { return vResult.values().toArray(new ITestNGMethod[0]); } - public static A findAnnotationSuperClasses( + public static @Nullable A findAnnotationSuperClasses( Class annotationClass, Class parameterClass) { Class c = parameterClass; while (c != null) { diff --git a/testng-core/src/main/java/org/testng/internal/annotations/BaseAnnotation.java b/testng-core/src/main/java/org/testng/internal/annotations/BaseAnnotation.java index ac43d7f6f3..6ec14f8e9f 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/BaseAnnotation.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/BaseAnnotation.java @@ -2,14 +2,15 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; +import org.jspecify.annotations.Nullable; public class BaseAnnotation { - private Class m_testClass; - private Method m_method; - private Constructor m_constructor; + private @Nullable Class m_testClass; + private @Nullable Method m_method; + private @Nullable Constructor m_constructor; - public Constructor getConstructor() { + public @Nullable Constructor getConstructor() { return m_constructor; } @@ -17,7 +18,7 @@ public void setConstructor(Constructor constructor) { m_constructor = constructor; } - public Method getMethod() { + public @Nullable Method getMethod() { return m_method; } @@ -25,7 +26,7 @@ public void setMethod(Method method) { m_method = method; } - public Class getTestClass() { + public @Nullable Class getTestClass() { return m_testClass; } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/BaseBeforeAfter.java b/testng-core/src/main/java/org/testng/internal/annotations/BaseBeforeAfter.java index c4383eafe8..ac14ee83eb 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/BaseBeforeAfter.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/BaseBeforeAfter.java @@ -1,22 +1,24 @@ package org.testng.internal.annotations; +import org.jspecify.annotations.Nullable; + public class BaseBeforeAfter extends TestOrConfiguration implements IBaseBeforeAfter { private boolean m_alwaysRun = false; private boolean m_inheritGroups = true; private String[] m_beforeGroups = {}; private String[] m_afterGroups = {}; - private String m_description; + private @Nullable String m_description; /** @return the description */ @Override - public String getDescription() { + public @Nullable String getDescription() { return m_description; } /** @param description the description to set */ @Override - public void setDescription(String description) { + public void setDescription(@Nullable String description) { m_description = description; } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/DataProviderAnnotation.java b/testng-core/src/main/java/org/testng/internal/annotations/DataProviderAnnotation.java index 680bcb6fb2..900ad921cc 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/DataProviderAnnotation.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/DataProviderAnnotation.java @@ -1,5 +1,6 @@ package org.testng.internal.annotations; +import java.util.Collections; import java.util.List; import org.testng.IRetryDataProvider; import org.testng.annotations.IDataProviderAnnotation; @@ -7,11 +8,12 @@ /** An implementation of IDataProvider. */ public class DataProviderAnnotation extends BaseAnnotation implements IDataProviderAnnotation { - private String m_name; + private String m_name = ""; private boolean m_parallel; - private List m_indices; + private List m_indices = Collections.emptyList(); private boolean m_bubbleUpFailures = false; - private Class retryUsing; + private Class retryUsing = + IRetryDataProvider.DisableDataProviderRetries.class; private boolean cachedDataForTestRetries = true; diff --git a/testng-core/src/main/java/org/testng/internal/annotations/DefaultAnnotationTransformer.java b/testng-core/src/main/java/org/testng/internal/annotations/DefaultAnnotationTransformer.java index d72a57afde..9eb899cdbb 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/DefaultAnnotationTransformer.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/DefaultAnnotationTransformer.java @@ -2,6 +2,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; +import org.jspecify.annotations.Nullable; import org.testng.IAnnotationTransformer; import org.testng.annotations.ITestAnnotation; @@ -15,7 +16,11 @@ public void transform( @Override public void transform( - ITestAnnotation annotation, Class testClass, Constructor cons, Method tm, Class clazz) { + ITestAnnotation annotation, + @Nullable Class testClass, + @Nullable Constructor cons, + @Nullable Method tm, + @Nullable Class clazz) { super.transform(annotation, testClass, cons, tm, clazz); } } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java b/testng-core/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java index 3feddb35e5..e2f098097d 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java @@ -1,17 +1,18 @@ package org.testng.internal.annotations; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.annotations.IFactoryAnnotation; import org.testng.annotations.Lazy; /** An implementation of IFactory */ public class FactoryAnnotation extends BaseAnnotation implements IFactoryAnnotation { - private String m_dataProvider = null; - private Class m_dataProviderClass; - private String m_dataProviderDynamicClass; + private String m_dataProvider = ""; + private @Nullable Class m_dataProviderClass; + private String m_dataProviderDynamicClass = ""; private boolean m_enabled = true; - private List m_indices; + private @Nullable List m_indices; private Lazy m_lazy = Lazy.UNSET; @Override @@ -24,12 +25,12 @@ public void setDataProvider(String dataProvider) { m_dataProvider = dataProvider; } - public void setDataProviderClass(Class dataProviderClass) { + public void setDataProviderClass(@Nullable Class dataProviderClass) { m_dataProviderClass = dataProviderClass; } @Override - public Class getDataProviderClass() { + public @Nullable Class getDataProviderClass() { return m_dataProviderClass; } @@ -54,7 +55,7 @@ public void setEnabled(boolean enabled) { } @Override - public List getIndices() { + public @Nullable List getIndices() { return m_indices; } @@ -69,7 +70,7 @@ public Lazy getLazy() { } @Override - public void setLazy(Lazy lazy) { + public void setLazy(@Nullable Lazy lazy) { m_lazy = lazy == null ? Lazy.UNSET : lazy; } } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/IAnnotationTransformer.java b/testng-core/src/main/java/org/testng/internal/annotations/IAnnotationTransformer.java index d3fa9fd035..3be32d839d 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/IAnnotationTransformer.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/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.ITestAnnotation; /** For backward compatibility. */ @@ -9,10 +10,10 @@ public interface IAnnotationTransformer extends org.testng.IAnnotationTransforme default void transform( ITestAnnotation annotation, - Class testClass, - Constructor testConstructor, - Method testMethod, - Class occurringClazz) { + @Nullable Class testClass, + @Nullable Constructor testConstructor, + @Nullable Method testMethod, + @Nullable Class occurringClazz) { // not implemented } } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/IBaseBeforeAfter.java b/testng-core/src/main/java/org/testng/internal/annotations/IBaseBeforeAfter.java index 240bc3eddb..78080c6f59 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/IBaseBeforeAfter.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/IBaseBeforeAfter.java @@ -1,5 +1,6 @@ package org.testng.internal.annotations; +import org.jspecify.annotations.Nullable; import org.testng.annotations.ITestOrConfiguration; /** Base interface for IBeforeSuite, IAfterSuite, etc... */ @@ -46,6 +47,7 @@ public interface IBaseBeforeAfter extends ITestOrConfiguration { * The description for this method. The string used will appear in the HTML report and also on * standard output if verbose > 2. */ + @Nullable String getDescription(); default boolean ignoreFailure() { diff --git a/testng-core/src/main/java/org/testng/internal/annotations/IgnoreListener.java b/testng-core/src/main/java/org/testng/internal/annotations/IgnoreListener.java index 8af2a78903..e87150c9c5 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/IgnoreListener.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/IgnoreListener.java @@ -3,6 +3,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.Arrays; +import org.jspecify.annotations.Nullable; import org.testng.annotations.ITestAnnotation; import org.testng.annotations.Ignore; import org.testng.internal.reflect.ReflectionHelper; @@ -18,10 +19,10 @@ public void transform( @Override public void transform( ITestAnnotation annotation, - Class testClass, - Constructor tc, - Method testMethod, - Class clazz) { + @Nullable Class testClass, + @Nullable Constructor tc, + @Nullable Method testMethod, + @Nullable Class clazz) { if (!annotation.getEnabled()) { return; } @@ -34,7 +35,7 @@ public void transform( ignoreTestAtClass(clazz, annotation); } - private static void ignoreTestAtClass(Class clazz, ITestAnnotation annotation) { + private static void ignoreTestAtClass(@Nullable Class clazz, ITestAnnotation annotation) { if (clazz != null) { ignoreTest(annotation, ReflectionHelper.findAnnotation(clazz, Ignore.class)); Package testPackage = clazz.getPackage(); @@ -44,7 +45,7 @@ private static void ignoreTestAtClass(Class clazz, ITestAnnotation annotation } } - private static void ignoreTest(ITestAnnotation annotation, Ignore ignore) { + private static void ignoreTest(ITestAnnotation annotation, @Nullable Ignore ignore) { if (ignore == null) { return; } @@ -66,7 +67,7 @@ private static void updateDescription(ITestAnnotation annotation, Ignore ignore) } @SuppressWarnings("deprecation") - private static Ignore findAnnotation(Package testPackage) { + private static @Nullable Ignore findAnnotation(@Nullable Package testPackage) { if (testPackage == null) { return null; } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java b/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java index 8651ae1c43..ad612e9664 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.Nullable; import org.testng.IAnnotationTransformer; import org.testng.ITestNGMethod; import org.testng.annotations.AfterClass; @@ -75,7 +76,8 @@ public JDK15AnnotationFinder(IAnnotationTransformer transformer) { m_annotationMap.put(IAfterMethod.class, AfterMethod.class); } - private A findAnnotationInSuperClasses(Class cls, Class a) { + private @Nullable A findAnnotationInSuperClasses( + Class cls, Class a) { // Hack for @Listeners: we don't look in superclasses for this annotation // because inheritance of this annotation causes aggregation instead of // overriding @@ -98,13 +100,13 @@ private A findAnnotationInSuperClasses(Class cls, Clas } @Override - public A findAnnotation(Method m, Class annotationClass) { + public @Nullable A findAnnotation(Method m, Class annotationClass) { return findAnnotation(null, m, annotationClass); } @Override - public A findAnnotation( - Class clazz, Method m, Class annotationClass) { + public @Nullable A findAnnotation( + @Nullable Class clazz, Method m, Class annotationClass) { final Class a = m_annotationMap.get(annotationClass); if (a == null) { throw new IllegalArgumentException( @@ -123,7 +125,8 @@ public A findAnnotation( } @Override - public A findAnnotation(ITestNGMethod tm, Class annotationClass) { + public @Nullable A findAnnotation( + ITestNGMethod tm, Class annotationClass) { final Class a = m_annotationMap.get(annotationClass); if (a == null) { throw new IllegalArgumentException( @@ -147,7 +150,7 @@ public A findAnnotation(ITestNGMethod tm, Class annot } @Override - public A findAnnotation( + public @Nullable A findAnnotation( ConstructorOrMethod com, Class annotationClass) { if (com.getConstructor() != null) { return findAnnotation(com.getConstructor(), annotationClass); @@ -159,11 +162,11 @@ public A findAnnotation( } private void transform( - IAnnotation a, - Class testClass, - Constructor testConstructor, - Method testMethod, - Class whichClass) { + @Nullable IAnnotation a, + @Nullable Class testClass, + @Nullable Constructor testConstructor, + @Nullable Method testMethod, + @Nullable Class whichClass) { if (!m_transformer.isEnabled()) { return; } @@ -192,19 +195,24 @@ private void transform( } @Override - public A findAnnotation(Class cls, Class annotationClass) { + public @Nullable A findAnnotation( + Class cls, Class annotationClass) { final Class a = m_annotationMap.get(annotationClass); if (a == null) { throw new IllegalArgumentException( "Java @Annotation class for '" + annotationClass + "' not found."); } Annotation annotation = findAnnotationInSuperClasses(cls, a); + if (annotation == null) { + return null; + } return findAnnotation( cls, annotation, annotationClass, cls, null, null, new Pair<>(annotation, cls), null); } @Override - public A findAnnotation(Constructor cons, Class annotationClass) { + public @Nullable A findAnnotation( + Constructor cons, Class annotationClass) { final Class a = m_annotationMap.get(annotationClass); if (a == null) { throw new IllegalArgumentException( @@ -266,15 +274,15 @@ private void findSuperInterface( } } - private A findAnnotation( + private @Nullable A findAnnotation( Class cls, - Annotation a, + @Nullable Annotation a, Class annotationClass, - Class testClass, - Constructor testConstructor, - Method testMethod, + @Nullable Class testClass, + @Nullable Constructor testConstructor, + @Nullable Method testMethod, Pair p, - Class whichClass) { + @Nullable Class whichClass) { if (a == null) { return null; } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java b/testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java index b67ec38af0..895908dd0c 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java @@ -6,7 +6,9 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.annotations.AfterClass; import org.testng.annotations.AfterGroups; @@ -41,13 +43,15 @@ */ public class JDK15TagFactory { - public A createTag( - Class cls, Method method, Annotation a, Class annotationClass) { + public @Nullable A createTag( + Class cls, @Nullable Method method, Annotation a, Class annotationClass) { IAnnotation result = null; if (a != null) { if (annotationClass == IDataProviderAnnotation.class) { - result = createDataProviderTag(method, a); + Method dataProviderMethod = + Objects.requireNonNull(method, "@DataProvider is only ever looked up on a method"); + result = createDataProviderTag(dataProviderMethod, a); } else if (annotationClass == IFactoryAnnotation.class) { result = createFactoryTag(cls, a); } else if (annotationClass == IParametersAnnotation.class) { @@ -80,7 +84,8 @@ public A createTag( return (A) result; } - private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class annotationClass) { + private @Nullable IAnnotation maybeCreateNewConfigurationTag( + Annotation a, Class annotationClass) { IAnnotation result = null; if (annotationClass == IBeforeSuite.class) { @@ -577,7 +582,7 @@ private String[] join(String[] strings, String[] strings2) { * annotation don't allow nulls, so each type has a different way of defining its own default. */ interface Default { - boolean isDefault(T t); + boolean isDefault(@Nullable T t); } private static final Default> DEFAULT_CLASS = c -> c == Object.class; @@ -590,7 +595,7 @@ interface Default { * the hierarchy (Object). */ @SuppressWarnings("unchecked") - private T findInherited( + private @Nullable T findInherited( T methodValue, Class cls, Class annotationClass, @@ -642,7 +647,7 @@ private String[] findInheritedStringArray(Class cls, String methodName) { return result.toArray(new String[0]); } - private Object invokeMethod(Annotation test, String methodName) { + private @Nullable Object invokeMethod(Annotation test, String methodName) { Object result = null; try { // Note: we should cache methods already looked up diff --git a/testng-core/src/main/java/org/testng/internal/annotations/ListenersAnnotation.java b/testng-core/src/main/java/org/testng/internal/annotations/ListenersAnnotation.java index 658d827634..53aae91c05 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/ListenersAnnotation.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/ListenersAnnotation.java @@ -5,7 +5,12 @@ public class ListenersAnnotation implements IListeners, IAnnotation { - private Class[] m_value; + @SuppressWarnings("unchecked") + private static final Class[] NO_LISTENERS = + (Class[]) new Class[0]; + + // Listeners#value() defaults to {}; JDK15TagFactory sets the real value right after construction. + private Class[] m_value = NO_LISTENERS; @Override public Class[] getValue() { diff --git a/testng-core/src/main/java/org/testng/internal/annotations/TestAnnotation.java b/testng-core/src/main/java/org/testng/internal/annotations/TestAnnotation.java index cbdf3d26d1..d153bb49f2 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/TestAnnotation.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/TestAnnotation.java @@ -1,5 +1,6 @@ package org.testng.internal.annotations; +import org.jspecify.annotations.Nullable; import org.testng.IRetryAnalyzer; import org.testng.annotations.CustomAttribute; import org.testng.annotations.ITestAnnotation; @@ -18,9 +19,9 @@ public class TestAnnotation extends TestOrConfiguration implements ITestAnnotati private String m_suiteName = ""; private String m_testName = ""; private boolean m_singleThreaded = false; - private Class m_dataProviderClass = null; - private String m_dataProviderDynamicClass = null; - private Class m_retryAnalyzerClass = null; + private @Nullable Class m_dataProviderClass; + private String m_dataProviderDynamicClass = ""; + private Class m_retryAnalyzerClass = DisabledRetryAnalyzer.class; private boolean m_skipFailedInvocations = false; private boolean m_ignoreMissingDependencies = false; private CustomAttribute[] m_attributes = {}; @@ -58,12 +59,12 @@ public void setDataProvider(String dataProvider) { } @Override - public Class getDataProviderClass() { + public @Nullable Class getDataProviderClass() { return m_dataProviderClass; } @Override - public void setDataProviderClass(Class dataProviderClass) { + public void setDataProviderClass(@Nullable Class dataProviderClass) { m_dataProviderClass = dataProviderClass; } diff --git a/testng-core/src/main/java/org/testng/internal/annotations/TestOrConfiguration.java b/testng-core/src/main/java/org/testng/internal/annotations/TestOrConfiguration.java index f924b887e0..9141792912 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/TestOrConfiguration.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/TestOrConfiguration.java @@ -1,5 +1,6 @@ package org.testng.internal.annotations; +import org.jspecify.annotations.Nullable; import org.testng.annotations.ITestOrConfiguration; public class TestOrConfiguration extends BaseAnnotation implements ITestOrConfiguration { @@ -8,7 +9,7 @@ public class TestOrConfiguration extends BaseAnnotation implements ITestOrConfig private boolean m_enabled = true; private String[] m_dependsOnGroups = {}; private String[] m_dependsOnMethods = {}; - private String m_description = ""; + private @Nullable String m_description = ""; private int m_priority; private long m_timeOut = 0; @@ -38,7 +39,7 @@ public void setGroups(String[] groups) { } @Override - public String getDescription() { + public @Nullable String getDescription() { return m_description; } @@ -58,7 +59,7 @@ public String[] getDependsOnMethods() { } @Override - public void setDescription(String description) { + public void setDescription(@Nullable String description) { m_description = description; } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java index 427b86686e..d53f28b4f0 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java @@ -9,6 +9,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.IMethodInstance; import org.testng.ITestNGMethod; import org.testng.internal.MethodInstance; @@ -112,7 +113,7 @@ private List methodsToMultipleMethodInstances(ITestNGMethod... m } private static boolean isSequential( - org.testng.annotations.ITestAnnotation test, XmlTest xmlTest) { + org.testng.annotations.@Nullable ITestAnnotation test, XmlTest xmlTest) { return test != null && test.getSingleThreaded() || XmlSuite.ParallelMode.CLASSES.equals(xmlTest.getParallel()); } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java index b3840dc953..9d83ec95cb 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java @@ -305,7 +305,9 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { handleConfigurationSkip( tm, testResult, - configurationAnnotation, + Objects.requireNonNull( + configurationAnnotation, + "a configuration method always carries a @Before/@After annotation"), arguments.getTestMethod(), arguments.getInstance(), arguments.getSuite());