Add Binder.requireAtInjectOnConstructors, to force Guice to require @Inject annotations on constructors.
Revision created by MOE tool push_codebase.
MOE_MIGRATION=4906
diff --git a/core/src/com/google/inject/Binder.java b/core/src/com/google/inject/Binder.java
index 8bf9a38..441b6ad 100644
--- a/core/src/com/google/inject/Binder.java
+++ b/core/src/com/google/inject/Binder.java
@@ -470,4 +470,17 @@
* @since 3.0
*/
void disableCircularProxies();
+
+ /**
+ * Requires that a {@literal @}{@link Inject} annotation exists on a constructor in order for
+ * Guice to consider it an eligible injectable class. By default, Guice will inject classes that
+ * have a no-args constructor if no {@literal @}{@link Inject} annotation exists on any
+ * constructor.
+ * <p>
+ * If the class is bound using {@link LinkedBindingBuilder#toConstructor}, Guice will still inject
+ * that constructor regardless of annotations.
+ *
+ * @since 4.0
+ */
+ void requireAtInjectOnConstructors();
}
diff --git a/core/src/com/google/inject/internal/BindingProcessor.java b/core/src/com/google/inject/internal/BindingProcessor.java
index 713f98d..a87e326 100644
--- a/core/src/com/google/inject/internal/BindingProcessor.java
+++ b/core/src/com/google/inject/internal/BindingProcessor.java
@@ -72,7 +72,7 @@
prepareBinding();
try {
ConstructorBindingImpl<T> onInjector = ConstructorBindingImpl.create(injector, key,
- binding.getConstructor(), source, scoping, errors, false);
+ binding.getConstructor(), source, scoping, errors, false, false);
scheduleInitialization(onInjector);
putBinding(onInjector);
} catch (ErrorsException e) {
diff --git a/core/src/com/google/inject/internal/ConstructorBindingImpl.java b/core/src/com/google/inject/internal/ConstructorBindingImpl.java
index 0bbce3f..f8fbfd2 100644
--- a/core/src/com/google/inject/internal/ConstructorBindingImpl.java
+++ b/core/src/com/google/inject/internal/ConstructorBindingImpl.java
@@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.inject.Binder;
import com.google.inject.ConfigurationException;
+import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.TypeLiteral;
import com.google.inject.internal.util.Classes;
@@ -71,7 +72,7 @@
*/
static <T> ConstructorBindingImpl<T> create(InjectorImpl injector, Key<T> key,
InjectionPoint constructorInjector, Object source, Scoping scoping, Errors errors,
- boolean failIfNotLinked)
+ boolean failIfNotLinked, boolean failIfNotExplicit)
throws ErrorsException {
int numErrors = errors.size();
@@ -96,6 +97,9 @@
if (constructorInjector == null) {
try {
constructorInjector = InjectionPoint.forConstructorOf(key.getTypeLiteral());
+ if (failIfNotExplicit && !hasAtInject((Constructor) constructorInjector.getMember())) {
+ errors.atInjectRequired(rawType);
+ }
} catch (ConfigurationException e) {
throw errors.merge(e.getErrorMessages()).toException();
}
@@ -120,6 +124,12 @@
return new ConstructorBindingImpl<T>(
injector, key, source, scopedFactory, scoping, factoryFactory, constructorInjector);
}
+
+ /** Returns true if the inject annotation is on the constructor. */
+ private static boolean hasAtInject(Constructor cxtor) {
+ return cxtor.isAnnotationPresent(Inject.class)
+ || cxtor.isAnnotationPresent(javax.inject.Inject.class);
+ }
@SuppressWarnings("unchecked") // the result type always agrees with the ConstructorInjector type
public void initialize(InjectorImpl injector, Errors errors) throws ErrorsException {
diff --git a/core/src/com/google/inject/internal/Errors.java b/core/src/com/google/inject/internal/Errors.java
index 4eddfe8..13c5cee 100644
--- a/core/src/com/google/inject/internal/Errors.java
+++ b/core/src/com/google/inject/internal/Errors.java
@@ -139,6 +139,13 @@
return addMessage("Explicit bindings are required and %s is not explicitly bound.", key);
}
+ public Errors atInjectRequired(Class clazz) {
+ return addMessage(
+ "Explicit @Inject annotations are required on constructors,"
+ + " but %s has no constructors annotated with @Inject.",
+ clazz);
+ }
+
public Errors converterReturnedNull(String stringValue, Object source,
TypeLiteral<?> type, TypeConverterBinding typeConverterBinding) {
return addMessage("Received null converting '%s' (bound at %s) to %s%n"
@@ -647,17 +654,17 @@
private static final Collection<Converter<?>> converters = ImmutableList.of(
new Converter<Class>(Class.class) {
- public String toString(Class c) {
+ @Override public String toString(Class c) {
return c.getName();
}
},
new Converter<Member>(Member.class) {
- public String toString(Member member) {
+ @Override public String toString(Member member) {
return Classes.toString(member);
}
},
new Converter<Key>(Key.class) {
- public String toString(Key key) {
+ @Override public String toString(Key key) {
if (key.getAnnotationType() != null) {
return key.getTypeLiteral() + " annotated with "
+ (key.getAnnotation() != null ? key.getAnnotation() : key.getAnnotationType());
diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java
index 49fd265..ee5e2d6 100644
--- a/core/src/com/google/inject/internal/InjectorImpl.java
+++ b/core/src/com/google/inject/internal/InjectorImpl.java
@@ -71,11 +71,14 @@
final Stage stage;
final boolean jitDisabled;
final boolean disableCircularProxies;
+ final boolean atInjectRequired;
- InjectorOptions(Stage stage, boolean jitDisabled, boolean disableCircularProxies) {
+ InjectorOptions(Stage stage, boolean jitDisabled, boolean disableCircularProxies,
+ boolean atInjectRequired) {
this.stage = stage;
this.jitDisabled = jitDisabled;
this.disableCircularProxies = disableCircularProxies;
+ this.atInjectRequired = atInjectRequired;
}
@Override
@@ -84,6 +87,7 @@
.add("stage", stage)
.add("jitDisabled", jitDisabled)
.add("disableCircularProxies", disableCircularProxies)
+ .add("atInjectRequired", atInjectRequired)
.toString();
}
}
@@ -179,7 +183,7 @@
if(isProvider(key)) {
try {
// This is safe because isProvider above ensures that T is a Provider<?>
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"unchecked", "cast"})
Key<?> providedKey = (Key<?>)getProvidedKey((Key)key, new Errors());
if(getExistingBinding(providedKey) != null) {
return getBinding(key);
@@ -651,7 +655,14 @@
}
- return ConstructorBindingImpl.create(this, key, null, source, scoping, errors, jitBinding && options.jitDisabled);
+ return ConstructorBindingImpl.create(this,
+ key,
+ null, /* use default constructor */
+ source,
+ scoping,
+ errors,
+ jitBinding && options.jitDisabled,
+ options.atInjectRequired);
}
/**
@@ -813,7 +824,7 @@
if (isProvider(key)) {
// These casts are safe. We know T extends Provider<X> and that given Key<Provider<X>>,
// createProviderBinding() will return BindingImpl<Provider<X>>.
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"unchecked", "cast"})
BindingImpl<T> binding = (BindingImpl<T>) createProviderBinding((Key) key, errors);
return binding;
}
@@ -822,7 +833,7 @@
if (isMembersInjector(key)) {
// These casts are safe. T extends MembersInjector<X> and that given Key<MembersInjector<X>>,
// createMembersInjectorBinding() will return BindingImpl<MembersInjector<X>>.
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"unchecked", "cast"})
BindingImpl<T> binding = (BindingImpl<T>) createMembersInjectorBinding((Key) key, errors);
return binding;
}
diff --git a/core/src/com/google/inject/internal/InjectorOptionsProcessor.java b/core/src/com/google/inject/internal/InjectorOptionsProcessor.java
index 4434dce..b21b696 100644
--- a/core/src/com/google/inject/internal/InjectorOptionsProcessor.java
+++ b/core/src/com/google/inject/internal/InjectorOptionsProcessor.java
@@ -22,6 +22,7 @@
import com.google.inject.Stage;
import com.google.inject.internal.InjectorImpl.InjectorOptions;
import com.google.inject.spi.DisableCircularProxiesOption;
+import com.google.inject.spi.RequireAtInjectOnConstructorsOption;
import com.google.inject.spi.RequireExplicitBindingsOption;
/**
@@ -33,6 +34,7 @@
private boolean disableCircularProxies = false;
private boolean jitDisabled = false;
+ private boolean atInjectRequired = false;
InjectorOptionsProcessor(Errors errors) {
super(errors);
@@ -49,6 +51,12 @@
jitDisabled = true;
return true;
}
+
+ @Override
+ public Boolean visit(RequireAtInjectOnConstructorsOption option) {
+ atInjectRequired = true;
+ return true;
+ }
InjectorOptions getOptions(Stage stage, InjectorOptions parentOptions) {
checkNotNull(stage, "stage must be set");
@@ -56,13 +64,15 @@
return new InjectorOptions(
stage,
jitDisabled,
- disableCircularProxies);
+ disableCircularProxies,
+ atInjectRequired);
} else {
checkState(stage == parentOptions.stage, "child & parent stage don't match");
return new InjectorOptions(
stage,
jitDisabled || parentOptions.jitDisabled,
- disableCircularProxies || parentOptions.disableCircularProxies);
+ disableCircularProxies || parentOptions.disableCircularProxies,
+ atInjectRequired || parentOptions.atInjectRequired);
}
}
diff --git a/core/src/com/google/inject/spi/DefaultElementVisitor.java b/core/src/com/google/inject/spi/DefaultElementVisitor.java
index cfceea7..573df35 100644
--- a/core/src/com/google/inject/spi/DefaultElementVisitor.java
+++ b/core/src/com/google/inject/spi/DefaultElementVisitor.java
@@ -94,4 +94,8 @@
public V visit(RequireExplicitBindingsOption option) {
return visitOther(option);
}
+
+ public V visit(RequireAtInjectOnConstructorsOption option) {
+ return visitOther(option);
+ }
}
diff --git a/core/src/com/google/inject/spi/ElementVisitor.java b/core/src/com/google/inject/spi/ElementVisitor.java
index 6ebc8cc..4dc65d7 100644
--- a/core/src/com/google/inject/spi/ElementVisitor.java
+++ b/core/src/com/google/inject/spi/ElementVisitor.java
@@ -17,6 +17,7 @@
package com.google.inject.spi;
import com.google.inject.Binding;
+import com.google.inject.Inject;
/**
* Visit elements.
@@ -105,4 +106,11 @@
* @since 3.0
*/
V visit(DisableCircularProxiesOption option);
+
+ /**
+ * Visit a require explicit {@literal @}{@link Inject} command.
+ *
+ * @since 4.0
+ */
+ V visit(RequireAtInjectOnConstructorsOption option);
}
diff --git a/core/src/com/google/inject/spi/Elements.java b/core/src/com/google/inject/spi/Elements.java
index e2737a1..553d65c 100644
--- a/core/src/com/google/inject/spi/Elements.java
+++ b/core/src/com/google/inject/spi/Elements.java
@@ -315,6 +315,10 @@
public void requireExplicitBindings() {
elements.add(new RequireExplicitBindingsOption(getSource()));
}
+
+ public void requireAtInjectOnConstructors() {
+ elements.add(new RequireAtInjectOnConstructorsOption(getSource()));
+ }
public void expose(Key<?> key) {
exposeInternal(key);
diff --git a/core/src/com/google/inject/spi/InjectionPoint.java b/core/src/com/google/inject/spi/InjectionPoint.java
index 891c42d..76197e3 100644
--- a/core/src/com/google/inject/spi/InjectionPoint.java
+++ b/core/src/com/google/inject/spi/InjectionPoint.java
@@ -444,6 +444,7 @@
this.field = field;
}
+ @Override
InjectionPoint toInjectionPoint() {
return new InjectionPoint(declaringType, field, optional);
}
@@ -463,6 +464,7 @@
this.method = method;
}
+ @Override
InjectionPoint toInjectionPoint() {
return new InjectionPoint(declaringType, method, optional);
}
diff --git a/core/src/com/google/inject/spi/RequireAtInjectOnConstructorsOption.java b/core/src/com/google/inject/spi/RequireAtInjectOnConstructorsOption.java
new file mode 100644
index 0000000..03d8c34
--- /dev/null
+++ b/core/src/com/google/inject/spi/RequireAtInjectOnConstructorsOption.java
@@ -0,0 +1,48 @@
+/**
+ * Copyright (C) 2012 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.inject.spi;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.inject.Binder;
+import com.google.inject.Inject;
+
+/**
+ * A request to require explicit {@literal @}{@link Inject} annotations on constructors.
+ *
+ * @author sameb@google.com (Sam Berlin)
+ * @since 4.0
+ */
+public final class RequireAtInjectOnConstructorsOption implements Element {
+ private final Object source;
+
+ RequireAtInjectOnConstructorsOption(Object source) {
+ this.source = checkNotNull(source, "source");
+ }
+
+ public Object getSource() {
+ return source;
+ }
+
+ public void applyTo(Binder binder) {
+ binder.withSource(getSource()).requireAtInjectOnConstructors();
+ }
+
+ public <T> T acceptVisitor(ElementVisitor<T> visitor) {
+ return visitor.visit(this);
+ }
+}
diff --git a/core/test/com/google/inject/AllTests.java b/core/test/com/google/inject/AllTests.java
index be5b875..d6b59f3 100644
--- a/core/test/com/google/inject/AllTests.java
+++ b/core/test/com/google/inject/AllTests.java
@@ -36,6 +36,7 @@
import com.google.inject.util.NoopOverrideTest;
import com.google.inject.util.ProvidersTest;
import com.google.inject.util.TypesTest;
+
import com.googlecode.guice.GuiceTck;
import com.googlecode.guice.Jsr330Test;
@@ -92,6 +93,7 @@
// ProxyFactoryTest is AOP-only
suite.addTestSuite(ReflectionTest.class);
suite.addTestSuite(RequestInjectionTest.class);
+ suite.addTestSuite(RequireAtInjectOnConstructorsTest.class);
suite.addTestSuite(ScopesTest.class);
suite.addTestSuite(SerializationTest.class);
suite.addTestSuite(SuperclassTest.class);
diff --git a/core/test/com/google/inject/RequireAtInjectOnConstructorsTest.java b/core/test/com/google/inject/RequireAtInjectOnConstructorsTest.java
new file mode 100644
index 0000000..e5f90c2
--- /dev/null
+++ b/core/test/com/google/inject/RequireAtInjectOnConstructorsTest.java
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2012 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+package com.google.inject;
+
+import junit.framework.TestCase;
+
+/**
+ * Tests for {@link Binder#requireAtInjectOnConstructors()}
+ *
+ * @author sameb@google.com (Sam Berlin)
+ */
+public class RequireAtInjectOnConstructorsTest extends TestCase {
+
+ public void testNoCxtors_explicitBinding() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(NoCxtors.class);
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ assertEquals(1, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testNoCxtors_jitBinding() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ try {
+ injector.getInstance(NoCxtors.class);
+ fail();
+ } catch (ConfigurationException ce) {
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "while locating " + NoCxtors.class.getName());
+ }
+ }
+
+ public void testNoCxtors_implicitBinding() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(Interface.class).to(NoCxtors.class);
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ assertEquals(1, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testNoCxtors_inheritedByPrivateModules() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ binder().requireAtInjectOnConstructors();
+ install(new PrivateModule() {
+ @Override
+ protected void configure() {
+ bind(NoCxtors.class);
+ }
+ });
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ assertEquals(1, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testNoCxtors_accumulatesAllErrors() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(NoCxtors.class);
+ bind(AnotherNoCxtors.class);
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ assertEquals(2, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure",
+ "2) Explicit @Inject annotations are required on constructors, but "
+ + AnotherNoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testNoCxtors_separateOptionsForPrivateModules() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(AnotherNoCxtors.class);
+ install(new PrivateModule() {
+ @Override
+ protected void configure() {
+ binder().requireAtInjectOnConstructors();
+ bind(NoCxtors.class);
+ }
+ });
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ // This is testing that the parent module doesn't fail because it isn't included
+ // in the error message.
+ assertEquals(1, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + NoCxtors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testManyConstructorsButNoneWithAtInject() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ManyConstructors.class);
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ fail();
+ } catch (CreationException ce) {
+ assertEquals(1, ce.getErrorMessages().size());
+ Asserts.assertContains(ce.getMessage(),
+ "1) Explicit @Inject annotations are required on constructors, but "
+ + ManyConstructors.class.getName() + " has no constructors annotated with @Inject",
+ "at " + RequireAtInjectOnConstructorsTest.class.getName() + "$", "configure");
+ }
+ }
+
+ public void testRequireAtInjectStillAllowsToConstructorBindings() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ try {
+ bind(ManyConstructors.class)
+ .toConstructor(ManyConstructors.class.getDeclaredConstructor());
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ binder().requireAtInjectOnConstructors();
+ }
+ });
+ injector.getInstance(ManyConstructors.class);
+ }
+
+ private static interface Interface {}
+ private static class NoCxtors implements Interface {}
+ private static class AnotherNoCxtors {}
+ private static class ManyConstructors {
+ @SuppressWarnings("unused") ManyConstructors() {}
+ @SuppressWarnings("unused") ManyConstructors(String a) {}
+ @SuppressWarnings("unused") ManyConstructors(int a) {}
+ }
+}
