Enable NullAway in junit-vintage-engine

Author: marcphilipp

junit-vintage-engine/junit-vintage-engine.gradle.kts

@@ -1,5 +1,6 @@
 plugins {
 	id("junitbuild.java-library-conventions")
+	id("junitbuild.java-nullability-conventions")
 	id("junitbuild.junit4-compatibility")
 	id("junitbuild.testing-conventions")
 	`java-test-fixtures`
@@ -14,6 +15,7 @@ dependencies {
 	api(libs.junit4)
 
 	compileOnlyApi(libs.apiguardian)
+	compileOnly(libs.jspecify)
 
 	testFixturesApi(platform(libs.groovy2.bom))
 	testFixturesApi(libs.spock1)

junit-vintage-engine/src/main/java/module-info.java

@@ -8,6 +8,8 @@
  * https://www.eclipse.org/legal/epl-v20.html
  */
 
+import org.jspecify.annotations.NullMarked;
+
 /**
  * Provides a {@link org.junit.platform.engine.TestEngine} for running JUnit 3
  * and 4 based tests on the platform.
@@ -16,6 +18,7 @@
  * @provides org.junit.platform.engine.TestEngine The {@code VintageTestEngine}
  * runs JUnit 3 and 4 based tests on the platform.
  */
+@NullMarked
 module org.junit.vintage.engine {
 	requires junit; // 4
 	requires static org.apiguardian.api;

junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/DescriptionUtils.java

@@ -11,6 +11,7 @@
 package org.junit.vintage.engine.descriptor;
 
 import org.apiguardian.api.API;
+import org.jspecify.annotations.Nullable;
 import org.junit.runner.Description;
 
 @API(status = API.Status.INTERNAL, since = "5.8")
@@ -19,6 +20,7 @@ public class DescriptionUtils {
 	private DescriptionUtils() {
 	}
 
+	@Nullable
 	public static String getMethodName(Description description) {
 		String displayName = description.getDisplayName();
 		int i = displayName.indexOf('(');

junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/RunnerTestDescriptor.java

@@ -26,6 +26,7 @@
 import java.util.function.Consumer;
 
 import org.apiguardian.api.API;
+import org.jspecify.annotations.Nullable;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.logging.Logger;
 import org.junit.platform.commons.logging.LoggerFactory;
@@ -54,6 +55,8 @@ public class RunnerTestDescriptor extends VintageTestDescriptor {
 	private Runner runner;
 	private final boolean ignored;
 	private boolean wasFiltered;
+
+	@Nullable
 	private List<Filter> filters = new ArrayList<>();
 
 	public RunnerTestDescriptor(UniqueId uniqueId, Class<?> testClass, Runner runner, boolean ignored) {

junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/TestSourceProvider.java

@@ -24,6 +24,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apiguardian.api.API;
+import org.jspecify.annotations.Nullable;
 import org.junit.platform.commons.support.ModifierSupport;
 import org.junit.platform.commons.util.LruCache;
 import org.junit.platform.engine.TestSource;
@@ -44,6 +45,7 @@ public class TestSourceProvider {
 	private final Map<Description, TestSource> testSourceCache = new ConcurrentHashMap<>();
 	private final Map<Class<?>, List<Method>> methodsCache = synchronizedMap(new LruCache<>(31));
 
+	@Nullable
 	public TestSource findTestSource(Description description) {
 		TestSource testSource = testSourceCache.computeIfAbsent(description, this::computeTestSource);
 		return testSource == NULL_SOURCE ? null : testSource;
@@ -72,6 +74,7 @@ private String sanitizeMethodName(String methodName) {
 		return methodName;
 	}
 
+	@Nullable
 	private Method findMethod(Class<?> testClass, String methodName) {
 		List<Method> methods = methodsCache.computeIfAbsent(testClass,
 			clazz -> findMethods(clazz, m -> true, TOP_DOWN)).stream() //

junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/VintageTestDescriptor.java

@@ -23,6 +23,7 @@
 import java.util.Set;
 
 import org.apiguardian.api.API;
+import org.jspecify.annotations.Nullable;
 import org.junit.experimental.categories.Category;
 import org.junit.platform.commons.util.ReflectionUtils;
 import org.junit.platform.engine.TestDescriptor;
@@ -45,15 +46,16 @@ public class VintageTestDescriptor extends AbstractTestDescriptor {
 
 	protected Description description;
 
-	public VintageTestDescriptor(UniqueId uniqueId, Description description, TestSource source) {
+	public VintageTestDescriptor(UniqueId uniqueId, Description description, @Nullable TestSource source) {
 		this(uniqueId, description, generateDisplayName(description), source);
 	}
 
-	VintageTestDescriptor(UniqueId uniqueId, Description description, String displayName, TestSource source) {
+	VintageTestDescriptor(UniqueId uniqueId, Description description, String displayName, @Nullable TestSource source) {
 		super(uniqueId, displayName, source);
 		this.description = description;
 	}
 
+	@SuppressWarnings("NullAway")
 	private static String generateDisplayName(Description description) {
 		String methodName = DescriptionUtils.getMethodName(description);
 		return isNotBlank(methodName) ? methodName : description.getDisplayName();

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java

@@ -13,6 +13,7 @@
 import java.lang.reflect.Method;
 import java.util.function.Predicate;
 
+import org.jspecify.annotations.Nullable;
 import org.junit.Ignore;
 import org.junit.internal.builders.AllDefaultPossibilitiesBuilder;
 import org.junit.internal.builders.AnnotatedBuilder;
@@ -108,6 +109,7 @@ private static class DefensiveAnnotatedBuilder extends AnnotatedBuilder {
 			super(suiteBuilder);
 		}
 
+		@Nullable
 		@Override
 		public Runner buildRunner(Class<? extends Runner> runnerClass, Class<?> testClass) throws Exception {
 			// Referenced by name because it might not be available at runtime.
@@ -127,6 +129,7 @@ private static class DefensiveJUnit4Builder extends JUnit4Builder {
 
 		private static final Predicate<Method> isPotentialJUnit4TestMethod = new IsPotentialJUnit4TestMethod();
 
+		@Nullable
 		@Override
 		public Runner runnerForClass(Class<?> testClass) throws Throwable {
 			if (containsTestMethods(testClass)) {
@@ -146,6 +149,7 @@ private boolean containsTestMethods(Class<?> testClass) {
 	 * @since 5.1
 	 */
 	private static class NullIgnoredBuilder extends IgnoredBuilder {
+		@Nullable
 		@Override
 		public Runner runnerForClass(Class<?> testClass) {
 			// don't ignore entire test classes just yet

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/MethodSelectorResolver.java

@@ -107,7 +107,7 @@ public boolean shouldRun(Description description) {
 
 			private boolean isParameterizedMethod(Description description) {
 				String methodName = DescriptionUtils.getMethodName(description);
-				return methodName.startsWith(desiredMethodName + "[");
+				return methodName != null && methodName.startsWith(desiredMethodName + "[");
 			}
 
 			@Override

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/UniqueIdFilter.java

@@ -18,6 +18,7 @@
 import java.util.Optional;
 import java.util.Set;
 
+import org.jspecify.annotations.Nullable;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
 import org.junit.runner.Description;
@@ -33,7 +34,10 @@ class UniqueIdFilter extends Filter {
 	private final RunnerTestDescriptor runnerTestDescriptor;
 	private final UniqueId uniqueId;
 
+	@Nullable
 	private Deque<Description> path;
+
+	@Nullable
 	private Set<Description> descendants;
 
 	UniqueIdFilter(RunnerTestDescriptor runnerTestDescriptor, UniqueId uniqueId) {
@@ -74,6 +78,7 @@ private Set<Description> determineDescendants(Optional<? extends TestDescriptor>
 	}
 
 	@Override
+	@SuppressWarnings({ "NullAway", "DataFlowIssue" })
 	public boolean shouldRun(Description description) {
 		ensureInitialized();
 		return path.contains(description) || descendants.contains(description);

junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/TestRun.java

@@ -11,6 +11,7 @@
 package org.junit.vintage.engine.execution;
 
 import static java.util.Collections.emptyList;
+import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
@@ -123,7 +124,7 @@ private Optional<VintageTestDescriptor> lookupUnambiguouslyOrApplyFallback(Descr
 	void markSkipped(TestDescriptor testDescriptor) {
 		skippedDescriptors.add(testDescriptor);
 		if (testDescriptor instanceof VintageTestDescriptor vintageDescriptor) {
-			descriptionToDescriptors.get(vintageDescriptor.getDescription()).incrementSkippedOrStarted();
+			getVintageDescriptors(vintageDescriptor).incrementSkippedOrStarted();
 		}
 	}
 
@@ -140,10 +141,15 @@ void markStarted(TestDescriptor testDescriptor, EventType eventType) {
 		startedDescriptors.add(testDescriptor);
 		if (testDescriptor instanceof VintageTestDescriptor vintageDescriptor) {
 			inProgressDescriptorsByStartingThread.get().addLast(vintageDescriptor);
-			descriptionToDescriptors.get(vintageDescriptor.getDescription()).incrementSkippedOrStarted();
+			getVintageDescriptors(vintageDescriptor).incrementSkippedOrStarted();
 		}
 	}
 
+	private VintageDescriptors getVintageDescriptors(VintageTestDescriptor vintageDescriptor) {
+		return requireNonNull(descriptionToDescriptors.get(vintageDescriptor.getDescription()),
+			() -> "No descriptors for " + vintageDescriptor);
+	}
+
 	boolean isNotStarted(TestDescriptor testDescriptor) {
 		return !startedDescriptors.contains(testDescriptor);
 	}
@@ -191,7 +197,7 @@ TestExecutionResult getStoredResultOrSuccessful(TestDescriptor testDescriptor) {
 		List<Throwable> failures = testExecutionResults
 				.stream()
 				.map(TestExecutionResult::getThrowable)
-				.map(Optional::get)
+				.map(Optional::orElseThrow)
 				.collect(toList());
 		// @formatter:on
 		MultipleFailuresError multipleFailuresError = new MultipleFailuresError("", failures);