fix issue 614 -- admittedly not the prettiest solution, but it works.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1526 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/core/src/com/google/inject/internal/BindingProcessor.java b/core/src/com/google/inject/internal/BindingProcessor.java
index 6de190f..28d04a6 100644
--- a/core/src/com/google/inject/internal/BindingProcessor.java
+++ b/core/src/com/google/inject/internal/BindingProcessor.java
@@ -54,11 +54,18 @@
private final List<CreationListener> creationListeners = Lists.newArrayList();
private final Initializer initializer;
private final List<Runnable> uninitializedBindings = Lists.newArrayList();
+
+ enum Phase { PASS_ONE, PASS_TWO; }
+ private Phase phase;
BindingProcessor(Errors errors, Initializer initializer) {
super(errors);
this.initializer = initializer;
}
+
+ void setPhase(Phase phase) {
+ this.phase = phase;
+ }
@Override public <T> Boolean visit(Binding<T> command) {
final Object source = command.getSource();
@@ -86,8 +93,8 @@
final Scoping scoping = Scoping.makeInjectable(
((BindingImpl<?>) command).getScoping(), injector, errors);
- command.acceptTargetVisitor(new BindingTargetVisitor<T, Void>() {
- public Void visit(ConstructorBinding<? extends T> binding) {
+ return command.acceptTargetVisitor(new BindingTargetVisitor<T, Boolean>() {
+ public Boolean visit(ConstructorBinding<? extends T> binding) {
try {
ConstructorBindingImpl<T> onInjector = ConstructorBindingImpl.create(injector, key,
binding.getConstructor(), source, scoping, errors, false);
@@ -97,10 +104,10 @@
errors.merge(e.getErrors());
putBinding(invalidBinding(injector, key, source));
}
- return null;
+ return true;
}
- public Void visit(InstanceBinding<? extends T> binding) {
+ public Boolean visit(InstanceBinding<? extends T> binding) {
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
T instance = binding.getInstance();
Initializable<T> ref = initializer.requestInjection(
@@ -110,10 +117,10 @@
= Scoping.scope(key, injector, factory, source, scoping);
putBinding(new InstanceBindingImpl<T>(injector, key, source, scopedFactory, injectionPoints,
instance));
- return null;
+ return true;
}
- public Void visit(ProviderInstanceBinding<? extends T> binding) {
+ public Boolean visit(ProviderInstanceBinding<? extends T> binding) {
Provider<? extends T> provider = binding.getProviderInstance();
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
Initializable<Provider<? extends T>> initializable = initializer
@@ -123,10 +130,10 @@
= Scoping.scope(key, injector, factory, source, scoping);
putBinding(new ProviderInstanceBindingImpl<T>(injector, key, source, scopedFactory, scoping,
provider, injectionPoints));
- return null;
+ return true;
}
- public Void visit(ProviderKeyBinding<? extends T> binding) {
+ public Boolean visit(ProviderKeyBinding<? extends T> binding) {
Key<? extends javax.inject.Provider<? extends T>> providerKey = binding.getProviderKey();
BoundProviderFactory<T> boundProviderFactory
= new BoundProviderFactory<T>(injector, providerKey, source);
@@ -135,10 +142,10 @@
key, injector, (InternalFactory<? extends T>) boundProviderFactory, source, scoping);
putBinding(new LinkedProviderBindingImpl<T>(
injector, key, source, scopedFactory, scoping, providerKey));
- return null;
+ return true;
}
- public Void visit(LinkedKeyBinding<? extends T> binding) {
+ public Boolean visit(LinkedKeyBinding<? extends T> binding) {
Key<? extends T> linkedKey = binding.getLinkedKey();
if (key.equals(linkedKey)) {
errors.recursiveBinding();
@@ -150,10 +157,10 @@
= Scoping.scope(key, injector, factory, source, scoping);
putBinding(
new LinkedBindingImpl<T>(injector, key, source, scopedFactory, scoping, linkedKey));
- return null;
+ return true;
}
- public Void visit(UntargettedBinding<? extends T> untargetted) {
+ public Boolean visit(UntargettedBinding<? extends T> untargetted) {
// Error: Missing implementation.
// Example: bind(Date.class).annotatedWith(Red.class);
// We can't assume abstract types aren't injectable. They may have an
@@ -161,7 +168,12 @@
if (key.getAnnotationType() != null) {
errors.missingImplementation(key);
putBinding(invalidBinding(injector, key, source));
- return null;
+ return true;
+ }
+
+ // We want to do UntargettedBindings in the second pass.
+ if (phase == Phase.PASS_ONE) {
+ return false;
}
// This cast is safe after the preceeding check.
@@ -175,18 +187,18 @@
putBinding(invalidBinding(injector, key, source));
}
- return null;
+ return true;
}
- public Void visit(ExposedBinding<? extends T> binding) {
+ public Boolean visit(ExposedBinding<? extends T> binding) {
throw new IllegalArgumentException("Cannot apply a non-module element");
}
- public Void visit(ConvertedConstantBinding<? extends T> binding) {
+ public Boolean visit(ConvertedConstantBinding<? extends T> binding) {
throw new IllegalArgumentException("Cannot apply a non-module element");
}
- public Void visit(ProviderBinding<? extends T> binding) {
+ public Boolean visit(ProviderBinding<? extends T> binding) {
throw new IllegalArgumentException("Cannot apply a non-module element");
}
@@ -202,11 +214,15 @@
});
}
});
-
- return true;
}
@Override public Boolean visit(PrivateElements privateElements) {
+ // Because we do two passes, we have to ignore the PrivateElements in the second
+ // pass. Otherwise we end up calling bindExposed twice for each one.
+ if (phase == Phase.PASS_TWO) {
+ return false;
+ }
+
for (Key<?> key : privateElements.getExposedKeys()) {
bindExposed(privateElements, key);
}
diff --git a/core/src/com/google/inject/internal/InjectorShell.java b/core/src/com/google/inject/internal/InjectorShell.java
index 9086b9a..151a1a1 100644
--- a/core/src/com/google/inject/internal/InjectorShell.java
+++ b/core/src/com/google/inject/internal/InjectorShell.java
@@ -24,6 +24,7 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.inject.Singleton;
import com.google.inject.Stage;
+import com.google.inject.internal.BindingProcessor.Phase;
import com.google.inject.internal.InjectorImpl.InjectorOptions;
import com.google.inject.internal.util.ImmutableSet;
import com.google.inject.internal.util.Lists;
@@ -169,6 +170,14 @@
bindInjector(injector);
bindLogger(injector);
+
+ // Do two passes over our bindings -- first to get all non UntargettedBindings,
+ // then to get the only UntargettedBindings. This is necessary because
+ // UntargettedBindings can create JIT bindings and need all their other
+ // dependencies set up ahead of time.
+ bindingProcessor.setPhase(Phase.PASS_ONE);
+ bindingProcessor.process(injector, elements);
+ bindingProcessor.setPhase(Phase.PASS_TWO);
bindingProcessor.process(injector, elements);
stopwatch.resetAndLog("Binding creation");
diff --git a/core/test/com/google/inject/BinderTest.java b/core/test/com/google/inject/BinderTest.java
index 6f97a03..435ab1b 100644
--- a/core/test/com/google/inject/BinderTest.java
+++ b/core/test/com/google/inject/BinderTest.java
@@ -268,8 +268,83 @@
assertSame(strings, injector.getInstance(new Key<String[]>() {}));
assertSame(strings, injector.getInstance(String[].class));
}
+
+ /**
+ * Binding something to two different things should give an error.
+ */
+ public void testSettingBindingTwice() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(String.class).toInstance("foo");
+ bind(String.class).toInstance("bar");
+ }
+ });
+ fail();
+ } catch(CreationException expected) {
+ assertContains(expected.getMessage(),
+ "1) A binding to java.lang.String was already configured at " + getClass().getName(),
+ "at " + getClass().getName(), ".configure(BinderTest.java:");
+ assertContains(expected.getMessage(), "1 error");
+ }
+ }
+
+ /**
+ * Binding an @ImplementedBy thing to something else should also fail.
+ */
+ public void testSettingAtImplementedByTwice() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(HasImplementedBy1.class);
+ bind(HasImplementedBy1.class).toInstance(new HasImplementedBy1() {});
+ }
+ });
+ fail();
+ } catch(CreationException expected) {
+ expected.printStackTrace();
+ assertContains(expected.getMessage(),
+ "1) A binding to " + HasImplementedBy1.class.getName()
+ + " was already configured at " + getClass().getName(),
+ "at " + getClass().getName(), ".configure(BinderTest.java:");
+ assertContains(expected.getMessage(), "1 error");
+ }
+ }
/**
+ * See issue 614, Problem One
+ * http://code.google.com/p/google-guice/issues/detail?id=614
+ */
+ public void testJitDependencyDoesntBlockOtherExplicitBindings() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(HasImplementedByThatNeedsAnotherImplementedBy.class);
+ bind(HasImplementedBy1.class).toInstance(new HasImplementedBy1() {});
+ }
+ });
+ injector.getAllBindings(); // just validate it doesn't throw.
+ // Also validate that we're using the explicit (and not @ImplementedBy) implementation
+ assertFalse(injector.getInstance(HasImplementedBy1.class) instanceof ImplementsHasImplementedBy1);
+ }
+
+ /**
+ * See issue 614, Problem Two
+ * http://code.google.com/p/google-guice/issues/detail?id=614
+ */
+ public void testJitDependencyCanUseExplicitDependencies() {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(HasImplementedByThatWantsExplicit.class);
+ bind(JustAnInterface.class).toInstance(new JustAnInterface() {});
+ }
+ });
+ }
+
+ /**
* Untargetted bindings should follow @ImplementedBy and @ProvidedBy
* annotations if they exist. Otherwise the class should be constructed
* directly.
@@ -423,6 +498,28 @@
static class ExtendsHasImplementedBy2 extends HasImplementedBy2 {}
static class JustAClass {}
+
+ @ImplementedBy(ImplementsHasImplementedByThatNeedsAnotherImplementedBy.class)
+ static interface HasImplementedByThatNeedsAnotherImplementedBy {
+ }
+
+ static class ImplementsHasImplementedByThatNeedsAnotherImplementedBy
+ implements HasImplementedByThatNeedsAnotherImplementedBy {
+ @Inject
+ ImplementsHasImplementedByThatNeedsAnotherImplementedBy(
+ HasImplementedBy1 h1n1) {}
+ }
+
+ @ImplementedBy(ImplementsHasImplementedByThatWantsExplicit.class)
+ static interface HasImplementedByThatWantsExplicit {
+ }
+
+ static class ImplementsHasImplementedByThatWantsExplicit
+ implements HasImplementedByThatWantsExplicit {
+ @Inject ImplementsHasImplementedByThatWantsExplicit(JustAnInterface jai) {}
+ }
+
+ static interface JustAnInterface {}
// public void testBindInterfaceWithoutImplementation() {
