NullnessMethodInvocationValidator should consider `jakarta.annotation.Nullable` annotations

[fjakop]

We switched to spring-data 3.5.1 recently and encountered errors on formerly (spring-data 3.4.4) working code

java.lang.NullPointerException: Return value is null but must not be null
	at org.springframework.data.util.NullnessMethodInvocationValidator.returnValueIsNull(NullnessMethodInvocationValidator.java:124)
...

The method in question is annotated with jakarta.annotation.Nullable, so this should not happen imho.
It's easily reproducible with this test code

package org.springframework.data.util;

import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.junit.jupiter.api.Test;
import org.springframework.core.ParameterNameDiscoverer;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

class MethodNullnessTest {

    private final ParameterNameDiscoverer discoverer = new ParameterNameDiscoverer() {
        @Override
        public String[] getParameterNames(Method method) {
            return null;
        }

        @Override
        public String[] getParameterNames(Constructor<?> ctor) {
            return null;
        }
    };

    @Test
    void isNullableReturn() throws NoSuchMethodException {

        assertFalse(NullnessMethodInvocationValidator.MethodNullness
                .of(Dummy.class.getDeclaredMethod("getNonnullData"), discoverer)
                .isNullableReturn());

        assertTrue(NullnessMethodInvocationValidator.MethodNullness
                .of(Dummy.class.getDeclaredMethod("getNullableData"), discoverer)
                .isNullableReturn());

    }

    static class Dummy {

        @Nonnull
        String getNonnullData() {
            return "";
        }

        @Nullable
        String getNullableData() {
            return "";
        }
    }
}

where the assertion for nullable = true always fails.

[mp911de]

jakarta.annotation.Nonnull (Jakarta Annotations) never were in scope for Spring Data's Nullness validation and the example doesn't work for me across 3.3 to 3.5 versions. We only support JSR305 (javax.annotation.Nonnull, javax.annotation.Nullable) as legacy annotation variant. JSR305 was never migrated to Jakarta.

Care to provide a reproducer for Spring Data 3.4.x?

[fjakop]

Reproducing for 3.4.x is not possible, since the class in question NullnessMethodInvocationValidator exists only from 3.5 on, so this particular testing strategy does not work. However, the issue originated of a more complex application.

I will try to create a reproducer for the complete scenario but I would also suggest to support jakarta.annotation.Nullable and jakarta.annotation.Nonnull in Spring Data, since the transition of JSR305 to Jakarta namespace is commonly adopted by various frameworks and tools.

I would think the support of Jakarta annotations is not that hard and would just require adding these classes to the respective sets in org.springframework.data.util.NullableUtils.

[mp911de]

You can use either ProxyFactory (or org.springframework.data.util.NullableUtils#isExplicitNullable(…) in a more isolated setup) to create a reproducer.

import static org.junit.jupiter.api.Assertions.*;


import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import org.junit.jupiter.api.Test;

import org.springframework.aop.framework.ProxyFactory;
import org.springframework.data.repository.core.support.MethodInvocationValidator;

public class MethodInvocationValidatorReproducer {

	@Test
	void reproducer() {

		ProxyFactory proxyFactory = new ProxyFactory();
		proxyFactory.addInterface(Something.class);
		proxyFactory.setTarget(new Dummy());
		proxyFactory.addAdvice(new MethodInvocationValidator());
		Something s = (Something) proxyFactory.getProxy();

		// should not throw an exception
		assertNull(s.getNullableData());
	}

	public static class Dummy implements Something {

		@Nonnull
		public String getNonnullData() {
			return "";
		}

		@Nullable
		public String getNullableData() {
			return null;
		}
	}

	public interface Something {

		@Nullable
		String getNullableData();
	}

}

I would also suggest to support jakarta.annotation.Nullable and jakarta.annotation.Nonnull in Spring Data, since the transition of JSR305 to Jakarta namespace is commonly adopted by various frameworks and tools.

JSR305 was never migrated to the Jakarta namespace.
We don't want folks to use nullness indication that has an incomplete scopewithout considering the entire JVM space where nullness applies. Spring has deliberately deprecated JSR-305 support (and with that, we're broadly checking annotation names) in favor of JSpecify.

I would think the support of Jakarta annotations is not that hard and would just require adding these classes to the respective sets in org.springframework.data.util.NullableUtils.

It's not about difficulty but about doing the right thing.

[fjakop]

Thanks for the clarification and the code, replacing the annotation with jakarta.annotation.Nullable leads exactly to our issue.
So our mitigation strategy would be to

• switch back to javax-scoped annotations
• use org.springframework.lang.Nullable

which both re-establish the pre 3.5.x behaviour.

[mp911de]

Exactly. Closing this ticket then.