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@449 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/BindCommandProcessor.java b/src/com/google/inject/BindCommandProcessor.java
index d03bacf..1ab63c6 100644
--- a/src/com/google/inject/BindCommandProcessor.java
+++ b/src/com/google/inject/BindCommandProcessor.java
@@ -50,6 +50,7 @@
Stage stage,
Map<Key<?>, BindingImpl<?>> bindings,
Map<Object, Void> outstandingInjections) {
+ super(injector.errorHandler);
this.injector = injector;
this.scopes = scopes;
this.stage = stage;
diff --git a/src/com/google/inject/BindInterceptorCommandProcessor.java b/src/com/google/inject/BindInterceptorCommandProcessor.java
index 93677c6..9b6ef1e 100644
--- a/src/com/google/inject/BindInterceptorCommandProcessor.java
+++ b/src/com/google/inject/BindInterceptorCommandProcessor.java
@@ -30,6 +30,7 @@
private final ProxyFactoryBuilder proxyFactoryBuilder;
BindInterceptorCommandProcessor(ErrorHandler errorHandler) {
+ super(errorHandler);
proxyFactoryBuilder = new ProxyFactoryBuilder(errorHandler);
}
diff --git a/src/com/google/inject/CommandProcessor.java b/src/com/google/inject/CommandProcessor.java
index a2ebd82..d92f507 100644
--- a/src/com/google/inject/CommandProcessor.java
+++ b/src/com/google/inject/CommandProcessor.java
@@ -33,19 +33,18 @@
*/
abstract class CommandProcessor implements Command.Visitor<Boolean> {
- private ErrorHandler errorHandler;
+ private final ErrorHandler errorHandler;
- public void processCommands(List<Command> commands, ErrorHandler errorHandler) {
+ protected CommandProcessor(ErrorHandler errorHandler) {
this.errorHandler = errorHandler;
- try {
- for (Iterator<Command> i = commands.iterator(); i.hasNext(); ) {
- Boolean allDone = i.next().acceptVisitor(this);
- if (allDone) {
- i.remove();
- }
+ }
+
+ public void processCommands(List<Command> commands) {
+ for (Iterator<Command> i = commands.iterator(); i.hasNext(); ) {
+ Boolean allDone = i.next().acceptVisitor(this);
+ if (allDone) {
+ i.remove();
}
- } finally {
- this.errorHandler = null;
}
}
diff --git a/src/com/google/inject/ConvertToTypesCommandProcessor.java b/src/com/google/inject/ConvertToTypesCommandProcessor.java
index db88d6b..9ad1376 100644
--- a/src/com/google/inject/ConvertToTypesCommandProcessor.java
+++ b/src/com/google/inject/ConvertToTypesCommandProcessor.java
@@ -18,8 +18,9 @@
package com.google.inject;
import com.google.inject.commands.ConvertToTypesCommand;
-import com.google.inject.internal.Strings;
+import com.google.inject.internal.ErrorHandler;
import com.google.inject.internal.MatcherAndConverter;
+import com.google.inject.internal.Strings;
import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.matcher.Matcher;
import com.google.inject.matcher.Matchers;
@@ -41,7 +42,9 @@
private final List<MatcherAndConverter<?>> converters;
- ConvertToTypesCommandProcessor(List<MatcherAndConverter<?>> converters) {
+ ConvertToTypesCommandProcessor(ErrorHandler errorHandler,
+ List<MatcherAndConverter<?>> converters) {
+ super(errorHandler);
this.converters = converters;
// Configure type converters.
diff --git a/src/com/google/inject/ErrorsCommandProcessor.java b/src/com/google/inject/ErrorsCommandProcessor.java
index a3238b3..aa3a6d1 100644
--- a/src/com/google/inject/ErrorsCommandProcessor.java
+++ b/src/com/google/inject/ErrorsCommandProcessor.java
@@ -18,6 +18,7 @@
import com.google.inject.commands.AddMessageErrorCommand;
import com.google.inject.commands.AddThrowableErrorCommand;
+import com.google.inject.internal.ErrorHandler;
import com.google.inject.internal.ErrorMessages;
import java.util.logging.Level;
@@ -34,8 +35,12 @@
private static final Logger logger
= Logger.getLogger(ErrorsCommandProcessor.class.getName());
+ ErrorsCommandProcessor(ErrorHandler errorHandler) {
+ super(errorHandler);
+ }
+
@Override public Boolean visitAddMessageError(AddMessageErrorCommand command) {
- addError(command.getSource(), command.getMessage(), command.getArguments());
+ addError(command.getSource(), command.getMessage(), command.getArguments().toArray());
return true;
}
diff --git a/src/com/google/inject/GetProviderProcessor.java b/src/com/google/inject/GetProviderProcessor.java
index 0fce817..2b1a275 100644
--- a/src/com/google/inject/GetProviderProcessor.java
+++ b/src/com/google/inject/GetProviderProcessor.java
@@ -29,6 +29,7 @@
private final InjectorImpl injector;
GetProviderProcessor(InjectorImpl injector) {
+ super(injector.errorHandler);
this.injector = injector;
}
diff --git a/src/com/google/inject/InjectorBuilder.java b/src/com/google/inject/InjectorBuilder.java
index cdca8d6..7f76615 100644
--- a/src/com/google/inject/InjectorBuilder.java
+++ b/src/com/google/inject/InjectorBuilder.java
@@ -120,38 +120,39 @@
* Builds the injector.
*/
private void buildCoreInjector() {
- new ErrorsCommandProcessor()
- .processCommands(commands, errorHandler);
+ new ErrorsCommandProcessor(errorHandler)
+ .processCommands(commands);
BindInterceptorCommandProcessor bindInterceptorCommandProcessor
= new BindInterceptorCommandProcessor(errorHandler);
- bindInterceptorCommandProcessor.processCommands(commands, errorHandler);
+ bindInterceptorCommandProcessor.processCommands(commands);
ConstructionProxyFactory proxyFactory = bindInterceptorCommandProcessor.createProxyFactory();
injector.reflection = reflectionFactory.create(errorHandler, proxyFactory);
stopwatch.resetAndLog("Interceptors creation");
- new ScopesCommandProcessor(injector.scopes)
- .processCommands(commands, errorHandler);
+ new ScopesCommandProcessor(errorHandler, injector.scopes)
+ .processCommands(commands);
stopwatch.resetAndLog("Scopes creation");
- new ConvertToTypesCommandProcessor(injector.converters)
- .processCommands(commands, errorHandler);
+ new ConvertToTypesCommandProcessor(errorHandler, injector.converters)
+ .processCommands(commands);
stopwatch.resetAndLog("Converters creation");
bindLogger();
bindCommandProcesor = new BindCommandProcessor(
injector, injector.scopes, stage, injector.explicitBindings,
injector.outstandingInjections);
- bindCommandProcesor.processCommands(commands, errorHandler);
+ bindCommandProcesor.processCommands(commands);
bindCommandProcesor.createUntargettedBindings();
stopwatch.resetAndLog("Binding creation");
injector.index();
stopwatch.resetAndLog("Binding indexing");
- requestStaticInjectionCommandProcessor = new RequestStaticInjectionCommandProcessor();
requestStaticInjectionCommandProcessor
- .processCommands(commands, errorHandler);
+ = new RequestStaticInjectionCommandProcessor(errorHandler);
+ requestStaticInjectionCommandProcessor
+ .processCommands(commands);
stopwatch.resetAndLog("Static injection");
}
@@ -170,7 +171,7 @@
stopwatch.resetAndLog("Instance member validation");
new GetProviderProcessor(injector)
- .processCommands(commands, errorHandler);
+ .processCommands(commands);
stopwatch.resetAndLog("Provider verification");
errorHandler.blowUpIfErrorsExist();
diff --git a/src/com/google/inject/RequestStaticInjectionCommandProcessor.java b/src/com/google/inject/RequestStaticInjectionCommandProcessor.java
index 401cb6a..6bdc85e 100644
--- a/src/com/google/inject/RequestStaticInjectionCommandProcessor.java
+++ b/src/com/google/inject/RequestStaticInjectionCommandProcessor.java
@@ -17,6 +17,7 @@
package com.google.inject;
import com.google.inject.commands.RequestStaticInjectionCommand;
+import com.google.inject.internal.ErrorHandler;
import java.util.ArrayList;
import java.util.List;
@@ -32,6 +33,10 @@
private final List<StaticInjection> staticInjections
= new ArrayList<StaticInjection>();
+ RequestStaticInjectionCommandProcessor(ErrorHandler errorHandler) {
+ super(errorHandler);
+ }
+
@Override public Boolean visitRequestStaticInjection(RequestStaticInjectionCommand command) {
for (Class<?> type : command.getTypes()) {
staticInjections.add(new StaticInjection(command.getSource(), type));
diff --git a/src/com/google/inject/ScopesCommandProcessor.java b/src/com/google/inject/ScopesCommandProcessor.java
index 583205e..a19fd05 100644
--- a/src/com/google/inject/ScopesCommandProcessor.java
+++ b/src/com/google/inject/ScopesCommandProcessor.java
@@ -18,9 +18,10 @@
import com.google.inject.commands.BindScopeCommand;
import com.google.inject.internal.Annotations;
+import com.google.inject.internal.ErrorHandler;
+import com.google.inject.internal.ErrorMessages;
import static com.google.inject.internal.Objects.nonNull;
import com.google.inject.internal.StackTraceElements;
-import com.google.inject.internal.ErrorMessages;
import java.lang.annotation.Annotation;
import java.util.Map;
@@ -35,7 +36,9 @@
private final Map<Class<? extends Annotation>, Scope> scopes;
- ScopesCommandProcessor(Map<Class<? extends Annotation>, Scope> scopes) {
+ ScopesCommandProcessor(ErrorHandler errorHandler,
+ Map<Class<? extends Annotation>, Scope> scopes) {
+ super(errorHandler);
this.scopes = scopes;
}