Skip to content

Commit

Permalink
Perform NullAway build-time checks in more modules
Browse files Browse the repository at this point in the history
This commit enables null-safety build-time checks in:
 - spring-jdbc
 - spring-r2dbc
 - spring-orm
 - spring-beans
 - spring-aop

See gh-32475
  • Loading branch information
sdeleuze committed Mar 26, 2024
1 parent 3b4f8db commit 5b660da
Show file tree
Hide file tree
Showing 46 changed files with 78 additions and 12 deletions.
5 changes: 3 additions & 2 deletions gradle/spring-module.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,11 @@ tasks.withType(JavaCompile).configureEach {
disableAllChecks = true
option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract")
option("NullAway:AnnotatedPackages", "org.springframework.core,org.springframework.expression," +
"org.springframework.web,org.springframework.jms,org.springframework.messaging")
"org.springframework.web,org.springframework.jms,org.springframework.messaging,org.springframework.jdbc," +
"org.springframework.r2dbc,org.springframework.orm,org.springframework.beans,org.springframework.aop")
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")
"org.springframework.javapoet,org.springframework.aot.nativex.substitution,org.springframework.aot.nativex.feature")
}
}
tasks.compileJava {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ private void configurePointcutParameters(String[] argumentNames, int argumentInd
* @param ex the exception thrown by the method execution (may be null)
* @return the empty array if there are no arguments
*/
@SuppressWarnings("NullAway")
protected Object[] argBinding(JoinPoint jp, @Nullable JoinPointMatch jpMatch,
@Nullable Object returnValue, @Nullable Throwable ex) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public BeanFactoryAspectInstanceFactory(BeanFactory beanFactory, String name, @N
this.beanFactory = beanFactory;
this.name = name;
Class<?> resolvedType = type;
if (type == null) {
if (resolvedType == null) {
resolvedType = beanFactory.getType(name);
Assert.notNull(resolvedType, "Unresolvable bean type - explicitly specify the aspect class");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public BeanFactoryAspectJAdvisorsBuilder(ListableBeanFactory beanFactory, Aspect
* @return the list of {@link org.springframework.aop.Advisor} beans
* @see #isEligibleBean
*/
@SuppressWarnings("NullAway")
public List<Advisor> buildAspectJAdvisors() {
List<String> aspectNames = this.aspectBeanNames;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ public int getDeclarationOrder() {
}

@Override
@SuppressWarnings("NullAway")
public boolean isBeforeAdvice() {
if (this.isBeforeAdvice == null) {
determineAdviceType();
Expand All @@ -203,6 +204,7 @@ public boolean isBeforeAdvice() {
}

@Override
@SuppressWarnings("NullAway")
public boolean isAfterAdvice() {
if (this.isAfterAdvice == null) {
determineAdviceType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public AsyncExecutionInterceptor(@Nullable Executor defaultExecutor, AsyncUncaug
*/
@Override
@Nullable
@SuppressWarnings("NullAway")
public Object invoke(final MethodInvocation invocation) throws Throwable {
Class<?> targetClass = (invocation.getThis() != null ? AopUtils.getTargetClass(invocation.getThis()) : null);
final Method userMethod = BridgeMethodResolver.getMostSpecificMethod(invocation.getMethod(), targetClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class ScopedProxyBeanRegistrationAotProcessor implements BeanRegistrationAotProc

@Override
@Nullable
@SuppressWarnings("NullAway")
public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) {
Class<?> beanClass = registeredBean.getBeanClass();
if (beanClass.equals(ScopedProxyFactoryBean.class)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.springframework.beans.factory.support.AbstractBeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -128,6 +129,7 @@ public static String getOriginalBeanName(@Nullable String targetBeanName) {
* the target bean within a scoped proxy.
* @since 4.1.4
*/
@Contract("null -> false")
public static boolean isScopedTarget(@Nullable String beanName) {
return (beanName != null && beanName.startsWith(TARGET_NAME_PREFIX));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.springframework.core.CoroutinesUtils;
import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodIntrospector;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
Expand Down Expand Up @@ -73,6 +74,7 @@ public abstract class AopUtils {
* @see #isJdkDynamicProxy
* @see #isCglibProxy
*/
@Contract("null -> false")
public static boolean isAopProxy(@Nullable Object object) {
return (object instanceof SpringProxy && (Proxy.isProxyClass(object.getClass()) ||
object.getClass().getName().contains(ClassUtils.CGLIB_CLASS_SEPARATOR)));
Expand All @@ -86,6 +88,7 @@ public static boolean isAopProxy(@Nullable Object object) {
* @param object the object to check
* @see java.lang.reflect.Proxy#isProxyClass
*/
@Contract("null -> false")
public static boolean isJdkDynamicProxy(@Nullable Object object) {
return (object instanceof SpringProxy && Proxy.isProxyClass(object.getClass()));
}
Expand All @@ -98,6 +101,7 @@ public static boolean isJdkDynamicProxy(@Nullable Object object) {
* @param object the object to check
* @see ClassUtils#isCglibProxy(Object)
*/
@Contract("null -> false")
public static boolean isCglibProxy(@Nullable Object object) {
return (object instanceof SpringProxy &&
object.getClass().getName().contains(ClassUtils.CGLIB_CLASS_SEPARATOR));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public AnnotationMatchingPointcut(@Nullable Class<? extends Annotation> classAnn
* @see AnnotationClassFilter#AnnotationClassFilter(Class, boolean)
* @see AnnotationMethodMatcher#AnnotationMethodMatcher(Class, boolean)
*/
@SuppressWarnings("NullAway")
public AnnotationMatchingPointcut(@Nullable Class<? extends Annotation> classAnnotationType,
@Nullable Class<? extends Annotation> methodAnnotationType, boolean checkInherited) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public void setRefreshCheckDelay(long refreshCheckDelay) {


@Override
@SuppressWarnings("NullAway")
public synchronized Class<?> getTargetClass() {
if (this.targetObject == null) {
refresh();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ public GenericTypeAwarePropertyDescriptor(Class<?> beanClass, String propertyNam
// by the JDK's JavaBeans Introspector...
Set<Method> ambiguousCandidates = new HashSet<>();
for (Method method : beanClass.getMethods()) {
if (method.getName().equals(writeMethodToUse.getName()) &&
!method.equals(writeMethodToUse) && !method.isBridge() &&
method.getParameterCount() == writeMethodToUse.getParameterCount()) {
if (method.getName().equals(this.writeMethod.getName()) &&
!method.equals(this.writeMethod) && !method.isBridge() &&
method.getParameterCount() == this.writeMethod.getParameterCount()) {
ambiguousCandidates.add(method);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ public void overrideDefaultEditor(Class<?> requiredType, PropertyEditor property
* @see #registerDefaultEditors
*/
@Nullable
@SuppressWarnings("NullAway")
public PropertyEditor getDefaultEditor(Class<?> requiredType) {
if (!this.defaultEditorsActive) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.springframework.beans.MutablePropertyValues;
import org.springframework.beans.PropertyValues;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ReflectionUtils;
Expand Down Expand Up @@ -182,6 +183,7 @@ public static InjectionMetadata forElements(Collection<InjectedElement> elements
* @return {@code true} indicating a refresh, {@code false} otherwise
* @see #needsRefresh(Class)
*/
@Contract("null, _ -> true")
public static boolean needsRefresh(@Nullable InjectionMetadata metadata, Class<?> clazz) {
return (metadata == null || metadata.needsRefresh(clazz));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,9 @@ public boolean equals(@Nullable Object other) {
if (!super.equals(other)) {
return false;
}
DependencyDescriptor otherDesc = (DependencyDescriptor) other;
return (this.required == otherDesc.required && this.eager == otherDesc.eager &&
this.nestingLevel == otherDesc.nestingLevel && this.containingClass == otherDesc.containingClass);
return (other instanceof DependencyDescriptor otherDesc && this.required == otherDesc.required &&
this.eager == otherDesc.eager && this.nestingLevel == otherDesc.nestingLevel &&
this.containingClass == otherDesc.containingClass);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ public void setBeanClassLoader(ClassLoader classLoader) {


@Override
@SuppressWarnings("NullAway")
public void afterPropertiesSet() throws ClassNotFoundException, NoSuchFieldException {
if (this.targetClass != null && this.targetObject != null) {
throw new IllegalArgumentException("Specify either targetClass or targetObject, not both");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public void setBeanFactory(BeanFactory beanFactory) {
this.beanFactory = beanFactory;
}


@SuppressWarnings("NullAway")
protected void doProcessProperties(ConfigurableListableBeanFactory beanFactoryToProcess,
StringValueResolver valueResolver) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public void setBeanName(String beanName) {


@Override
@SuppressWarnings("NullAway")
public void setBeanFactory(BeanFactory beanFactory) {
this.beanFactory = beanFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ else if (this.currentBeanDefinition != null) {
}
}

@SuppressWarnings("NullAway")
private GroovyDynamicElementReader createDynamicElementReader(String namespace) {
XmlReaderContext readerContext = this.groovyDslXmlBeanDefinitionReader.createReaderContext(
new DescriptiveResource("Groovy"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public ConstructorResolver(AbstractAutowireCapableBeanFactory beanFactory) {
* or {@code null} if none (-> use constructor argument values from bean definition)
* @return a BeanWrapper for the new instance
*/
@SuppressWarnings("NullAway")
public BeanWrapper autowireConstructor(String beanName, RootBeanDefinition mbd,
@Nullable Constructor<?>[] chosenCtors, @Nullable Object[] explicitArgs) {

Expand Down Expand Up @@ -391,6 +392,7 @@ private boolean isStaticCandidate(Method method, Class<?> factoryClass) {
* method, or {@code null} if none (-> use constructor argument values from bean definition)
* @return a BeanWrapper for the new instance
*/
@SuppressWarnings("NullAway")
public BeanWrapper instantiateUsingFactoryMethod(
String beanName, RootBeanDefinition mbd, @Nullable Object[] explicitArgs) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.log.LogMessage;
import org.springframework.core.metrics.StartupStep;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
Expand Down Expand Up @@ -1487,6 +1488,7 @@ else if (descriptor.supportsLazyResolution()) {
}

@Nullable
@SuppressWarnings("NullAway")
public Object doResolveDependency(DependencyDescriptor descriptor, @Nullable String beanName,
@Nullable Set<String> autowiredBeanNames, @Nullable TypeConverter typeConverter) throws BeansException {

Expand Down Expand Up @@ -2066,6 +2068,7 @@ protected boolean matchesBeanName(String beanName, @Nullable String candidateNam
* i.e. whether the candidate points back to the original bean or to a factory method
* on the original bean.
*/
@Contract("null, _ -> false;_, null -> false;")
private boolean isSelfReference(@Nullable String beanName, @Nullable String candidateName) {
return (beanName != null && candidateName != null &&
(beanName.equals(candidateName) || (containsBeanDefinition(candidateName) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ protected Object getSingleton(String beanName, boolean allowEarlyReference) {
* with, if necessary
* @return the registered singleton object
*/
@SuppressWarnings("NullAway")
public Object getSingleton(String beanName, ObjectFactory<?> singletonFactory) {
Assert.notNull(beanName, "Bean name must not be null");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public boolean isAutowireCandidate(BeanDefinitionHolder bdHolder, DependencyDesc
* Match the given dependency type with its generic type information against the given
* candidate bean definition.
*/
@SuppressWarnings("NullAway")
protected boolean checkGenericTypeMatch(BeanDefinitionHolder bdHolder, DependencyDescriptor descriptor) {
ResolvableType dependencyType = descriptor.getResolvableType();
if (dependencyType.getType() instanceof Class) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ public BeanDefinitionHolder parseBeanDefinitionElement(Element ele) {
* {@link org.springframework.beans.factory.parsing.ProblemReporter}.
*/
@Nullable
@SuppressWarnings("NullAway")
public BeanDefinitionHolder parseBeanDefinitionElement(Element ele, @Nullable BeanDefinition containingBean) {
String id = ele.getAttribute(ID_ATTRIBUTE);
String nameAttr = ele.getAttribute(NAME_ATTRIBUTE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ enum InstrumentedMethod {
/**
* {@link Class#getField(String)}.
*/
@SuppressWarnings("NullAway")
CLASS_GETFIELD(Class.class, "getField", HintType.REFLECTION,
invocation -> {
Field field = invocation.getReturnValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class ArgumentTypePreparedStatementSetter implements PreparedStatementSet
* @param args the arguments to set
* @param argTypes the corresponding SQL types of the arguments
*/
@SuppressWarnings("NullAway")
public ArgumentTypePreparedStatementSetter(@Nullable Object[] args, @Nullable int[] argTypes) {
if ((args != null && argTypes == null) || (args == null && argTypes != null) ||
(args != null && args.length != argTypes.length)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.SqlParameter;
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
import org.springframework.lang.Nullable;

/**
* A SimpleJdbcCall is a multithreaded, reusable object representing a call
Expand Down Expand Up @@ -148,36 +149,42 @@ public SimpleJdbcCall withNamedBinding() {
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeFunction(Class<T> returnType, Object... args) {
return (T) doExecute(args).get(getScalarOutParameterName());
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeFunction(Class<T> returnType, Map<String, ?> args) {
return (T) doExecute(args).get(getScalarOutParameterName());
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeFunction(Class<T> returnType, SqlParameterSource args) {
return (T) doExecute(args).get(getScalarOutParameterName());
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeObject(Class<T> returnType, Object... args) {
return (T) doExecute(args).get(getScalarOutParameterName());
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeObject(Class<T> returnType, Map<String, ?> args) {
return (T) doExecute(args).get(getScalarOutParameterName());
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public <T> T executeObject(Class<T> returnType, SqlParameterSource args) {
return (T) doExecute(args).get(getScalarOutParameterName());
Expand Down
Loading

0 comments on commit 5b660da

Please sign in to comment.