Skip to content

Commit

Permalink
Refactor Errors.checkForNull to InternalProvisionException
Browse files Browse the repository at this point in the history
this more or less just moves the logic, but i also refactored the initial if-statement out of the implementation and into the callers.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178762789
  • Loading branch information
lukesandberg authored and ronshapiro committed Jan 25, 2018
1 parent f6cb899 commit 234fbab
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 64 deletions.
61 changes: 0 additions & 61 deletions core/src/com/google/inject/internal/Errors.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -24,16 +24,12 @@
import com.google.inject.Binding; import com.google.inject.Binding;
import com.google.inject.ConfigurationException; import com.google.inject.ConfigurationException;
import com.google.inject.CreationException; import com.google.inject.CreationException;
import com.google.inject.Guice;
import com.google.inject.Injector; import com.google.inject.Injector;
import com.google.inject.Key; import com.google.inject.Key;
import com.google.inject.Provides;
import com.google.inject.ProvisionException; import com.google.inject.ProvisionException;
import com.google.inject.Scope; import com.google.inject.Scope;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
import com.google.inject.internal.util.SourceProvider; import com.google.inject.internal.util.SourceProvider;
import com.google.inject.internal.util.StackTraceElements;
import com.google.inject.spi.Dependency;
import com.google.inject.spi.ElementSource; import com.google.inject.spi.ElementSource;
import com.google.inject.spi.Message; import com.google.inject.spi.Message;
import com.google.inject.spi.ScopeBinding; import com.google.inject.spi.ScopeBinding;
Expand All @@ -48,14 +44,10 @@
import java.lang.reflect.Type; import java.lang.reflect.Type;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.Formatter; import java.util.Formatter;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;


/** /**
* A collection of error messages. If this type is passed as a method parameter, the method is * A collection of error messages. If this type is passed as a method parameter, the method is
Expand All @@ -74,8 +66,6 @@
*/ */
public final class Errors implements Serializable { public final class Errors implements Serializable {


private static final Logger logger = Logger.getLogger(Guice.class.getName());

/** When a binding is not found, show at most this many bindings with the same type */ /** When a binding is not found, show at most this many bindings with the same type */
private static final int MAX_MATCHING_TYPES_REPORTED = 3; private static final int MAX_MATCHING_TYPES_REPORTED = 3;


Expand Down Expand Up @@ -118,9 +108,6 @@ static void checkConfiguration(boolean condition, String format, Object... args)
.addAll(Primitives.allWrapperTypes()) .addAll(Primitives.allWrapperTypes())
.build(); .build();


private static final Set<Dependency<?>> warnedDependencies =
Collections.newSetFromMap(new ConcurrentHashMap<Dependency<?>, Boolean>());

/** The root errors object. Used to access the list of error messages. */ /** The root errors object. Used to access the list of error messages. */
private final Errors root; private final Errors root;


Expand Down Expand Up @@ -656,54 +643,6 @@ public int compare(Message a, Message b) {
}.sortedCopy(root.errors); }.sortedCopy(root.errors);
} }


// TODO(lukes): move @Provides logic into @Provides methods to help simplify this monster

/**
* Returns {@code value} if it is non-null or allowed to be null. Otherwise a message is added and
* an {@code InternalProvisionException} is thrown.
*/
public static <T> T checkForNull(T value, Object source, Dependency<?> dependency)
throws InternalProvisionException {
if (value != null || dependency.isNullable()) {
return value;
}

// Hack to allow null parameters to @Provides methods, for backwards compatibility.
if (dependency.getInjectionPoint().getMember() instanceof Method) {
Method annotated = (Method) dependency.getInjectionPoint().getMember();
if (annotated.isAnnotationPresent(Provides.class)) {
switch (InternalFlags.getNullableProvidesOption()) {
case ERROR:
break; // break out & let the below exception happen
case IGNORE:
return value; // user doesn't care about injecting nulls to non-@Nullables.
case WARN:
// Warn only once, otherwise we spam logs too much.
if (warnedDependencies.add(dependency)) {
logger.log(
Level.WARNING,
"Guice injected null into {0} (a {1}), please mark it @Nullable."
+ " Use -Dguice_check_nullable_provides_params=ERROR to turn this into an"
+ " error.",
new Object[] {
Messages.formatParameter(dependency), Messages.convert(dependency.getKey())
});
}
return value;
}
}
}

Object formattedDependency =
(dependency.getParameterIndex() != -1)
? Messages.formatParameter(dependency)
: StackTraceElements.forMember(dependency.getInjectionPoint().getMember());

throw InternalProvisionException.create(
"null returned by binding at %s%n but %s is not @Nullable", source, formattedDependency)
.addSource(source);
}

