Large internal change:
I've replaced MissingDependencyException with ResolveFailedException, and increased the number of places where it's used. We used to have a lot of code that looked like this:
if (someBadCondition) {
addError(whatBadThingHappenedMessage);
return invalidBinding();
}
Now we do something simpler - we just throw a ResolveFailedException:
if (someBadCondition) {
throw new ResolveFailedException(whatBadThingHappenedMessage);
}
The motivation was to fix some seemingly-unrelated logic: optional bindings to providers weren't working. The problem was that the code that was calling 'addError' didn't know that it was an optional binding, and therefore not an error. This change is pretty much the only thing that could fix that problem, so I think it's worthwhile.
As an added treat, the change also fixed 2 of our 3 known test failures:
1. messaging for injecting abstract class used to be lame. Now it's good.
2. we never used to complain when leaving a dangling binding, like so:
bind(Collection.class).to(List.class);
Now we get an error when doing the list binding, and this bubbles all the way up as it should.
Since the change is big I suspect there might be some bug fallout. I'm going to run it against my standard test case (a giant application that uses Guice) to see if there's any regressions. Anything that has regressed will get its own testcase in the coming week or so...
git-svn-id: https://google-guice.googlecode.com/svn/trunk@448 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/BindCommandProcessor.java b/src/com/google/inject/BindCommandProcessor.java
index dd1c351..d03bacf 100644
--- a/src/com/google/inject/BindCommandProcessor.java
+++ b/src/com/google/inject/BindCommandProcessor.java
@@ -20,15 +20,11 @@
import com.google.inject.commands.BindConstantCommand;
import com.google.inject.commands.BindScoping;
import com.google.inject.commands.BindTarget;
-import com.google.inject.internal.Annotations;
-import com.google.inject.internal.Objects;
-import com.google.inject.internal.StackTraceElements;
-import com.google.inject.internal.ErrorMessages;
+import com.google.inject.internal.*;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.*;
-import java.util.logging.Logger;
/**
* Handles {@link Binder#bind} and {@link Binder#bindConstant} commands.
@@ -167,17 +163,13 @@
@SuppressWarnings("unchecked")
Class<T> clazz = (Class<T>) type;
- BindingImpl<T> binding = injector.createBindingFromType(clazz, scope, source);
- // TODO: Should we clean up the binding left behind in jitBindings?
-
- if (binding == null) {
- injector.errorHandler.handle(
- source, ErrorMessages.CANNOT_INJECT_ABSTRACT_TYPE, clazz);
+ try {
+ BindingImpl<T> binding = injector.createBindingFromType(clazz, scope, source);
+ createBinding(source, shouldPreload, binding);
+ } catch (ResolveFailedException e) {
+ injector.errorHandler.handle(source, e.getMessage());
createBinding(source, shouldPreload, invalidBinding(injector, key, source));
- return;
}
-
- createBinding(source, shouldPreload, binding);
}
});
diff --git a/src/com/google/inject/BoundProviderFactory.java b/src/com/google/inject/BoundProviderFactory.java
index 65136d3..f39e8ae 100644
--- a/src/com/google/inject/BoundProviderFactory.java
+++ b/src/com/google/inject/BoundProviderFactory.java
@@ -18,6 +18,7 @@
import com.google.inject.BindCommandProcessor.CreationListener;
import com.google.inject.internal.ErrorMessages;
+import com.google.inject.internal.ResolveFailedException;
/**
* Delegates to a custom factory which is also bound in the injector.
@@ -48,7 +49,11 @@
public void notify(final InjectorImpl injector) {
injector.withDefaultSource(source, new Runnable() {
public void run() {
- providerFactory = injector.getInternalFactory(providerKey);
+ try {
+ providerFactory = injector.getInternalFactory(providerKey);
+ } catch (ResolveFailedException e) {
+ injector.errorHandler.handle(source, e.getMessage());
+ }
}
});
}
diff --git a/src/com/google/inject/ClassBindingImpl.java b/src/com/google/inject/ClassBindingImpl.java
index edf776f..9eb2756 100644
--- a/src/com/google/inject/ClassBindingImpl.java
+++ b/src/com/google/inject/ClassBindingImpl.java
@@ -16,11 +16,12 @@
package com.google.inject;
-import com.google.inject.spi.ClassBinding;
-import com.google.inject.spi.BindingVisitor;
-import com.google.inject.spi.Dependency;
-import com.google.inject.internal.ToStringBuilder;
import com.google.inject.InjectorImpl.SingleParameterInjector;
+import com.google.inject.internal.ToStringBuilder;
+import com.google.inject.spi.BindingVisitor;
+import com.google.inject.spi.ClassBinding;
+import com.google.inject.spi.Dependency;
+
import java.util.Collection;
/**
@@ -30,9 +31,13 @@
class ClassBindingImpl<T> extends BindingImpl<T>
implements ClassBinding<T> {
+ private final InjectorImpl.LateBoundConstructor<T> lateBoundConstructor;
+
ClassBindingImpl(InjectorImpl injector, Key<T> key, Object source,
- InternalFactory<? extends T> internalFactory, Scope scope) {
+ InternalFactory<? extends T> internalFactory, Scope scope,
+ InjectorImpl.LateBoundConstructor<T> lateBoundConstructor) {
super(injector, key, source, internalFactory, scope);
+ this.lateBoundConstructor = lateBoundConstructor;
}
public void accept(BindingVisitor<? super T> visitor) {
@@ -46,10 +51,14 @@
}
public Collection<Dependency<?>> getDependencies() {
+ if (lateBoundConstructor == null) {
+ throw new AssertionError();
+ }
+
Class<T> boundClass = getBoundClass();
Collection<Dependency<?>> injectors
= injector.getModifiableFieldAndMethodDependenciesFor(boundClass);
- ConstructorInjector<T> constructor = injector.getConstructor(boundClass);
+ ConstructorInjector<T> constructor = lateBoundConstructor.constructorInjector;
if (constructor.parameterInjectors != null) {
for (SingleParameterInjector<?> parameterInjector
: constructor.parameterInjectors) {
diff --git a/src/com/google/inject/ConstructorInjector.java b/src/com/google/inject/ConstructorInjector.java
index 3e68a69..94ce9dd 100644
--- a/src/com/google/inject/ConstructorInjector.java
+++ b/src/com/google/inject/ConstructorInjector.java
@@ -18,6 +18,7 @@
import com.google.inject.internal.ConstructionProxy;
import com.google.inject.internal.ErrorMessages;
+import com.google.inject.internal.ResolveFailedException;
import java.lang.reflect.InvocationTargetException;
@@ -60,8 +61,8 @@
constructionProxy.getMember(),
constructionProxy.getParameters());
}
- catch (InjectorImpl.MissingDependencyException e) {
- e.handle(injector.errorHandler);
+ catch (ResolveFailedException e) {
+ injector.errorHandler.handle(constructionProxy.getMember(), e.getMessage());
return null;
}
}
diff --git a/src/com/google/inject/DefaultErrorHandler.java b/src/com/google/inject/DefaultErrorHandler.java
index 52f5195..36b74dd 100644
--- a/src/com/google/inject/DefaultErrorHandler.java
+++ b/src/com/google/inject/DefaultErrorHandler.java
@@ -37,6 +37,8 @@
final Collection<Message> errorMessages = new ArrayList<Message>();
public void handle(Object source, String message) {
+ source = ErrorMessages.convert(source);
+
if (state == State.RUNTIME) {
throw new ConfigurationException("Error at " + source + " " + message);
@@ -52,10 +54,7 @@
* Implements formatting. Converts known types to readable strings.
*/
public final void handle(Object source, String message, Object... arguments) {
- for (int i = 0; i < arguments.length; i++) {
- arguments[i] = ErrorMessages.convert(arguments[i]);
- }
- handle(source, String.format(message, arguments));
+ handle(source, ErrorMessages.format(message, arguments));
}
void blowUpIfErrorsExist() {
diff --git a/src/com/google/inject/FactoryProxy.java b/src/com/google/inject/FactoryProxy.java
index f6767b1..db60412 100644
--- a/src/com/google/inject/FactoryProxy.java
+++ b/src/com/google/inject/FactoryProxy.java
@@ -17,6 +17,7 @@
package com.google.inject;
+import com.google.inject.internal.ResolveFailedException;
import com.google.inject.internal.ToStringBuilder;
/**
@@ -41,7 +42,11 @@
public void notify(final InjectorImpl injector) {
injector.withDefaultSource(source, new Runnable() {
public void run() {
- targetFactory = injector.getInternalFactory(targetKey);
+ try {
+ targetFactory = injector.getInternalFactory(targetKey);
+ } catch (ResolveFailedException e) {
+ injector.errorHandler.handle(source, e.getMessage());
+ }
}
});
}
diff --git a/src/com/google/inject/InjectorImpl.java b/src/com/google/inject/InjectorImpl.java
index 11bc35b..c2b2822 100644
--- a/src/com/google/inject/InjectorImpl.java
+++ b/src/com/google/inject/InjectorImpl.java
@@ -17,26 +17,15 @@
package com.google.inject;
import com.google.inject.internal.*;
-import com.google.inject.spi.BindingVisitor;
-import com.google.inject.spi.ConvertedConstantBinding;
-import com.google.inject.spi.Dependency;
-import com.google.inject.spi.ProviderBinding;
-import com.google.inject.spi.SourceProviders;
+import com.google.inject.spi.*;
import com.google.inject.util.Providers;
-import java.lang.annotation.Annotation;
-import java.lang.reflect.AnnotatedElement;
-import java.lang.reflect.Field;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Member;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
-import java.util.*;
-import java.util.concurrent.Callable;
import net.sf.cglib.reflect.FastClass;
import net.sf.cglib.reflect.FastMethod;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.*;
+import java.util.*;
+
/**
* Default {@link Injector} implementation.
*
@@ -167,13 +156,24 @@
}
/**
+ * Returns the binding for {@code key}, or {@code null} if that binding
+ * cannot be resolved.
+ */
+ public <T> BindingImpl<T> getBinding(Key<T> key) {
+ try {
+ return getBindingOrThrow(key);
+ } catch(ResolveFailedException e) {
+ return null;
+ }
+ }
+
+ /**
* Gets a binding implementation. First, it check to see if the parent has
* a binding. If the parent has a binding and the binding is scoped, it
* will use that binding. Otherwise, this checks for an explicit binding.
* If no explicit binding is found, it looks for a just-in-time binding.
*/
-
- public <T> BindingImpl<T> getBinding(Key<T> key) {
+ public <T> BindingImpl<T> getBindingOrThrow(Key<T> key) throws ResolveFailedException {
if (parentInjector != null) {
BindingImpl<T> bindingImpl = getParentBinding(key);
if (bindingImpl != null) {
@@ -191,6 +191,7 @@
return getJitBindingImpl(key);
}
+
/**
* Checks the parent injector for a scoped binding, and if available, creates
* an appropriate binding local to this injector and remembers it.
@@ -247,7 +248,7 @@
* Provider<X> binding, etc.
*/
@SuppressWarnings("unchecked")
- <T> BindingImpl<T> getJitBindingImpl(Key<T> key) {
+ <T> BindingImpl<T> getJitBindingImpl(Key<T> key) throws ResolveFailedException {
synchronized (jitBindings) {
// Support null values.
if (!jitBindings.containsKey(key)) {
@@ -277,12 +278,12 @@
* from Binding<T>.
*/
private <T> BindingImpl<Provider<T>> createProviderBinding(
- Key<Provider<T>> key) {
+ Key<Provider<T>> key) throws ResolveFailedException {
Type providerType = key.getTypeLiteral().getType();
// If the Provider has no type parameter (raw Provider)...
if (!(providerType instanceof ParameterizedType)) {
- return null;
+ throw new ResolveFailedException(ErrorMessages.CANNOT_INJECT_RAW_PROVIDER);
}
Type entryType
@@ -292,15 +293,7 @@
@SuppressWarnings("unchecked")
Key<T> providedKey = (Key<T>) key.ofType(entryType);
- BindingImpl<T> providedBinding = getBinding(providedKey);
-
- // If binding isn't found...
- if (providedBinding == null) {
- handleMissingBinding(SourceProviders.defaultSource(), key);
- return invalidBinding(key);
- }
-
- return new ProviderBindingImpl<T>(this, key, providedBinding);
+ return new ProviderBindingImpl<T>(this, key, getBindingOrThrow(providedKey));
}
void handleMissingBinding(Object source, Key<?> key) {
@@ -389,7 +382,8 @@
* <p>If the type is not elligible for conversion or a constant string
* binding is not found, this method returns null.
*/
- private <T> BindingImpl<T> convertConstantStringBinding(Key<T> key) {
+ private <T> BindingImpl<T> convertConstantStringBinding(Key<T> key)
+ throws ResolveFailedException {
// Find a constant string binding.
Key<String> stringKey = key.ofType(String.class);
BindingImpl<String> stringBinding = getExplicitBindingImpl(stringKey);
@@ -406,11 +400,8 @@
for (MatcherAndConverter<?> converter : converters) {
if (converter.getTypeMatcher().matches(type)) {
if (matchingConverter != null) {
- // More than one matching converter!
- errorHandler.handle(SourceProviders.defaultSource(),
- ErrorMessages.AMBIGUOUS_TYPE_CONVERSION, stringValue, type,
- matchingConverter, converter);
- return invalidBinding(key);
+ throw new ResolveFailedException(ErrorMessages.AMBIGUOUS_TYPE_CONVERSION,
+ stringValue, type, matchingConverter, converter);
}
matchingConverter = converter;
@@ -430,7 +421,7 @@
.convert(stringValue, key.getTypeLiteral());
if (converted == null) {
- throw new RuntimeException("Converter returned null.");
+ throw new ResolveFailedException(ErrorMessages.CONVERTER_RETURNED_NULL);
}
// We have to filter out primitive types because an Integer is not an
@@ -438,18 +429,16 @@
// and know that they work anyway.
if (!type.rawType.isPrimitive()
&& !type.getRawType().isInstance(converted)) {
- throw new RuntimeException("Converter returned " + converted
- + " but we expected a[n] " + type + ".");
+ throw new ResolveFailedException(ErrorMessages.CONVERSION_TYPE_ERROR, converted, type);
}
return new ConvertedConstantBindingImpl<T>(
this, key, converted, stringBinding);
+ } catch (ResolveFailedException e) {
+ throw e;
} catch (Exception e) {
- // Conversion error.
- errorHandler.handle(SourceProviders.defaultSource(),
- ErrorMessages.CONVERSION_ERROR, stringValue,
+ throw new ResolveFailedException(ErrorMessages.CONVERSION_ERROR, stringValue,
stringBinding.getSource(), type, matchingConverter, e.getMessage());
- return invalidBinding(key);
}
}
@@ -496,15 +485,16 @@
}
}
- <T> BindingImpl<T> createBindingFromType(Class<T> type) {
+ <T> BindingImpl<T> createBindingFromType(Class<T> type)
+ throws ResolveFailedException {
return createBindingFromType(type, null, SourceProviders.defaultSource());
}
<T> BindingImpl<T> createBindingFromType(Class<T> type, Scope scope,
- Object source) {
+ Object source) throws ResolveFailedException {
// Don't try to inject primitives, arrays, or enums.
if (type.isArray() || type.isEnum() || type.isPrimitive()) {
- return null;
+ throw new ResolveFailedException(ErrorMessages.MISSING_BINDING, type);
}
// Handle @ImplementedBy
@@ -529,19 +519,18 @@
* a scope on the type if none is specified.
*/
<T> BindingImpl<T> createBindingForInjectableType(Class<T> type,
- Scope scope, Object source) {
+ Scope scope, Object source) throws ResolveFailedException {
+
// We can't inject abstract classes.
// TODO: Method interceptors could actually enable us to implement
// abstract types. Should we remove this restriction?
if (Modifier.isAbstract(type.getModifiers())) {
- return null;
+ throw new ResolveFailedException(ErrorMessages.CANNOT_INJECT_ABSTRACT_TYPE, type);
}
// Error: Inner class.
if (Classes.isInnerClass(type)) {
- errorHandler.handle(SourceProviders.defaultSource(),
- ErrorMessages.CANNOT_INJECT_INNER_CLASS, type);
- return invalidBinding(Key.get(type));
+ throw new ResolveFailedException(ErrorMessages.CANNOT_INJECT_INNER_CLASS, type);
}
if (scope == null) {
@@ -556,7 +545,7 @@
= Scopes.scope(key, this, lateBoundConstructor, scope);
BindingImpl<T> binding
- = new ClassBindingImpl<T>(this, key, source, scopedFactory, scope);
+ = new ClassBindingImpl<T>(this, key, source, scopedFactory, scope, lateBoundConstructor);
// Put the partially constructed binding in the map a little early. This
// enables us to handle circular dependencies.
@@ -564,29 +553,25 @@
// Note: We don't need to synchronize on jitBindings during injector
// creation.
jitBindings.put(key, binding);
-
+ boolean successful = false;
try {
lateBoundConstructor.bind(this, type);
- return binding;
+ successful = true;
+ } finally {
+ if (!successful) {
+ jitBindings.remove(key);
+ }
}
- catch (RuntimeException e) {
- // Clean up state.
- jitBindings.remove(key);
- throw e;
- }
- catch (Throwable t) {
- // Clean up state.
- jitBindings.remove(key);
- throw new AssertionError(t);
- }
+
+ return binding;
}
static class LateBoundConstructor<T> implements InternalFactory<T> {
ConstructorInjector<T> constructorInjector;
- void bind(
- InjectorImpl injector, Class<T> implementation) {
+ void bind(InjectorImpl injector, Class<T> implementation)
+ throws ResolveFailedException {
this.constructorInjector = injector.getConstructor(implementation);
}
@@ -601,17 +586,15 @@
}
/**
- * Creates a binding for a type annotated with @ImplementedBy.
+ * Creates a binding for a type annotated with @ProvidedBy.
*/
<T> BindingImpl<T> createProvidedByBinding(final Class<T> type,
- ProvidedBy providedBy) {
+ ProvidedBy providedBy) throws ResolveFailedException {
final Class<? extends Provider<?>> providerType = providedBy.value();
// Make sure it's not the same type. TODO: Can we check for deeper loops?
if (providerType == type) {
- errorHandler.handle(StackTraceElements.forType(type),
- ErrorMessages.RECURSIVE_PROVIDER_TYPE, type);
- return invalidBinding(type);
+ throw new ResolveFailedException(ErrorMessages.RECURSIVE_PROVIDER_TYPE);
}
// TODO: Make sure the provided type extends type. We at least check
@@ -623,13 +606,7 @@
Key<? extends Provider<T>> providerKey
= (Key<? extends Provider<T>>) Key.get(providerType);
final BindingImpl<? extends Provider<?>> providerBinding
- = getBinding(providerKey);
-
- if (providerBinding == null) {
- errorHandler.handle(StackTraceElements.forType(type),
- ErrorMessages.BINDING_NOT_FOUND, type);
- return invalidBinding(type);
- }
+ = getBindingOrThrow(providerKey);
InternalFactory<T> internalFactory = new InternalFactory<T>() {
public T get(InternalContext context, InjectionPoint injectionPoint) {
@@ -655,7 +632,7 @@
* Creates a binding for a type annotated with @ImplementedBy.
*/
<T> BindingImpl<T> createImplementedByBinding(Class<T> type,
- ImplementedBy implementedBy) {
+ ImplementedBy implementedBy) throws ResolveFailedException {
// TODO: Use scope annotation on type if present. Right now, we always
// use NO_SCOPE.
@@ -663,16 +640,12 @@
// Make sure it's not the same type. TODO: Can we check for deeper cycles?
if (implementationType == type) {
- errorHandler.handle(StackTraceElements.forType(type),
- ErrorMessages.RECURSIVE_IMPLEMENTATION_TYPE, type);
- return invalidBinding(type);
+ throw new ResolveFailedException(ErrorMessages.RECURSIVE_IMPLEMENTATION_TYPE);
}
// Make sure implementationType extends type.
if (!type.isAssignableFrom(implementationType)) {
- errorHandler.handle(StackTraceElements.forType(type),
- ErrorMessages.NOT_A_SUBTYPE, implementationType, type);
- return invalidBinding(type);
+ throw new ResolveFailedException(ErrorMessages.NOT_A_SUBTYPE, implementationType, type);
}
// After the preceding check, this cast is safe.
@@ -681,13 +654,7 @@
// Look up the target binding.
final BindingImpl<? extends T> targetBinding
- = getBinding(Key.get(subclass));
-
- if (targetBinding == null) {
- errorHandler.handle(StackTraceElements.forType(type),
- ErrorMessages.BINDING_NOT_FOUND, type);
- return invalidBinding(type);
- }
+ = getBindingOrThrow(Key.get(subclass));
InternalFactory<T> internalFactory = new InternalFactory<T>() {
public T get(InternalContext context, InjectionPoint<?> injectionPoint) {
@@ -700,7 +667,7 @@
Key.get(subclass));
}
- <T> BindingImpl<T> createBindingJustInTime(Key<T> key) {
+ <T> BindingImpl<T> createBindingJustInTime(Key<T> key) throws ResolveFailedException {
// Handle cases where T is a Provider<?>.
if (isProvider(key)) {
// These casts are safe. We know T extends Provider<X> and that given
@@ -727,8 +694,14 @@
// If the key has an annotation...
if (key.hasAnnotationType()) {
// Look for a binding without annotation attributes or return null.
- return key.hasAttributes()
- ? getBinding(key.withoutAttributes()) : null;
+ if (key.hasAttributes()) {
+ try {
+ return getBindingOrThrow(key.withoutAttributes());
+ } catch (ResolveFailedException ignored) {
+ // throw with a more appropriate message below
+ }
+ }
+ throw new ResolveFailedException(ErrorMessages.MISSING_BINDING, key);
}
// Create a binding based on the raw type.
@@ -737,9 +710,9 @@
return createBindingFromType(clazz);
}
- <T> InternalFactory<? extends T> getInternalFactory(Key<T> key) {
- BindingImpl<T> binding = getBinding(key);
- return binding == null ? null : binding.internalFactory;
+ <T> InternalFactory<? extends T> getInternalFactory(Key<T> key)
+ throws ResolveFailedException {
+ return getBindingOrThrow(key).internalFactory;
}
/**
@@ -777,7 +750,7 @@
addInjectorsForMembers(Arrays.asList(methods), statics, injectors,
new SingleInjectorFactory<Method>() {
public SingleMemberInjector create(InjectorImpl injector,
- Method method) throws MissingDependencyException {
+ Method method) throws ResolveFailedException {
return new SingleMethodInjector(injector, method);
}
});
@@ -788,7 +761,7 @@
addInjectorsForMembers(Arrays.asList(fields), statics, injectors,
new SingleInjectorFactory<Field>() {
public SingleMemberInjector create(InjectorImpl injector,
- Field field) throws MissingDependencyException {
+ Field field) throws ResolveFailedException {
return new SingleFieldInjector(injector, field);
}
});
@@ -804,10 +777,10 @@
try {
injectors.add(injectorFactory.create(this, member));
}
- catch (MissingDependencyException e) {
+ catch (ResolveFailedException e) {
if (!inject.optional()) {
// TODO: Report errors for more than one parameter per member.
- e.handle(errorHandler);
+ errorHandler.handle(member, e.getMessage());
}
}
}
@@ -826,7 +799,7 @@
interface SingleInjectorFactory<M extends Member & AnnotatedElement> {
SingleMemberInjector create(InjectorImpl injector, M member)
- throws MissingDependencyException;
+ throws ResolveFailedException;
}
private boolean isStatic(Member member) {
@@ -866,7 +839,7 @@
final InjectionPoint<?> injectionPoint;
public SingleFieldInjector(final InjectorImpl injector, Field field)
- throws MissingDependencyException {
+ throws ResolveFailedException {
this.field = field;
// Ewwwww...
@@ -874,17 +847,11 @@
final Key<?> key = Keys.get(
field.getGenericType(), field, field.getAnnotations(), errorHandler);
- factory = SourceProviders.withDefault(StackTraceElements.forMember(field),
- new Callable<InternalFactory<?>>() {
- public InternalFactory<?> call() throws Exception {
+ factory = new ResolvingCallable<InternalFactory<?>>() {
+ public InternalFactory<?> call() throws ResolveFailedException {
return injector.getInternalFactory(key);
}
- }
- );
-
- if (factory == null) {
- throw new MissingDependencyException(key, field);
- }
+ }.runWithDefaultSource(StackTraceElements.forMember(field));
this.injectionPoint = InjectionPoint.newInstance(field,
Nullability.forAnnotations(field.getAnnotations()), key, injector);
@@ -928,7 +895,7 @@
*/
SingleParameterInjector<?>[] getParametersInjectors(Member member,
List<Parameter<?>> parameters)
- throws MissingDependencyException {
+ throws ResolveFailedException {
SingleParameterInjector<?>[] parameterInjectors
= new SingleParameterInjector<?>[parameters.size()];
int index = 0;
@@ -941,19 +908,13 @@
<T> SingleParameterInjector<T> createParameterInjector(
final Parameter<T> parameter, Member member)
- throws MissingDependencyException {
- InternalFactory<? extends T> factory =
- SourceProviders.withDefault(StackTraceElements.forMember(member),
- new Callable<InternalFactory<? extends T>>() {
- public InternalFactory<? extends T> call() throws Exception {
- return getInternalFactory(parameter.getKey());
- }
- }
- );
-
- if (factory == null) {
- throw new MissingDependencyException(parameter.getKey(), member);
- }
+ throws ResolveFailedException {
+ InternalFactory<? extends T> factory
+ = new ResolvingCallable<InternalFactory<? extends T>>() {
+ public InternalFactory<? extends T> call() throws ResolveFailedException {
+ return getInternalFactory(parameter.getKey());
+ }
+ }.runWithDefaultSource(StackTraceElements.forMember(member));
InjectionPoint<T> injectionPoint = InjectionPoint.newInstance(
member, parameter.getIndex(), parameter.getNullability(), parameter.getKey(), this);
@@ -966,7 +927,7 @@
final SingleParameterInjector<?>[] parameterInjectors;
public SingleMethodInjector(InjectorImpl injector, final Method method)
- throws MissingDependencyException {
+ throws ResolveFailedException {
// We can't use FastMethod if the method is private.
if (Modifier.isPrivate(method.getModifiers())
|| Modifier.isProtected(method.getModifiers())) {
@@ -1031,19 +992,16 @@
IllegalAccessException, InvocationTargetException;
}
- final Map<Class<?>, ConstructorInjector> constructors
- = new ReferenceCache<Class<?>, ConstructorInjector>() {
+ final Map<Class<?>, Object> constructors
+ = new ReferenceCache<Class<?>, Object>() {
@SuppressWarnings("unchecked")
- protected ConstructorInjector<?> create(Class<?> implementation) {
+ protected Object create(Class<?> implementation) {
if (!Classes.isConcrete(implementation)) {
- errorHandler.handle(SourceProviders.defaultSource(),
+ return new ResolveFailedException(
ErrorMessages.CANNOT_INJECT_ABSTRACT_TYPE, implementation);
- return ConstructorInjector.invalidConstructor();
}
if (Classes.isInnerClass(implementation)) {
- errorHandler.handle(SourceProviders.defaultSource(),
- ErrorMessages.CANNOT_INJECT_INNER_CLASS, implementation);
- return ConstructorInjector.invalidConstructor();
+ return new ResolveFailedException(ErrorMessages.CANNOT_INJECT_INNER_CLASS, implementation);
}
return new ConstructorInjector(InjectorImpl.this, implementation);
@@ -1125,13 +1083,9 @@
return getProvider(Key.get(type));
}
- <T> Provider<T> maybeGetProvider(final Key<T> key) {
+ <T> Provider<T> getProviderOrThrow(final Key<T> key) throws ResolveFailedException {
final InternalFactory<? extends T> factory = getInternalFactory(key);
- if (factory == null) {
- return null;
- }
-
return new Provider<T>() {
public T get() {
return callInContext(new ContextualCallable<T>() {
@@ -1160,14 +1114,12 @@
}
public <T> Provider<T> getProvider(final Key<T> key) {
- Provider<T> provider = maybeGetProvider(key);
-
- if (provider == null) {
+ try {
+ return getProviderOrThrow(key);
+ } catch (ResolveFailedException e) {
throw new ConfigurationException(
- "Missing binding to " + ErrorMessages.convert(key) + ".");
+ "Missing binding to " + ErrorMessages.convert(key) + ": " + e.getMessage());
}
-
- return provider;
}
public <T> T getInstance(Key<T> key) {
@@ -1211,8 +1163,16 @@
* Gets a constructor function for a given implementation class.
*/
@SuppressWarnings("unchecked")
- <T> ConstructorInjector<T> getConstructor(Class<T> implementation) {
- return constructors.get(implementation);
+ <T> ConstructorInjector<T> getConstructor(Class<T> implementation)
+ throws ResolveFailedException {
+ Object o = constructors.get(implementation);
+ if (o instanceof ResolveFailedException) {
+ throw (ResolveFailedException) o;
+ } else if (o instanceof ConstructorInjector<?>) {
+ return (ConstructorInjector<T>) o;
+ } else {
+ throw new AssertionError();
+ }
}
/**
@@ -1238,21 +1198,6 @@
getModifiableFieldAndMethodDependenciesFor(clazz));
}
- class MissingDependencyException extends Exception {
-
- final Key<?> key;
- final Member member;
-
- MissingDependencyException(Key<?> key, Member member) {
- this.key = key;
- this.member = member;
- }
-
- void handle(ErrorHandler errorHandler) {
- handleMissingBinding(member, key);
- }
- }
-
public String toString() {
return new ToStringBuilder(Injector.class)
.add("bindings", explicitBindings)
diff --git a/src/com/google/inject/internal/ErrorMessages.java b/src/com/google/inject/internal/ErrorMessages.java
index afbabd6..213f341 100644
--- a/src/com/google/inject/internal/ErrorMessages.java
+++ b/src/com/google/inject/internal/ErrorMessages.java
@@ -42,6 +42,13 @@
return o;
}
+ public static String format(String message, Object... arguments) {
+ for (int i = 0; i < arguments.length; i++) {
+ arguments[i] = ErrorMessages.convert(arguments[i]);
+ }
+ return String.format(message, arguments);
+ }
+
@SuppressWarnings("unchecked") // for generic array creation.
static Collection<Converter<?>> createConverters() {
return Arrays.asList(
@@ -112,6 +119,12 @@
"Binding to %s not found. Annotations on other"
+ " bindings to that type include: %s";
+ public static final String CONVERTER_RETURNED_NULL
+ = "Converter returned null.";
+
+ public static final String CONVERSION_TYPE_ERROR
+ = "Converter returned %s but we expected a[n] %s.";
+
public static final String CONVERSION_ERROR = "Error converting '%s'"
+ " (bound at %s) to %s using %s. Reason: %s";
@@ -132,10 +145,10 @@
public static final String NOT_A_SUBTYPE = "%s doesn't extend %s.";
- public static final String RECURSIVE_IMPLEMENTATION_TYPE = "@DefaultImplementation"
+ public static final String RECURSIVE_IMPLEMENTATION_TYPE = "@ImplementedBy"
+ " points to the same class it annotates.";
- public static final String RECURSIVE_PROVIDER_TYPE = "@DefaultProvider"
+ public static final String RECURSIVE_PROVIDER_TYPE = "@ProvidedBy"
+ " points to the same class it annotates.";
public static final String ERROR_INJECTING_MEMBERS_SEE_LOG = "An error of type %s"
@@ -237,6 +250,9 @@
public static final String CANNOT_INJECT_NULL_INTO_MEMBER =
"null returned by binding at %s%n but %s is not @Nullable";
+ public static final String CANNOT_INJECT_RAW_PROVIDER =
+ "Cannot inject a Provider that has no type parameter";
+
public static String getRootMessage(Throwable t) {
Throwable cause = t.getCause();
return cause == null
diff --git a/src/com/google/inject/internal/ResolveFailedException.java b/src/com/google/inject/internal/ResolveFailedException.java
new file mode 100644
index 0000000..8e7663c
--- /dev/null
+++ b/src/com/google/inject/internal/ResolveFailedException.java
@@ -0,0 +1,32 @@
+/**
+ * Copyright (C) 2008 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.internal;
+
+/**
+ * Indicates that resolving a binding failed. This is thrown when resolving a
+ * new binding, either at injector-creation time or when resolving a
+ * just-in-time binding.
+ *
+ * @author jessewilson@google.com (Jesse Wilson)
+ */
+public class ResolveFailedException extends Exception {
+
+ public ResolveFailedException(String message, Object... arguments) {
+ super(ErrorMessages.format(message, arguments));
+ }
+}
diff --git a/src/com/google/inject/internal/ResolvingCallable.java b/src/com/google/inject/internal/ResolvingCallable.java
new file mode 100644
index 0000000..bd86ce3
--- /dev/null
+++ b/src/com/google/inject/internal/ResolvingCallable.java
@@ -0,0 +1,44 @@
+/**
+ * Copyright (C) 2008 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.internal;
+
+import com.google.inject.spi.SourceProviders;
+
+import java.util.concurrent.Callable;
+
+/**
+ * An arbitrary body of code that throws a {@link ResolveFailedException}.
+ * Only necessary because it's difficult to throw specific types with Callable.
+ *
+ * @author jessewilson@google.com (Jesse Wilson)
+ */
+public abstract class ResolvingCallable<T> implements Callable<T> {
+ public abstract T call() throws ResolveFailedException;
+
+ public T runWithDefaultSource(Object source) throws ResolveFailedException {
+ try {
+ return SourceProviders.withDefaultChecked(source, this);
+ } catch (ResolveFailedException e) {
+ throw e;
+ } catch (RuntimeException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new AssertionError();
+ }
+ }
+}
diff --git a/src/com/google/inject/spi/SourceProviders.java b/src/com/google/inject/spi/SourceProviders.java
index 80fbe9e..85a0273 100644
--- a/src/com/google/inject/spi/SourceProviders.java
+++ b/src/com/google/inject/spi/SourceProviders.java
@@ -106,11 +106,7 @@
*/
public static void withDefault(
final Object source, Runnable r) {
- withDefault(new SourceProvider() {
- public Object source() {
- return source;
- }
- }, r);
+ withDefault(sourceProviderFor(source), r);
}
/**
@@ -138,17 +134,40 @@
}
/**
+ * Sets the default source provider, runs the given command, and then
+ * restores the previous default source provider. Unlike
+ * {@link #withDefault}, this method doesn't wrap exceptions.
+ */
+ public static <T> T withDefaultChecked(Object source, Callable<T> c)
+ throws Exception {
+ // We use a holder so we perform only 1 thread local access instead of 3.
+ SourceProvider[] holder = localSourceProvider.get();
+ SourceProvider previous = holder[0];
+ try {
+ holder[0] = sourceProviderFor(source);
+ return c.call();
+ } finally {
+ holder[0] = previous;
+ }
+ }
+
+ private static SourceProvider sourceProviderFor(final Object source) {
+ return new SourceProvider() {
+ public Object source() {
+ return source;
+ }
+ };
+ }
+
+
+ /**
* Sets the default source, runs the given command, and then
* restores the previous default source provider. Wraps checked exceptions
* with a RuntimeException.
*/
public static <T> T withDefault(
final Object source, Callable<T> c) {
- return withDefault(new SourceProvider() {
- public Object source() {
- return source;
- }
- }, c);
+ return withDefault(sourceProviderFor(source), c);
}
static class StacktraceSourceProvider implements SourceProvider {
diff --git a/test/com/google/inject/BindingTest.java b/test/com/google/inject/BindingTest.java
index d43817b..807d7fc 100644
--- a/test/com/google/inject/BindingTest.java
+++ b/test/com/google/inject/BindingTest.java
@@ -16,18 +16,10 @@
package com.google.inject;
-import junit.framework.TestCase;
-import com.google.inject.spi.BindingVisitor;
-import com.google.inject.spi.LinkedBinding;
-import com.google.inject.spi.InstanceBinding;
-import com.google.inject.spi.ProviderInstanceBinding;
-import com.google.inject.spi.LinkedProviderBinding;
-import com.google.inject.spi.ClassBinding;
-import com.google.inject.spi.ConstantBinding;
-import com.google.inject.spi.ProviderBinding;
-import com.google.inject.spi.ConvertedConstantBinding;
-import com.google.inject.spi.Dependency;
import com.google.inject.name.Names;
+import com.google.inject.spi.*;
+import junit.framework.TestCase;
+
import java.util.Collection;
import java.util.List;
@@ -209,10 +201,10 @@
bind(Collection.class).to(List.class);
}
});
- // Despite the fact that the binding to List.class cannot be resolved,
- // we successfully return a (broken) injector. This is a known bug.
- fail("known bug: Dangling linked binding");
+ fail();
} catch (CreationException expected) {
+ assertTrue(expected.getMessage().contains(
+ "Injecting into abstract types is not supported."));
}
}
}
\ No newline at end of file
diff --git a/test/com/google/inject/ErrorMessagesTest.java b/test/com/google/inject/ErrorMessagesTest.java
index f2bec70..f328f22 100644
--- a/test/com/google/inject/ErrorMessagesTest.java
+++ b/test/com/google/inject/ErrorMessagesTest.java
@@ -59,11 +59,7 @@
injector.getInstance(AbstractClass.class);
fail();
} catch(ConfigurationException e) {
- // The error returned is "Missing binding to AbstractClass", which is
- // true, but as helpful as it could be in this context. This is a
- // known bug
- assertTrue("known bug: unhelpful message for abstract classes",
- e.getMessage().contains("Injecting into abstract types is not supported."));
+ assertTrue(e.getMessage().contains("Injecting into abstract types is not supported."));
}
}
