Refine null-safety in the spring-aop module

Closes gh-34154
This commit is contained in:
Sébastien Deleuze 2024-12-26 13:53:25 +01:00
parent b4a2cdf028
commit 92f1f55957
10 changed files with 23 additions and 17 deletions

View File

@ -545,14 +545,13 @@ public abstract class AbstractAspectJAdvice implements Advice, AspectJPrecedence
* @param ex the exception thrown by the method execution (may be null) * @param ex the exception thrown by the method execution (may be null)
* @return the empty array if there are no arguments * @return the empty array if there are no arguments
*/ */
@SuppressWarnings("NullAway") protected @Nullable Object[] argBinding(JoinPoint jp, @Nullable JoinPointMatch jpMatch,
protected Object[] argBinding(JoinPoint jp, @Nullable JoinPointMatch jpMatch,
@Nullable Object returnValue, @Nullable Throwable ex) { @Nullable Object returnValue, @Nullable Throwable ex) {
calculateArgumentBindings(); calculateArgumentBindings();
// AMC start // AMC start
Object[] adviceInvocationArgs = new Object[this.parameterTypes.length]; @Nullable Object[] adviceInvocationArgs = new Object[this.parameterTypes.length];
int numBound = 0; int numBound = 0;
if (this.joinPointArgumentIndex != -1) { if (this.joinPointArgumentIndex != -1) {
@ -571,6 +570,7 @@ public abstract class AbstractAspectJAdvice implements Advice, AspectJPrecedence
for (PointcutParameter parameter : parameterBindings) { for (PointcutParameter parameter : parameterBindings) {
String name = parameter.getName(); String name = parameter.getName();
Integer index = this.argumentBindings.get(name); Integer index = this.argumentBindings.get(name);
Assert.notNull(index, "Index must not be null");
adviceInvocationArgs[index] = parameter.getBinding(); adviceInvocationArgs[index] = parameter.getBinding();
numBound++; numBound++;
} }
@ -578,12 +578,14 @@ public abstract class AbstractAspectJAdvice implements Advice, AspectJPrecedence
// binding from returning clause // binding from returning clause
if (this.returningName != null) { if (this.returningName != null) {
Integer index = this.argumentBindings.get(this.returningName); Integer index = this.argumentBindings.get(this.returningName);
Assert.notNull(index, "Index must not be null");
adviceInvocationArgs[index] = returnValue; adviceInvocationArgs[index] = returnValue;
numBound++; numBound++;
} }
// binding from thrown exception // binding from thrown exception
if (this.throwingName != null) { if (this.throwingName != null) {
Integer index = this.argumentBindings.get(this.throwingName); Integer index = this.argumentBindings.get(this.throwingName);
Assert.notNull(index, "Index must not be null");
adviceInvocationArgs[index] = ex; adviceInvocationArgs[index] = ex;
numBound++; numBound++;
} }

View File

@ -84,7 +84,6 @@ public class BeanFactoryAspectJAdvisorsBuilder {
* @return the list of {@link org.springframework.aop.Advisor} beans * @return the list of {@link org.springframework.aop.Advisor} beans
* @see #isEligibleBean * @see #isEligibleBean
*/ */
@SuppressWarnings("NullAway")
public List<Advisor> buildAspectJAdvisors() { public List<Advisor> buildAspectJAdvisors() {
List<String> aspectNames = this.aspectBeanNames; List<String> aspectNames = this.aspectBeanNames;
@ -158,6 +157,7 @@ public class BeanFactoryAspectJAdvisorsBuilder {
} }
else { else {
MetadataAwareAspectInstanceFactory factory = this.aspectFactoryCache.get(aspectName); MetadataAwareAspectInstanceFactory factory = this.aspectFactoryCache.get(aspectName);
Assert.notNull(factory, "Factory must not be null");
advisors.addAll(this.advisorFactory.getAdvisors(factory)); advisors.addAll(this.advisorFactory.getAdvisors(factory));
} }
} }

View File

@ -75,9 +75,11 @@ final class InstantiationModelAwarePointcutAdvisorImpl
private @Nullable Advice instantiatedAdvice; private @Nullable Advice instantiatedAdvice;
private @Nullable Boolean isBeforeAdvice; @SuppressWarnings("NullAway.Init")
private Boolean isBeforeAdvice;
private @Nullable Boolean isAfterAdvice; @SuppressWarnings("NullAway.Init")
private Boolean isAfterAdvice;
public InstantiationModelAwarePointcutAdvisorImpl(AspectJExpressionPointcut declaredPointcut, public InstantiationModelAwarePointcutAdvisorImpl(AspectJExpressionPointcut declaredPointcut,
@ -192,7 +194,6 @@ final class InstantiationModelAwarePointcutAdvisorImpl
} }
@Override @Override
@SuppressWarnings("NullAway")
public boolean isBeforeAdvice() { public boolean isBeforeAdvice() {
if (this.isBeforeAdvice == null) { if (this.isBeforeAdvice == null) {
determineAdviceType(); determineAdviceType();
@ -201,7 +202,6 @@ final class InstantiationModelAwarePointcutAdvisorImpl
} }
@Override @Override
@SuppressWarnings("NullAway")
public boolean isAfterAdvice() { public boolean isAfterAdvice() {
if (this.isAfterAdvice == null) { if (this.isAfterAdvice == null) {
determineAdviceType(); determineAdviceType();

View File

@ -304,7 +304,7 @@ public abstract class AsyncExecutionAspectSupport implements BeanFactoryAware {
* @param method the method that was invoked * @param method the method that was invoked
* @param params the parameters used to invoke the method * @param params the parameters used to invoke the method
*/ */
protected void handleError(Throwable ex, Method method, Object... params) throws Exception { protected void handleError(Throwable ex, Method method, @Nullable Object... params) throws Exception {
if (Future.class.isAssignableFrom(method.getReturnType())) { if (Future.class.isAssignableFrom(method.getReturnType())) {
ReflectionUtils.rethrowException(ex); ReflectionUtils.rethrowException(ex);
} }

View File

@ -97,7 +97,6 @@ public class AsyncExecutionInterceptor extends AsyncExecutionAspectSupport imple
* otherwise. * otherwise.
*/ */
@Override @Override
@SuppressWarnings("NullAway")
public @Nullable Object invoke(final MethodInvocation invocation) throws Throwable { public @Nullable Object invoke(final MethodInvocation invocation) throws Throwable {
Class<?> targetClass = (invocation.getThis() != null ? AopUtils.getTargetClass(invocation.getThis()) : null); Class<?> targetClass = (invocation.getThis() != null ? AopUtils.getTargetClass(invocation.getThis()) : null);
final Method userMethod = BridgeMethodResolver.getMostSpecificMethod(invocation.getMethod(), targetClass); final Method userMethod = BridgeMethodResolver.getMostSpecificMethod(invocation.getMethod(), targetClass);
@ -116,7 +115,8 @@ public class AsyncExecutionInterceptor extends AsyncExecutionAspectSupport imple
} }
} }
catch (ExecutionException ex) { catch (ExecutionException ex) {
handleError(ex.getCause(), userMethod, invocation.getArguments()); Throwable cause = ex.getCause();
handleError(cause == null ? ex : cause, userMethod, invocation.getArguments());
} }
catch (Throwable ex) { catch (Throwable ex) {
handleError(ex, userMethod, invocation.getArguments()); handleError(ex, userMethod, invocation.getArguments());

View File

@ -18,6 +18,8 @@ package org.springframework.aop.interceptor;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import org.jspecify.annotations.Nullable;
/** /**
* A strategy for handling uncaught exceptions thrown from asynchronous methods. * A strategy for handling uncaught exceptions thrown from asynchronous methods.
* *
@ -38,6 +40,6 @@ public interface AsyncUncaughtExceptionHandler {
* @param method the asynchronous method * @param method the asynchronous method
* @param params the parameters used to invoke the method * @param params the parameters used to invoke the method
*/ */
void handleUncaughtException(Throwable ex, Method method, Object... params); void handleUncaughtException(Throwable ex, Method method, @Nullable Object... params);
} }

View File

@ -20,6 +20,7 @@ import java.lang.reflect.Method;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.jspecify.annotations.Nullable;
/** /**
* A default {@link AsyncUncaughtExceptionHandler} that simply logs the exception. * A default {@link AsyncUncaughtExceptionHandler} that simply logs the exception.
@ -34,7 +35,8 @@ public class SimpleAsyncUncaughtExceptionHandler implements AsyncUncaughtExcepti
@Override @Override
public void handleUncaughtException(Throwable ex, Method method, Object... params) { @SuppressWarnings("NullAway") // https://github.com/uber/NullAway/issues/1113
public void handleUncaughtException(Throwable ex, Method method, @Nullable Object... params) {
if (logger.isErrorEnabled()) { if (logger.isErrorEnabled()) {
logger.error("Unexpected exception occurred invoking async method: " + method, ex); logger.error("Unexpected exception occurred invoking async method: " + method, ex);
} }

View File

@ -53,7 +53,7 @@ class ScopedProxyBeanRegistrationAotProcessor implements BeanRegistrationAotProc
@Override @Override
@SuppressWarnings("NullAway") @SuppressWarnings("NullAway") // Lambda
public @Nullable BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) { public @Nullable BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) {
Class<?> beanClass = registeredBean.getBeanClass(); Class<?> beanClass = registeredBean.getBeanClass();
if (beanClass.equals(ScopedProxyFactoryBean.class)) { if (beanClass.equals(ScopedProxyFactoryBean.class)) {

View File

@ -88,7 +88,7 @@ public class AnnotationMatchingPointcut implements Pointcut {
* @see AnnotationClassFilter#AnnotationClassFilter(Class, boolean) * @see AnnotationClassFilter#AnnotationClassFilter(Class, boolean)
* @see AnnotationMethodMatcher#AnnotationMethodMatcher(Class, boolean) * @see AnnotationMethodMatcher#AnnotationMethodMatcher(Class, boolean)
*/ */
@SuppressWarnings("NullAway") @SuppressWarnings("NullAway") // Dataflow analysis limitation
public AnnotationMatchingPointcut(@Nullable Class<? extends Annotation> classAnnotationType, public AnnotationMatchingPointcut(@Nullable Class<? extends Annotation> classAnnotationType,
@Nullable Class<? extends Annotation> methodAnnotationType, boolean checkInherited) { @Nullable Class<? extends Annotation> methodAnnotationType, boolean checkInherited) {

View File

@ -42,7 +42,8 @@ public abstract class AbstractRefreshableTargetSource implements TargetSource, R
/** Logger available to subclasses. */ /** Logger available to subclasses. */
protected final Log logger = LogFactory.getLog(getClass()); protected final Log logger = LogFactory.getLog(getClass());
protected @Nullable Object targetObject; @SuppressWarnings("NullAway.Init")
protected Object targetObject;
private long refreshCheckDelay = -1; private long refreshCheckDelay = -1;
@ -65,7 +66,6 @@ public abstract class AbstractRefreshableTargetSource implements TargetSource, R
@Override @Override
@SuppressWarnings("NullAway")
public synchronized Class<?> getTargetClass() { public synchronized Class<?> getTargetClass() {
if (this.targetObject == null) { if (this.targetObject == null) {
refresh(); refresh();