Merge pull request square#456 from google/moe_writing_branch_from_fbb684005f94867c54a07e824985836d845ce654

Moe sync 9/6

Author: ronshapiro

compiler/src/it/functional-tests/src/main/java/test/multipackage/MultibindsComponent.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2016 The Dagger Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package test.multipackage;
+
+import dagger.Component;
+import test.multipackage.a.AMultibindsModule;
+import test.multipackage.a.UsesInaccessible;
+
+/**
+ * A component that tests the interaction between multiple packages and {@code @Multibinding}s.
+ * Specifically, we want:
+ *
+ * <ul>
+ * <li>A {@code @Multibinding} for an empty set of a type not accessible from this package.
+ * <li>A {@code @Multibinding} for an empty map of a type not accessible from this package.
+ * <li>A public type that injects the empty set and map of inaccessible objects.
+ * </ul>
+ */
+@Component(modules = {AMultibindsModule.class})
+interface MultibindsComponent {
+  UsesInaccessible usesInaccessible();
+}

compiler/src/it/functional-tests/src/main/java/test/multipackage/a/AModule.java

@@ -25,7 +25,6 @@
 import dagger.multibindings.StringKey;
 import java.util.HashSet;
 import java.util.Set;
-import javax.inject.Inject;
 
 @Module
 public abstract class AModule {
@@ -49,9 +48,4 @@ static Set<Inaccessible> provideSetOfInaccessibles() {
   @IntoMap
   @StringKey("inaccessible")
   abstract Inaccessible provideInaccessibleToMap(Inaccessible inaccessible);
-
-  static class Inaccessible {
-    @Inject Inaccessible() {}
-  }
-
 }

compiler/src/it/functional-tests/src/main/java/test/multipackage/a/AMultibindsModule.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2015 The Dagger Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package test.multipackage.a;
+
+import dagger.Module;
+import dagger.multibindings.Multibinds;
+import java.util.Map;
+import java.util.Set;
+
+/** A module that {@code @Multibinds} a set and a map of {@link Inaccessible}. */
+@Module
+public abstract class AMultibindsModule {
+  @Multibinds
+  abstract Set<Inaccessible> inaccessibleSet();
+
+  @Multibinds
+  abstract Map<String, Inaccessible> inaccessibleMap();
+}

compiler/src/it/functional-tests/src/main/java/test/multipackage/a/Inaccessible.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2016 The Dagger Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package test.multipackage.a;
+
+import javax.inject.Inject;
+
+final class Inaccessible {
+  @Inject Inaccessible() {}
+}

compiler/src/it/functional-tests/src/main/java/test/multipackage/a/UsesInaccessible.java

@@ -19,7 +19,6 @@
 import java.util.Map;
 import java.util.Set;
 import javax.inject.Inject;
-import test.multipackage.a.AModule.Inaccessible;
 
 @SuppressWarnings("unused")
 public class UsesInaccessible {

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

@@ -23,6 +23,7 @@
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.squareup.javapoet.MethodSpec.constructorBuilder;
 import static com.squareup.javapoet.MethodSpec.methodBuilder;
+import static com.squareup.javapoet.TypeSpec.classBuilder;
 import static dagger.internal.codegen.AbstractComponentWriter.InitializationState.DELEGATED;
 import static dagger.internal.codegen.AbstractComponentWriter.InitializationState.INITIALIZED;
 import static dagger.internal.codegen.AbstractComponentWriter.InitializationState.UNINITIALIZED;
@@ -140,13 +141,15 @@ abstract class AbstractComponentWriter {
   protected final BindingGraph graph;
   protected final ImmutableMap<ComponentDescriptor, String> subcomponentNames;
   private final Map<BindingKey, InitializationState> initializationStates = new HashMap<>();
-  protected TypeSpec.Builder component;
+  protected final TypeSpec.Builder component;
   private final UniqueNameSet componentFieldNames = new UniqueNameSet();
   private final Map<BindingKey, MemberSelect> memberSelects = new HashMap<>();
   private final Map<BindingKey, MemberSelect> producerFromProviderMemberSelects = new HashMap<>();
   private final Map<BindingKey, RequestFulfillment> requestFulfillments = Maps.newLinkedHashMap();
   protected final MethodSpec.Builder constructor = constructorBuilder().addModifiers(PRIVATE);
   protected Optional<ClassName> builderName = Optional.absent();
+  private final OptionalFactories optionalFactories;
+  private boolean done;
 
   /**
    * For each component requirement, the builder field. This map is empty for subcomponents that do
@@ -162,28 +165,37 @@ abstract class AbstractComponentWriter {
    */
   protected final Map<TypeElement, MemberSelect> componentContributionFields = Maps.newHashMap();
 
-  /**
-   * The factory classes that implement {@code Provider<Optional<T>>} within the component. If the
-   * key is {@link Optional#absent()}, the class provides absent values.
-   */
-  private final Map<Optional<DependencyRequest.Kind>, TypeSpec> optionalFactoryClasses =
-      new HashMap<>();
-
   AbstractComponentWriter(
       Types types,
       Elements elements,
       Key.Factory keyFactory,
       CompilerOptions compilerOptions,
       ClassName name,
       BindingGraph graph,
-      ImmutableMap<ComponentDescriptor, String> subcomponentNames) {
+      ImmutableMap<ComponentDescriptor, String> subcomponentNames,
+      OptionalFactories optionalFactories) {
     this.types = types;
     this.elements = elements;
     this.keyFactory = keyFactory;
     this.compilerOptions = compilerOptions;
+    this.component = classBuilder(name);
     this.name = name;
     this.graph = graph;
     this.subcomponentNames = subcomponentNames;
+    this.optionalFactories = optionalFactories;
+  }
+
+  protected AbstractComponentWriter(
+      AbstractComponentWriter parent, ClassName name, BindingGraph graph) {
+    this(
+        parent.types,
+        parent.elements,
+        parent.keyFactory,
+        parent.compilerOptions,
+        name,
+        graph,
+        parent.subcomponentNames,
+        parent.optionalFactories);
   }
 
   protected final TypeElement componentDefinitionType() {
@@ -267,22 +279,27 @@ private void setInitializationState(BindingKey bindingKey, InitializationState s
    * component must be regenerated, use a new instance.
    */
   final TypeSpec.Builder write() {
-    checkState(component == null, "ComponentWriter has already been generated.");
-    component = createComponentClass();
+    checkState(!done, "ComponentWriter has already been generated.");
+    decorateComponent();
     addBuilder();
     addFactoryMethods();
     addFrameworkFields();
     initializeFrameworkTypes();
     implementInterfaceMethods();
     addSubcomponents();
     component.addMethod(constructor.build());
+    if (graph.componentDescriptor().kind().isTopLevel()) {
+      optionalFactories.addMembers(component);
+    }
+    done = true;
     return component;
   }
 
   /**
-   * Creates the component implementation class.
+   * Adds Javadoc, modifiers, supertypes, and annotations to the component implementation class
+   * declaration.
    */
-  protected abstract TypeSpec.Builder createComponentClass();
+  protected abstract void decorateComponent();
 
   /**
    * Adds a builder type.
@@ -737,7 +754,7 @@ private void implementInterfaceMethods() {
     }
   }
 
-  public MethodSpec.Builder methodSpecForComponentMethod(
+  private MethodSpec.Builder methodSpecForComponentMethod(
       ExecutableElement method, ExecutableType methodType) {
     String methodName = method.getSimpleName().toString();
     MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(methodName);
@@ -1279,41 +1296,16 @@ private CodeBlock initializeFactoryForSyntheticOptionalBinding(ContributionBindi
     }
 
     if (binding.dependencies().isEmpty()) {
-      return CodeBlock.of(
-          "$N.instance()", optionalFactoryClass(Optional.<DependencyRequest.Kind>absent()));
+      return optionalFactories.absentOptionalProvider();
     } else {
       TypeMirror valueType = OptionalType.from(binding.key()).valueType();
-      DependencyRequest.Kind optionalValueKind =
-          DependencyRequest.extractKindAndType(valueType).kind();
+      DependencyRequest.Kind valueKind = DependencyRequest.extractKindAndType(valueType).kind();
       FrameworkDependency frameworkDependency =
           getOnlyElement(frameworkDependenciesForBinding(binding));
       CodeBlock dependencyArgument =
           getDependencyArgument(frameworkDependency).getExpressionFor(name);
-      return CodeBlock.of(
-          "$N.of($L)", optionalFactoryClass(Optional.of(optionalValueKind)), dependencyArgument);
-    }
-  }
-
-  /**
-   * Returns the nested class that implements {@code Provider<Optional<T>>} for optional bindings.
-   * Adds it to the root component if it hasn't already been added.
-   *
-   * <p>If {@code optionalValueKind} is absent, returns a {@link Provider} class that returns {@link
-   * Optional#absent()}.
-   *
-   * <p>If {@code optionalValueKind} is present, returns a {@link Provider} class where {@code T}
-   * represents dependency requests of that kind.
-   */
-  protected TypeSpec optionalFactoryClass(Optional<DependencyRequest.Kind> optionalValueKind) {
-    if (!optionalFactoryClasses.containsKey(optionalValueKind)) {
-      TypeSpec factory =
-          optionalValueKind.isPresent()
-              ? OptionalFactoryClasses.presentFactoryClass(optionalValueKind.get())
-              : OptionalFactoryClasses.ABSENT_FACTORY_CLASS;
-      component.addType(factory);
-      optionalFactoryClasses.put(optionalValueKind, factory);
+      return optionalFactories.presentOptionalProvider(valueKind, dependencyArgument);
     }
-    return optionalFactoryClasses.get(optionalValueKind);
   }
 
   private static String simpleVariableName(TypeElement typeElement) {

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

@@ -64,7 +64,8 @@ final class ComponentWriter extends AbstractComponentWriter {
         compilerOptions,
         name,
         graph,
-        new UniqueSubcomponentNamesGenerator(graph).generate());
+        new UniqueSubcomponentNamesGenerator(graph).generate(),
+        new OptionalFactories());
   }
 
   /**
@@ -142,10 +143,9 @@ private static ImmutableListMultimap<ComponentDescriptor, String> qualifiedNames
   }
 
   @Override
-  protected TypeSpec.Builder createComponentClass() {
-    TypeSpec.Builder component = classBuilder(name).addModifiers(PUBLIC, FINAL);
+  protected void decorateComponent() {
+    component.addModifiers(PUBLIC, FINAL);
     addSupertype(component, componentDefinitionType());
-    return component;
   }
 
   @Override

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

@@ -26,7 +26,6 @@
 import static dagger.internal.codegen.TypeNames.MAP_PROVIDER_FACTORY;
 import static dagger.internal.codegen.TypeNames.MEMBERS_INJECTOR;
 import static dagger.internal.codegen.TypeNames.MEMBERS_INJECTORS;
-import static dagger.internal.codegen.TypeNames.SET;
 
 import com.google.common.collect.ImmutableList;
 import com.squareup.javapoet.ClassName;
@@ -144,10 +143,7 @@ static MemberSelect emptyFrameworkMapFactory(
    */
   static MemberSelect emptySetProvider(ClassName setFactoryType, SetType setType) {
     return new ParameterizedStaticMethod(
-        setFactoryType,
-        ImmutableList.of(setType.elementType()),
-        CodeBlock.of("empty()"),
-        SET);
+        setFactoryType, ImmutableList.of(setType.elementType()), CodeBlock.of("empty()"), FACTORY);
   }
 
   private static final class ParameterizedStaticMethod extends MemberSelect {