public int size() { public int size() {
return root.errors == null ? 0 : root.errors.size(); return root.errors == null ? 0 : root.errors.size();
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ public InternalFactoryToProviderAdapter(Provider<? extends T> provider, Object s
public T get(InternalContext context, Dependency<?> dependency, boolean linked) public T get(InternalContext context, Dependency<?> dependency, boolean linked)
throws InternalProvisionException { throws InternalProvisionException {
try { try {
return Errors.checkForNull(provider.get(), source, dependency); T t = provider.get();
if (t == null && !dependency.isNullable()) {
InternalProvisionException.onNullInjectedIntoNonNullableDependency(source, dependency);
}
return t;
} catch (RuntimeException userException) { } catch (RuntimeException userException) {
throw InternalProvisionException.errorInProvider(userException).addSource(source); throw InternalProvisionException.errorInProvider(userException).addSource(source);
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -19,16 +19,26 @@


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.inject.Guice;
import com.google.inject.Key; import com.google.inject.Key;
import com.google.inject.MembersInjector; import com.google.inject.MembersInjector;
import com.google.inject.Provides;
import com.google.inject.ProvisionException; import com.google.inject.ProvisionException;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
import com.google.inject.internal.util.SourceProvider; import com.google.inject.internal.util.SourceProvider;
import com.google.inject.internal.util.StackTraceElements;
import com.google.inject.spi.Dependency;
import com.google.inject.spi.InjectionListener; import com.google.inject.spi.InjectionListener;
import com.google.inject.spi.Message; import com.google.inject.spi.Message;
import java.lang.reflect.Method;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;


/** /**
* A checked exception for provisioning errors. * A checked exception for provisioning errors.
Expand All @@ -54,6 +64,10 @@
* ProvisionException). * ProvisionException).
*/ */
public final class InternalProvisionException extends Exception { public final class InternalProvisionException extends Exception {
private static final Logger logger = Logger.getLogger(Guice.class.getName());
private static final Set<Dependency<?>> warnedDependencies =
Collections.newSetFromMap(new ConcurrentHashMap<Dependency<?>, Boolean>());

// TODO(lukes): rename ErrorsException to InternalConfigurationException after // TODO(lukes): rename ErrorsException to InternalConfigurationException after
// InternalProvisionException is fully integrated. // InternalProvisionException is fully integrated.


Expand Down Expand Up @@ -118,6 +132,48 @@ public static InternalProvisionException errorNotifyingInjectionListener(
cause, "Error notifying InjectionListener %s of %s.%n Reason: %s", listener, type, cause); cause, "Error notifying InjectionListener %s of %s.%n Reason: %s", listener, type, cause);
} }


/**
* Returns {@code value} if it is non-null or allowed to be null. Otherwise a message is added and
* an {@code InternalProvisionException} is thrown.
*/
static void onNullInjectedIntoNonNullableDependency(Object source, Dependency<?> dependency)
throws InternalProvisionException {
// Hack to allow null parameters to @Provides methods, for backwards compatibility.
if (dependency.getInjectionPoint().getMember() instanceof Method) {
Method annotated = (Method) dependency.getInjectionPoint().getMember();
if (annotated.isAnnotationPresent(Provides.class)) {
switch (InternalFlags.getNullableProvidesOption()) {
case ERROR:
break; // break out & let the below exception happen
case IGNORE:
return; // user doesn't care about injecting nulls to non-@Nullables.
case WARN:
// Warn only once, otherwise we spam logs too much.
if (warnedDependencies.add(dependency)) {
logger.log(
Level.WARNING,
"Guice injected null into {0} (a {1}), please mark it @Nullable."
+ " Use -Dguice_check_nullable_provides_params=ERROR to turn this into an"
+ " error.",
new Object[] {
Messages.formatParameter(dependency), Messages.convert(dependency.getKey())
});
}
return;
}
}
}

Object formattedDependency =
(dependency.getParameterIndex() != -1)
? Messages.formatParameter(dependency)
: StackTraceElements.forMember(dependency.getInjectionPoint().getMember());

throw InternalProvisionException.create(
"null returned by binding at %s%n but %s is not @Nullable", source, formattedDependency)
.addSource(source);
}

private final List<Object> sourcesToPrepend = new ArrayList<>(); private final List<Object> sourcesToPrepend = new ArrayList<>();
private final ImmutableList<Message> errors; private final ImmutableList<Message> errors;


Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ protected T provision(
Dependency<?> dependency, Dependency<?> dependency,
ConstructionContext<T> constructionContext) ConstructionContext<T> constructionContext)
throws InternalProvisionException { throws InternalProvisionException {
T t = Errors.checkForNull(provider.get(), source, dependency); T t = provider.get();
if (t == null && !dependency.isNullable()) {
InternalProvisionException.onNullInjectedIntoNonNullableDependency(source, dependency);
}
constructionContext.setProxyDelegates(t); constructionContext.setProxyDelegates(t);
return t; return t;
} }
Expand Down
4 changes: 3 additions & 1 deletion core/src/com/google/inject/internal/ProviderMethod.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ protected T doProvision(InternalContext context, Dependency<?> dependency)
throws InternalProvisionException { throws InternalProvisionException {
try { try {
T t = doProvision(SingleParameterInjector.getAll(context, parameterInjectors)); T t = doProvision(SingleParameterInjector.getAll(context, parameterInjectors));
Errors.checkForNull(t, getMethod(), dependency); if (t == null && !dependency.isNullable()) {
InternalProvisionException.onNullInjectedIntoNonNullableDependency(getMethod(), dependency);
}
return t; return t;
} catch (IllegalAccessException e) { } catch (IllegalAccessException e) {
throw new AssertionError(e); throw new AssertionError(e);
Expand Down

0 comments on commit 234fbab

Please sign in to comment.