Merge pull request google#471 from google/moe_sync_2016_09_26

MOE sync

compiler/pom.xml

@@ -65,14 +65,13 @@
     <dependency>
       <groupId>com.google.auto.service</groupId>
       <artifactId>auto-service</artifactId>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
-
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
@@ -81,9 +80,9 @@
     <dependency>
       <groupId>${project.groupId}</groupId>
       <artifactId>dagger</artifactId>
+      <classifier>tests</classifier>
       <version>${project.version}</version>
       <scope>test</scope>
-      <classifier>tests</classifier>
     </dependency>
     <dependency>
       <groupId>com.google.testing.compile</groupId>
@@ -98,7 +97,6 @@
     <dependency>
       <groupId>com.squareup</groupId>
       <artifactId>javapoet</artifactId>
-      <version>1.7.0</version>
     </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
@@ -158,10 +156,6 @@
             <pomInclude>*/pom.xml</pomInclude>
           </pomIncludes>
           <localRepositoryPath>${project.build.directory}/it-repo</localRepositoryPath>
-          <filterProperties>
-            <dagger.version>${project.version}</dagger.version>
-            <dagger.groupId>${project.groupId}</dagger.groupId>
-          </filterProperties>
           <streamLogs>true</streamLogs>
         </configuration>
         <executions>

compiler/src/it/functional-tests/pom.xml

@@ -46,14 +46,12 @@ limitations under the License.
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>${auto.value.version}</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>com.google.auto.factory</groupId>
       <artifactId>auto-factory</artifactId>
-      <version>${auto.factory.version}</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
 
     <dependency>

compiler/src/it/guava-functional-tests/pom.xml

@@ -42,14 +42,12 @@ limitations under the License.
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>${auto.value.version}</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>com.google.auto.factory</groupId>
       <artifactId>auto-factory</artifactId>
-      <version>${auto.factory.version}</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
 
     <dependency>

compiler/src/it/producers-functional-tests/pom.xml

@@ -51,8 +51,7 @@ limitations under the License.
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>${auto.value.version}</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>junit</groupId>

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

