Refine null-safety in the spring-test module

sdeleuze · sdeleuze · commit a442c180f195 · 2024-12-26T17:49:17.000+01:00
Closes gh-34161
diff --git a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java
@@ -410,7 +410,7 @@ private String[] getScripts(Sql sql, Class<?> testClass, @Nullable Method testMe
 	 * Detect a default SQL script by implementing the algorithm defined in
 	 * {@link Sql#scripts}.
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	private String detectDefaultScript(Class<?> testClass, @Nullable Method testMethod, boolean classLevel) {
 		Assert.state(classLevel || testMethod != null, "Method-level @Sql requires a testMethod");
 
diff --git a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java
@@ -373,7 +373,7 @@ private static Store getStore(ExtensionContext context) {
 	 * the supplied {@link TestContextManager}.
 	 * @since 6.1
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // org.junit.jupiter.api.extension.ExecutableInvoker is not null marked
 	private static void registerMethodInvoker(TestContextManager testContextManager, ExtensionContext context) {
 		testContextManager.getTestContext().setMethodInvoker(context.getExecutableInvoker()::invoke);
 	}
diff --git a/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java
@@ -84,7 +84,7 @@ protected void dirtyContext(TestContext testContext, @Nullable HierarchyMode hie
 	 * @since 4.2
 	 * @see #dirtyContext
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	protected void beforeOrAfterTestMethod(TestContext testContext, MethodMode requiredMethodMode,
 			ClassMode requiredClassMode) throws Exception {
 
@@ -136,7 +136,7 @@ else if (logger.isDebugEnabled()) {
 	 * @since 4.2
 	 * @see #dirtyContext
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	protected void beforeOrAfterTestClass(TestContext testContext, ClassMode requiredClassMode) throws Exception {
 		Assert.notNull(testContext, "TestContext must not be null");
 		Assert.notNull(requiredClassMode, "requiredClassMode must not be null");
diff --git a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java
@@ -232,7 +232,7 @@ static Map<String, List<ContextConfigurationAttributes>> buildContextHierarchyMa
 	 * @throws IllegalArgumentException if the supplied class is {@code null} or if
 	 * {@code @ContextConfiguration} is not <em>present</em> on the supplied class
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	static List<ContextConfigurationAttributes> resolveContextConfigurationAttributes(Class<?> testClass) {
 		Assert.notNull(testClass, "Class must not be null");
 
diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java
@@ -134,7 +134,7 @@ else if (!duplicationDetected(currentAttributes, previousAttributes)) {
 		return mergedAttributes;
 	}
 
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	private static boolean duplicationDetected(TestPropertySourceAttributes currentAttributes,
 			@Nullable TestPropertySourceAttributes previousAttributes) {
 
diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java
@@ -194,7 +194,7 @@ public final int getOrder() {
 	 * @see #getTransactionManager(TestContext, String)
 	 */
 	@Override
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public void beforeTestMethod(final TestContext testContext) throws Exception {
 		Method testMethod = testContext.getTestMethod();
 		Class<?> testClass = testContext.getTestClass();
diff --git a/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java b/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java
@@ -103,7 +103,6 @@ public MediaTypeAssert isCompatibleWith(String mediaType) {
 	}
 
 
-	@SuppressWarnings("NullAway")
 	private MediaType parseMediaType(String value) {
 		try {
 			return MediaType.parseMediaType(value);
diff --git a/spring-test/src/main/java/org/springframework/test/json/AbstractJsonContentAssert.java b/spring-test/src/main/java/org/springframework/test/json/AbstractJsonContentAssert.java
@@ -489,7 +489,7 @@ public SELF withCharset(@Nullable Charset charset) {
 		return (this.actual != null ? this.actual.getJson() : null);
 	}
 
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	private String toNonNullJsonString() {
 		String jsonString = toJsonString();
 		Assertions.assertThat(jsonString).as("JSON content").isNotNull();
diff --git a/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java b/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java
@@ -173,7 +173,7 @@ public static void setField(
 	 * @see ReflectionUtils#setField(Field, Object, Object)
 	 * @see AopTestUtils#getUltimateTargetObject(Object)
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public static void setField(@Nullable Object targetObject, @Nullable Class<?> targetClass,
 			@Nullable String name, @Nullable Object value, @Nullable Class<?> type) {
 
@@ -258,7 +258,7 @@ public static void setField(@Nullable Object targetObject, @Nullable Class<?> ta
 	 * @see ReflectionUtils#getField(Field, Object)
 	 * @see AopTestUtils#getUltimateTargetObject(Object)
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public static @Nullable Object getField(@Nullable Object targetObject, @Nullable Class<?> targetClass, String name) {
 		Assert.isTrue(targetObject != null || targetClass != null,
 			"Either targetObject or targetClass for the field must be specified");
diff --git a/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java b/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java
@@ -109,7 +109,7 @@ public static void assertModelAttributeValue(ModelAndView mav, String modelName,
 	 * @param mav the ModelAndView to test against (never {@code null})
 	 * @param expectedModel the expected model
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public static void assertModelAttributeValues(ModelAndView mav, Map<String, Object> expectedModel) {
 		Map<String, Object> model = mav.getModel();
 
diff --git a/spring-test/src/main/java/org/springframework/test/web/UriAssert.java b/spring-test/src/main/java/org/springframework/test/web/UriAssert.java
@@ -80,22 +80,32 @@ public UriAssert matchesAntPattern(String uriPattern) {
 		return this;
 	}
 
-	@SuppressWarnings("NullAway")
+
 	private String buildUri(String uriTemplate, Object... uriVars) {
 		try {
 			return UriComponentsBuilder.fromUriString(uriTemplate)
 					.buildAndExpand(uriVars).encode().toUriString();
 		}
 		catch (Exception ex) {
+			String message = ex.getMessage();
 			throw Failures.instance().failure(this.info,
-					new ShouldBeValidUriTemplate(uriTemplate, ex.getMessage()));
+					message == null ?
+							new ShouldBeValidUriTemplate(uriTemplate) :
+							new ShouldBeValidUriTemplateWithMessage(uriTemplate, message));
 		}
 	}
 
 
 	private static final class ShouldBeValidUriTemplate extends BasicErrorMessageFactory {
 
-		private ShouldBeValidUriTemplate(String uriTemplate, String errorMessage) {
+		private ShouldBeValidUriTemplate(String uriTemplate) {
+			super("%nExpecting:%n  %s%nTo be a valid URI template%n", uriTemplate);
+		}
+	}
+
+	private static final class ShouldBeValidUriTemplateWithMessage extends BasicErrorMessageFactory {
+
+		private ShouldBeValidUriTemplateWithMessage(String uriTemplate, String errorMessage) {
 			super("%nExpecting:%n  %s%nTo be a valid URI template but got:%n  %s%n", uriTemplate, errorMessage);
 		}
 	}
diff --git a/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java b/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java
@@ -159,7 +159,7 @@ public static RequestMatcher queryParamList(String name, Matcher<? super List<St
 	 * @see #queryParam(String, String...)
 	 */
 	@SafeVarargs
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public static RequestMatcher queryParam(String name, Matcher<? super String>... matchers) {
 		return request -> {
 			MultiValueMap<String, String> params = getQueryParams(request);
@@ -187,7 +187,7 @@ public static RequestMatcher queryParam(String name, Matcher<? super String>...
 	 * @see #queryParamList(String, Matcher)
 	 * @see #queryParam(String, Matcher...)
 	 */
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public static RequestMatcher queryParam(String name, String... expectedValues) {
 		return request -> {
 			MultiValueMap<String, String> params = getQueryParams(request);
diff --git a/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java b/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java
@@ -62,7 +62,7 @@ class WiretapConnector implements ClientHttpConnector {
 
 
 	@Override
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	public Mono<ClientHttpResponse> connect(HttpMethod method, URI uri,
 			Function<? super ClientHttpRequest, Mono<Void>> requestCallback) {
 
@@ -180,7 +180,7 @@ public Publisher<? extends Publisher<? extends DataBuffer>> getNestedPublisherTo
 			return this.publisherNested;
 		}
 
-		@SuppressWarnings("NullAway")
+		@SuppressWarnings("NullAway") // Dataflow analysis limitation
 		public Mono<byte[]> getContent() {
 			return Mono.defer(() -> {
 				if (this.content.scan(Scannable.Attr.TERMINATED) == Boolean.TRUE) {
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java b/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java
@@ -124,13 +124,12 @@ public void setAsyncResult(@Nullable Object asyncResult) {
 	}
 
 	@Override
-	public Object getAsyncResult() {
+	public @Nullable Object getAsyncResult() {
 		return getAsyncResult(-1);
 	}
 
 	@Override
-	@SuppressWarnings("NullAway")
-	public Object getAsyncResult(long timeToWait) {
+	public @Nullable Object getAsyncResult(long timeToWait) {
 		if (this.mockRequest.getAsyncContext() != null && timeToWait == -1) {
 			long requestTimeout = this.mockRequest.getAsyncContext().getTimeout();
 			timeToWait = requestTimeout == -1 ? Long.MAX_VALUE : requestTimeout;
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java b/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java
@@ -85,7 +85,7 @@ public interface MvcResult {
 	 * {@link #getAsyncResult(long)} to specify the amount of time to wait.
 	 * @throws IllegalStateException if the async result was not set
 	 */
-	Object getAsyncResult();
+	@Nullable Object getAsyncResult();
 
 	/**
 	 * Get the result of async execution and wait if necessary.
@@ -96,6 +96,6 @@ public interface MvcResult {
 	 * 	MockAsyncContext#setTimeout} for more details.
 	 * @throws IllegalStateException if the async result was not set
 	 */
-	Object getAsyncResult(long timeToWait);
+	@Nullable Object getAsyncResult(long timeToWait);
 
 }
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java
@@ -212,7 +212,7 @@ public MvcTestResultAssert hasViewName(String viewName) {
 		return this.actual.getMvcResult().getResolvedException();
 	}
 
-	@SuppressWarnings("NullAway")
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	private ModelAndView getModelAndView() {
 		ModelAndView modelAndView = getMvcResult().getModelAndView();
 		Assertions.assertThat(modelAndView).as("ModelAndView").isNotNull();
diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java b/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java
@@ -292,12 +292,12 @@ public FlashMap getFlashMap() {
 		}
 
 		@Override
-		public Object getAsyncResult() {
+		public @Nullable Object getAsyncResult() {
 			return this.mvcResult.getAsyncResult();
 		}
 
 		@Override
-		public Object getAsyncResult(long timeToWait) {
+		public @Nullable Object getAsyncResult(long timeToWait) {
 			return this.mvcResult.getAsyncResult(timeToWait);
 		}
 
diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/StubMvcResult.java b/spring-test/src/test/java/org/springframework/test/web/servlet/StubMvcResult.java
@@ -16,6 +16,8 @@
 
 package org.springframework.test.web.servlet;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.web.servlet.FlashMap;
@@ -127,12 +129,12 @@ public void setResponse(MockHttpServletResponse response) {
 	}
 
 	@Override
-	public Object getAsyncResult() {
+	public @Nullable Object getAsyncResult() {
 		return null;
 	}
 
 	@Override
-	public Object getAsyncResult(long timeToWait) {
+	public @Nullable Object getAsyncResult(long timeToWait) {
 		return null;
 	}
 
diff --git a/spring-web/src/main/java/org/springframework/http/InvalidMediaTypeException.java b/spring-web/src/main/java/org/springframework/http/InvalidMediaTypeException.java
@@ -51,6 +51,14 @@ public InvalidMediaTypeException(String mediaType, @Nullable String message) {
 		this.mediaType = ex.getMimeType();
 	}
 
+	/**
+	 * Returns the detail message string of this exception instance (never {@code null}).
+	 */
+	@Override
+	@SuppressWarnings("NullAway") // InvalidMediaTypeException message is never null
+	public String getMessage() {
+		return super.getMessage();
+	}
 
 	/**
 	 * Return the offending media type.

Original file line number	Diff line number	Diff line change
`@@ -373,7 +373,7 @@ private static Store getStore(ExtensionContext context) {`
`373`	`373`	`* the supplied {@link TestContextManager}.`
`374`	`374`	`* @since 6.1`
`375`	`375`	`*/`
`376`		`- @SuppressWarnings("NullAway")`
	`376`	`+ @SuppressWarnings("NullAway") // org.junit.jupiter.api.extension.ExecutableInvoker is not null marked`
`377`	`377`	`private static void registerMethodInvoker(TestContextManager testContextManager, ExtensionContext context) {`
`378`	`378`	`testContextManager.getTestContext().setMethodInvoker(context.getExecutableInvoker()::invoke);`
`379`	`379`	`}`
Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ else if (!duplicationDetected(currentAttributes, previousAttributes)) {`
`134`	`134`	`return mergedAttributes;`
`135`	`135`	`}`
`136`	`136`
`137`		`- @SuppressWarnings("NullAway")`
	`137`	`+ @SuppressWarnings("NullAway") // Dataflow analysis limitation`
`138`	`138`	`private static boolean duplicationDetected(TestPropertySourceAttributes currentAttributes,`
`139`	`139`	`@Nullable TestPropertySourceAttributes previousAttributes) {`
`140`	`140`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,6 @@ public MediaTypeAssert isCompatibleWith(String mediaType) {`
`103`	`103`	`}`
`104`	`104`
`105`	`105`
`106`		`- @SuppressWarnings("NullAway")`
`107`	`106`	`private MediaType parseMediaType(String value) {`
`108`	`107`	`try {`
`109`	`108`	`return MediaType.parseMediaType(value);`
Original file line number	Diff line number	Diff line change
`@@ -489,7 +489,7 @@ public SELF withCharset(@Nullable Charset charset) {`
`489`	`489`	`return (this.actual != null ? this.actual.getJson() : null);`
`490`	`490`	`}`
`491`	`491`
`492`		`- @SuppressWarnings("NullAway")`
	`492`	`+ @SuppressWarnings("NullAway") // Dataflow analysis limitation`
`493`	`493`	`private String toNonNullJsonString() {`
`494`	`494`	`String jsonString = toJsonString();`
`495`	`495`	`Assertions.assertThat(jsonString).as("JSON content").isNotNull();`