Throw an error if there is no MapKey annotation on @IntoMap dagge…

…r provider methods. PiperOrigin-RevId: 359792774
google · Feb 26, 2021 · 2872237 · 2872237
1 parent 9dc98cb
commit 2872237
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 3 deletions.
diff --git a/core/src/com/google/inject/internal/ProviderMethodsModule.java b/core/src/com/google/inject/internal/ProviderMethodsModule.java
@@ -295,15 +295,17 @@ private <T> ProviderMethod<T> createProviderMethod(
     @SuppressWarnings("unchecked") // Define T as the method's return type.
     TypeLiteral<T> returnType = (TypeLiteral<T>) typeLiteral.getReturnType(method);
     Key<T> key = getKey(errors, returnType, method, method.getAnnotations());
+    boolean prepareMethodError = false;
     try {
       key = scanner.prepareMethod(binder, annotation, key, point);
     } catch (Throwable t) {
+      prepareMethodError = true;
       binder.addError(t);
     }
 
     if (Modifier.isAbstract(method.getModifiers())) {
       checkState(
-          key == null,
+          prepareMethodError || key == null,
           "%s returned a non-null key (%s) for %s. prepareMethod() must return null for abstract"
               + " methods",
           scanner,

diff --git a/extensions/dagger-adapter/src/com/google/inject/daggeradapter/DaggerMethodScanner.java b/extensions/dagger-adapter/src/com/google/inject/daggeradapter/DaggerMethodScanner.java
@@ -15,6 +15,7 @@
  */
 package com.google.inject.daggeradapter;
 
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.inject.daggeradapter.Annotations.getAnnotatedAnnotation;
@@ -37,6 +38,7 @@
 import com.google.inject.spi.ModuleAnnotatedMethodScanner;
 import dagger.Binds;
 import dagger.BindsOptionalOf;
+import dagger.MapKey;
 import dagger.Provides;
 import dagger.multibindings.IntoMap;
 import dagger.multibindings.IntoSet;
@@ -46,6 +48,7 @@
 import java.lang.reflect.ParameterizedType;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import javax.inject.Scope;
 
@@ -139,8 +142,13 @@ private static <K, V> Key<V> processMapBinding(Binder binder, Key<V> key, Method
   }
 
   private static <K> MapKeyData<K> mapKeyData(Method method) {
-    Annotation mapKey = getAnnotatedAnnotation(method, dagger.MapKey.class).get();
-    dagger.MapKey mapKeyDefinition = mapKey.annotationType().getAnnotation(dagger.MapKey.class);
+    Optional<Annotation> mapKeyOpt = getAnnotatedAnnotation(method, MapKey.class);
+    checkState(
+        mapKeyOpt.isPresent(),
+        "Missing @MapKey annotation on method %s (make sure the annotation has RUNTIME rentention)",
+        method);
+    Annotation mapKey = mapKeyOpt.get();
+    MapKey mapKeyDefinition = mapKey.annotationType().getAnnotation(MapKey.class);
     if (!mapKeyDefinition.unwrapValue()) {
       return MapKeyData.create(TypeLiteral.get(mapKey.annotationType()), mapKey);
     }

diff --git a/extensions/dagger-adapter/test/com/google/inject/daggeradapter/IntoMapTest.java b/extensions/dagger-adapter/test/com/google/inject/daggeradapter/IntoMapTest.java
@@ -17,8 +17,10 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import static org.junit.Assert.assertThrows;
 
 import com.google.inject.Binder;
+import com.google.inject.CreationException;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Key;
@@ -27,6 +29,7 @@
 import com.google.inject.multibindings.MapBinder;
 import com.google.inject.util.Providers;
 import dagger.Binds;
+import dagger.MapKey;
 import dagger.multibindings.IntoMap;
 import dagger.multibindings.StringKey;
 import java.lang.annotation.Retention;
@@ -198,4 +201,52 @@ public void testBinds() {
     Map<String, Object> map = injector.getInstance(new Key<Map<String, Object>>() {});
     assertThat(map).containsExactly("hello", "world");
   }
+
+  @MapKey
+  @interface CustomMapKey {
+    String value() default "";
+  }
+
+  @dagger.Module
+  static class MissingRuntimeRetentionModule {
+    @dagger.Provides
+    @IntoMap
+    @CustomMapKey("hello")
+    String binds() {
+      return "world";
+    }
+  }
+
+  @dagger.Module
+  abstract static class MissingRuntimeRetentionBindsModule {
+    @dagger.Binds
+    @IntoMap
+    @CustomMapKey("hello")
+    abstract Object binds(String value);
+
+    @dagger.Provides
+    static String provideValue() {
+      return "world";
+    }
+  }
+
+  public void testMissingRuntimeRetention() {
+    CreationException exception =
+        assertThrows(
+            CreationException.class,
+            () -> Guice.createInjector(DaggerAdapter.from(new MissingRuntimeRetentionModule())));
+    assertThat(exception.getErrorMessages()).hasSize(1);
+    assertThat(exception).hasMessageThat().contains("Missing @MapKey annotation on method");
+  }
+
+  public void testMissingRuntimeRetentionUsingBinds() {
+    CreationException exception =
+        assertThrows(
+            CreationException.class,
+            () ->
+                Guice.createInjector(DaggerAdapter.from(MissingRuntimeRetentionBindsModule.class)));
+    exception.printStackTrace();
+    assertThat(exception.getErrorMessages()).hasSize(1);
+    assertThat(exception).hasMessageThat().contains("Missing @MapKey annotation on method");
+  }
 }