@@ -579,7 +579,7 @@ private Optional<MemberSelect> staticMemberSelect(ResolvedBindings resolvedBindi
               MapType mapType = MapType.from(contributionBinding.key());
               return Optional.of(
                   emptyFrameworkMapFactory(
-                      frameworkMapFactoryClassName(bindingType),
+                      bindingType,
                       mapType.keyType(),
                       mapType.unwrappedValueType(bindingType.frameworkClass())));
 
@@ -1283,10 +1283,6 @@ private CodeBlock initializeFactoryForMapMultibinding(ContributionBinding bindin
               useRawTypes,
               frameworkDependency.frameworkClass(),
               getDependencyArgument(frameworkDependency).getExpressionFor(name));
-      if (binding.bindingType().frameworkClass().equals(Producer.class)
-          && frameworkDependency.frameworkClass().equals(Provider.class)) {
-        value = CodeBlock.of("$T.producerFromProvider($L)", PRODUCERS, value);
-      }
       codeBlocks.add(
           CodeBlock.of(
               ".put($L, $L)", getMapKeyExpression(contributionBinding.mapKey().get()), value));

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

@@ -183,6 +183,19 @@ ResolvedBindings previousResolvedBindings() {
       return Iterators.get(path.descendingIterator(), 1).resolvedBindings();
     }
 
+    /**
+     * Returns the contribution bindings resolved for the second-most-recent request in the given
+     * path; that is, returns those bindings that depend on the latest request in the path.
+     */
+    FluentIterable<ContributionBinding> contributionsDependingOnLatestRequest() {
+      if (size() <= 1) {
+        return FluentIterable.from(ImmutableList.<ContributionBinding>of());
+      }
+      return FluentIterable.from(previousResolvedBindings().bindings())
+          .filter(binding -> binding.implicitDependencies().contains(currentDependencyRequest()))
+          .filter(ContributionBinding.class);
+    }
+
     /**
      * {@code true} if there is a dependency cycle, which means that the
      * {@linkplain #currentDependencyRequest() current request}'s binding key occurs earlier in the
@@ -347,14 +360,18 @@ private void traverseDependencyRequest(DependencyRequest request, DependencyPath
           validateResolvedBindings(path);
 
           // Validate all dependencies within the component that owns the binding.
-          for (Map.Entry<ComponentDescriptor, ? extends Binding> entry :
-              path.currentResolvedBindings().bindingsByComponent()) {
-            Validation validation = validationForComponent(entry.getKey());
-            Binding binding = entry.getValue();
-            for (DependencyRequest nextRequest : binding.implicitDependencies()) {
-              validation.traverseDependencyRequest(nextRequest, path);
-            }
-          }
+          path.currentResolvedBindings()
+              .allBindings()
+              .asMap()
+              .forEach(
+                  (component, bindings) -> {
+                    Validation validation = validationForComponent(component);
+                    for (Binding binding : bindings) {
+                      for (DependencyRequest nextRequest : binding.implicitDependencies()) {
+                        validation.traverseDependencyRequest(nextRequest, path);
+                      }
+                    }
+                  });
         }
       } finally {
         path.pop();
@@ -418,9 +435,10 @@ private void validateResolvedBindings(DependencyPath path) {
           }
           if (contributionBinding.bindingType().equals(PRODUCTION)
               && doesPathRequireProvisionOnly(path)) {
-            reportProviderMayNotDependOnProducer(path);
+            reportProviderMayNotDependOnProducer(path, contributionBinding);
             return;
           }
+          // TODO(dpb,beder): Validate this during @Inject/@Provides/@Produces validation.
           if (compilerOptions.usesProducers()) {
             Key productionImplementationExecutorKey =
                 keyFactory.forProductionImplementationExecutor();
@@ -553,12 +571,13 @@ private void validateNullability(DependencyPath path, Set<ContributionBinding> b
 
       for (ContributionBinding binding : bindings) {
         if (binding.nullableType().isPresent()) {
-          reportBuilder.addItem(
-              nullableToNonNullable(typeName, bindingDeclarationFormatter.format(binding))
-                  + "\n at: "
-                  + dependencyRequestFormatter.toDependencyTrace(path),
-              compilerOptions.nullableValidationKind(),
-              path.entryPointElement());
+          owningReportBuilder(path.contributionsDependingOnLatestRequest().append(binding))
+              .addItem(
+                  nullableToNonNullable(typeName, bindingDeclarationFormatter.format(binding))
+                      + "\n at: "
+                      + dependencyRequestFormatter.toDependencyTrace(path),
+                  compilerOptions.nullableValidationKind(),
+                  path.entryPointElement());
         }
       }
     }
@@ -605,6 +624,7 @@ private void validateMapKeyAnnotationTypes(
     }
 
     /** Reports errors if a members injection binding is invalid. */
+    // TODO(dpb): Can this be done while validating @Inject?
     private void validateMembersInjectionBinding(
         final MembersInjectionBinding binding, final DependencyPath path) {
       binding
@@ -967,24 +987,26 @@ void validateComponentScope() {
 
     @SuppressWarnings("resource") // Appendable is a StringBuilder.
     // TODO(b/29509141): Clarify the error.
-    private void reportProviderMayNotDependOnProducer(DependencyPath path) {
-      StringBuilder errorMessage = new StringBuilder();
+    private void reportProviderMayNotDependOnProducer(
+        DependencyPath path, ContributionBinding productionBinding) {
       if (path.size() == 1) {
-        new Formatter(errorMessage)
-            .format(
+        reportBuilder.addError(
+            String.format(
                 PROVIDER_ENTRY_POINT_MAY_NOT_DEPEND_ON_PRODUCER_FORMAT,
-                formatCurrentDependencyRequestKey(path));
+                formatCurrentDependencyRequestKey(path)),
+            path.entryPointElement());
       } else {
         FluentIterable<ContributionBinding> dependentProvisions =
             provisionsDependingOnLatestRequest(path);
         // TODO(beder): Consider displaying all dependent provisions in the error message. If we
         // do that, should we display all productions that depend on them also?
-        new Formatter(errorMessage)
-            .format(
-                PROVIDER_MAY_NOT_DEPEND_ON_PRODUCER_FORMAT,
-                dependentProvisions.iterator().next().key());
+        owningReportBuilder(dependentProvisions.append(productionBinding))
+            .addError(
+                String.format(
+                    PROVIDER_MAY_NOT_DEPEND_ON_PRODUCER_FORMAT,
+                    dependentProvisions.iterator().next().key()),
+                path.entryPointElement());
       }
-      reportBuilder.addError(errorMessage.toString(), path.entryPointElement());
     }
 
     /**
@@ -1033,7 +1055,7 @@ private void reportMissingBinding(DependencyPath path) {
               topLevelGraph(), path.currentDependencyRequest().bindingKey())) {
         errorMessage.append('\n').append(suggestion);
       }
-      reportBuilder.addError(errorMessage.toString(), path.entryPointElement());
+      topLevelReport().addError(errorMessage.toString(), path.entryPointElement());
     }
 
     @SuppressWarnings("resource") // Appendable is a StringBuilder.
@@ -1070,13 +1092,13 @@ private void reportDuplicateBindings(DependencyPath path) {
     }
 
     /**
-     * Returns the report builder for the rootmost component that contains any of the duplicate
-     * bindings.
+     * Returns the report builder for the rootmost component that contains any of the {@code
+     * bindings}.
      */
     private ValidationReport.Builder<TypeElement> owningReportBuilder(
-        Iterable<ContributionBinding> duplicateBindings) {
+        Iterable<ContributionBinding> bindings) {
       ImmutableSet.Builder<ComponentDescriptor> owningComponentsBuilder = ImmutableSet.builder();
-      for (ContributionBinding binding : duplicateBindings) {
+      for (ContributionBinding binding : bindings) {
         ResolvedBindings resolvedBindings =
             subject.resolvedBindings().get(BindingKey.contribution(binding.key()));
         owningComponentsBuilder.addAll(
@@ -1088,8 +1110,7 @@ private ValidationReport.Builder<TypeElement> owningReportBuilder(
           return validation.reportBuilder;
         }
       }
-      throw new AssertionError(
-          "cannot find owning component for duplicate bindings: " + duplicateBindings);
+      throw new AssertionError("cannot find owning component for bindings: " + bindings);
     }
 
     /**
@@ -1128,15 +1149,16 @@ private void reportMultipleContributionTypes(DependencyPath path) {
             builder, declarationsByType.get(contributionType), 2, DUPLICATE_SIZE_LIMIT);
         builder.append('\n');
       }
-      reportBuilder.addError(builder.toString(), path.entryPointElement());
+      owningReportBuilder(resolvedBindings.contributionBindings())
+          .addError(builder.toString(), path.entryPointElement());
     }
 
     private void reportDuplicateMapKeys(
         DependencyPath path, Collection<ContributionBinding> mapBindings) {
       StringBuilder builder = new StringBuilder();
       builder.append(duplicateMapKeysError(formatCurrentDependencyRequestKey(path)));
       bindingDeclarationFormatter.formatIndentedList(builder, mapBindings, 1, DUPLICATE_SIZE_LIMIT);
-      reportBuilder.addError(builder.toString(), path.entryPointElement());
+      owningReportBuilder(mapBindings).addError(builder.toString(), path.entryPointElement());
     }
 
     private void reportInconsistentMapKeyAnnotations(
@@ -1159,24 +1181,25 @@ private void reportInconsistentMapKeyAnnotations(
 
         bindingDeclarationFormatter.formatIndentedList(builder, bindings, 2, DUPLICATE_SIZE_LIMIT);
       }
-      reportBuilder.addError(builder.toString(), path.entryPointElement());
+      owningReportBuilder(mapBindingsByAnnotationType.values())
+          .addError(builder.toString(), path.entryPointElement());
     }
 
     private void reportCycle(DependencyPath path) {
       if (!providersBreakingCycle(path).isEmpty()) {
         return;
       }
       // TODO(cgruber): Provide a hint for the start and end of the cycle.
-      TypeElement componentType =
-          MoreElements.asType(path.entryPointElement().getEnclosingElement());
-      reportBuilder.addItem(
-          String.format(
-              CONTAINS_DEPENDENCY_CYCLE_FORMAT,
-              componentType.getQualifiedName(),
-              path.entryPointElement().getSimpleName(),
-              dependencyRequestFormatter.toDependencyTrace(path)),
-          ERROR,
-          path.entryPointElement());
+      owningReportBuilder(
+              path.cycle()
+                  .transform(ResolvedRequest::resolvedBindings)
+                  .transformAndConcat(ResolvedBindings::contributionBindings))
+          .addItem(
+              String.format(
+                  CONTAINS_DEPENDENCY_CYCLE_FORMAT,
+                  dependencyRequestFormatter.toDependencyTrace(path)),
+              ERROR,
+              path.entryPointElement());
     }
 
     /**
@@ -1299,11 +1322,8 @@ private boolean doesPathRequireProvisionOnly(DependencyPath path) {
    * that is, returns those provision bindings that depend on the latest request in the path.
    */
   private FluentIterable<ContributionBinding> provisionsDependingOnLatestRequest(
-      final DependencyPath path) {
-    return FluentIterable.from(path.previousResolvedBindings().bindings())
-        .filter(PROVISION::isOfType)
-        .filter(binding -> binding.implicitDependencies().contains(path.currentDependencyRequest()))
-        .filter(ContributionBinding.class);
+      DependencyPath path) {
+    return path.contributionsDependingOnLatestRequest().filter(PROVISION::isOfType);
   }
 
   private String formatContributionType(ContributionType type) {

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

@@ -288,7 +288,7 @@ static String provisionMayNotDependOnProducerType(TypeMirror type) {
   static final String MEMBERS_INJECTION_WITH_UNBOUNDED_TYPE =
       "Type parameters must be bounded for members injection. %s required by %s, via:\n%s";
 
-  static final String CONTAINS_DEPENDENCY_CYCLE_FORMAT = "%s.%s() contains a dependency cycle:\n%s";
+  static final String CONTAINS_DEPENDENCY_CYCLE_FORMAT = "Found a dependency cycle:\n%s";
 
   static String nullableToNonNullable(String typeName, String bindingString) {
     return String.format(