Fix of #700

Ensure the order of calls to Binder.bind(...) in combination with @ does not affect whether classes are bound explicitly or with jIT binding.

Signed-off-by: Harald Fassler <harald.fassler+9974@gmail.com>

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

@@ -56,7 +56,7 @@
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -610,7 +610,7 @@ <T> void initializeJitBinding(BindingImpl<T> binding, Errors errors) throws Erro
           // so that cached exceptions while constructing it get stored.
           // See TypeListenerTest#testTypeListenerThrows
           removeFailedJitBinding(binding, null);
-          cleanup(binding, new HashSet<Key<?>>());
+          cleanup(binding, new HashMap<>());
         }
       }
     }
@@ -622,13 +622,15 @@ <T> void initializeJitBinding(BindingImpl<T> binding, Errors errors) throws Erro
    * added to allow circular dependency support, so dependencies may pass where they should have
    * failed.
    */
-  private boolean cleanup(BindingImpl<?> binding, Set<Key<?>> encountered) {
+  private boolean cleanup(BindingImpl<?> binding, Map<Key<?>, Boolean> encountered) {
     boolean bindingFailed = false;
     Set<Dependency<?>> deps = getInternalDependencies(binding);
     for (Dependency<?> dep : deps) {
       Key<?> depKey = dep.getKey();
       InjectionPoint ip = dep.getInjectionPoint();
-      if (encountered.add(depKey)) { // only check if we haven't looked at this key yet
+      Boolean failedBefore = encountered.get(depKey);
+      if (failedBefore == null) { // only check if we haven't looked at this key yet
+        encountered.put(depKey, false);
         BindingImpl<?> depBinding = jitBindingData.getJitBinding(depKey);
         if (depBinding != null) { // if the binding still exists, validate
           boolean failed = cleanup(depBinding, encountered); // if children fail, we fail
@@ -640,6 +642,7 @@ private boolean cleanup(BindingImpl<?> binding, Set<Key<?>> encountered) {
             }
           }
           if (failed) {
+            encountered.put(depKey, true);
             removeFailedJitBinding(depBinding, ip);
             bindingFailed = true;
           }
@@ -649,6 +652,9 @@ private boolean cleanup(BindingImpl<?> binding, Set<Key<?>> encountered) {
           bindingFailed = true;
         }
       }
+      else if (Boolean.TRUE.equals(failedBefore)) {
+        bindingFailed = true;
+      }
     }
     return bindingFailed;
   }
@@ -812,14 +818,16 @@ private <T> BindingImpl<T> createImplementedByBinding(
     final Key<? extends T> targetKey = Key.get(subclass);
     Object source = rawType;
     FactoryProxy<T> factory = new FactoryProxy<>(this, key, targetKey, source);
-    factory.notify(errors); // causes the factory to initialize itself internally
+    // factory.notify(...) causes the factory to initialize itself internally
+    DelayedInitialize initialize = (injector, errors1) -> factory.notify(errors1);
     return new LinkedBindingImpl<T>(
         this,
         key,
         source,
         Scoping.<T>scope(key, this, factory, source, scoping),
         scoping,
-        targetKey);
+        targetKey,
+        initialize);
   }
 
   /**
@@ -941,6 +949,7 @@ private <T> BindingImpl<T> createJustInTimeBinding(
         createUninitializedBinding(key, Scoping.UNSCOPED, source, errors, true);
     errors.throwIfNewErrors(numErrorsBefore);
     initializeJitBinding(binding, errors);
+    errors.throwIfNewErrors(numErrorsBefore);
     return binding;
   }
 

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

@@ -31,9 +31,10 @@
 import java.util.Set;
 
 final class LinkedBindingImpl<T> extends BindingImpl<T>
-    implements LinkedKeyBinding<T>, HasDependencies {
+    implements LinkedKeyBinding<T>, HasDependencies, DelayedInitialize {
 
   final Key<? extends T> targetKey;
+  private DelayedInitialize delayedInitialize;
 
   LinkedBindingImpl(
       InjectorImpl injector,
@@ -42,15 +43,34 @@ final class LinkedBindingImpl<T> extends BindingImpl<T>
       InternalFactory<? extends T> internalFactory,
       Scoping scoping,
       Key<? extends T> targetKey) {
+    this(injector, key, source, internalFactory, scoping, targetKey, null);
+  }
+
+  LinkedBindingImpl(
+          InjectorImpl injector,
+          Key<T> key,
+          Object source,
+          InternalFactory<? extends T> internalFactory,
+          Scoping scoping,
+          Key<? extends T> targetKey,
+          DelayedInitialize delayedInitialize) {
     super(injector, key, source, internalFactory, scoping);
     this.targetKey = targetKey;
+    this.delayedInitialize = delayedInitialize;
   }
 
   LinkedBindingImpl(Object source, Key<T> key, Scoping scoping, Key<? extends T> targetKey) {
     super(source, key, scoping);
     this.targetKey = targetKey;
   }
 
+  @Override
+  public void initialize(InjectorImpl injector, Errors errors) throws ErrorsException {
+    if (this.delayedInitialize != null) {
+      this.delayedInitialize.initialize(injector, errors);
+    }
+  }
+
   @Override
   public <V> V acceptTargetVisitor(BindingTargetVisitor<? super T, V> visitor) {
     return visitor.visit(this);

core/test/com/google/inject/BinderTest.java

@@ -439,6 +439,27 @@ protected void configure() {
         });
   }
 
+  /**
+   * Test bugfix of #700.<br />
+   * Test, that {@code JustAClass} is not bound with JIT binding
+   * when {@code bind(HasImplementedByThatWantsExplicitClass.class}} is called before {@code bind(JustAClass.class)}.
+   * <br /><br />
+   * The error before the fix was:
+   * <pre>
+   * 1) [Guice/JitBindingAlreadySet]: A just-in-time binding to BinderTest$JustAClass was already configured on a parent injector.
+   *   at BinderTest$25.configure(BinderTest.java:458)
+   * </pre>
+   */
+  public void testJitDependencyCanUseExplicitDependenciesInAnyOrder() {
+    Guice.createInjector(new AbstractModule() {
+        @Override
+        protected void configure() {
+          bind(HasImplementedByThatWantsExplicitClass.class);
+          bind(JustAClass.class);
+        }
+    });
+  }
+
   /**
    * Untargetted bindings should follow @ImplementedBy and @ProvidedBy annotations if they exist.
    * Otherwise the class should be constructed directly.
@@ -515,20 +536,23 @@ protected void configure() {
   }
 
   public void testBindingToProvider() {
+    String simpleName = null;
     try {
-      Guice.createInjector(
-          new AbstractModule() {
+      Module module = new AbstractModule() {
             @Override
             protected void configure() {
               bind(new TypeLiteral<Provider<String>>() {}).toInstance(Providers.of("A"));
             }
-          });
+        };
+      String className = module.getClass().getName();
+      simpleName = className.substring(className.lastIndexOf('.') + 1);
+      Guice.createInjector(module);
       fail();
     } catch (CreationException expected) {
       assertContains(
           expected.getMessage(),
           "Binding to Provider is not allowed.",
-          "at BinderTest$28.configure");
+          String.format("at %s.configure", simpleName));
     }
   }
 
@@ -662,6 +686,15 @@ static class ImplementsHasImplementedByThatWantsExplicit
     ImplementsHasImplementedByThatWantsExplicit(JustAnInterface jai) {}
   }
 
+  @ImplementedBy(ImplementsHasImplementedByThatWantsExplicitClass.class)
+  static interface HasImplementedByThatWantsExplicitClass {}
+
+  static class ImplementsHasImplementedByThatWantsExplicitClass
+      implements HasImplementedByThatWantsExplicitClass {
+    @Inject
+    ImplementsHasImplementedByThatWantsExplicitClass(JustAClass jac) {}
+  }
+
   static interface JustAnInterface {}
 
   //  public void testBindInterfaceWithoutImplementation() {