diff --git a/testng-core-api/src/main/java/org/testng/xml/DefaultXmlWeaver.java b/testng-core-api/src/main/java/org/testng/xml/DefaultXmlWeaver.java index 099c7003f..f4badef8a 100644 --- a/testng-core-api/src/main/java/org/testng/xml/DefaultXmlWeaver.java +++ b/testng-core-api/src/main/java/org/testng/xml/DefaultXmlWeaver.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.Properties; import javax.xml.XMLConstants; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.internal.Utils; import org.testng.reporters.XMLStringBuffer; @@ -80,7 +81,7 @@ public class DefaultXmlWeaver implements IWeaveXml { /** Immutable, so a single instance can serve every {@code asXmlFragment} call. */ private static final DefaultXmlWeaver LEGACY_FRAGMENT_WEAVER = new DefaultXmlWeaver(); - private final String defaultComment; + private final @Nullable String defaultComment; /** Writes the name of each named tag as a trailing XML comment, as TestNG always has. */ public DefaultXmlWeaver() { @@ -92,7 +93,7 @@ public DefaultXmlWeaver() { * tag's own {@code name} attribute. Pass the empty string to write no comment at all, which * is what {@link CommentDisabledXmlWeaver} does. */ - protected DefaultXmlWeaver(String defaultComment) { + protected DefaultXmlWeaver(@Nullable String defaultComment) { this.defaultComment = defaultComment; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlClass.java b/testng-core-api/src/main/java/org/testng/xml/XmlClass.java index 47d5c2587..60d9444f9 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlClass.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlClass.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.collections.Objects; import org.testng.internal.ClassHelper; @@ -13,22 +14,24 @@ public class XmlClass implements Cloneable { private List m_includedMethods = new ArrayList<>(); private List m_excludedMethods = new ArrayList<>(); - private String m_name = null; - private Class m_class = null; + // Assigned by init, which every constructor calls directly: NullAway traces an initializer + // helper one hop only, so reaching init through a delegating overload stops it seeing this. + private String m_name; + private @Nullable Class m_class; /** The index of this class in the <test> tag */ private int m_index; /** True if the classes need to be loaded */ private boolean m_loadClasses = true; private Map m_parameters = new HashMap<>(); - private XmlTest m_xmlTest; + private @Nullable XmlTest m_xmlTest; public XmlClass() { init("", null, 0, false /* load classes */); } public XmlClass(String name) { - init(name, null, 0); + init(name, null, 0, true /* load classes */); } public XmlClass(String name, boolean loadClasses) { @@ -51,11 +54,7 @@ public XmlClass(String className, int index, boolean loadClasses) { init(className, null, index, loadClasses); } - private void init(String className, Class cls, int index) { - init(className, cls, index, true /* load classes */); - } - - private void init(String className, Class cls, int index, boolean resolveClass) { + private void init(String className, @Nullable Class cls, int index, boolean resolveClass) { m_name = className; m_class = cls; m_index = index; @@ -65,20 +64,21 @@ private void init(String className, Class cls, int index, boolean resolveClass) } } - private void loadClass() { - m_class = ClassHelper.forName(m_name); + /** Resolves {@link #m_name}, caches it in {@link #m_class} and hands it back. */ + private Class loadClass() { + Class cls = ClassHelper.forName(m_name); - if (null == m_class) { + if (null == cls) { throw new TestNGException("Cannot find class in classpath: " + m_name); } + m_class = cls; + return cls; } /** @return Returns the className. */ public Class getSupportClass() { - if (m_class == null) { - loadClass(); - } - return m_class; + Class cls = m_class; + return cls == null ? loadClass() : cls; } /** @param className The className to set. */ diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlDefine.java b/testng-core-api/src/main/java/org/testng/xml/XmlDefine.java index b3b46adf2..1c0f0c678 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlDefine.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlDefine.java @@ -2,16 +2,17 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; public class XmlDefine { - private String m_name; + private @Nullable String m_name; - public void setName(String name) { + public void setName(@Nullable String name) { m_name = name; } - public String getName() { + public @Nullable String getName() { return m_name; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlGroups.java b/testng-core-api/src/main/java/org/testng/xml/XmlGroups.java index afca78d0d..643dffa5c 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlGroups.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlGroups.java @@ -2,11 +2,12 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; public class XmlGroups { private List m_defines = new ArrayList<>(); - private XmlRun m_run; + private @Nullable XmlRun m_run; private List m_dependencies = new ArrayList<>(); public List getDefines() { @@ -21,11 +22,15 @@ public void setDefines(List defines) { m_defines = defines; } - public XmlRun getRun() { + /** + * @return the {@code } element, or {@code null} when none has been set. A {@code } + * can legitimately carry only {@code } or {@code } elements. + */ + public @Nullable XmlRun getRun() { return m_run; } - public void setRun(XmlRun run) { + public void setRun(@Nullable XmlRun run) { m_run = run; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlInclude.java b/testng-core-api/src/main/java/org/testng/xml/XmlInclude.java index 0d46d88e6..369cc82fd 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlInclude.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlInclude.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import org.jspecify.annotations.Nullable; public class XmlInclude { @@ -17,10 +18,10 @@ public class XmlInclude { // out as "17 1". The generated suite should not depend on that. private final Set m_factoryInstances = new TreeSet<>(); private final int m_index; - private String m_description; + private @Nullable String m_description; private final Map m_parameters = new HashMap<>(); - private XmlClass m_xmlClass; + private @Nullable XmlClass m_xmlClass; public XmlInclude() { this("", 0); @@ -40,7 +41,7 @@ public XmlInclude(String n, List list, int index) { m_index = index; } - public void setDescription(String description) { + public void setDescription(@Nullable String description) { m_description = description; } @@ -49,7 +50,7 @@ public void setParameters(Map parameters) { m_parameters.putAll(parameters); } - public String getDescription() { + public @Nullable String getDescription() { return m_description; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlMethodSelector.java b/testng-core-api/src/main/java/org/testng/xml/XmlMethodSelector.java index 0892f93a4..7e567b9e8 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlMethodSelector.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlMethodSelector.java @@ -1,5 +1,7 @@ package org.testng.xml; +import org.jspecify.annotations.Nullable; + /** This class describes the tag <method-selector> in testng.xml. */ public class XmlMethodSelector { @@ -7,18 +9,18 @@ public class XmlMethodSelector { public static final int DEFAULT_PRIORITY = 0; // Either this: - private String m_className; + private @Nullable String m_className; private int m_priority = DEFAULT_PRIORITY; // Or that: - private XmlScript m_script; + private @Nullable XmlScript m_script; // For YAML - public void setClassName(String s) { + public void setClassName(@Nullable String s) { m_className = s; } - public String getClassName() { + public @Nullable String getClassName() { return m_className; } @@ -32,11 +34,11 @@ public void setName(String name) { m_className = name; } - public XmlScript getScript() { + public @Nullable XmlScript getScript() { return m_script; } - public void setScript(XmlScript script) { + public void setScript(@Nullable XmlScript script) { m_script = script; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java index 8212441e1..6bc633efa 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.internal.PackageUtils; import org.testng.internal.Utils; import org.testng.internal.protocols.UnhandledIOException; @@ -10,10 +11,10 @@ /** This class describes the tag <package> in testng.xml. */ public class XmlPackage { - private String m_name; + private @Nullable String m_name; private List m_include = new ArrayList<>(); private List m_exclude = new ArrayList<>(); - private List m_xmlClasses = null; + private @Nullable List m_xmlClasses; public XmlPackage() {} @@ -43,21 +44,23 @@ public void setInclude(List include) { } /** @return the name */ - public String getName() { + public @Nullable String getName() { return m_name; } /** @param name the name to set */ - public void setName(String name) { + public void setName(@Nullable String name) { m_name = name; } public List getXmlClasses() { - if (null == m_xmlClasses) { - m_xmlClasses = initializeXmlClasses(); + List xmlClasses = m_xmlClasses; + if (null == xmlClasses) { + xmlClasses = initializeXmlClasses(); + m_xmlClasses = xmlClasses; } - return m_xmlClasses; + return xmlClasses; } private List initializeXmlClasses() { diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlScript.java b/testng-core-api/src/main/java/org/testng/xml/XmlScript.java index 63d072f67..5416b4504 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlScript.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlScript.java @@ -1,23 +1,25 @@ package org.testng.xml; +import org.jspecify.annotations.Nullable; + public class XmlScript { - private String language; - private String expression; + private @Nullable String language; + private @Nullable String expression; - public void setLanguage(String language) { + public void setLanguage(@Nullable String language) { this.language = language; } - public void setExpression(String expression) { + public void setExpression(@Nullable String expression) { this.expression = expression; } - public String getExpression() { + public @Nullable String getExpression() { return expression; } - public String getLanguage() { + public @Nullable String getLanguage() { return language; } } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java b/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java index 5561ba34d..1123e2e5a 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import org.jspecify.annotations.Nullable; import org.testng.ITestObjectFactory; import org.testng.internal.RuntimeBehavior; import org.testng.internal.Utils; @@ -87,7 +88,7 @@ public enum FailurePolicy { this.name = name; } - public static FailurePolicy getValidPolicy(String policy) { + public static @Nullable FailurePolicy getValidPolicy(@Nullable String policy) { if (policy == null) { return null; } @@ -104,7 +105,8 @@ public String toString() { } } - private String m_test; + /** Never assigned; {@link #getTest()} has always returned null. */ + private @Nullable String m_test; /** The default suite name TODO CQ is this OK as a default name. */ private static final String DEFAULT_SUITE_NAME = "Default Suite"; @@ -115,7 +117,7 @@ public String toString() { /** The suite verbose flag (0 to 10). */ public static final Integer DEFAULT_VERBOSE = 1; - private Integer m_verbose = null; + private @Nullable Integer m_verbose; public static final ParallelMode DEFAULT_PARALLEL = ParallelMode.NONE; private ParallelMode m_parallel = DEFAULT_PARALLEL; @@ -162,7 +164,7 @@ public String toString() { * Whether {@code @Factory} produced instances are created lazily. {@code null} means "unset at * the suite level" so the {@link org.testng.TestNG} configuration decides (default eager). */ - private Boolean m_lazyFactory = null; + private @Nullable Boolean m_lazyFactory; public static final Boolean DEFAULT_ALLOW_RETURN_VALUES = Boolean.FALSE; private Boolean m_allowReturnValues = DEFAULT_ALLOW_RETURN_VALUES; @@ -180,27 +182,27 @@ public String toString() { private Map m_parameters = new HashMap<>(); /** Name of the XML file. */ - private String m_fileName; + private @Nullable String m_fileName; /** Time out for methods/tests. */ - private String m_timeOut; + private @Nullable String m_timeOut; /** List of child XML suites specified using tags. */ private final List m_childSuites = new ArrayList<>(); /** Parent XML suite if this suite was specified in another suite using tag. */ - private XmlSuite m_parentSuite; + private @Nullable XmlSuite m_parentSuite; private List m_suiteFiles = new ArrayList<>(); - private Class m_objectFactoryClass; + private @Nullable Class m_objectFactoryClass; private List m_listeners = new ArrayList<>(); public static final Boolean DEFAULT_PRESERVE_ORDER = Boolean.TRUE; private Boolean m_preserveOrder = DEFAULT_PRESERVE_ORDER; - private XmlMethodSelectors m_xmlMethodSelectors; + private @Nullable XmlMethodSelectors m_xmlMethodSelectors; private boolean parsed = false; public void setParsed(boolean parsed) { @@ -213,12 +215,12 @@ public boolean isParsed() { } /** @return The fileName. */ - public String getFileName() { + public @Nullable String getFileName() { return m_fileName; } /** @param fileName The fileName to set. */ - public void setFileName(String fileName) { + public void setFileName(@Nullable String fileName) { m_fileName = fileName; } @@ -239,7 +241,7 @@ public String getGuiceStage() { return m_guiceStage; } - public Class getObjectFactoryClass() { + public @Nullable Class getObjectFactoryClass() { return m_objectFactoryClass; } @@ -259,7 +261,8 @@ public boolean isShareThreadPoolForDataProviders() { return shareThreadPoolForDataProviders; } - public void setObjectFactoryClass(Class objectFactoryClass) { + public void setObjectFactoryClass( + @Nullable Class objectFactoryClass) { m_objectFactoryClass = objectFactoryClass; } @@ -342,7 +345,7 @@ public void setName(String name) { * * @return The test. */ - public String getTest() { + public @Nullable String getTest() { return m_test; } @@ -440,9 +443,10 @@ public Map getAllParameters() { * Returns the parameter defined in this suite only. * * @param name The parameter name. - * @return The parameter defined in this suite only. + * @return The parameter defined in this suite only, or {@code null} if this suite does not define + * it. Unlike {@link XmlTest#getParameter(String)} this does not consult a parent suite. */ - public String getParameter(String name) { + public @Nullable String getParameter(String name) { return m_parameters.get(name); } @@ -491,7 +495,7 @@ public List getPackages() { return getXmlPackages(); } - public void setMethodSelectors(XmlMethodSelectors xms) { + public void setMethodSelectors(@Nullable XmlMethodSelectors xms) { m_xmlMethodSelectors = xms; } @@ -510,11 +514,11 @@ public List getLocalListeners() { return m_listeners; } - public void setXmlMethodSelectors(XmlMethodSelectors xms) { + public void setXmlMethodSelectors(@Nullable XmlMethodSelectors xms) { m_xmlMethodSelectors = xms; } - public XmlMethodSelectors getXmlMethodSelectors() { + public @Nullable XmlMethodSelectors getXmlMethodSelectors() { return m_xmlMethodSelectors; } @@ -590,7 +594,7 @@ public XmlSuite shallowCopy() { * * @param timeOut The timeout. */ - public void setTimeOut(String timeOut) { + public void setTimeOut(@Nullable String timeOut) { m_timeOut = timeOut; } @@ -599,7 +603,7 @@ public void setTimeOut(String timeOut) { * * @return The timeout. */ - public String getTimeOut() { + public @Nullable String getTimeOut() { return m_timeOut; } @@ -662,12 +666,12 @@ public int getDataProviderThreadCount() { return m_dataProviderThreadCount; } - public void setParentSuite(XmlSuite parentSuite) { + public void setParentSuite(@Nullable XmlSuite parentSuite) { m_parentSuite = parentSuite; updateParameters(); } - public XmlSuite getParentSuite() { + public @Nullable XmlSuite getParentSuite() { return m_parentSuite; } @@ -850,32 +854,41 @@ public List getIncludedGroups() { } } - private void initGroupsRun() { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); + /** Creates the {@code } and {@code } pair on first use, and returns the run. */ + private XmlRun groupsRun() { + XmlGroups groups = groups(); + XmlRun run = groups.getRun(); + if (run == null) { + run = new XmlRun(); + groups.setRun(run); } - if (m_xmlGroups.getRun() == null) { - m_xmlGroups.setRun(new XmlRun()); + return run; + } + + /** Creates the {@code } element on first use, and returns it. */ + private XmlGroups groups() { + XmlGroups groups = m_xmlGroups; + if (groups == null) { + groups = new XmlGroups(); + m_xmlGroups = groups; } + return groups; } public void addIncludedGroup(String g) { - initGroupsRun(); - m_xmlGroups.getRun().onInclude(g); + groupsRun().onInclude(g); } /** @param g - The list of groups to include. */ public void setIncludedGroups(List g) { - initGroupsRun(); - List includes = m_xmlGroups.getRun().getIncludes(); + List includes = groupsRun().getIncludes(); includes.clear(); includes.addAll(g); } /** @param g The excludedGrousps to set. */ public void setExcludedGroups(List g) { - initGroupsRun(); - List excludes = m_xmlGroups.getRun().getExcludes(); + List excludes = groupsRun().getExcludes(); excludes.clear(); excludes.addAll(g); } @@ -895,8 +908,7 @@ public List getExcludedGroups() { } public void addExcludedGroup(String g) { - initGroupsRun(); - m_xmlGroups.getRun().onExclude(g); + groupsRun().onExclude(g); } public Boolean getGroupByInstances() { @@ -913,11 +925,11 @@ public void setGroupByInstances(boolean f) { * at the suite level, in which case the {@link org.testng.TestNG} configuration decides * (default eager). */ - public Boolean getLazyFactory() { + public @Nullable Boolean getLazyFactory() { return m_lazyFactory; } - public void setLazyFactory(Boolean lazyFactory) { + public void setLazyFactory(@Nullable Boolean lazyFactory) { m_lazyFactory = lazyFactory; } @@ -933,9 +945,9 @@ public void setAllowReturnValues(Boolean allowReturnValues) { m_allowReturnValues = allowReturnValues; } - private XmlGroups m_xmlGroups; + private @Nullable XmlGroups m_xmlGroups; - public void setGroups(XmlGroups xmlGroups) { + public void setGroups(@Nullable XmlGroups xmlGroups) { m_xmlGroups = xmlGroups; } @@ -959,7 +971,8 @@ public void onMethodSelectorElement(String language, String name, String priorit System.out.println("Language:" + language); } - public XmlGroups getGroups() { + /** @return the {@code } element, or {@code null} if the suite declares none. */ + public @Nullable XmlGroups getGroups() { return m_xmlGroups; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlTest.java b/testng-core-api/src/main/java/org/testng/xml/XmlTest.java index 597c57956..71163648b 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlTest.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlTest.java @@ -6,9 +6,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.regex.Pattern; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; /** This class describes the tag <test> in testng.xml. */ @@ -16,31 +18,31 @@ public class XmlTest implements Cloneable { public static final int DEFAULT_TIMEOUT_MS = Integer.MAX_VALUE; - private XmlSuite m_suite; - private String m_name; + private @Nullable XmlSuite m_suite; + private @Nullable String m_name; private Integer m_verbose = XmlSuite.DEFAULT_VERBOSE; private int m_threadCount = -1; private List m_xmlClasses = new ArrayList<>(); private Map m_parameters = new HashMap<>(); - private XmlSuite.ParallelMode m_parallel; + private XmlSuite.@Nullable ParallelMode m_parallel; private List m_methodSelectors = new ArrayList<>(); // test level packages private List m_xmlPackages = new ArrayList<>(); - private String m_timeOut; + private @Nullable String m_timeOut; private Boolean m_skipFailedInvocationCounts = XmlSuite.DEFAULT_SKIP_FAILED_INVOCATION_COUNTS; - private Map> m_failedInvocationNumbers = null; // lazily initialized + private @Nullable Map> m_failedInvocationNumbers; // lazily initialized private Boolean m_preserveOrder = XmlSuite.DEFAULT_PRESERVE_ORDER; private int m_index; - private Boolean m_groupByInstances; + private @Nullable Boolean m_groupByInstances; - private Boolean m_allowReturnValues = null; + private @Nullable Boolean m_allowReturnValues; private Map m_xmlDependencyGroups = new HashMap<>(); @@ -148,12 +150,12 @@ public void setXmlClasses(List classes) { } /** @return Returns the name. */ - public String getName() { + public @Nullable String getName() { return m_name; } /** @param name The name to set. */ - public void setName(String name) { + public void setName(@Nullable String name) { m_name = name; } @@ -170,26 +172,35 @@ public void setThreadCount(int threadCount) { m_threadCount = threadCount; } - public void setIncludedGroups(List g) { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); + /** Creates the {@code } and {@code } pair on first use, and returns the run. */ + private XmlRun groupsRun() { + XmlGroups groups = groups(); + XmlRun run = groups.getRun(); + if (run == null) { + run = new XmlRun(); + groups.setRun(run); } - if (m_xmlGroups.getRun() == null) { - m_xmlGroups.setRun(new XmlRun()); + return run; + } + + /** Creates the {@code } element on first use, and returns it. */ + private XmlGroups groups() { + XmlGroups groups = m_xmlGroups; + if (groups == null) { + groups = new XmlGroups(); + m_xmlGroups = groups; } - List includes = m_xmlGroups.getRun().getIncludes(); + return groups; + } + + public void setIncludedGroups(List g) { + List includes = groupsRun().getIncludes(); includes.clear(); includes.addAll(g); } public void setExcludedGroups(List g) { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); - } - if (m_xmlGroups.getRun() == null) { - m_xmlGroups.setRun(new XmlRun()); - } - List excludes = m_xmlGroups.getRun().getExcludes(); + List excludes = groupsRun().getExcludes(); excludes.clear(); excludes.addAll(g); } @@ -204,21 +215,21 @@ public List getExcludedGroups() { } public void addIncludedGroup(String g) { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); - m_xmlGroups.setRun(new XmlRun()); + XmlGroups groups = m_xmlGroups; + if (groups == null) { + groups = new XmlGroups(); + groups.setRun(new XmlRun()); + m_xmlGroups = groups; } - m_xmlGroups.getRun().getIncludes().add(g); + // Unlike addExcludedGroup, a element that has no yet is not repaired here. The + // asymmetry is kept: setGroups and addMetaGroup can both leave one in that state, and calling + // this method afterwards has always thrown. + Objects.requireNonNull(groups.getRun(), " has no to add an included group to") + .onInclude(g); } public void addExcludedGroup(String g) { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); - } - if (m_xmlGroups.getRun() == null) { - m_xmlGroups.setRun(new XmlRun()); - } - m_xmlGroups.getRun().getExcludes().add(g); + groupsRun().onExclude(g); } /** @return Returns the verbose. */ @@ -259,13 +270,10 @@ public boolean skipFailedInvocationCounts() { } public void addMetaGroup(String name, List metaGroup) { - if (m_xmlGroups == null) { - m_xmlGroups = new XmlGroups(); - } XmlDefine define = new XmlDefine(); define.setName(name); define.getIncludes().addAll(metaGroup); - m_xmlGroups.getDefines().add(define); + groups().getDefines().add(define); } public void addMetaGroup(String name, String... metaGroup) { @@ -301,7 +309,7 @@ public void addParameter(String key, String value) { m_parameters.put(key, value); } - public String getParameter(String name) { + public @Nullable String getParameter(String name) { String result = m_parameters.get(name); if (null == result) { result = getSuite().getParameter(name); @@ -334,7 +342,7 @@ public XmlSuite.ParallelMode getParallel() { return Optional.ofNullable(m_parallel).orElse(getSuite().getParallel()); } - public String getTimeOut() { + public @Nullable String getTimeOut() { String result = getSuite().getTimeOut(); if (null != m_timeOut) { result = m_timeOut; @@ -356,11 +364,11 @@ public void setTimeOut(long timeOut) { m_timeOut = Long.toString(timeOut); } - private void setTimeOut(String timeOut) { + private void setTimeOut(@Nullable String timeOut) { m_timeOut = timeOut; } - public void setScript(XmlScript script) { + public void setScript(@Nullable XmlScript script) { List selectors = getMethodSelectors(); if (!selectors.isEmpty()) { XmlMethodSelector xms = selectors.get(0); @@ -372,7 +380,8 @@ public void setScript(XmlScript script) { } } - public XmlScript getScript() { + /** @return the script of the first method selector, or {@code null} if none carries one. */ + public @Nullable XmlScript getScript() { List selectors = getMethodSelectors(); if (selectors.isEmpty()) { return null; @@ -423,20 +432,23 @@ public Object clone() { * @return The invocation numbers of the method */ public List getInvocationNumbers(String method) { - if (m_failedInvocationNumbers == null) { - m_failedInvocationNumbers = new HashMap<>(); + Map> cached = m_failedInvocationNumbers; + if (cached == null) { + cached = new HashMap<>(); + m_failedInvocationNumbers = cached; for (XmlClass c : getXmlClasses()) { for (XmlInclude xi : c.getIncludedMethods()) { List invocationNumbers = xi.getInvocationNumbers(); if (!invocationNumbers.isEmpty()) { String methodName = c.getName() + "." + xi.getName(); - m_failedInvocationNumbers.put(methodName, invocationNumbers); + cached.put(methodName, invocationNumbers); } } } } - return Optional.ofNullable(m_failedInvocationNumbers.get(method)).orElse(new ArrayList<>()); + List numbers = cached.get(method); + return numbers != null ? numbers : new ArrayList<>(); } public void setPreserveOrder(Boolean preserveOrder) { @@ -532,20 +544,27 @@ public boolean equals(Object obj) { return XmlSuite.f(); } } else { - if (other.m_xmlGroups == null) { + XmlGroups otherGroups = other.m_xmlGroups; + if (otherGroups == null) { return false; } - if ((m_xmlGroups.getRun() == null && other.m_xmlGroups != null) - || m_xmlGroups.getRun() != null && other.m_xmlGroups == null) { + // Was a two-armed condition whose second arm re-tested other.m_xmlGroups, already known + // non-null one line above; only the first arm could ever fire. + XmlRun run = m_xmlGroups.getRun(); + if (run == null) { return false; } - if (!m_xmlGroups.getRun().getExcludes().equals(other.m_xmlGroups.getRun().getExcludes())) { + // The other side's is not tested, exactly as before: comparing against a + // that has none has always thrown here. + XmlRun otherRun = + Objects.requireNonNull(otherGroups.getRun(), "the compared has no "); + if (!run.getExcludes().equals(otherRun.getExcludes())) { return XmlSuite.f(); } - if (!m_xmlGroups.getRun().getIncludes().equals(other.m_xmlGroups.getRun().getIncludes())) { + if (!run.getIncludes().equals(otherRun.getIncludes())) { return XmlSuite.f(); } - if (!m_xmlGroups.getDefines().equals(other.m_xmlGroups.getDefines())) { + if (!m_xmlGroups.getDefines().equals(otherGroups.getDefines())) { return false; } } @@ -665,13 +684,13 @@ public void setXmlSuite(XmlSuite suite) { m_suite = suite; } - private XmlGroups m_xmlGroups; + private @Nullable XmlGroups m_xmlGroups; public void setGroups(XmlGroups xmlGroups) { m_xmlGroups = xmlGroups; } - public XmlGroups getXmlGroups() { + public @Nullable XmlGroups getXmlGroups() { return m_xmlGroups; } diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java index e3e501d13..7157853c2 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java @@ -1,5 +1,7 @@ package org.testng.xml; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.internal.ClassHelper; import org.testng.internal.RuntimeBehavior; @@ -7,7 +9,7 @@ /** A Utility class that helps represent a {@link XmlSuite} and {@link XmlTest} as String. */ final class XmlWeaver { - private static IWeaveXml instance = null; + private static @Nullable IWeaveXml instance; private static final boolean testMode = RuntimeBehavior.isTestMode(); private XmlWeaver() {} @@ -16,7 +18,11 @@ private static IWeaveXml getInstance() { if (testMode) { // Do not resort to caching when running Unit tests for TestNG, because we have to check // both implementations. If we cache the instance, then its not possible to do that. - return attemptDefaultImplementationInstantiation(); + // The requireNonNull records what the callers have always assumed: in test mode a third + // party weaver is not instantiated, and dereferencing the result threw here already. + return Objects.requireNonNull( + attemptDefaultImplementationInstantiation(), + "test mode does not instantiate a third party weaver named by -Dtestng.xml.weaver"); } return instantiateIfRequired(); } @@ -70,7 +76,7 @@ static String asXml(XmlTest xmlTest, String indent) { return getInstance().asXml(xmlTest, indent); } - private static IWeaveXml attemptDefaultImplementationInstantiation() { + private static @Nullable IWeaveXml attemptDefaultImplementationInstantiation() { String clazz = getClassName(); if (clazz.equals(DefaultXmlWeaver.class.getName())) { return new DefaultXmlWeaver(); diff --git a/testng-core-api/src/main/java/org/testng/xml/package-info.java b/testng-core-api/src/main/java/org/testng/xml/package-info.java new file mode 100644 index 000000000..2d7182933 --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/xml/package-info.java @@ -0,0 +1,5 @@ +/** The suite model that testng.xml describes, its parser, and the weaver that writes it back. */ +@NullMarked +package org.testng.xml; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-core/src/main/java/org/testng/reporters/jq/BaseMultiSuitePanel.java b/testng-core/src/main/java/org/testng/reporters/jq/BaseMultiSuitePanel.java index e861423f4..56c6a6a11 100644 --- a/testng-core/src/main/java/org/testng/reporters/jq/BaseMultiSuitePanel.java +++ b/testng-core/src/main/java/org/testng/reporters/jq/BaseMultiSuitePanel.java @@ -6,7 +6,7 @@ public abstract class BaseMultiSuitePanel extends BasePanel implements INavigatorPanel { - abstract String getHeader(ISuite suite); + abstract @Nullable String getHeader(ISuite suite); abstract String getContent(ISuite suite, XMLStringBuffer xsb); diff --git a/testng-core/src/main/java/org/testng/reporters/jq/TestNgXmlPanel.java b/testng-core/src/main/java/org/testng/reporters/jq/TestNgXmlPanel.java index 615c2320b..01ee16b24 100644 --- a/testng-core/src/main/java/org/testng/reporters/jq/TestNgXmlPanel.java +++ b/testng-core/src/main/java/org/testng/reporters/jq/TestNgXmlPanel.java @@ -1,5 +1,6 @@ package org.testng.reporters.jq; +import org.jspecify.annotations.Nullable; import org.testng.ISuite; import org.testng.internal.Utils; import org.testng.reporters.XMLStringBuffer; @@ -16,7 +17,7 @@ public String getPrefix() { } @Override - public String getHeader(ISuite suite) { + public @Nullable String getHeader(ISuite suite) { return suite.getXmlSuite().getFileName(); } diff --git a/testng-core/src/main/java/org/testng/xml/IFileParser.java b/testng-core/src/main/java/org/testng/xml/IFileParser.java index cbc582a83..88973f7b2 100644 --- a/testng-core/src/main/java/org/testng/xml/IFileParser.java +++ b/testng-core/src/main/java/org/testng/xml/IFileParser.java @@ -1,9 +1,14 @@ package org.testng.xml; import java.io.InputStream; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; public interface IFileParser { - T parse(String filePath, InputStream is, boolean loadClasses) throws TestNGException; + /** + * @param is the file's contents, or {@code null} for a parser that reads a source of its own -- + * {@code Parser} only opens a stream for a {@code file:} scheme. + */ + T parse(String filePath, @Nullable InputStream is, boolean loadClasses) throws TestNGException; } diff --git a/testng-core/src/main/java/org/testng/xml/SuiteXmlParser.java b/testng-core/src/main/java/org/testng/xml/SuiteXmlParser.java index f59dcdce9..b93171a0e 100644 --- a/testng-core/src/main/java/org/testng/xml/SuiteXmlParser.java +++ b/testng-core/src/main/java/org/testng/xml/SuiteXmlParser.java @@ -2,6 +2,8 @@ import java.io.IOException; import java.io.InputStream; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.xml.internal.Parser; import org.xml.sax.SAXException; @@ -9,13 +11,20 @@ public class SuiteXmlParser extends XMLParser implements ISuiteParser { @Override - public XmlSuite parse(String currentFile, InputStream inputStream, boolean loadClasses) { + public XmlSuite parse( + String currentFile, @Nullable InputStream inputStream, boolean loadClasses) { TestNGContentHandler contentHandler = new TestNGContentHandler(currentFile, loadClasses); try { - parse(inputStream, contentHandler); + // Nullable on the interface for the parsers that read a source of their own. This one is + // reached either through accept(), which requires a file: scheme and so a stream, or as + // Parser's fallback for a scheme nothing claims -- which has never been readable here. + parse( + Objects.requireNonNull(inputStream, "a file: suite is read from an open stream"), + contentHandler); - return contentHandler.getSuite(); + return Objects.requireNonNull( + contentHandler.getSuite(), "the document declares no element"); } catch (SAXException | IOException e) { throw new TestNGException(e); } diff --git a/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java b/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java index 9a15dba9b..2d89d693d 100644 --- a/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java +++ b/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java @@ -18,9 +18,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Stack; import javax.xml.XMLConstants; +import org.jspecify.annotations.Nullable; import org.testng.ITestObjectFactory; import org.testng.TestNGException; import org.testng.internal.RuntimeBehavior; @@ -46,22 +48,25 @@ public class TestNGContentHandler extends DefaultHandler implements LexicalHandler { private static final int DTD_CONNECTION_TIMEOUT_MILLIS = 10_000; - private XmlSuite m_currentSuite = null; - private XmlTest m_currentTest = null; - private XmlDefine m_currentDefine = null; - private XmlRun m_currentRun = null; - private List m_currentClasses = null; + /** Only the end tag half of the {@code xml*} methods below is called without attributes. */ + private static final String START_TAG_ATTRIBUTES = "a start tag carries its attributes"; + + private @Nullable XmlSuite m_currentSuite; + private @Nullable XmlTest m_currentTest; + private @Nullable XmlDefine m_currentDefine; + private @Nullable XmlRun m_currentRun; + private @Nullable List m_currentClasses; private int m_currentTestIndex = 0; private int m_currentClassIndex = 0; private int m_currentIncludeIndex = 0; - private List m_currentPackages = null; - private XmlPackage m_currentPackage = null; + private @Nullable List m_currentPackages; + private @Nullable XmlPackage m_currentPackage; private final List m_suites = new ArrayList<>(); - private XmlGroups m_currentGroups = null; - private Map m_currentTestParameters = null; - private Map m_currentSuiteParameters = null; - private Map m_currentClassParameters = null; - private Include m_currentInclude; + private @Nullable XmlGroups m_currentGroups; + private @Nullable Map m_currentTestParameters; + private @Nullable Map m_currentSuiteParameters; + private @Nullable Map m_currentClassParameters; + private @Nullable Include m_currentInclude; // Borrowed this implementation from this SO post : https://stackoverflow.com/a/29751441/679824 private final EntityResolver m_redirectionAwareResolver = @@ -126,16 +131,16 @@ enum Location { private final Stack m_locations = new Stack<>(); private boolean isSuiteFileTag = false; - private XmlClass m_currentClass = null; - private ArrayList m_currentIncludedMethods = null; - private List m_currentExcludedMethods = null; - private ArrayList m_currentSelectors = null; - private XmlMethodSelector m_currentSelector = null; - private String m_currentLanguage = null; - private String m_currentExpression = null; + private @Nullable XmlClass m_currentClass; + private @Nullable ArrayList m_currentIncludedMethods; + private @Nullable List m_currentExcludedMethods; + private @Nullable ArrayList m_currentSelectors; + private @Nullable XmlMethodSelector m_currentSelector; + private @Nullable String m_currentLanguage; + private @Nullable String m_currentExpression; private final List m_suiteFiles = new ArrayList<>(); private boolean m_enabledTest; - private List m_listeners; + private @Nullable List m_listeners; private final String m_fileName; private final boolean m_loadClasses; @@ -281,36 +286,82 @@ private InputStream loadDtdUsingClassLoader() { return Thread.currentThread().getContextClassLoader().getResourceAsStream(Parser.TESTNG_DTD); } + // Every m_currentXxx field declared above is set when its element's start tag is seen and cleared + // at the matching end tag, so it is non-null for the whole of that element's body. A null means + // the document is not well formed, which SAX reports separately. The accessors below assert it so + // the failure names the element instead of arriving as a bare NullPointerException. + private XmlSuite currentSuite() { + return Objects.requireNonNull(m_currentSuite, "no is being parsed"); + } + + private XmlTest currentTest() { + return Objects.requireNonNull(m_currentTest, "no is being parsed"); + } + + private XmlClass currentClass() { + return Objects.requireNonNull(m_currentClass, "no is being parsed"); + } + + private XmlGroups currentGroups() { + return Objects.requireNonNull(m_currentGroups, "no is being parsed"); + } + + private XmlMethodSelector currentSelector() { + return Objects.requireNonNull(m_currentSelector, "no is being parsed"); + } + + private List currentSelectors() { + return Objects.requireNonNull(m_currentSelectors, "no is being parsed"); + } + + private Include currentInclude() { + return Objects.requireNonNull(m_currentInclude, "no is being parsed"); + } + + private Map currentSuiteParameters() { + return Objects.requireNonNull(m_currentSuiteParameters, "no is being parsed"); + } + + private Map currentTestParameters() { + return Objects.requireNonNull(m_currentTestParameters, "no is being parsed"); + } + + private Map currentClassParameters() { + return Objects.requireNonNull(m_currentClassParameters, "no is being parsed"); + } + /** Parse */ - private void xmlSuiteFile(boolean start, Attributes attributes) { + private void xmlSuiteFile(boolean start, @Nullable Attributes startAttributes) { if (start) { - String path = attributes.getValue("path"); + String path = Objects.requireNonNull(startAttributes, START_TAG_ATTRIBUTES).getValue("path"); pushLocation(Location.SUITE); m_suiteFiles.add(path); isSuiteFileTag = true; } else { - m_currentSuite.setSuiteFiles(m_suiteFiles); + currentSuite().setSuiteFiles(m_suiteFiles); popLocation(); isSuiteFileTag = false; } } /** Parse */ - private void xmlSuite(boolean start, Attributes attributes) { + private void xmlSuite(boolean start, @Nullable Attributes startAttributes) { if (start) { pushLocation(Location.SUITE); + Attributes attributes = Objects.requireNonNull(startAttributes, START_TAG_ATTRIBUTES); String name = attributes.getValue("name"); if (isStringBlank(name)) { throw new TestNGException("The tag must define the name attribute"); } - m_currentSuite = new XmlSuite(); - m_currentSuite.setFileName(m_fileName); - m_currentSuite.setName(name); + XmlSuite suite = new XmlSuite(); + m_currentSuite = suite; + suite.setFileName(m_fileName); + suite.setName(name); m_currentSuiteParameters = new HashMap<>(); String verbose = attributes.getValue("verbose"); if (null != verbose) { - m_currentSuite.setVerbose(Integer.parseInt(verbose)); + suite.setVerbose(Integer.parseInt(verbose)); } String jUnit = attributes.getValue("junit"); if (null != jUnit) { @@ -320,7 +371,7 @@ private void xmlSuite(boolean start, Attributes attributes) { if (parallel != null) { XmlSuite.ParallelMode mode = XmlSuite.ParallelMode.getValidParallel(parallel); if (mode != null) { - m_currentSuite.setParallel(mode); + suite.setParallel(mode); } else { Utils.log( "Parser", @@ -330,36 +381,36 @@ private void xmlSuite(boolean start, Attributes attributes) { } String parentModule = attributes.getValue("parent-module"); if (parentModule != null) { - m_currentSuite.setParentModule(parentModule); + suite.setParentModule(parentModule); } String guiceStage = attributes.getValue("guice-stage"); if (guiceStage != null) { - m_currentSuite.setGuiceStage(guiceStage); + suite.setGuiceStage(guiceStage); } XmlSuite.FailurePolicy configFailurePolicy = XmlSuite.FailurePolicy.getValidPolicy(attributes.getValue("configfailurepolicy")); if (null != configFailurePolicy) { - m_currentSuite.setConfigFailurePolicy(configFailurePolicy); + suite.setConfigFailurePolicy(configFailurePolicy); } String groupByInstances = attributes.getValue("group-by-instances"); if (groupByInstances != null) { - m_currentSuite.setGroupByInstances(Boolean.parseBoolean(groupByInstances)); + suite.setGroupByInstances(Boolean.parseBoolean(groupByInstances)); } String lazyFactory = attributes.getValue("lazy-factory"); if (lazyFactory != null) { - m_currentSuite.setLazyFactory(Boolean.parseBoolean(lazyFactory)); + suite.setLazyFactory(Boolean.parseBoolean(lazyFactory)); } String skip = attributes.getValue("skipfailedinvocationcounts"); if (skip != null) { - m_currentSuite.setSkipFailedInvocationCounts(Boolean.parseBoolean(skip)); + suite.setSkipFailedInvocationCounts(Boolean.parseBoolean(skip)); } String threadCount = attributes.getValue("thread-count"); if (null != threadCount) { - m_currentSuite.setThreadCount(Integer.parseInt(threadCount)); + suite.setThreadCount(Integer.parseInt(threadCount)); } String dataProviderThreadCount = attributes.getValue("data-provider-thread-count"); if (null != dataProviderThreadCount) { - m_currentSuite.setDataProviderThreadCount(Integer.parseInt(dataProviderThreadCount)); + suite.setDataProviderThreadCount(Integer.parseInt(dataProviderThreadCount)); } String shareThreadPoolForDataProviders = @@ -367,24 +418,22 @@ private void xmlSuite(boolean start, Attributes attributes) { Optional.ofNullable(shareThreadPoolForDataProviders) .ifPresent( it -> - m_currentSuite.setShareThreadPoolForDataProviders( + suite.setShareThreadPoolForDataProviders( Boolean.parseBoolean(shareThreadPoolForDataProviders))); String useGlobalThreadPool = attributes.getValue("use-global-thread-pool"); Optional.ofNullable(useGlobalThreadPool) .ifPresent( - it -> - m_currentSuite.shouldUseGlobalThreadPool( - Boolean.parseBoolean(useGlobalThreadPool))); + it -> suite.shouldUseGlobalThreadPool(Boolean.parseBoolean(useGlobalThreadPool))); String timeOut = attributes.getValue("time-out"); if (null != timeOut) { - m_currentSuite.setTimeOut(timeOut); + suite.setTimeOut(timeOut); } String objectFactory = attributes.getValue("object-factory"); if (null != objectFactory && m_loadClasses) { try { - m_currentSuite.setObjectFactoryClass( + suite.setObjectFactoryClass( (Class) Class.forName(objectFactory)); } catch (Exception e) { Utils.log( @@ -395,45 +444,50 @@ private void xmlSuite(boolean start, Attributes attributes) { } String preserveOrder = attributes.getValue("preserve-order"); if (preserveOrder != null) { - m_currentSuite.setPreserveOrder(Boolean.valueOf(preserveOrder)); + suite.setPreserveOrder(Boolean.valueOf(preserveOrder)); } String allowReturnValues = attributes.getValue("allow-return-values"); if (allowReturnValues != null) { - m_currentSuite.setAllowReturnValues(Boolean.valueOf(allowReturnValues)); + suite.setAllowReturnValues(Boolean.valueOf(allowReturnValues)); } } else { - m_currentSuite.setParameters(m_currentSuiteParameters); - m_suites.add(m_currentSuite); + XmlSuite suite = currentSuite(); + suite.setParameters(currentSuiteParameters()); + m_suites.add(suite); m_currentSuiteParameters = null; popLocation(); } } /** Parse */ - private void xmlDefine(boolean start, Attributes attributes) { + private void xmlDefine(boolean start, @Nullable Attributes startAttributes) { if (start) { - String name = attributes.getValue("name"); - m_currentDefine = new XmlDefine(); - m_currentDefine.setName(name); + String name = Objects.requireNonNull(startAttributes, START_TAG_ATTRIBUTES).getValue("name"); + XmlDefine define = new XmlDefine(); + define.setName(name); + m_currentDefine = define; } else { // define is only defined within the context of XmlGroups - m_currentGroups.addDefine(m_currentDefine); + currentGroups() + .addDefine(Objects.requireNonNull(m_currentDefine, "no is being parsed")); m_currentDefine = null; } } /** Parse