Skip to content

Commit fd53d2a

Browse files
jhoellerjhoeller
committed
Consistent use of @nullable in spring-test
This commit also removes nullability from two common spots: ResolvableType.getType() and TargetSource.getTarget(), both of which are never effectively null with any regular implementation. For such scenarios, a non-null empty type/target is the cleaner contract. Issue: SPR-15540
1 parent ee5fa26 commit fd53d2a

File tree

134 files changed

+812
-777
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

134 files changed

+812
-777
lines changed

‎spring-aop/src/main/java/org/springframework/aop/TargetSource.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
package org.springframework.aop;
1818

19-
import org.springframework.lang.Nullable;
20-
2119
/**
2220
* A {@code TargetSource} is used to obtain the current "target" of
2321
* an AOP invocation, which will be invoked via reflection if no around
@@ -59,7 +57,6 @@ public interface TargetSource extends TargetClassAware {
5957
* @return the target object, which contains the joinpoint
6058
* @throws Exception if the target object can't be resolved
6159
*/
62-
@Nullable
6360
Object getTarget() throws Exception;
6461

6562
/**

‎spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,12 @@ private Callback[] getCallbacks(Class<?> rootClass) throws Exception {
326326
// TODO: small memory optimization here (can skip creation for methods with no advice)
327327
for (int x = 0; x < methods.length; x++) {
328328
List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(methods[x], rootClass);
329-
fixedCallbacks[x] = new FixedChainStaticTargetInterceptor(
330-
chain, this.advised.getTargetSource().getTarget(), this.advised.getTargetClass());
329+
Object target = this.advised.getTargetSource().getTarget();
330+
Class<?> targetClass = this.advised.getTargetClass();
331+
if (targetClass == null) {
332+
targetClass = target.getClass();
333+
}
334+
fixedCallbacks[x] = new FixedChainStaticTargetInterceptor(chain, target, targetClass);
331335
this.fixedInterceptorMap.put(methods[x].toString(), x);
332336
}
333337

@@ -374,7 +378,7 @@ private static boolean implementsInterface(Method method, Set<Class<?>> ifcs) {
374378
* {@code proxy} and also verifies that {@code null} is not returned as a primitive.
375379
*/
376380
@Nullable
377-
private static Object processReturnType(Object proxy, @Nullable Object target, Method method, @Nullable Object retVal) {
381+
private static Object processReturnType(Object proxy, Object target, Method method, @Nullable Object retVal) {
378382
// Massage return value if necessary
379383
if (retVal != null && retVal == target &&
380384
!RawTargetAccess.class.isAssignableFrom(method.getDeclaringClass())) {
@@ -409,7 +413,7 @@ private static class StaticUnadvisedInterceptor implements MethodInterceptor, Se
409413

410414
private final Object target;
411415

412-
public StaticUnadvisedInterceptor(@Nullable Object target) {
416+
public StaticUnadvisedInterceptor(Object target) {
413417
this.target = target;
414418
}
415419

@@ -430,7 +434,7 @@ private static class StaticUnadvisedExposedInterceptor implements MethodIntercep
430434

431435
private final Object target;
432436

433-
public StaticUnadvisedExposedInterceptor(@Nullable Object target) {
437+
public StaticUnadvisedExposedInterceptor(Object target) {
434438
this.target = target;
435439
}
436440

@@ -472,9 +476,7 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
472476
return processReturnType(proxy, target, method, retVal);
473477
}
474478
finally {
475-
if (target != null) {
476-
this.targetSource.releaseTarget(target);
477-
}
479+
this.targetSource.releaseTarget(target);
478480
}
479481
}
480482
}
@@ -503,9 +505,7 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
503505
}
504506
finally {
505507
AopContext.setCurrentProxy(oldProxy);
506-
if (target != null) {
507-
this.targetSource.releaseTarget(target);
508-
}
508+
this.targetSource.releaseTarget(target);
509509
}
510510
}
511511
}
@@ -612,9 +612,7 @@ private static class FixedChainStaticTargetInterceptor implements MethodIntercep
612612

