MOE Sync 2020-07-30

core/src/com/google/inject/internal/AbstractBindingProcessor.java

@@ -81,6 +81,10 @@ protected void putBinding(BindingImpl<?> binding) {
       if (injector.getBindingData().getExplicitBinding(key) != null) {
         try {
           if (!isOkayDuplicate(original, binding, injector.getBindingData())) {
+            if (InternalFlags.enableExperimentalErrorMessages()) {
+              errors.bindingAlreadySet(binding, original);
+              return;
+            }
             errors.bindingAlreadySet(key, original.getSource());
             return;
           }

core/src/com/google/inject/internal/BindingAlreadySetError.java

@@ -0,0 +1,54 @@
+package com.google.inject.internal;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.Binding;
+import com.google.inject.spi.ErrorDetail;
+import java.util.ArrayList;
+import java.util.Formatter;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/** Error reported by Guice when a key is bound at multiple places the injector. */
+final class BindingAlreadySetError extends ErrorDetail<BindingAlreadySetError> {
+  private final Binding<?> binding;
+  private final Binding<?> original;
+
+  BindingAlreadySetError(Binding<?> binding, Binding<?> original, List<Object> sources) {
+    super(
+        String.format("%s was bound multiple times.", Messages.convert(binding.getKey())),
+        sources,
+        null);
+    this.binding = binding;
+    this.original = original;
+  }
+
+  @Override
+  public boolean isMergeable(ErrorDetail<?> otherError) {
+    return otherError instanceof BindingAlreadySetError
+        && ((BindingAlreadySetError) otherError).binding.getKey().equals(binding.getKey());
+  }
+
+  @Override
+  public void format(int index, List<ErrorDetail<?>> mergeableErrors, Formatter formatter) {
+    List<List<Object>> sourcesList = new ArrayList<>();
+    sourcesList.add(ImmutableList.of(original.getSource()));
+    sourcesList.add(ImmutableList.of(binding.getSource()));
+    sourcesList.addAll(
+        mergeableErrors.stream()
+            .map(e -> ((BindingAlreadySetError) e).binding.getSource())
+            .map(ImmutableList::of)
+            .collect(Collectors.toList()));
+
+    formatter.format("%s) %s %s%n%n", index, "[Guice/BindingAlreadySet]", getMessage());
+    formatter.format("Bound at the following %s locations:%n", sourcesList.size());
+    for (int i = 0; i < sourcesList.size(); i++) {
+      ErrorFormatter.formatSources(i + 1, sourcesList.get(i), formatter);
+    }
+    formatter.format("%n");
+  }
+
+  @Override
+  public BindingAlreadySetError withSources(List<Object> newSources) {
+    return new BindingAlreadySetError(binding, original, newSources);
+  }
+}

core/src/com/google/inject/internal/Errors.java

@@ -511,6 +511,12 @@ public Errors recursiveBinding() {
     return addMessage(ErrorId.RECURSIVE_BINDING, "Binding points to itself.");
   }
 
+  Errors bindingAlreadySet(Binding<?> binding, Binding<?> original) {
+    BindingAlreadySetError error = new BindingAlreadySetError(binding, original, getSources());
+    return addMessage(
+        new Message(GuiceInternal.GUICE_INTERNAL, ErrorId.BINDING_ALREADY_SET, error));
+  }
+
   public Errors bindingAlreadySet(Key<?> key, Object source) {
     return addMessage(
         ErrorId.BINDING_ALREADY_SET,

core/src/com/google/inject/internal/MissingImplementationError.java

@@ -13,7 +13,8 @@ final class MissingImplementationError extends ErrorDetail<MissingImplementation
   private final Key<?> key;
 
   public MissingImplementationError(Key<?> key, List<Object> sources) {
-    super("", sources, null);
+    super(
+        String.format("No implementation for %s was bound.", Messages.convert(key)), sources, null);
     this.key = key;
   }
 
@@ -29,16 +30,13 @@ public void format(int index, List<ErrorDetail<?>> mergeableErrors, Formatter fo
     sourcesList.add(getSources());
     sourcesList.addAll(
         mergeableErrors.stream().map(ErrorDetail::getSources).collect(Collectors.toList()));
-    formatter.format(
-        "%s) [Guice/MissingImplementation]: No implementation for %s was bound.%n",
-        index, Messages.convert(key));
-
     List<List<Object>> filteredSourcesList =
         sourcesList.stream()
             .map(this::trimSource)
             .filter(sources -> !sources.isEmpty())
             .collect(Collectors.toList());
 
+    formatter.format("%s) %s: %s%n", index, "[Guice/MissingImplementation]", getMessage());
     if (!filteredSourcesList.isEmpty()) {
       formatter.format("%n%s%n", "Requested by:");
       int sourceListIndex = 1;

core/src/com/google/inject/internal/RealMultibinder.java

@@ -81,6 +81,13 @@ static <T> TypeLiteral<Collection<javax.inject.Provider<T>>> collectionOfJavaxPr
     return (TypeLiteral<Collection<javax.inject.Provider<T>>>) TypeLiteral.get(type);
   }
 
+  @SuppressWarnings("unchecked")
+  static <T> TypeLiteral<Set<? extends T>> setOfExtendsOf(TypeLiteral<T> elementType) {
+    Type extendsType = Types.subtypeOf(elementType.getType());
+    Type setOfExtendsType = Types.setOf(extendsType);
+    return (TypeLiteral<Set<? extends T>>) TypeLiteral.get(setOfExtendsType);
+  }
+
   private final BindingSelection<T> bindingSelection;
   private final Binder binder;
 
@@ -100,6 +107,7 @@ public void configure(Binder binder) {
     binder
         .bind(bindingSelection.getCollectionOfProvidersKey())
         .toProvider(collectionOfProvidersProvider);
+    binder.bind(bindingSelection.getSetOfExtendsKey()).to(bindingSelection.getSetKey());
 
     // The collection this exposes is internally an ImmutableList, so it's OK to massage
     // the guice Provider to javax Provider in the value (since the guice Provider implements
@@ -296,7 +304,8 @@ public Key<Set<T>> getSetKey() {
     public Set<Key<?>> getAlternateSetKeys() {
       return ImmutableSet.of(
           (Key<?>) bindingSelection.getCollectionOfProvidersKey(),
-          (Key<?>) bindingSelection.getCollectionOfJavaxProvidersKey());
+          (Key<?>) bindingSelection.getCollectionOfJavaxProvidersKey(),
+          (Key<?>) bindingSelection.getSetOfExtendsKey());
     }
 
     @Override
@@ -332,6 +341,7 @@ private static final class BindingSelection<T> {
     private String setName;
     private Key<Collection<Provider<T>>> collectionOfProvidersKey;
     private Key<Collection<javax.inject.Provider<T>>> collectionOfJavaxProvidersKey;
+    private Key<Set<? extends T>> setOfExtendsKey;
     private Key<Boolean> permitDuplicatesKey;
 
     private boolean isInitialized;
@@ -457,6 +467,14 @@ Key<Collection<javax.inject.Provider<T>>> getCollectionOfJavaxProvidersKey() {
       return local;
     }
 
+    Key<Set<? extends T>> getSetOfExtendsKey() {
+      Key<Set<? extends T>> local = setOfExtendsKey;
+      if (local == null) {
+        local = setOfExtendsKey = setKey.ofType(setOfExtendsOf(elementType));
+      }
+      return local;
+    }
+
     boolean isInitialized() {
       return isInitialized;
     }
@@ -496,7 +514,8 @@ boolean containsElement(com.google.inject.spi.Element element) {
             || binding.getKey().equals(getPermitDuplicatesKey())
             || binding.getKey().equals(setKey)
             || binding.getKey().equals(collectionOfProvidersKey)
-            || binding.getKey().equals(collectionOfJavaxProvidersKey);
+            || binding.getKey().equals(collectionOfJavaxProvidersKey)
+            || binding.getKey().equals(setOfExtendsKey);
       } else {
         return false;
       }

core/src/com/google/inject/multibindings/MultibinderBinding.java

@@ -27,12 +27,12 @@
 /**
  * A binding for a Multibinder.
  *
- * <p>Although Multibinders may be injected through a variety of generic types ({@code Set<V>} and
- * {@code Collection<Provider<V>>}), a MultibinderBinding exists only on the Binding associated with
- * the {@code Set<V>} key. Injectable types can be discovered using {@link #getSetKey} (which will
- * return the {@code Set<V>} key), or{@link #getAlternateSetKeys} (which will return the other keys
- * that can inject this data). Other bindings can be validated to be derived from this
- * MultibinderBinding using {@link #containsElement(Element)}.
+ * <p>Although Multibinders may be injected through a variety of generic types ({@code Set<V>},
+ * {@code Collection<Provider<V>>}, and {@code Set<? extends V>} ), a MultibinderBinding exists only
+ * on the Binding associated with the {@code Set<V>} key. Injectable types can be discovered using
+ * {@link #getSetKey} (which will return the {@code Set<V>} key), or {@link #getAlternateSetKeys}
+ * (which will return the other keys that can inject this data). Other bindings can be validated to
+ * be derived from this MultibinderBinding using {@link #containsElement(Element)}.
  *
  * @param <T> The fully qualified type of the set, including Set. For example: {@code
  *     MultibinderBinding<Set<Boolean>>}
@@ -46,8 +46,8 @@ public interface MultibinderBinding<T> {
 
   /**
    * Returns the keys of other bindings that represent this set. This will return an entry for
-   * {@code Collection<com.google.inject.Provider<V>>} and {@code
-   * Collection<javax.inject.Provider<V>>}.
+   * {@code Collection<com.google.inject.Provider<V>>}, {@code
+   * Collection<javax.inject.Provider<V>>}, and {@code Set<? extends V>}.
    *
    * @since 4.2.3
    */

core/test/com/google/inject/internal/MapBinderTest.java

@@ -163,6 +163,10 @@ protected void configure() {
                     collectionOf(
                         Types.javaxProviderOf(
                             mapEntryOf(String.class, Types.providerOf(String.class))))),
+                // Set<? extends Map.Entry<K, Provider<V>>>
+                Key.get(
+                    Types.setOf(
+                        Types.subtypeOf(mapEntryOf(String.class, Types.providerOf(String.class))))),
                 // @Named(...) Boolean
                 Key.get(
                     Boolean.class,

core/test/com/google/inject/internal/MultibinderTest.java

@@ -1530,6 +1530,93 @@ void setter(String s) {
         });
   }
 
+  public void testMultibinderWithWildcard() {
+    Module module =
+        new AbstractModule() {
+          @Override
+          protected void configure() {
+            Multibinder<String> multibinder = Multibinder.newSetBinder(binder(), String.class);
+            multibinder.addBinding().toInstance("a");
+            multibinder.addBinding().toInstance("b");
+            multibinder.addBinding().toInstance("c");
+          }
+        };
+    Injector injector = Guice.createInjector(module);
+
+    Set<String> set = injector.getInstance(new Key<Set<String>>() {});
+    assertEquals(ImmutableSet.of("a", "b", "c"), set);
+
+    Set<? extends String> setOfWildcard = injector.getInstance(new Key<Set<? extends String>>() {});
+    assertEquals(ImmutableSet.of("a", "b", "c"), setOfWildcard);
+  }
+
+  /**
+   * Injection of {@code Set<? extends T>} wasn't added until 2020-07. It's possible that
+   * applications already have a binding to that type. If they do, confirm that Guice fails fast
+   * with a duplicate binding error.
+   */
+  public void testMultibinderConflictsWithExistingWildcard() {
+    Module module =
+        new AbstractModule() {
+          @Override
+          protected void configure() {
+            Multibinder<String> multibinder = Multibinder.newSetBinder(binder(), String.class);
+            multibinder.addBinding().toInstance("a");
+            multibinder.addBinding().toInstance("b");
+            multibinder.addBinding().toInstance("c");
+          }
+
+          @Provides
+          public Set<? extends String> provideStrings() {
+            return ImmutableSet.of("d", "e", "f");
+          }
+        };
+
+    try {
+      Guice.createInjector(module);
+      fail();
+    } catch (CreationException e) {
+      assertTrue(
+          e.getMessage()
+              .contains(
+                  "A binding to java.util.Set<? extends java.lang.String> was already configured"));
+    }
+  }
+
+  /**
+   * This is the same as the previous test, but it gets at the conflicting set through a multibinder
+   * rather than through a regular binding. It's unlikely that application developers would do this
+   * in practice, but if they do we want to make sure it is detected and fails fast.
+   */
+  public void testMultibinderConflictsWithExistingMultibinder() {
+    Module module =
+        new AbstractModule() {
+          @Override
+          protected void configure() {
+            Multibinder<String> multibinder = Multibinder.newSetBinder(binder(), String.class);
+            multibinder.addBinding().toInstance("a");
+            multibinder.addBinding().toInstance("b");
+            multibinder.addBinding().toInstance("c");
+
+            Multibinder<String> multibinder2 =
+                Multibinder.newSetBinder(
+                    binder(), (TypeLiteral<String>) TypeLiteral.get(Types.subtypeOf(String.class)));
+            multibinder2.addBinding().toInstance("d");
+            multibinder2.addBinding().toInstance("e");
+          }
+        };
+
+    try {
+      Guice.createInjector(module);
+      fail();
+    } catch (CreationException e) {
+      assertTrue(
+          e.getMessage()
+              .contains(
+                  "A binding to java.util.Set<? extends java.lang.String> was already configured"));
+    }
+  }
+
   private <T> Collection<T> collectValues(
       Collection<? extends javax.inject.Provider<T>> providers) {
     Collection<T> values = Lists.newArrayList();