Merge pull request square#437 from google/moe_sync_0810

Moe sync 08/10

Author: gk5885

.travis.yml

@@ -3,6 +3,7 @@ language: android
 jdk:
   - oraclejdk7
   - openjdk7
+  - oraclejdk8
 
 android:
   components:

compiler/src/main/java/dagger/internal/codegen/AbstractComponentWriter.java

@@ -659,26 +659,29 @@ private void implementInterfaceMethods() {
     for (ComponentMethodDescriptor componentMethod :
         graph.componentDescriptor().componentMethods()) {
       if (componentMethod.dependencyRequest().isPresent()) {
-        DependencyRequest interfaceRequest = componentMethod.dependencyRequest().get();
-        ExecutableElement requestElement =
-            MoreElements.asExecutable(interfaceRequest.requestElement());
-        ExecutableType requestType = MoreTypes.asExecutable(types.asMemberOf(
-            MoreTypes.asDeclared(componentDefinitionType().asType()), requestElement));
-        MethodSignature signature = MethodSignature.fromExecutableType(
-            requestElement.getSimpleName().toString(), requestType);
+        ExecutableElement methodElement =
+            MoreElements.asExecutable(componentMethod.methodElement());
+        ExecutableType requestType =
+            MoreTypes.asExecutable(
+                types.asMemberOf(
+                    MoreTypes.asDeclared(componentDefinitionType().asType()), methodElement));
+        MethodSignature signature =
+            MethodSignature.fromExecutableType(
+                methodElement.getSimpleName().toString(), requestType);
         if (!interfaceMethods.contains(signature)) {
           interfaceMethods.add(signature);
           MethodSpec.Builder interfaceMethod =
-              methodBuilder(requestElement.getSimpleName().toString())
+              methodBuilder(methodElement.getSimpleName().toString())
                   .addAnnotation(Override.class)
                   .addModifiers(PUBLIC)
                   .returns(TypeName.get(requestType.getReturnType()));
+          DependencyRequest interfaceRequest = componentMethod.dependencyRequest().get();
           BindingKey bindingKey = interfaceRequest.bindingKey();
           MemberSelect memberSelect = getMemberSelect(bindingKey);
           CodeBlock memberSelectCodeBlock = memberSelect.getExpressionFor(name);
           switch (interfaceRequest.kind()) {
             case MEMBERS_INJECTOR:
-              List<? extends VariableElement> parameters = requestElement.getParameters();
+              List<? extends VariableElement> parameters = methodElement.getParameters();
               if (parameters.isEmpty()) {
                 // we're returning the framework type
                 interfaceMethod.addStatement("return $L", memberSelectCodeBlock);

compiler/src/main/java/dagger/internal/codegen/Binding.java

@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import dagger.internal.codegen.BindingType.HasBindingType;
 import java.util.List;
 import java.util.Set;
@@ -61,15 +62,31 @@ Class<?> frameworkClass() {
   public abstract Key key();
 
   /**
-   * The explicit set of {@link DependencyRequest dependencies} required to satisfy this binding.
+   * The explicit set of {@link DependencyRequest dependencies} required to satisfy this binding as
+   * defined by the user-defined injection sites.
    */
   abstract ImmutableSet<DependencyRequest> dependencies();
 
   /**
-   * The set of {@link DependencyRequest dependencies} required to satisfy this binding. This is a
-   * superset of {@link #dependencies()}.  This returns an unmodifiable set.
+   * The set of {@link DependencyRequest dependencies} that are added by the framework rather than a
+   * user-defined injection site. This returns an unmodifiable set.
    */
-  abstract Set<DependencyRequest> implicitDependencies();
+  // TODO(gak): this will eventually get migrated to FrameworkDependency
+  Set<DependencyRequest> frameworkDependencies() {
+    return ImmutableSet.of();
+  }
+
+  /**
+   * The set of {@link DependencyRequest dependencies} required to satisfy this binding. This is the
+   * union of {@link #dependencies()} and {@link #frameworkDependencies()}. This returns an
+   * unmodifiable set.
+   */
+  final Set<DependencyRequest> implicitDependencies() {
+    Set<DependencyRequest> frameworkDependencies = frameworkDependencies();
+    return frameworkDependencies.isEmpty()
+        ? dependencies()
+        : Sets.union(frameworkDependencies, dependencies());
+  }
 
   /**
    * Returns the name of the package in which this binding must be managed. E.g.: a binding
@@ -129,7 +146,7 @@ public Void visitWildcard(WildcardType t, ImmutableSet.Builder<String> p) {
   }
 
   /**
-   * if this binding's key's type parameters are different from those of the
+   * If this binding's key's type parameters are different from those of the
    * {@link #bindingTypeElement()}, this is the binding for the {@link #bindingTypeElement()}'s
    * unresolved type.
    */

compiler/src/main/java/dagger/internal/codegen/BindingGraph.java

@@ -547,28 +547,49 @@ private ImmutableSet<ContributionBinding> createDelegateBindings(
           ImmutableSet<DelegateDeclaration> delegateDeclarations) {
         ImmutableSet.Builder<ContributionBinding> builder = ImmutableSet.builder();
         for (DelegateDeclaration delegateDeclaration : delegateDeclarations) {
-          DependencyRequest delegateRequest = delegateDeclaration.delegateRequest();
-          ResolvedBindings resolvedDelegate = lookUpBindings(delegateRequest);
-          for (ContributionBinding explicitDelegate : resolvedDelegate.contributionBindings()) {
-            switch (explicitDelegate.bindingType()) {
-              case PRODUCTION:
-                builder.add(
-                    productionBindingFactory.delegate(
-                        delegateDeclaration, (ProductionBinding) explicitDelegate));
-                break;
-              case PROVISION:
-                builder.add(
-                    provisionBindingFactory.delegate(
-                        delegateDeclaration, (ProvisionBinding) explicitDelegate));
-                break;
-              default:
-                throw new AssertionError();
-            }
-          }
+          builder.add(createDelegateBinding(delegateDeclaration));
         }
         return builder.build();
       }
 
+      /**
+       * Creates one (and only one) delegate binding for a delegate declaration, based on the
+       * resolved bindings of the right-hand-side of a {@link dagger.Binds} method. If there are
+       * duplicate bindings for the dependency key, there should still be only one binding for the
+       * delegate key.
+       */
+      private ContributionBinding createDelegateBinding(DelegateDeclaration delegateDeclaration) {
+        ResolvedBindings resolvedDelegate = lookUpBindings(delegateDeclaration.delegateRequest());
+        if (resolvedDelegate.contributionBindings().isEmpty()) {
+          // This is guaranteed to result in a missing binding error, so it doesn't matter if the
+          // binding is a Provision or Production, except if it is a @IntoMap method, in which
+          // case the key will be of type Map<K, Provider<V>>, which will be "upgraded" into a
+          // Map<K, Producer<V>> if it's requested in a ProductionComponent. This may result in a
+          // strange error, that the RHS needs to be provided with an @Inject or @Provides
+          // annotated method, but a user should be able to figure out if a @Produces annotation
+          // is needed.
+          // TODO(gak): revisit how we model missing delegates if/when we clean up how we model
+          // binding declarations
+          return provisionBindingFactory.missingDelegate(delegateDeclaration);
+        }
+        // It doesn't matter which of these is selected, since they will later on produce a
+        // duplicate binding error.
+        // TODO(ronshapiro): Once compile-testing has a CompilationResult, add a test which asserts
+        // that a duplicate binding for the RHS does not result in a duplicate binding for the LHS.
+        ContributionBinding explicitDelegate =
+            resolvedDelegate.contributionBindings().iterator().next();
+        switch (explicitDelegate.bindingType()) {
+          case PRODUCTION:
+            return productionBindingFactory.delegate(
+                delegateDeclaration, (ProductionBinding) explicitDelegate);
+          case PROVISION:
+            return provisionBindingFactory.delegate(
+                delegateDeclaration, (ProvisionBinding) explicitDelegate);
+          default:
+            throw new AssertionError("bindingType: " + explicitDelegate);
+        }
+      }
+
       private ImmutableSetMultimap<ComponentDescriptor, ContributionBinding>
           indexBindingsByOwningComponent(
               DependencyRequest request, Iterable<? extends ContributionBinding> bindings) {

compiler/src/main/java/dagger/internal/codegen/BindingGraphValidator.java

@@ -160,7 +160,7 @@ static final class DependencyPath {
 
     /** The entry point. */
     Element entryPointElement() {
-      return path.getFirst().dependencyRequest().requestElement();
+      return path.getFirst().dependencyRequest().requestElement().get();
     }
 
     /** The current dependency request, which is a transitive dependency of the entry point. */
@@ -392,8 +392,7 @@ private void validateResolvedBindings(DependencyPath path) {
             throw new AssertionError(
                 "contribution binding keys should never have members injection bindings");
           }
-          validateNullability(
-              path.currentDependencyRequest(), resolvedBindings.contributionBindings());
+          validateNullability(path, resolvedBindings.contributionBindings());
           if (resolvedBindings.contributionBindings().size() > 1) {
             reportDuplicateBindings(path);
             return;
@@ -517,9 +516,12 @@ private ImmutableListMultimap<ContributionType, BindingDeclaration> declarations
           .build();
     }
 
-    /** Ensures that if the request isn't nullable, then each contribution is also not nullable. */
-    private void validateNullability(DependencyRequest request, Set<ContributionBinding> bindings) {
-      if (request.isNullable()) {
+    /**
+     * Ensures that if the current request isn't nullable, then each contribution is also not
+     * nullable.
+     */
+    private void validateNullability(DependencyPath path, Set<ContributionBinding> bindings) {
+      if (path.currentDependencyRequest().isNullable()) {
         return;
       }
 
@@ -528,16 +530,16 @@ private void validateNullability(DependencyRequest request, Set<ContributionBind
        * (Maybe this happens if the code was already compiled before this point?)
        * ... we manually print out the request in that case, otherwise the error
        * message is kind of useless. */
-      String typeName = TypeName.get(request.key().type()).toString();
+      String typeName = TypeName.get(path.currentDependencyRequest().key().type()).toString();
 
       for (ContributionBinding binding : bindings) {
         if (binding.nullableType().isPresent()) {
           reportBuilder.addItem(
               nullableToNonNullable(typeName, bindingDeclarationFormatter.format(binding))
                   + "\n at: "
-                  + dependencyRequestFormatter.format(request),
+                  + dependencyRequestFormatter.toDependencyTrace(path),
               compilerOptions.nullableValidationKind(),
-              request.requestElement());
+              path.entryPointElement());
         }
       }
     }
@@ -583,10 +585,9 @@ private void validateMapKeyAnnotationTypes(
       }
     }
 
-    /**
-     * Reports errors if a members injection binding is invalid.
-     */
-    private void validateMembersInjectionBinding(Binding binding, final DependencyPath path) {
+    /** Reports errors if a members injection binding is invalid. */
+    private void validateMembersInjectionBinding(
+        final MembersInjectionBinding binding, final DependencyPath path) {
       binding
           .key()
           .type()
@@ -595,8 +596,7 @@ private void validateMembersInjectionBinding(Binding binding, final DependencyPa
                 @Override
                 protected Void defaultAction(TypeMirror e, Void p) {
                   reportBuilder.addError(
-                      "Invalid members injection request.",
-                      path.currentDependencyRequest().requestElement());
+                      "Invalid members injection request.", binding.membersInjectedType());
                   return null;
                 }
 
@@ -1180,7 +1180,7 @@ private ImmutableSet<DependencyRequest> providersBreakingCycle(
       return FluentIterable.from(cycle)
           .skip(1)
           .transform(ResolvedRequest.DEPENDENCY_REQUEST)
-          .filter(not(DependencyRequest.IS_SYNTHETIC))
+          .filter(DependencyRequest.HAS_REQUEST_ELEMENT)
           .filter(
               new Predicate<DependencyRequest>() {
                 @Override

compiler/src/main/java/dagger/internal/codegen/ContributionBinding.java

@@ -20,7 +20,6 @@
 import static dagger.internal.codegen.ContributionBinding.FactoryCreationStrategy.CLASS_CONSTRUCTOR;
 import static dagger.internal.codegen.ContributionBinding.FactoryCreationStrategy.DELEGATE;
 import static dagger.internal.codegen.ContributionBinding.FactoryCreationStrategy.ENUM_INSTANCE;
-import static dagger.internal.codegen.ContributionBinding.Kind.IS_SYNTHETIC_KIND;
 import static dagger.internal.codegen.MapKeys.unwrapValue;
 import static dagger.internal.codegen.MoreAnnotationMirrors.unwrapOptionalEquivalence;
 
@@ -33,7 +32,6 @@
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Multimaps;
-import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import dagger.Component;
@@ -59,28 +57,9 @@
  */
 abstract class ContributionBinding extends Binding implements HasContributionType {
 
-  @Override
-  Set<DependencyRequest> implicitDependencies() {
-    // Optimization: If we don't need the memberInjectionRequest, don't create more objects.
-    if (!membersInjectionRequest().isPresent()) {
-      return dependencies();
-    } else {
-      // Optimization: Avoid creating an ImmutableSet+Builder just to union two things together.
-      return Sets.union(membersInjectionRequest().asSet(), dependencies());
-    }
-  }
-
   /** Returns the type that specifies this' nullability, absent if not nullable. */
   abstract Optional<DeclaredType> nullableType();
 
-  /**
-   * Returns whether this binding is synthetic, i.e., not explicitly tied to code, but generated
-   * implicitly by the framework.
-   */
-  boolean isSyntheticBinding() {
-    return IS_SYNTHETIC_KIND.apply(bindingKind());
-  }
-
   /**
    * A function that returns the kind of a binding.
    */
@@ -92,9 +71,6 @@ public Kind apply(ContributionBinding binding) {
         }
       };
 
-  /** If this provision requires members injection, this will be the corresponding request. */
-  abstract Optional<DependencyRequest> membersInjectionRequest();
-
   abstract Optional<Equivalence.Wrapper<AnnotationMirror>> wrappedMapKey();
 
   final Optional<AnnotationMirror> mapKey() {
@@ -166,17 +142,6 @@ enum Kind {
     COMPONENT_PRODUCTION,
     ;
 
-    /**
-     * A predicate that tests whether a kind is for synthetic bindings.
-     */
-    static final Predicate<Kind> IS_SYNTHETIC_KIND =
-        Predicates.in(
-            immutableEnumSet(
-                SYNTHETIC_MAP,
-                SYNTHETIC_MULTIBOUND_SET,
-                SYNTHETIC_MULTIBOUND_MAP,
-                SYNTHETIC_DELEGATE_BINDING));
-
     /**
      * A predicate that tests whether a kind is for synthetic multibindings.
      */
@@ -320,8 +285,6 @@ abstract static class Builder<B extends Builder<B>> {
 
     abstract B nullableType(Optional<DeclaredType> nullableType);
 
-    abstract B membersInjectionRequest(Optional<DependencyRequest> membersInjectionRequest);
-
     abstract B wrappedMapKey(Optional<Equivalence.Wrapper<AnnotationMirror>> wrappedMapKey);
 
     abstract B bindingKind(ContributionBinding.Kind kind);