613613
private final Class<?> targetClass;
614614

615-
public FixedChainStaticTargetInterceptor(
616-
List<Object> adviceChain, @Nullable Object target, @Nullable Class<?> targetClass) {
617-
615+
public FixedChainStaticTargetInterceptor(List<Object> adviceChain, Object target, Class<?> targetClass) {
618616
this.adviceChain = adviceChain;
619617
this.target = target;
620618
this.targetClass = targetClass;
@@ -650,20 +648,16 @@ public DynamicAdvisedInterceptor(AdvisedSupport advised) {
650648
public Object intercept(Object proxy, Method method, Object[] args, MethodProxy methodProxy) throws Throwable {
651649
Object oldProxy = null;
652650
boolean setProxyContext = false;
653-
Class<?> targetClass = null;
654651
Object target = null;
655652
try {
656653
if (this.advised.exposeProxy) {
657654
// Make invocation available if necessary.
658655
oldProxy = AopContext.setCurrentProxy(proxy);
659656
setProxyContext = true;
660657
}
661-
// May be null. Get as late as possible to minimize the time we
662-
// "own" the target, in case it comes from a pool...
658+
// Get as late as possible to minimize the time we "own" the target, in case it comes from a pool...
663659
target = getTarget();
664-
if (target != null) {
665-
targetClass = target.getClass();
666-
}
660+
Class<?> targetClass = target.getClass();
667661
List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(method, targetClass);
668662
Object retVal;
669663
// Check whether we only have one InvokerInterceptor: that is,
@@ -709,7 +703,6 @@ public int hashCode() {
709703
return this.advised.hashCode();
710704
}
711705

712-
@Nullable
713706
protected Object getTarget() throws Exception {
714707
return this.advised.getTargetSource().getTarget();
715708
}
@@ -729,9 +722,8 @@ private static class CglibMethodInvocation extends ReflectiveMethodInvocation {
729722

730723
private final boolean publicMethod;
731724

732-
public CglibMethodInvocation(Object proxy, @Nullable Object target, Method method,
733-
Object[] arguments, @Nullable Class<?> targetClass,
734-
List<Object> interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) {
725+
public CglibMethodInvocation(Object proxy, Object target, Method method, Object[] arguments,
726+
Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) {
735727

736728
super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers);
737729
this.methodProxy = methodProxy;

‎spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
159159
boolean setProxyContext = false;
160160

161161
TargetSource targetSource = this.advised.targetSource;
162-
Class<?> targetClass = null;
163162
Object target = null;
164163

165164
try {
@@ -189,12 +188,10 @@ else if (!this.advised.opaque && method.getDeclaringClass().isInterface() &&
189188
setProxyContext = true;
190189
}
191190

192-
// May be null. Get as late as possible to minimize the time we "own" the target,
191+
// Get as late as possible to minimize the time we "own" the target,
193192
// in case it comes from a pool.
194193
target = targetSource.getTarget();
195-
if (target != null) {
196-
targetClass = target.getClass();
197-
}
194+
Class<?> targetClass = target.getClass();
198195

199196
// Get the interception chain for this method.
200197
List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(method, targetClass);

‎spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ public class ReflectiveMethodInvocation implements ProxyMethodInvocation, Clonea
103103
* but would complicate the code. And it would work only for static pointcuts.
104104
*/
105105
protected ReflectiveMethodInvocation(
106-
Object proxy, @Nullable Object target, Method method, Object[] arguments,
107-
@Nullable Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers) {
106+
Object proxy, Object target, Method method, Object[] arguments,
107+
Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers) {
108108

109109
this.proxy = proxy;
110110
this.target = target;

‎spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ public static List<Advisor> findAdvisorsThatCanApply(List<Advisor> candidateAdvi
329329
* @throws org.springframework.aop.AopInvocationException in case of a reflection error
330330
*/
331331
@Nullable
332-
public static Object invokeJoinpointUsingReflection(@Nullable Object target, Method method, Object[] args)
332+
public static Object invokeJoinpointUsingReflection(Object target, Method method, Object[] args)
333333
throws Throwable {
334334

335335
// Use reflection to invoke the method.

‎spring-aop/src/main/java/org/springframework/aop/target/EmptyTargetSource.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -32,6 +32,13 @@
3232
*/
3333
public class EmptyTargetSource implements TargetSource, Serializable {
3434

35+
/**
36+
* The canonical (Singleton) instance of this {@link EmptyTargetSource}.
37+
*/
38+
public static final EmptyTargetSource INSTANCE = new EmptyTargetSource(null, true);
39+
40+
private static final Object EMPTY_TARGET = new Object();
41+
3542
/** use serialVersionUID from Spring 1.2 for interoperability */
3643
private static final long serialVersionUID = 3680494563553489691L;
3744

@@ -40,12 +47,6 @@ public class EmptyTargetSource implements TargetSource, Serializable {
4047
// Static factory methods
4148
//---------------------------------------------------------------------
4249

43-
/**
44-
* The canonical (Singleton) instance of this {@link EmptyTargetSource}.
45-
*/
46-
public static final EmptyTargetSource INSTANCE = new EmptyTargetSource(null, true);
47-
48-
4950
/**
5051
* Return an EmptyTargetSource for the given target Class.
5152
* @param targetClass the target Class (may be {@code null})
@@ -87,6 +88,7 @@ private EmptyTargetSource(@Nullable Class<?> targetClass, boolean isStatic) {
8788
this.isStatic = isStatic;
8889
}
8990

91+
9092
/**
9193
* Always returns the specified target Class, or {@code null} if none.
9294
*/
@@ -104,11 +106,11 @@ public boolean isStatic() {
104106
}
105107

106108
/**
107-
* Always returns {@code null}.
109+
* Always returns {@code DUMMY_TARGET}.
108110
*/
109111
@Override
110112
public Object getTarget() {
111-
return null;
113+
return EMPTY_TARGET;
112114
}
113115

114116
/**

‎spring-aspects/src/main/java/org/springframework/transaction/aspectj/AbstractTransactionAspect.aj

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -73,12 +73,9 @@ public abstract aspect AbstractTransactionAspect extends TransactionAspectSuppor
7373
}
7474
});
7575
}
76-
catch (RuntimeException ex) {
76+
catch (RuntimeException | Error ex) {
7777
throw ex;
7878
}
79-
catch (Error err) {
80-
throw err;
81-
}
8279
catch (Throwable thr) {
8380
Rethrower.rethrow(thr);
8481
throw new IllegalStateException("Should never get here", thr);

‎spring-beans/src/main/java/org/springframework/beans/factory/wiring/BeanConfigurerSupport.java

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,11 @@ public void configureBean(Object beanInstance) {
141141
!this.beanFactory.containsBean(bwi.getBeanName()))) {
142142
// Perform autowiring (also applying standard factory / post-processor callbacks).
143143
this.beanFactory.autowireBeanProperties(beanInstance, bwi.getAutowireMode(), bwi.getDependencyCheck());
144-
Object result = this.beanFactory.initializeBean(beanInstance, bwi.getBeanName());
145-
checkExposedObject(result, beanInstance);
144+
this.beanFactory.initializeBean(beanInstance, bwi.getBeanName());
146145
}
147146
else {
148147
// Perform explicit wiring based on the specified bean definition.
149-
Object result = this.beanFactory.configureBean(beanInstance, bwi.getBeanName());
150-
checkExposedObject(result, beanInstance);
148+
this.beanFactory.configureBean(beanInstance, bwi.getBeanName());
151149
}
152150
}
153151
catch (BeanCreationException ex) {
@@ -168,13 +166,4 @@ public void configureBean(Object beanInstance) {
168166
}
169167
}
170168

171-
private void checkExposedObject(@Nullable Object exposedObject, Object originalBeanInstance) {
172-
if (exposedObject != originalBeanInstance) {
173-
throw new IllegalStateException("Post-processor tried to replace bean instance of type [" +
174-
originalBeanInstance.getClass().getName() + "] with (proxy) object of type [" +
175-
(exposedObject != null ? exposedObject.getClass().getName() : null) +
176-
"] - not supported for aspect-configured classes!");
177-
}
178-
}
179-
180169
}

‎spring-context/src/main/java/org/springframework/context/annotation/ComponentScanAnnotationParser.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.springframework.core.type.filter.AssignableTypeFilter;
3939
import org.springframework.core.type.filter.RegexPatternTypeFilter;
4040
import org.springframework.core.type.filter.TypeFilter;
41-
import org.springframework.lang.Nullable;
4241
import org.springframework.util.Assert;
4342
import org.springframework.util.ClassUtils;
4443
import org.springframework.util.StringUtils;
@@ -64,7 +63,7 @@ class ComponentScanAnnotationParser {
6463
private final BeanDefinitionRegistry registry;
6564

6665

67-
public ComponentScanAnnotationParser(@Nullable Environment environment, @Nullable ResourceLoader resourceLoader,
66+
public ComponentScanAnnotationParser(Environment environment, ResourceLoader resourceLoader,
6867
BeanNameGenerator beanNameGenerator, BeanDefinitionRegistry registry) {
6968

7069
this.environment = environment;
@@ -75,9 +74,6 @@ public ComponentScanAnnotationParser(@Nullable Environment environment, @Nullabl
7574

7675

7776
public Set<BeanDefinitionHolder> parse(AnnotationAttributes componentScan, final String declaringClass) {
78-
Assert.state(this.environment != null, "Environment must not be null");
79-
Assert.state(this.resourceLoader != null, "ResourceLoader must not be null");
80-
8177
ClassPathBeanDefinitionScanner scanner = new ClassPathBeanDefinitionScanner(this.registry,
8278
componentScan.getBoolean("useDefaultFilters"), this.environment, this.resourceLoader);
8379

‎spring-context/src/test/java/org/springframework/jmx/IJmxTestBean.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,19 +22,19 @@
2222
*/
2323
public interface IJmxTestBean {
2424

25-
public int add(int x, int y);
25+
int add(int x, int y);
2626

27-
public long myOperation();
27+
long myOperation();
2828

29-
public int getAge();
29+
int getAge();
3030

31-
public void setAge(int age);
31+
void setAge(int age);
3232

33-
public void setName(String name) throws Exception;
33+
void setName(String name) throws Exception;
3434

35-
public String getName();
35+
String getName();
3636

3737
// used to test invalid methods that exist in the proxy interface
38-
public void dontExposeMe();
38+
void dontExposeMe();
3939

4040
}

‎spring-context/src/test/java/org/springframework/remoting/rmi/RmiSupportTests.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -449,24 +449,21 @@ protected Remote lookupStub() {
449449
}
450450

451451

452-
public static interface IBusinessBean {
453-
454-
public void setName(String name);
452+
public interface IBusinessBean {
455453

454+
void setName(String name);
456455
}
457456

458457

459-
public static interface IWrongBusinessBean {
460-
461-
public void setOtherName(String name);
458+
public interface IWrongBusinessBean {
462459

460+
void setOtherName(String name);
463461
}
464462

465463

466-
public static interface IRemoteBean extends Remote {
467-
468-
public void setName(String name) throws RemoteException;
464+
public interface IRemoteBean extends Remote {
469465

466+
void setName(String name) throws RemoteException;
470467
}
471468

472469

0 commit comments

Comments
 (0)