Modifiable methods should always return publicly accessible types in case that their eventual subclass implementations need to implement the method in a package in which the type isn't accessible.
It's also possible that the return type isn't accessible in the base class implementation.
RELNOTES=n/a
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221797313
diff --git a/java/dagger/internal/codegen/BindingMethodImplementation.java b/java/dagger/internal/codegen/BindingMethodImplementation.java
index 618b1bd..814bf3a 100644
--- a/java/dagger/internal/codegen/BindingMethodImplementation.java
+++ b/java/dagger/internal/codegen/BindingMethodImplementation.java
@@ -17,7 +17,6 @@
package dagger.internal.codegen;
import static com.google.common.base.Preconditions.checkNotNull;
-import static dagger.internal.codegen.RequestKinds.requestType;
import com.google.common.base.Supplier;
import com.squareup.javapoet.ClassName;
@@ -79,14 +78,6 @@
&& binding.contributedPrimitiveType().isPresent()) {
return binding.contributedPrimitiveType().get();
}
- return types.accessibleType(requestedType(), componentName);
- }
-
- private TypeMirror requestedType() {
- if (request.requestKind().isPresent()) {
- return requestType(request.requestKind().get(), binding.contributedType(), types);
- }
- return types.wrapType(
- binding.contributedType(), request.frameworkType().get().frameworkClass());
+ return request.accessibleType(binding, componentName, types);
}
}
diff --git a/java/dagger/internal/codegen/BindingRequest.java b/java/dagger/internal/codegen/BindingRequest.java
index 5245d6f..9bbb0b0 100644
--- a/java/dagger/internal/codegen/BindingRequest.java
+++ b/java/dagger/internal/codegen/BindingRequest.java
@@ -16,14 +16,16 @@
package dagger.internal.codegen;
-import static dagger.internal.codegen.RequestKinds.requestTypeName;
+import static com.google.common.base.Preconditions.checkArgument;
+import static dagger.internal.codegen.RequestKinds.requestType;
import com.google.auto.value.AutoValue;
-import com.squareup.javapoet.TypeName;
+import com.squareup.javapoet.ClassName;
import dagger.model.DependencyRequest;
import dagger.model.Key;
import dagger.model.RequestKind;
import java.util.Optional;
+import javax.lang.model.type.TypeMirror;
/**
* A request for a binding, which may be in the form of a request for a dependency to pass to a
@@ -32,7 +34,6 @@
*/
@AutoValue
abstract class BindingRequest {
-
/** Creates a {@link BindingRequest} for the given {@link DependencyRequest}. */
static BindingRequest bindingRequest(DependencyRequest dependencyRequest) {
return bindingRequest(dependencyRequest.key(), dependencyRequest.kind());
@@ -83,13 +84,30 @@
return requestKind.equals(requestKind().orElse(null));
}
- /** Returns the type name for the requested type. */
- final TypeName typeName() {
- TypeName keyTypeName = TypeName.get(key().type());
+ /**
+ * Returns the {@link Accessibility#isTypePubliclyAccessible(TypeMirror)} publicly accessible
+ * type} of this request.
+ */
+ final TypeMirror publiclyAccessibleRequestType(DaggerTypes types) {
+ return types.publiclyAccessibleType(requestedType(key().type(), types));
+ }
+
+ /**
+ * Returns the {@link DaggerTypes#accessibleType(TypeMirror, ClassName) accessible type} of
+ * instances created by a {@code binding} for this request. This may differ from the accessible
+ * type of the key for multibindings.
+ */
+ final TypeMirror accessibleType(
+ ContributionBinding binding, ClassName requestingClass, DaggerTypes types) {
+ checkArgument(binding.key().equals(key()));
+ return types.accessibleType(requestedType(binding.contributedType(), types), requestingClass);
+ }
+
+ final TypeMirror requestedType(TypeMirror contributedType, DaggerTypes types) {
if (requestKind().isPresent()) {
- return requestTypeName(requestKind().get(), keyTypeName);
+ return requestType(requestKind().get(), contributedType, types);
}
- return frameworkType().get().frameworkClassOf(keyTypeName);
+ return types.wrapType(contributedType, frameworkType().get().frameworkClass());
}
/** Returns a name that can be used for the kind of request this is. */
diff --git a/java/dagger/internal/codegen/ComponentBindingExpressions.java b/java/dagger/internal/codegen/ComponentBindingExpressions.java
index 5858d1c..f58382c 100644
--- a/java/dagger/internal/codegen/ComponentBindingExpressions.java
+++ b/java/dagger/internal/codegen/ComponentBindingExpressions.java
@@ -582,6 +582,7 @@
this,
membersInjectionMethods,
componentRequirementFields,
+ types,
elements));
case MEMBERS_INJECTOR:
diff --git a/java/dagger/internal/codegen/DaggerTypes.java b/java/dagger/internal/codegen/DaggerTypes.java
index a0f352f..e0947c1 100644
--- a/java/dagger/internal/codegen/DaggerTypes.java
+++ b/java/dagger/internal/codegen/DaggerTypes.java
@@ -29,6 +29,7 @@
import com.squareup.javapoet.ClassName;
import java.util.List;
import java.util.Optional;
+import java.util.function.Predicate;
import javax.inject.Inject;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;
@@ -144,12 +145,44 @@
}
}
- /** Returns a {@link TypeMirror} for the binding that is accessible to the component. */
- protected final TypeMirror accessibleType(TypeMirror type, ClassName requestingClass) {
- if (Accessibility.isTypeAccessibleFrom(type, requestingClass.packageName())) {
+ /**
+ * Returns a publicly accessible type based on {@code type}:
+ *
+ * <ul>
+ * <li>If {@code type} is publicly accessible, returns it.
+ * <li>If not, but {@code type}'s raw type is publicly accessible, returns the raw type.
+ * <li>Otherwise returns {@link Object}.
+ * </ul>
+ */
+ protected TypeMirror publiclyAccessibleType(TypeMirror type) {
+ return accessibleType(
+ type, Accessibility::isTypePubliclyAccessible, Accessibility::isRawTypePubliclyAccessible);
+ }
+
+ /**
+ * Returns an accessible type in {@code requestingClass}'s package based on {@code type}:
+ *
+ * <ul>
+ * <li>If {@code type} is accessible from the package, returns it.
+ * <li>If not, but {@code type}'s raw type is accessible from the package, returns the raw type.
+ * <li>Otherwise returns {@link Object}.
+ * </ul>
+ */
+ protected TypeMirror accessibleType(TypeMirror type, ClassName requestingClass) {
+ return accessibleType(
+ type,
+ t -> Accessibility.isTypeAccessibleFrom(t, requestingClass.packageName()),
+ t -> Accessibility.isRawTypeAccessible(t, requestingClass.packageName()));
+ }
+
+ private TypeMirror accessibleType(
+ TypeMirror type,
+ Predicate<TypeMirror> accessibilityPredicate,
+ Predicate<TypeMirror> rawTypeAccessibilityPredicate) {
+ if (accessibilityPredicate.test(type)) {
return type;
} else if (type.getKind().equals(TypeKind.DECLARED)
- && Accessibility.isRawTypeAccessible(type, requestingClass.packageName())) {
+ && rawTypeAccessibilityPredicate.test(type)) {
return getDeclaredType(MoreTypes.asTypeElement(type));
} else {
return elements.getTypeElement(Object.class).asType();
diff --git a/java/dagger/internal/codegen/GeneratedInstanceBindingExpression.java b/java/dagger/internal/codegen/GeneratedInstanceBindingExpression.java
index e82af0e..1e295d8 100644
--- a/java/dagger/internal/codegen/GeneratedInstanceBindingExpression.java
+++ b/java/dagger/internal/codegen/GeneratedInstanceBindingExpression.java
@@ -37,13 +37,15 @@
ResolvedBindings resolvedBindings,
BindingRequest request,
Optional<ModifiableBindingMethod> matchingModifiableBindingMethod,
- Optional<ComponentMethodDescriptor> matchingComponentMethod) {
+ Optional<ComponentMethodDescriptor> matchingComponentMethod,
+ DaggerTypes types) {
super(
componentImplementation,
ModifiableBindingType.GENERATED_INSTANCE,
request,
matchingModifiableBindingMethod,
- matchingComponentMethod);
+ matchingComponentMethod,
+ types);
this.componentImplementation = componentImplementation;
this.binding = resolvedBindings.contributionBinding();
this.request = request;
diff --git a/java/dagger/internal/codegen/InjectionMethods.java b/java/dagger/internal/codegen/InjectionMethods.java
index 5d6e0c0..a419024 100644
--- a/java/dagger/internal/codegen/InjectionMethods.java
+++ b/java/dagger/internal/codegen/InjectionMethods.java
@@ -18,6 +18,7 @@
import static com.google.common.base.CaseFormat.LOWER_CAMEL;
import static com.google.common.base.CaseFormat.UPPER_CAMEL;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static dagger.internal.codegen.Accessibility.isElementAccessibleFrom;
import static dagger.internal.codegen.Accessibility.isRawTypeAccessible;
@@ -174,16 +175,32 @@
* requires the use of an injection method.
*/
static boolean requiresInjectionMethod(
- ProvisionBinding binding, CompilerOptions compilerOptions, String callingPackage) {
+ ProvisionBinding binding,
+ ImmutableList<Expression> arguments,
+ CompilerOptions compilerOptions,
+ String callingPackage,
+ DaggerTypes types) {
ExecutableElement method = MoreElements.asExecutable(binding.bindingElement().get());
return !binding.injectionSites().isEmpty()
|| binding.shouldCheckForNull(compilerOptions)
|| !isElementAccessibleFrom(method, callingPackage)
- || method
- .getParameters()
- .stream()
- .map(VariableElement::asType)
- .anyMatch(type -> !isRawTypeAccessible(type, callingPackage));
+ || !areParametersAssignable(method, arguments, types)
+ // This check should be removable once we drop support for -source 7
+ || method.getParameters().stream()
+ .map(VariableElement::asType)
+ .anyMatch(type -> !isRawTypeAccessible(type, callingPackage));
+ }
+
+ private static boolean areParametersAssignable(
+ ExecutableElement element, ImmutableList<Expression> arguments, DaggerTypes types) {
+ List<? extends VariableElement> parameters = element.getParameters();
+ checkArgument(parameters.size() == arguments.size());
+ for (int i = 0; i < parameters.size(); i++) {
+ if (!types.isAssignable(arguments.get(i).type(), parameters.get(i).asType())) {
+ return false;
+ }
+ }
+ return true;
}
/**
diff --git a/java/dagger/internal/codegen/MissingBindingExpression.java b/java/dagger/internal/codegen/MissingBindingExpression.java
index c3822e4..1b414ff 100644
--- a/java/dagger/internal/codegen/MissingBindingExpression.java
+++ b/java/dagger/internal/codegen/MissingBindingExpression.java
@@ -34,13 +34,15 @@
ComponentImplementation componentImplementation,
BindingRequest request,
Optional<ModifiableBindingMethod> matchingModifiableBindingMethod,
- Optional<ComponentMethodDescriptor> matchingComponentMethod) {
+ Optional<ComponentMethodDescriptor> matchingComponentMethod,
+ DaggerTypes types) {
super(
componentImplementation,
ModifiableBindingType.MISSING,
request,
matchingModifiableBindingMethod,
- matchingComponentMethod);
+ matchingComponentMethod,
+ types);
this.componentImplementation = componentImplementation;
this.request = request;
}
diff --git a/java/dagger/internal/codegen/ModifiableAbstractMethodBindingExpression.java b/java/dagger/internal/codegen/ModifiableAbstractMethodBindingExpression.java
index a7de8e3..9b2b336 100644
--- a/java/dagger/internal/codegen/ModifiableAbstractMethodBindingExpression.java
+++ b/java/dagger/internal/codegen/ModifiableAbstractMethodBindingExpression.java
@@ -22,9 +22,11 @@
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.MethodSpec;
+import com.squareup.javapoet.TypeName;
import dagger.internal.codegen.ComponentDescriptor.ComponentMethodDescriptor;
import dagger.internal.codegen.ModifiableBindingMethods.ModifiableBindingMethod;
import java.util.Optional;
+import javax.lang.model.type.TypeMirror;
/**
* A {@link BindingExpression} that invokes a method that encapsulates a binding that cannot be
@@ -37,6 +39,7 @@
private final ComponentImplementation componentImplementation;
private final ModifiableBindingType modifiableBindingType;
private final BindingRequest request;
+ private final TypeMirror returnType;
private Optional<String> methodName;
ModifiableAbstractMethodBindingExpression(
@@ -44,10 +47,12 @@
ModifiableBindingType modifiableBindingType,
BindingRequest request,
Optional<ModifiableBindingMethod> matchingModifiableBindingMethod,
- Optional<ComponentMethodDescriptor> matchingComponentMethod) {
+ Optional<ComponentMethodDescriptor> matchingComponentMethod,
+ DaggerTypes types) {
this.componentImplementation = componentImplementation;
this.modifiableBindingType = modifiableBindingType;
this.request = request;
+ this.returnType = returnType(request, matchingComponentMethod, types);
this.methodName =
initializeMethodName(matchingComponentMethod, matchingModifiableBindingMethod);
}
@@ -72,7 +77,7 @@
@Override
final Expression getDependencyExpression(ClassName requestingClass) {
addUnimplementedMethod();
- return Expression.create(request.key().type(), CodeBlock.of("$L()", methodName.get()));
+ return Expression.create(returnType, CodeBlock.of("$L()", methodName.get()));
}
private void addUnimplementedMethod() {
@@ -84,12 +89,39 @@
request,
MethodSpec.methodBuilder(methodName.get())
.addModifiers(PUBLIC, ABSTRACT)
- .returns(request.typeName())
+ .returns(TypeName.get(returnType))
.build(),
false /* finalized */);
}
}
+ /**
+ * The return type of this abstract method expression:
+ *
+ * <ul>
+ * <li>If there's a {@code matchingComponentMethod}, use its return type.
+ * <li>Otherwise, use the {@linkplain DaggerTypes#publiclyAccessibleType(TypeMirror) publicly
+ * accessible type} of the request. We can't use the {@linkplain
+ * Accessibility#isTypeAccessibleFrom(TypeMirror, String) type accessible from the current
+ * implementation's package} because a subclass implementation may be in a different package
+ * from which the request type is not accessible.
+ * </ul>
+ */
+ private static TypeMirror returnType(
+ BindingRequest bindingRequest,
+ Optional<ComponentMethodDescriptor> matchingComponentMethod,
+ DaggerTypes types) {
+ if (matchingComponentMethod.isPresent()) {
+ TypeMirror returnType =
+ matchingComponentMethod.get().dependencyRequest().get().requestElement().get().asType();
+ return returnType.getKind().isPrimitive()
+ ? returnType
+ : bindingRequest.requestedType(bindingRequest.key().type(), types);
+ } else {
+ return bindingRequest.publiclyAccessibleRequestType(types);
+ }
+ }
+
/** Returns a unique 'getter' method name for the current component. */
abstract String chooseMethodName();
}
diff --git a/java/dagger/internal/codegen/ModifiableBindingExpressions.java b/java/dagger/internal/codegen/ModifiableBindingExpressions.java
index 30d11e0..14ce801 100644
--- a/java/dagger/internal/codegen/ModifiableBindingExpressions.java
+++ b/java/dagger/internal/codegen/ModifiableBindingExpressions.java
@@ -190,7 +190,8 @@
resolvedBindings,
request,
matchingModifiableBindingMethod,
- matchingComponentMethod);
+ matchingComponentMethod,
+ types);
}
// Otherwise return a concrete implementation.
return bindingExpressions.wrapInMethod(
@@ -205,7 +206,8 @@
componentImplementation,
request,
matchingModifiableBindingMethod,
- matchingComponentMethod);
+ matchingComponentMethod,
+ types);
}
// Otherwise we assume that it is valid to have a missing binding as it is part of a
// dependency chain that has been passively pruned.
diff --git a/java/dagger/internal/codegen/SimpleMethodBindingExpression.java b/java/dagger/internal/codegen/SimpleMethodBindingExpression.java
index f35d0a0..6778998 100644
--- a/java/dagger/internal/codegen/SimpleMethodBindingExpression.java
+++ b/java/dagger/internal/codegen/SimpleMethodBindingExpression.java
@@ -24,6 +24,8 @@
import static dagger.internal.codegen.TypeNames.rawTypeName;
import com.google.auto.common.MoreTypes;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.MethodSpec;
@@ -31,6 +33,7 @@
import dagger.internal.codegen.InjectionMethods.ProvisionMethod;
import dagger.model.DependencyRequest;
import java.util.Optional;
+import java.util.function.Function;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.type.DeclaredType;
@@ -46,6 +49,7 @@
private final ComponentBindingExpressions componentBindingExpressions;
private final MembersInjectionMethods membersInjectionMethods;
private final ComponentRequirementFields componentRequirementFields;
+ private final DaggerTypes types;
private final DaggerElements elements;
SimpleMethodBindingExpression(
@@ -54,6 +58,7 @@
ComponentBindingExpressions componentBindingExpressions,
MembersInjectionMethods membersInjectionMethods,
ComponentRequirementFields componentRequirementFields,
+ DaggerTypes types,
DaggerElements elements) {
super(resolvedBindings);
this.compilerOptions = compilerOptions;
@@ -65,23 +70,36 @@
this.componentBindingExpressions = componentBindingExpressions;
this.membersInjectionMethods = membersInjectionMethods;
this.componentRequirementFields = componentRequirementFields;
+ this.types = types;
this.elements = elements;
}
@Override
Expression getDependencyExpression(ClassName requestingClass) {
- return requiresInjectionMethod(provisionBinding, compilerOptions, requestingClass.packageName())
- ? invokeInjectionMethod(requestingClass)
- : invokeMethod(requestingClass);
+ ImmutableMap<DependencyRequest, Expression> arguments =
+ ImmutableMap.copyOf(
+ Maps.asMap(
+ provisionBinding.dependencies(),
+ request -> dependencyArgument(request, requestingClass)));
+ Function<DependencyRequest, CodeBlock> argumentsFunction =
+ request -> arguments.get(request).codeBlock();
+ return requiresInjectionMethod(
+ provisionBinding,
+ arguments.values().asList(),
+ compilerOptions,
+ requestingClass.packageName(),
+ types)
+ ? invokeInjectionMethod(argumentsFunction, requestingClass)
+ : invokeMethod(argumentsFunction, requestingClass);
}
- private Expression invokeMethod(ClassName requestingClass) {
+ private Expression invokeMethod(
+ Function<DependencyRequest, CodeBlock> argumentsFunction,
+ ClassName requestingClass) {
// TODO(dpb): align this with the contents of InlineMethods.create
CodeBlock arguments =
- provisionBinding
- .dependencies()
- .stream()
- .map(request -> dependencyArgument(request, requestingClass))
+ provisionBinding.dependencies().stream()
+ .map(argumentsFunction)
.collect(toParametersCodeBlock());
ExecutableElement method = asExecutable(provisionBinding.bindingElement().get());
CodeBlock invocation;
@@ -113,20 +131,19 @@
return rawTypeName(typeName);
}
- private Expression invokeInjectionMethod(ClassName requestingClass) {
+ private Expression invokeInjectionMethod(
+ Function<DependencyRequest, CodeBlock> argumentsFunction, ClassName requestingClass) {
return injectMembers(
ProvisionMethod.invoke(
provisionBinding,
- request -> dependencyArgument(request, requestingClass),
+ argumentsFunction,
requestingClass,
moduleReference(requestingClass),
compilerOptions));
}
- private CodeBlock dependencyArgument(DependencyRequest dependency, ClassName requestingClass) {
- return componentBindingExpressions
- .getDependencyArgumentExpression(dependency, requestingClass)
- .codeBlock();
+ private Expression dependencyArgument(DependencyRequest dependency, ClassName requestingClass) {
+ return componentBindingExpressions.getDependencyArgumentExpression(dependency, requestingClass);
}
private Expression injectMembers(CodeBlock instance) {