root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Revise use of Objects.requireNonNull() 

Historically, we have rarely intentionally thrown a
NullPointerException in the Spring Framework. Instead, we prefer to
throw either an IllegalArgumentException or IllegalStateException
instead of a NullPointerException.

However, changes to the code in recent times have introduced the use of
Objects.requireNonNull(Object) which throws a NullPointerException
without an explicit error message.

The latter ends up providing less context than a NullPointerException
thrown by the JVM (since Java 14) due to actually de-referencing a
null-pointer. See https://openjdk.org/jeps/358.

In light of that, this commit revises our current use of
Objects.requireNonNull(Object) by removing it or replacing it with
Assert.notNull().

However, we still use Objects.requireNonNull(T, String) in a few places
where we are required to throw a NullPointerException in order to
comply with a third-party contract such as Reactive Streams.

Closes gh-32430
This commit is contained in:
Sam Brannen 2024-03-13 14:31:33 +01:00
parent d6422d368a
commit 986c4fd926
11 changed files with 37 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java
View File

@ -19,7 +19,6 @@ package org.springframework.core;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.Objects;
import kotlin.Unit;
import kotlin.coroutines.CoroutineContext;
@ -112,7 +111,8 @@ public abstract class CoroutinesUtils {
public static Publisher<?> invokeSuspendingFunction(CoroutineContext context, Method method, Object target,
Object... args) {
Assert.isTrue(KotlinDetector.isSuspendingFunction(method), "'method' must be a suspending function");
KFunction<?> function = Objects.requireNonNull(ReflectJvmMapping.getKotlinFunction(method));
KFunction<?> function = ReflectJvmMapping.getKotlinFunction(method);
Assert.notNull(function, () -> "Failed to get Kotlin function for method: " + method);
if (method.isAccessible() && !KCallablesJvm.isAccessible(function)) {
KCallablesJvm.setAccessible(function, true);
}

6
spring-core/src/main/java/org/springframework/core/NamedThreadLocal.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* 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.
@ -16,7 +16,6 @@
package org.springframework.core;
import java.util.Objects;
import java.util.function.Supplier;
import org.springframework.util.Assert;
@ -76,7 +75,8 @@ public class NamedThreadLocal<T> extends ThreadLocal<T> {
SuppliedNamedThreadLocal(String name, Supplier<? extends T> supplier) {
super(name);
this.supplier = Objects.requireNonNull(supplier);
Assert.notNull(supplier, "Supplier must not be null");
this.supplier = supplier;
}
@Override

7
spring-core/src/main/java/org/springframework/core/ReactiveAdapterRegistry.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* 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.
@ -19,7 +19,6 @@ package org.springframework.core;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
@ -383,8 +382,8 @@ public class ReactiveAdapterRegistry {
Method multiPublisher = ClassUtils.getMethod(
io.smallrye.mutiny.groups.MultiCreate.class, "publisher", Flow.Publisher.class);
registry.registerReactiveType(uniDesc,
uni -> FlowAdapters.toPublisher((Flow.Publisher<Object>) Objects.requireNonNull(
ReflectionUtils.invokeMethod(uniToPublisher, ((io.smallrye.mutiny.Uni<?>) uni).convert()))),
uni -> FlowAdapters.toPublisher((Flow.Publisher<Object>)
ReflectionUtils.invokeMethod(uniToPublisher, ((io.smallrye.mutiny.Uni<?>) uni).convert())),
publisher -> ReflectionUtils.invokeMethod(uniPublisher, io.smallrye.mutiny.Uni.createFrom(),
FlowAdapters.toFlowPublisher(publisher)));
registry.registerReactiveType(multiDesc,

4
spring-core/src/main/java/org/springframework/core/io/buffer/OutputStreamPublisher.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* 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.
@ -66,6 +66,8 @@ final class OutputStreamPublisher implements Publisher<DataBuffer> {
@Override
public void subscribe(Subscriber<? super DataBuffer> subscriber) {
// We don't use Assert.notNull(), because a NullPointerException is required
// for Reactive Streams compliance.
Objects.requireNonNull(subscriber, "Subscriber must not be null");
OutputStreamSubscription subscription = new OutputStreamSubscription(

8
spring-test/src/main/java/org/springframework/test/web/reactive/server/CookieAssertions.java
View File

@ -17,7 +17,6 @@
package org.springframework.test.web.reactive.server;
import java.time.Duration;
import java.util.Objects;
import java.util.function.Consumer;
import org.hamcrest.Matcher;
@ -213,10 +212,13 @@ public class CookieAssertions {
private ResponseCookie getCookie(String name) {
ResponseCookie cookie = this.exchangeResult.getResponseCookies().getFirst(name);
if (cookie == null) {
if (cookie != null) {
return cookie;
}
else {
this.exchangeResult.assertWithDiagnostics(() -> fail("No cookie with name '" + name + "'"));
}
return Objects.requireNonNull(cookie);
throw new IllegalStateException("This code path should not be reachable");
}
private static String getMessage(String cookie) {

8
spring-test/src/main/java/org/springframework/test/web/reactive/server/HeaderAssertions.java
View File

@ -19,7 +19,6 @@ package org.springframework.test.web.reactive.server;
import java.net.URI;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import org.hamcrest.Matcher;
@ -200,10 +199,13 @@ public class HeaderAssertions {
private List<String> getRequiredValues(String name) {
List<String> values = getHeaders().get(name);
if (CollectionUtils.isEmpty(values)) {
if (!CollectionUtils.isEmpty(values)) {
return values;
}
else {
this.exchangeResult.assertWithDiagnostics(() -> fail(getMessage(name) + " not found"));
}
return Objects.requireNonNull(values);
throw new IllegalStateException("This code path should not be reachable");
}
/**

3
spring-test/src/test/java/org/springframework/test/context/bean/override/OverrideMetadataTests.java
View File

@ -18,7 +18,6 @@ package org.springframework.test.context.bean.override;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.util.Objects;
import org.junit.jupiter.api.Test;
@ -49,7 +48,7 @@ class OverrideMetadataTests {
private static OverrideMetadata exampleOverride() throws Exception {
Field field = OverrideMetadataTests.class.getDeclaredField("annotated");
return new ConcreteOverrideMetadata(Objects.requireNonNull(field), field.getAnnotation(NonNull.class),
return new ConcreteOverrideMetadata(field, field.getAnnotation(NonNull.class),
ResolvableType.forClass(String.class), BeanOverrideStrategy.REPLACE_DEFINITION);
}

10
spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java
View File

@ -19,7 +19,6 @@ package org.springframework.test.context.bean.override.convention;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Objects;
import org.junit.jupiter.api.Test;
@ -91,7 +90,8 @@ class TestBeanOverrideProcessorTests {
Class<?> clazz = ExplicitMethodNameConf.class;
Class<?> returnType = ExampleService.class;
Field field = clazz.getField("a");
TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class));
TestBean overrideAnnotation = field.getAnnotation(TestBean.class);
assertThat(overrideAnnotation).isNotNull();
TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor();
assertThatIllegalStateException()
@ -106,7 +106,8 @@ class TestBeanOverrideProcessorTests {
void createMetaDataForKnownExplicitMethod() throws Exception {
Class<?> returnType = ExampleService.class;
Field field = ExplicitMethodNameConf.class.getField("b");
TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class));
TestBean overrideAnnotation = field.getAnnotation(TestBean.class);
assertThat(overrideAnnotation).isNotNull();
TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor();
assertThat(processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(returnType)))
@ -117,7 +118,8 @@ class TestBeanOverrideProcessorTests {
void createMetaDataWithDeferredCheckForExistenceOfConventionBasedFactoryMethod() throws Exception {
Class<?> returnType = ExampleService.class;
Field field = MethodConventionConf.class.getField("field");
TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class));
TestBean overrideAnnotation = field.getAnnotation(TestBean.class);
assertThat(overrideAnnotation).isNotNull();
TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor();
// When in convention-based mode, createMetadata() will not verify that

4
spring-web/src/main/java/org/springframework/http/client/OutputStreamPublisher.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* 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.
@ -150,6 +150,8 @@ final class OutputStreamPublisher<T> implements Flow.Publisher<T> {
@Override
public void subscribe(Flow.Subscriber<? super T> subscriber) {
// We don't use Assert.notNull(), because a NullPointerException is required
// for Reactive Streams compliance.
Objects.requireNonNull(subscriber, "Subscriber must not be null");
OutputStreamSubscription<T> subscription = new OutputStreamSubscription<>(subscriber, this.outputStreamHandler,

5
spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java
View File

@ -18,7 +18,6 @@ package org.springframework.web.method.annotation;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import jakarta.servlet.ServletException;
@ -35,6 +34,7 @@ import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodParameter;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.web.bind.ServletRequestBindingException;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.ValueConstants;
@ -342,7 +342,8 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
* or an optional parameter (with a default value in the Kotlin declaration).
*/
public static boolean hasDefaultValue(MethodParameter parameter) {
Method method = Objects.requireNonNull(parameter.getMethod());
Method method = parameter.getMethod();
Assert.notNull(method, () -> "Retrieved null method from MethodParameter: " + parameter);
KFunction<?> function = ReflectJvmMapping.getKotlinFunction(method);
if (function != null) {
int index = 0;

5
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java
View File

@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.method.annotation;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import kotlin.reflect.KFunction;
@ -37,6 +36,7 @@ import org.springframework.core.MethodParameter;
import org.springframework.core.ReactiveAdapterRegistry;
import org.springframework.lang.Nullable;
import org.springframework.ui.Model;
import org.springframework.util.Assert;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.ValueConstants;
import org.springframework.web.reactive.BindingContext;
@ -334,7 +334,8 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr
* or an optional parameter (with a default value in the Kotlin declaration).
*/
public static boolean hasDefaultValue(MethodParameter parameter) {
Method method = Objects.requireNonNull(parameter.getMethod());
Method method = parameter.getMethod();
Assert.notNull(method, () -> "Retrieved null method from MethodParameter: " + parameter);
KFunction<?> function = ReflectJvmMapping.getKotlinFunction(method);
if (function != null) {
int index = 0;