Permalink
Browse files

@Nullable all the way: null-safety at field level

This commits extends nullability declarations to the field level, formalizing the interaction between methods and their underlying fields and therefore avoiding any nullability mismatch.

Issue: SPR-15720
  • Loading branch information...
jhoeller committed Jun 29, 2017
1 parent c4694c3 commit cc74a2891a4d2a4c7bcec059f20c35aa80bcf668
Showing 936 changed files with 6,071 additions and 2,787 deletions.
@@ -104,11 +104,11 @@ public static JoinPoint currentJoinPoint() {
private final AspectInstanceFactory aspectInstanceFactory;
/**
* The name of the aspect (ref bean) in which this advice was defined (used
* when determining advice precedence so that we can determine
* The name of the aspect (ref bean) in which this advice was defined
* (used when determining advice precedence so that we can determine
* whether two pieces of advice come from the same aspect).
*/
private String aspectName;
private String aspectName = "";
/**
* The order of declaration of this advice within the aspect.
@@ -119,13 +119,16 @@ public static JoinPoint currentJoinPoint() {
* This will be non-null if the creator of this advice object knows the argument names
* and sets them explicitly
*/
private String[] argumentNames = null;
@Nullable
private String[] argumentNames;
/** Non-null if after throwing advice binds the thrown value */
private String throwingName = null;
@Nullable
private String throwingName;
/** Non-null if after returning advice binds the return value */
private String returningName = null;
@Nullable
private String returningName;
private Class<?> discoveredReturningType = Object.class;
@@ -143,10 +146,12 @@ public static JoinPoint currentJoinPoint() {
*/
private int joinPointStaticPartArgumentIndex = -1;
@Nullable
private Map<String, Integer> argumentBindings;
private boolean argumentsIntrospected = false;
@Nullable
private Type discoveredReturningGenericType;
// Note: Unlike return type, no such generic information is needed for the throwing type,
// since Java doesn't allow exception types to be parameterized.
@@ -464,6 +469,7 @@ protected ParameterNameDiscoverer createParameterNameDiscoverer() {
}
private void bindExplicitArguments(int numArgumentsLeftToBind) {
Assert.state(this.argumentNames != null, "No argument names available");
this.argumentBindings = new HashMap<>();
int numExpectedArgumentNames = this.aspectJAdviceMethod.getParameterCount();
@@ -504,36 +510,36 @@ private void bindExplicitArguments(int numArgumentsLeftToBind) {
}
// configure the pointcut expression accordingly.
configurePointcutParameters(argumentIndexOffset);
configurePointcutParameters(this.argumentNames, argumentIndexOffset);
}
/**
* All parameters from argumentIndexOffset onwards are candidates for
* pointcut parameters - but returning and throwing vars are handled differently
* and must be removed from the list if present.
*/
private void configurePointcutParameters(int argumentIndexOffset) {
private void configurePointcutParameters(String[] argumentNames, int argumentIndexOffset) {
int numParametersToRemove = argumentIndexOffset;
if (this.returningName != null) {
numParametersToRemove++;
}
if (this.throwingName != null) {
numParametersToRemove++;
}
String[] pointcutParameterNames = new String[this.argumentNames.length - numParametersToRemove];
String[] pointcutParameterNames = new String[argumentNames.length - numParametersToRemove];
Class<?>[] pointcutParameterTypes = new Class<?>[pointcutParameterNames.length];
Class<?>[] methodParameterTypes = this.aspectJAdviceMethod.getParameterTypes();
int index = 0;
for (int i = 0; i < this.argumentNames.length; i++) {
for (int i = 0; i < argumentNames.length; i++) {
if (i < argumentIndexOffset) {
continue;
}
if (this.argumentNames[i].equals(this.returningName) ||
this.argumentNames[i].equals(this.throwingName)) {
if (argumentNames[i].equals(this.returningName) ||
argumentNames[i].equals(this.throwingName)) {
continue;
}
pointcutParameterNames[index] = this.argumentNames[i];
pointcutParameterNames[index] = argumentNames[i];
pointcutParameterTypes[index] = methodParameterTypes[i];
index++;
}
@@ -114,6 +114,7 @@
* returning {@code null} in the case that the parameter names cannot be discovered.
*
* @author Adrian Colyer
* @author Juergen Hoeller
* @since 2.0
*/
public class AspectJAdviceParameterNameDiscoverer implements ParameterNameDiscoverer {
@@ -155,26 +156,23 @@
}
/** The pointcut expression associated with the advice, as a simple String */
@Nullable
private String pointcutExpression;
private boolean raiseExceptions;
/**
* If the advice is afterReturning, and binds the return value, this is the parameter name used.
*/
/** If the advice is afterReturning, and binds the return value, this is the parameter name used */
@Nullable
private String returningName;
/**
* If the advice is afterThrowing, and binds the thrown value, this is the parameter name used.
*/
/** If the advice is afterThrowing, and binds the thrown value, this is the parameter name used */
@Nullable
private String throwingName;
/**
* The pointcut expression associated with the advice, as a simple String.
*/
private String pointcutExpression;
private Class<?>[] argumentTypes;
private Class<?>[] argumentTypes = new Class<?>[0];
private String[] parameterNameBindings;
private String[] parameterNameBindings = new String[0];
private int numberOfRemainingUnboundArguments;
@@ -202,7 +200,7 @@ public void setRaiseExceptions(boolean raiseExceptions) {
* returning variable name must be specified.
* @param returningName the name of the returning variable
*/
public void setReturningName(String returningName) {
public void setReturningName(@Nullable String returningName) {
this.returningName = returningName;
}
@@ -211,7 +209,7 @@ public void setReturningName(String returningName) {
* throwing variable name must be specified.
* @param throwingName the name of the throwing variable
*/
public void setThrowingName(String throwingName) {
public void setThrowingName(@Nullable String throwingName) {
this.throwingName = throwingName;
}
@@ -781,6 +779,7 @@ private void findAndBind(Class<?> argumentType, String varName) {
private int numTokensConsumed;
@Nullable
private String text;
public PointcutBody(int tokens, @Nullable String text) {
@@ -102,16 +102,20 @@
private static final Log logger = LogFactory.getLog(AspectJExpressionPointcut.class);
@Nullable
private Class<?> pointcutDeclarationScope;
private String[] pointcutParameterNames = new String[0];
private Class<?>[] pointcutParameterTypes = new Class<?>[0];
@Nullable
private BeanFactory beanFactory;
@Nullable
private transient ClassLoader pointcutClassLoader;
@Nullable
private transient PointcutExpression pointcutExpression;
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);
@@ -169,13 +173,13 @@ public void setBeanFactory(BeanFactory beanFactory) {
@Override
public ClassFilter getClassFilter() {
checkReadyToMatch();
obtainPointcutExpression();
return this;
}
@Override
public MethodMatcher getMethodMatcher() {
checkReadyToMatch();
obtainPointcutExpression();
return this;
}
@@ -184,14 +188,15 @@ public MethodMatcher getMethodMatcher() {
* Check whether this pointcut is ready to match,
* lazily building the underlying AspectJ pointcut expression.
*/
private void checkReadyToMatch() {
private PointcutExpression obtainPointcutExpression() {
if (getExpression() == null) {
throw new IllegalStateException("Must set property 'expression' before attempting to match");
}
if (this.pointcutExpression == null) {
this.pointcutClassLoader = determinePointcutClassLoader();
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
}
return this.pointcutExpression;
}
/**
@@ -258,16 +263,15 @@ private String replaceBooleanOperators(String pcExpr) {
* Return the underlying AspectJ pointcut expression.
*/
public PointcutExpression getPointcutExpression() {
checkReadyToMatch();
return this.pointcutExpression;
return obtainPointcutExpression();
}
@Override
public boolean matches(Class<?> targetClass) {
checkReadyToMatch();
PointcutExpression pointcutExpression = obtainPointcutExpression();
try {
try {
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
return pointcutExpression.couldMatchJoinPointsInType(targetClass);
}
catch (ReflectionWorldException ex) {
logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex);
@@ -286,7 +290,7 @@ public boolean matches(Class<?> targetClass) {
@Override
public boolean matches(Method method, @Nullable Class<?> targetClass, boolean beanHasIntroductions) {
checkReadyToMatch();
obtainPointcutExpression();
Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
ShadowMatch shadowMatch = getShadowMatch(targetMethod, method);
@@ -321,13 +325,12 @@ public boolean matches(Method method, @Nullable Class<?> targetClass) {
@Override
public boolean isRuntime() {
checkReadyToMatch();
return this.pointcutExpression.mayNeedDynamicTest();
return obtainPointcutExpression().mayNeedDynamicTest();
}
@Override
public boolean matches(Method method, @Nullable Class<?> targetClass, Object... args) {
checkReadyToMatch();
obtainPointcutExpression();
ShadowMatch shadowMatch = getShadowMatch(AopUtils.getMostSpecificMethod(method, targetClass), method);
ShadowMatch originalShadowMatch = getShadowMatch(method, method);
@@ -436,7 +439,7 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
if (shadowMatch == null) {
try {
try {
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
}
catch (ReflectionWorldException ex) {
// Failed to introspect target method, probably because it has been loaded
@@ -454,7 +457,7 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
if (shadowMatch == null && targetMethod != originalMethod) {
methodToMatch = originalMethod;
try {
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);
}
catch (ReflectionWorldException ex3) {
// Could neither introspect the target class nor the proxy class ->
@@ -519,18 +522,16 @@ public int hashCode() {
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("AspectJExpressionPointcut: ");
if (this.pointcutParameterNames != null && this.pointcutParameterTypes != null) {
sb.append("(");
for (int i = 0; i < this.pointcutParameterTypes.length; i++) {
sb.append(this.pointcutParameterTypes[i].getName());
sb.append(" ");
sb.append(this.pointcutParameterNames[i]);
if ((i+1) < this.pointcutParameterTypes.length) {
sb.append(", ");
}
sb.append("(");
for (int i = 0; i < this.pointcutParameterTypes.length; i++) {
sb.append(this.pointcutParameterTypes[i].getName());
sb.append(" ");
sb.append(this.pointcutParameterNames[i]);
if ((i+1) < this.pointcutParameterTypes.length) {
sb.append(", ");
}
sb.append(")");
}
sb.append(")");
sb.append(" ");
if (getExpression() != null) {
sb.append(getExpression());
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2017 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.
@@ -21,6 +21,7 @@
import org.springframework.aop.Pointcut;
import org.springframework.aop.PointcutAdvisor;
import org.springframework.core.Ordered;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
@@ -38,6 +39,7 @@
private final Pointcut pointcut;
@Nullable
private Integer order;
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2017 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.
@@ -57,12 +57,15 @@
private final ProxyMethodInvocation methodInvocation;
@Nullable
private Object[] defensiveCopyOfArgs;
/** Lazily initialized signature object */
@Nullable
private Signature signature;
/** Lazily initialized source location object */
@Nullable
private SourceLocation sourceLocation;
@@ -178,6 +181,7 @@ public String toString() {
*/
private class MethodSignatureImpl implements MethodSignature {
@Nullable
private volatile String[] parameterNames;
@Override
@@ -216,6 +220,7 @@ public Method getMethod() {
}
@Override
@Nullable
public String[] getParameterNames() {
if (this.parameterNames == null) {
this.parameterNames = parameterNameDiscoverer.getParameterNames(getMethod());
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2017 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.
@@ -37,6 +37,7 @@
import org.aspectj.weaver.reflect.ShadowMatchImpl;
import org.aspectj.weaver.tools.ShadowMatch;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
@@ -78,6 +79,7 @@
}
@Nullable
private final Test runtimeTest;
Oops, something went wrong.

2 comments on commit cc74a28

@mattsheppard

This comment has been minimized.

mattsheppard replied May 9, 2018

Not sure why yet, and no doubt it's something I've done, but upgrading an app from java 8 to java 10 I'm hitting the following here...

Caused by: java.lang.NoSuchMethodError: javax.annotation.Resource.lookup()Ljava/lang/String;
        at org.springframework.context.annotation.CommonAnnotationBeanPostProcessor$ResourceElement.<init>(CommonAnnotationBeanPostProcessor.java:609)
        at org.springframework.context.annotation.CommonAnnotationBeanPostProcessor.lambda$buildResourceMetadata$0(CommonAnnotationBeanPostProcessor.java:373)
        at org.springframework.util.ReflectionUtils.doWithLocalFields(ReflectionUtils.java:692)
        at org.springframework.context.annotation.CommonAnnotationBeanPostProcessor.buildResourceMetadata(CommonAnnotationBeanPostProcessor.java:355)
        at org.springframework.context.annotation.CommonAnnotationBeanPostProcessor.findResourceMetadata(CommonAnnotationBeanPostProcessor.java:339)
        at org.springframework.context.annotation.CommonAnnotationBeanPostProcessor.postProcessMergedBeanDefinition(CommonAnnotationBeanPostProcessor.java:298)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyMergedBeanDefinitionPostProcessors(AbstractAutowireCapableBeanFactory.java:1016)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:553)
        ... 47 more

Just recording it in case it's a useful breadcrumb to someone else later.

@mattsheppard

This comment has been minimized.

mattsheppard replied May 9, 2018

Yeah, seems to have been an old copy of jsr250-api-1.0.jar on the classpath - Presumably the old version left the lookupValue as null and for whatever reason that didn't matter to this particular app.

Please sign in to comment.