diff --git a/integration-tests/src/test/java/com/example/IntegrationTest.java b/integration-tests/src/test/java/com/example/IntegrationTest.java index 90700b52..260497f6 100644 --- a/integration-tests/src/test/java/com/example/IntegrationTest.java +++ b/integration-tests/src/test/java/com/example/IntegrationTest.java @@ -62,7 +62,6 @@ public void bindsProvider() { @Test @IgnoreCodegen - @ReflectBug("check not implemented") public void bindsProviderNullabilityMismatch() { BindsProviderNullabilityMismatch component = backend.create(BindsProviderNullabilityMismatch.class); @@ -153,7 +152,7 @@ public void optionalBindingNullable() { assertThat(e) .hasMessageThat() .isEqualTo( - "@Provides[com.example.OptionalBindingNullable$Module1.foo(…)] " + "@Nullable @Provides[com.example.OptionalBindingNullable$Module1.foo(…)] " + "returned null which is not allowed for optional bindings"); } } diff --git a/reflect/src/main/java/dagger/reflect/ComponentInvocationHandler.java b/reflect/src/main/java/dagger/reflect/ComponentInvocationHandler.java index 11454a59..6396ebf5 100644 --- a/reflect/src/main/java/dagger/reflect/ComponentInvocationHandler.java +++ b/reflect/src/main/java/dagger/reflect/ComponentInvocationHandler.java @@ -16,6 +16,7 @@ package dagger.reflect; import static dagger.reflect.Reflection.findQualifier; +import static dagger.reflect.Reflection.hasNullable; import static dagger.reflect.Reflection.newProxy; import dagger.MembersInjector; @@ -89,7 +90,8 @@ private static ComponentInvocationHandler.MethodInvocationHandler createMethodIn if (parameterTypes.length == 0) { Key key = Key.of(findQualifier(method.getDeclaredAnnotations()), returnType); LinkedBinding binding = scope.getBinding(key); - return new ProvisionMethodInvocationHandler(binding); + return new ProvisionMethodInvocationHandler( + binding, hasNullable(method.getDeclaredAnnotations())); } if (parameterTypes.length == 1) { @@ -123,13 +125,22 @@ private interface MethodInvocationHandler { private static final class ProvisionMethodInvocationHandler implements MethodInvocationHandler { private final LinkedBinding binding; + private final boolean nullable; - ProvisionMethodInvocationHandler(LinkedBinding binding) { + ProvisionMethodInvocationHandler(LinkedBinding binding, boolean nullable) { this.binding = binding; + this.nullable = nullable; } @Override public @Nullable Object invoke(Object[] args) { + if (binding instanceof LinkedProvidesBinding) { + LinkedProvidesBinding binding = (LinkedProvidesBinding) this.binding; + if (!binding.nullableMatch(nullable)) { + // TODO add details about requestor. + throw new IllegalStateException(binding + " nullability did not match"); + } + } return binding.get(); } } diff --git a/reflect/src/main/java/dagger/reflect/LinkedProvidesBinding.java b/reflect/src/main/java/dagger/reflect/LinkedProvidesBinding.java index 76c110f8..4cd11e65 100644 --- a/reflect/src/main/java/dagger/reflect/LinkedProvidesBinding.java +++ b/reflect/src/main/java/dagger/reflect/LinkedProvidesBinding.java @@ -10,11 +10,14 @@ public final class LinkedProvidesBinding extends LinkedBinding { private final @Nullable Object instance; private final Method method; private final LinkedBinding[] dependencies; + private final boolean nullable; - LinkedProvidesBinding(@Nullable Object instance, Method method, LinkedBinding[] dependencies) { + LinkedProvidesBinding( + @Nullable Object instance, Method method, LinkedBinding[] dependencies, boolean nullable) { this.instance = instance; this.method = method; this.dependencies = dependencies; + this.nullable = nullable; } @Override @@ -26,11 +29,30 @@ public final class LinkedProvidesBinding extends LinkedBinding { // The binding is associated with the return type of method as key. @SuppressWarnings("unchecked") T value = (T) tryInvoke(instance, method, arguments); + // We could add a check to ensure that value is null only if this Binding is annotated with + // nullable. However, we cannot because not all Nullable annotations have runtime retention so + // it would return false positives. return value; } + boolean nullableMatch(boolean nullable) { + if (this.nullable && !nullable) { + return false; + } + if (!this.nullable && nullable) { + return false; + } + return true; + } + @Override public String toString() { - return "@Provides[" + method.getDeclaringClass().getName() + '.' + method.getName() + "(…)]"; + String nullableString = nullable ? "@Nullable " : ""; + return nullableString + + "@Provides[" + + method.getDeclaringClass().getName() + + '.' + + method.getName() + + "(…)]"; } } diff --git a/reflect/src/main/java/dagger/reflect/Reflection.java b/reflect/src/main/java/dagger/reflect/Reflection.java index 7bb4bb9d..96a35204 100644 --- a/reflect/src/main/java/dagger/reflect/Reflection.java +++ b/reflect/src/main/java/dagger/reflect/Reflection.java @@ -121,6 +121,19 @@ static boolean hasAnnotation(Annotation[] annotations, Class T requireAnnotation(Class cls, Class annotationClass) { T annotation = cls.getAnnotation(annotationClass); diff --git a/reflect/src/main/java/dagger/reflect/ReflectiveDependencyParser.java b/reflect/src/main/java/dagger/reflect/ReflectiveDependencyParser.java index ce623c89..efeaa082 100644 --- a/reflect/src/main/java/dagger/reflect/ReflectiveDependencyParser.java +++ b/reflect/src/main/java/dagger/reflect/ReflectiveDependencyParser.java @@ -1,6 +1,7 @@ package dagger.reflect; import static dagger.reflect.Reflection.findQualifier; +import static dagger.reflect.Reflection.hasNullable; import dagger.reflect.Binding.LinkedBinding; import java.lang.annotation.Annotation; @@ -21,7 +22,9 @@ static void parse(Class cls, Object instance, Scope.Builder scopeBuilder) { Type type = method.getGenericReturnType(); Key key = Key.of(qualifier, type); - Binding binding = new LinkedProvidesBinding<>(instance, method, NO_BINDINGS); + Binding binding = + new LinkedProvidesBinding<>( + instance, method, NO_BINDINGS, hasNullable(method.getDeclaredAnnotations())); scopeBuilder.addBinding(key, binding); } diff --git a/reflect/src/main/java/dagger/reflect/UnlinkedProvidesBinding.java b/reflect/src/main/java/dagger/reflect/UnlinkedProvidesBinding.java index 70427afb..663331f3 100644 --- a/reflect/src/main/java/dagger/reflect/UnlinkedProvidesBinding.java +++ b/reflect/src/main/java/dagger/reflect/UnlinkedProvidesBinding.java @@ -1,6 +1,7 @@ package dagger.reflect; import static dagger.reflect.Reflection.findQualifier; +import static dagger.reflect.Reflection.hasNullable; import java.lang.annotation.Annotation; import java.lang.reflect.Method; @@ -25,7 +26,8 @@ public LinkedBinding link(Linker linker, Scope scope) { Key key = Key.of(findQualifier(parameterAnnotations[i]), parameterTypes[i]); dependencies[i] = linker.get(key); } - return new LinkedProvidesBinding<>(instance, method, dependencies); + return new LinkedProvidesBinding<>( + instance, method, dependencies, hasNullable(method.getDeclaredAnnotations())); } @Override