Refine null-safety with NullAway build-time checks

This commit introduces null-safety checks for spring-core at build-time
in order to validate the consistency of Spring null-safety annotations
and generate errors when inconsistencies are detected during a build
(similar to what is done with Checkstyle).

In order to make that possible, this commit also introduces a new
org.springframework.lang.Contract annotation inspired from
org.jetbrains.annotations.Contract, which allows to specify semantics
of methods like Assert#notNull in order to prevent artificial
additional null checks in Spring Framework code base.

This commit only checks org.springframework.core package, follow-up
commits will also extend the analysis to other modules, after related
null-safety refinements.

See gh-32475

Author: sdeleuze

build.gradle

@@ -9,6 +9,7 @@ plugins {
 	id 'de.undercouch.download' version '5.4.0'
 	id 'me.champeau.jmh' version '0.7.2' apply false
 	id 'me.champeau.mrjar' version '0.1.1'
+	id "net.ltgt.errorprone" version "3.1.0" apply false
 }
 
 ext {

gradle/spring-module.gradle

@@ -6,12 +6,15 @@ apply plugin: 'org.springframework.build.optional-dependencies'
 // apply plugin: 'com.github.johnrengelman.shadow'
 apply plugin: 'me.champeau.jmh'
 apply from: "$rootDir/gradle/publications.gradle"
+apply plugin: 'net.ltgt.errorprone'
 
 dependencies {
 	jmh 'org.openjdk.jmh:jmh-core:1.37'
 	jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.37'
 	jmh 'org.openjdk.jmh:jmh-generator-bytecode:1.37'
 	jmh 'net.sf.jopt-simple:jopt-simple'
+	errorprone 'com.uber.nullaway:nullaway:0.10.24'
+	errorprone 'com.google.errorprone:error_prone_core:2.9.0'
 }
 
 pluginManager.withPlugin("kotlin") {
@@ -109,3 +112,18 @@ publishing {
 // Disable publication of test fixture artifacts.
 components.java.withVariantsFromConfiguration(configurations.testFixturesApiElements) { skip() }
 components.java.withVariantsFromConfiguration(configurations.testFixturesRuntimeElements) { skip() }
+
+tasks.withType(JavaCompile).configureEach {
+    options.errorprone {
+    	disableAllChecks = true
+        option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract")
+        option("NullAway:AnnotatedPackages", "org.springframework.core")
+        option("NullAway:UnannotatedSubPackages", "org.springframework.instrument,org.springframework.context.index," +
+				"org.springframework.asm,org.springframework.cglib,org.springframework.objenesis," +
+				"org.springframework.javapoet,org.springframework.aot.nativex.substitution")
+    }
+}
+tasks.compileJava {
+    // The check defaults to a warning, bump it up to an error for the main sources
+    options.errorprone.error("NullAway")
+}

spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java

@@ -22,6 +22,7 @@
 import java.util.Map;
 import java.util.Objects;
 
+import org.springframework.lang.Contract;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ConcurrentReferenceHashMap;
@@ -80,6 +81,7 @@ Annotation[] findRepeatedAnnotations(Annotation annotation) {
 
 
 	@Override
+	@Contract("null -> false")
 	public boolean equals(@Nullable Object other) {
 		if (other == this) {
 			return true;

spring-core/src/main/java/org/springframework/lang/Contract.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2002-2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.lang;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Inspired from {@code org.jetbrains.annotations.Contract}, this variant has been introduce in the
+ * {@code org.springframework.lang} package to avoid requiring an extra dependency, while still following the same semantics.
+ *
+ * <p>Specifies some aspects of the method behavior depending on the arguments. Can be used by tools for advanced data flow analysis.
+ * Note that this annotation just describes how the code works and doesn't add any functionality by means of code generation.
+ *
+ * <p>Method contract has the following syntax:<br/>
+ *  contract ::= (clause ';')* clause<br/>
+ *  clause ::= args '->' effect<br/>
+ *  args ::= ((arg ',')* arg )?<br/>
+ *  arg ::= value-constraint<br/>
+ *  value-constraint ::= 'any' | 'null' | '!null' | 'false' | 'true'<br/>
+ *  effect ::= value-constraint | 'fail'
+ *
+ * The constraints denote the following:<br/>
+ * <ul>
+ * <li> _ - any value
+ * <li> null - null value
+ * <li> !null - a value statically proved to be not-null
+ * <li> true - true boolean value
+ * <li> false - false boolean value
+ * <li> fail - the method throws an exception, if the arguments satisfy argument constraints
+ * </ul>
+ * <p>Examples:
+ * <code>@Contract("_, null -> null")</code> - method returns null if its second argument is null<br/>
+ * <code>@Contract("_, null -> null; _, !null -> !null")</code> - method returns null if its second argument is null and not-null otherwise<br/>
+ * <code>@Contract("true -> fail")</code> - a typical assertFalse method which throws an exception if <code>true</code> is passed to it<br/>
+ *
+ * @author Sebastien Deleuze
+ * @since 6.2
+ * @see <a href="https://github.com/uber/NullAway/wiki/Configuration#custom-contract-annotations">NullAway custom contract annotations</a>
+ */
+@Documented
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.METHOD)
+public @interface Contract {
+
+	/**
+	 * Contains the contract clauses describing causal relations between call arguments and the returned value.
+	 */
+	String value() default "";
+
+	/**
+	 * Specifies if this method is pure, i.e. has no visible side effects. This may be used for more precise data flow analysis, and
+	 * to check that the method's return value is actually used in the call place.
+	 */
+	boolean pure() default false;
+}

spring-core/src/main/java/org/springframework/util/Assert.java

@@ -20,6 +20,7 @@
 import java.util.Map;
 import java.util.function.Supplier;
 
+import org.springframework.lang.Contract;
 import org.springframework.lang.Nullable;
 
 /**
@@ -71,6 +72,7 @@ public abstract class Assert {
 	 * @param message the exception message to use if the assertion fails
 	 * @throws IllegalStateException if {@code expression} is {@code false}
 	 */
+	@Contract("false, _ -> fail")
 	public static void state(boolean expression, String message) {
 		if (!expression) {
 			throw new IllegalStateException(message);
@@ -92,6 +94,7 @@ public static void state(boolean expression, String message) {
 	 * @throws IllegalStateException if {@code expression} is {@code false}
 	 * @since 5.0
 	 */
+	@Contract("false, _ -> fail")
 	public static void state(boolean expression, Supplier<String> messageSupplier) {
 		if (!expression) {
 			throw new IllegalStateException(nullSafeGet(messageSupplier));
@@ -106,6 +109,7 @@ public static void state(boolean expression, Supplier<String> messageSupplier) {
 	 * @param message the exception message to use if the assertion fails
 	 * @throws IllegalArgumentException if {@code expression} is {@code false}
 	 */
+	@Contract("false, _ -> fail")
 	public static void isTrue(boolean expression, String message) {
 		if (!expression) {
 			throw new IllegalArgumentException(message);
@@ -124,6 +128,7 @@ public static void isTrue(boolean expression, String message) {
 	 * @throws IllegalArgumentException if {@code expression} is {@code false}
 	 * @since 5.0
 	 */
+	@Contract("false, _ -> fail")
 	public static void isTrue(boolean expression, Supplier<String> messageSupplier) {
 		if (!expression) {
 			throw new IllegalArgumentException(nullSafeGet(messageSupplier));
@@ -137,6 +142,7 @@ public static void isTrue(boolean expression, Supplier<String> messageSupplier)
 	 * @param message the exception message to use if the assertion fails
 	 * @throws IllegalArgumentException if the object is not {@code null}
 	 */
+	@Contract("!null, _ -> fail")
 	public static void isNull(@Nullable Object object, String message) {
 		if (object != null) {
 			throw new IllegalArgumentException(message);
@@ -154,6 +160,7 @@ public static void isNull(@Nullable Object object, String message) {
 	 * @throws IllegalArgumentException if the object is not {@code null}
 	 * @since 5.0
 	 */
+	@Contract("!null, _ -> fail")
 	public static void isNull(@Nullable Object object, Supplier<String> messageSupplier) {
 		if (object != null) {
 			throw new IllegalArgumentException(nullSafeGet(messageSupplier));
@@ -167,6 +174,7 @@ public static void isNull(@Nullable Object object, Supplier<String> messageSuppl
 	 * @param message the exception message to use if the assertion fails
 	 * @throws IllegalArgumentException if the object is {@code null}
 	 */
+	@Contract("null, _ -> fail")
 	public static void notNull(@Nullable Object object, String message) {
 		if (object == null) {
 			throw new IllegalArgumentException(message);
@@ -185,6 +193,7 @@ public static void notNull(@Nullable Object object, String message) {
 	 * @throws IllegalArgumentException if the object is {@code null}
 	 * @since 5.0
 	 */
+	@Contract("null, _ -> fail")
 	public static void notNull(@Nullable Object object, Supplier<String> messageSupplier) {
 		if (object == null) {
 			throw new IllegalArgumentException(nullSafeGet(messageSupplier));

spring-core/src/main/java/org/springframework/util/ClassUtils.java

@@ -544,6 +544,7 @@ public static boolean isPrimitiveWrapperArray(Class<?> clazz) {
 	 * @param clazz the class to check
 	 * @return the original class, or a primitive wrapper for the original primitive type
 	 */
+	@SuppressWarnings("NullAway")
 	public static Class<?> resolvePrimitiveIfNecessary(Class<?> clazz) {
 		Assert.notNull(clazz, "Class must not be null");
 		return (clazz.isPrimitive() && clazz != void.class ? primitiveTypeToWrapperMap.get(clazz) : clazz);

spring-core/src/main/java/org/springframework/util/CollectionUtils.java

@@ -34,6 +34,7 @@
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 
+import org.springframework.lang.Contract;
 import org.springframework.lang.Nullable;
 
 /**
@@ -71,6 +72,7 @@ public static boolean isEmpty(@Nullable Collection<?> collection) {
 	 * @param map the Map to check
 	 * @return whether the given Map is empty
 	 */
+	@Contract("null -> true")
 	public static boolean isEmpty(@Nullable Map<?, ?> map) {
 		return (map == null || map.isEmpty());
 	}

spring-core/src/main/java/org/springframework/util/ConcurrentLruCache.java

@@ -45,7 +45,7 @@
  * @param <V> the type of the cached values, does not allow null values
  * @see #get(Object)
  */
-@SuppressWarnings({"unchecked"})
+@SuppressWarnings({"unchecked", "NullAway"})
 public final class ConcurrentLruCache<K, V> {
 
 	private final int capacity;

spring-core/src/main/java/org/springframework/util/MimeType.java

@@ -573,6 +573,7 @@ public int compareTo(MimeType other) {
 			else {
 				String thisValue = getParameters().get(thisAttribute);
 				String otherValue = other.getParameters().get(otherAttribute);
+				Assert.notNull(thisValue, "Parameter for " + thisAttribute + " must not be null");
 				if (otherValue == null) {
 					otherValue = "";
 				}

spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java

@@ -213,6 +213,7 @@ public static MimeType parseMimeType(String mimeType) {
 		return cachedMimeTypes.get(mimeType);
 	}
 
+	@SuppressWarnings("NullAway")
 	private static MimeType parseMimeTypeInternal(String mimeType) {
 		int index = mimeType.indexOf(';');
 		String fullType = (index >= 0 ? mimeType.substring(0, index) : mimeType).trim();

spring-core/src/main/java/org/springframework/util/NumberUtils.java

@@ -238,6 +238,7 @@ else if (BigDecimal.class == targetClass || Number.class == targetClass) {
 	 * @see #convertNumberToTargetClass
 	 * @see #parseNumber(String, Class)
 	 */
+	@SuppressWarnings("NullAway")
 	public static <T extends Number> T parseNumber(
 			String text, Class<T> targetClass, @Nullable NumberFormat numberFormat) {
 

spring-core/src/main/java/org/springframework/util/StringUtils.java

@@ -36,6 +36,7 @@
 import java.util.TimeZone;
 import java.util.stream.Collectors;
 
+import org.springframework.lang.Contract;
 import org.springframework.lang.Nullable;
 
 /**
@@ -122,6 +123,7 @@ public static boolean isEmpty(@Nullable Object str) {
 	 * @see #hasLength(String)
 	 * @see #hasText(CharSequence)
 	 */
+	@Contract("null -> false")
 	public static boolean hasLength(@Nullable CharSequence str) {
 		return (str != null && str.length() > 0);
 	}
@@ -135,6 +137,7 @@ public static boolean hasLength(@Nullable CharSequence str) {
 	 * @see #hasLength(CharSequence)
 	 * @see #hasText(String)
 	 */
+	@Contract("null -> false")
 	public static boolean hasLength(@Nullable String str) {
 		return (str != null && !str.isEmpty());
 	}
@@ -158,6 +161,7 @@ public static boolean hasLength(@Nullable String str) {
 	 * @see #hasLength(CharSequence)
 	 * @see Character#isWhitespace
 	 */
+	@Contract("null -> false")
 	public static boolean hasText(@Nullable CharSequence str) {
 		if (str == null) {
 			return false;
@@ -188,6 +192,7 @@ public static boolean hasText(@Nullable CharSequence str) {
 	 * @see #hasLength(String)
 	 * @see Character#isWhitespace
 	 */
+	@Contract("null -> false")
 	public static boolean hasText(@Nullable String str) {
 		return (str != null && !str.isBlank());
 	}

spring-core/src/main/java/org/springframework/util/concurrent/ListenableFuture.java

@@ -62,6 +62,7 @@ public interface ListenableFuture<T> extends Future<T> {
 	 * Expose this {@link ListenableFuture} as a JDK {@link CompletableFuture}.
 	 * @since 5.0
 	 */
+	@SuppressWarnings("NullAway")
 	default CompletableFuture<T> completable() {
 		CompletableFuture<T> completable = new DelegatingCompletableFuture<>(this);
 		addCallback(completable::complete, completable::completeExceptionally);

spring-core/src/main/java/org/springframework/util/concurrent/ListenableFutureTask.java

@@ -70,6 +70,7 @@ public void addCallback(SuccessCallback<? super T> successCallback, FailureCallb
 	}
 
 	@Override
+	@SuppressWarnings("NullAway")
 	public CompletableFuture<T> completable() {
 		CompletableFuture<T> completable = new DelegatingCompletableFuture<>(this);
 		this.callbacks.addSuccessCallback(completable::complete);