Navigation Menu

Skip to content

Commit

Permalink
Prune modules that do not provide bindings that require a module inst…
Browse files Browse the repository at this point in the history 
…ance. The methods that would set those modules remain on the builder, but are @deprecated no-op methods.

-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=103132967
  • Loading branch information
@gk5885 @cgruber
gk5885 authored and cgruber committed Oct 21, 2015
1 parent fb7a9d4 commit 9b81e10
Show file tree
Hide file tree
Showing 12 changed files with 302 additions and 238 deletions.
26 changes: 23 additions & 3 deletions compiler/src/it/functional-tests/src/test/java/test/staticprovides/StaticProvidesTest.java
View file
Expand Up @@ -15,14 +15,17 @@
*/
package test.staticprovides;

import static com.google.common.truth.Truth.assertThat;

import com.google.common.collect.ImmutableSet;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Arrays;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;

@RunWith(JUnit4.class)
public class StaticProvidesTest {
private final StaticTestComponent component = DaggerStaticTestComponent.create();
Expand All @@ -33,4 +36,21 @@ public class StaticProvidesTest {
SomeStaticModule.class + ".contributeStringFromAStaticMethod",
SomeStaticModule.class + ".contributeStringFromAnInstanceMethod"));
}

@Test public void allStaticProvidesModules_noFieldInComponentBuilder() {
for (Field field : DaggerStaticTestComponent.Builder.class.getDeclaredFields()) {
assertWithMessage(field.getName())
.that(field.getType()).isNotEqualTo(AllStaticModule.class);
}
}

@Test public void allStaticProvidesModules_deprecatedMethodInComponentBuilder() {
for (Method method : DaggerStaticTestComponent.Builder.class.getDeclaredMethods()) {
if (Arrays.asList(method.getParameterTypes()).contains(AllStaticModule.class)) {
assertWithMessage(method.getName())
.that(method.isAnnotationPresent(Deprecated.class))
.isTrue();
}
}
}
}
40 changes: 18 additions & 22 deletions compiler/src/it/producers-functional-tests/src/test/java/test/ProducerFactoryTest.java
View file
Expand Up @@ -15,14 +15,23 @@
*/
package test;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;

import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;

import dagger.producers.Producer;
import dagger.producers.monitoring.ProducerMonitor;
import dagger.producers.monitoring.ProducerToken;
import dagger.producers.monitoring.ProductionComponentMonitor;
import java.util.concurrent.ExecutionException;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -31,13 +40,7 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import java.util.concurrent.ExecutionException;

@RunWith(JUnit4.class)
public class ProducerFactoryTest {
Expand All @@ -53,10 +56,9 @@ public void setUpMocks() {
@Test
public void noArgMethod() throws Exception {
ProducerToken token = ProducerToken.create(SimpleProducerModule_StrFactory.class);
SimpleProducerModule module = new SimpleProducerModule();
Producer<String> producer =
new SimpleProducerModule_StrFactory(
componentMonitor, module, MoreExecutors.directExecutor());
componentMonitor, MoreExecutors.directExecutor());
assertThat(producer.get().get()).isEqualTo("str");
InOrder order = inOrder(componentMonitor, monitor);
order.verify(componentMonitor).producerMonitorFor(token);
Expand All @@ -67,12 +69,11 @@ public void noArgMethod() throws Exception {
}

@Test public void singleArgMethod() throws Exception {
SimpleProducerModule module = new SimpleProducerModule();
SettableFuture<Integer> intFuture = SettableFuture.create();
Producer<Integer> intProducer = producerOfFuture(intFuture);
Producer<String> producer =
new SimpleProducerModule_StrWithArgFactory(
componentMonitor, module, MoreExecutors.directExecutor(), intProducer);
componentMonitor, MoreExecutors.directExecutor(), intProducer);
assertThat(producer.get().isDone()).isFalse();
intFuture.set(42);
assertThat(producer.get().get()).isEqualTo("str with arg");
Expand All @@ -82,13 +83,12 @@ public void noArgMethod() throws Exception {
public void successMonitor() throws Exception {
ProducerToken token = ProducerToken.create(SimpleProducerModule_SettableFutureStrFactory.class);

SimpleProducerModule module = new SimpleProducerModule();
SettableFuture<String> strFuture = SettableFuture.create();
SettableFuture<SettableFuture<String>> strFutureFuture = SettableFuture.create();
Producer<SettableFuture<String>> strFutureProducer = producerOfFuture(strFutureFuture);
Producer<String> producer =
new SimpleProducerModule_SettableFutureStrFactory(
componentMonitor, module, MoreExecutors.directExecutor(), strFutureProducer);
componentMonitor, MoreExecutors.directExecutor(), strFutureProducer);
assertThat(producer.get().isDone()).isFalse();

InOrder order = inOrder(componentMonitor, monitor);
Expand All @@ -110,13 +110,12 @@ public void successMonitor() throws Exception {
public void failureMonitor() throws Exception {
ProducerToken token = ProducerToken.create(SimpleProducerModule_SettableFutureStrFactory.class);

SimpleProducerModule module = new SimpleProducerModule();
SettableFuture<String> strFuture = SettableFuture.create();
SettableFuture<SettableFuture<String>> strFutureFuture = SettableFuture.create();
Producer<SettableFuture<String>> strFutureProducer = producerOfFuture(strFutureFuture);
Producer<String> producer =
new SimpleProducerModule_SettableFutureStrFactory(
componentMonitor, module, MoreExecutors.directExecutor(), strFutureProducer);
componentMonitor, MoreExecutors.directExecutor(), strFutureProducer);
assertThat(producer.get().isDone()).isFalse();

InOrder order = inOrder(componentMonitor, monitor);
Expand Down Expand Up @@ -144,10 +143,9 @@ public void failureMonitor() throws Exception {
public void failureMonitorDueToThrowingProducer() throws Exception {
ProducerToken token = ProducerToken.create(SimpleProducerModule_ThrowingProducerFactory.class);

SimpleProducerModule module = new SimpleProducerModule();
Producer<String> producer =
new SimpleProducerModule_ThrowingProducerFactory(
componentMonitor, module, MoreExecutors.directExecutor());
componentMonitor, MoreExecutors.directExecutor());
assertThat(producer.get().isDone()).isTrue();

InOrder order = inOrder(componentMonitor, monitor);
Expand All @@ -168,9 +166,8 @@ public void failureMonitorDueToThrowingProducer() throws Exception {

@Test
public void nullComponentMonitor() throws Exception {
SimpleProducerModule module = new SimpleProducerModule();
Producer<String> producer =
new SimpleProducerModule_StrFactory(null, module, MoreExecutors.directExecutor());
new SimpleProducerModule_StrFactory(null, MoreExecutors.directExecutor());
assertThat(producer.get().get()).isEqualTo("str");
verifyZeroInteractions(componentMonitor, monitor);
}
Expand All @@ -180,10 +177,9 @@ public void nullMonitor() throws Exception {
when(componentMonitor.producerMonitorFor(any(ProducerToken.class))).thenReturn(null);

ProducerToken token = ProducerToken.create(SimpleProducerModule_StrFactory.class);
SimpleProducerModule module = new SimpleProducerModule();
Producer<String> producer =
new SimpleProducerModule_StrFactory(
componentMonitor, module, MoreExecutors.directExecutor());
componentMonitor, MoreExecutors.directExecutor());
assertThat(producer.get().get()).isEqualTo("str");
verify(componentMonitor).producerMonitorFor(token);
verifyZeroInteractions(monitor);
Expand Down
43 changes: 32 additions & 11 deletions compiler/src/main/java/dagger/internal/codegen/AbstractComponentWriter.java
View file
Expand Up @@ -110,6 +110,7 @@
import static dagger.internal.codegen.Util.getProvidedValueTypeOfMap;
import static dagger.internal.codegen.Util.isMapWithNonProvidedValues;
import static dagger.internal.codegen.writer.Snippet.memberSelectSnippet;
import static dagger.internal.codegen.writer.Snippet.nullCheck;
import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.PUBLIC;
Expand Down Expand Up @@ -226,7 +227,7 @@ protected MemberSelect getMemberSelect(BindingKey key) {
protected Optional<MemberSelect> getMultibindingContributionSnippet(ContributionBinding binding) {
return Optional.fromNullable(multibindingContributionSnippets.get(binding));
}

/**
* Returns the initialization state of the factory field for a binding key in this component.
*/
Expand Down Expand Up @@ -303,8 +304,16 @@ protected void addBuilder() {
constructorWriter.addParameter(builderWriter, "builder");
constructorWriter.body().addSnippet("assert builder != null;");

builderFields = addBuilderMethods(builderWriter, builderSpec, buildMethod);
buildMethod.body().addSnippet("return new %s(this);", name);
}

private ImmutableMap<TypeElement, FieldWriter> addBuilderMethods(
ClassWriter builderWriter, Optional<BuilderSpec> builderSpec, MethodWriter buildMethod) {
ImmutableMap.Builder<TypeElement, FieldWriter> builderFieldsBuilder = ImmutableMap.builder();
for (TypeElement contributionElement : graph.componentRequirements()) {
ImmutableSet<TypeElement> componentRequirements = graph.componentRequirements();

for (TypeElement contributionElement : componentRequirements) {
String contributionName = simpleVariableName(contributionElement);
FieldWriter builderField = builderWriter.addField(contributionElement, contributionName);
builderField.addModifiers(PRIVATE);
Expand Down Expand Up @@ -344,10 +353,7 @@ protected void addBuilder() {
builderMethod.addParameter(contributionElement, contributionName);
builderMethod
.body()
.addSnippet("if (%s == null) {", contributionName)
.addSnippet(
" throw new NullPointerException(%s);", StringLiteral.forValue(contributionName))
.addSnippet("}")
.addSnippet(nullCheck(contributionName))
.addSnippet("this.%s = %s;", builderField.name(), contributionName);
if (!builderMethod.returnType().equals(VoidName.VOID)) {
builderMethod.body().addSnippet("return this;");
Expand All @@ -359,7 +365,7 @@ protected void addBuilder() {
* component requirements that are in the builder spec but _not_ owned by the component must
* be inherited. */
for (TypeElement inheritedRequirement :
Sets.difference(builderSpec.get().methodMap().keySet(), graph.componentRequirements())) {
Sets.difference(builderSpec.get().methodMap().keySet(), componentRequirements)) {
MethodWriter builderMethod =
addBuilderMethodFromSpec(
builderWriter, builderSpec.get().methodMap().get(inheritedRequirement));
Expand All @@ -375,10 +381,23 @@ protected void addBuilder() {
"%s cannot be set because it is inherited from the enclosing component"),
ClassName.fromTypeElement(inheritedRequirement));
}
} else {
for (TypeElement ownedButNotRequired :
Sets.difference(graph.ownedModuleTypes(), componentRequirements)) {
String contributionName = simpleVariableName(ownedButNotRequired);
MethodWriter builderMethod =
builderWriter.addMethod(builderWriter, contributionName);
builderMethod.addModifiers(PUBLIC);
builderMethod.annotate(Deprecated.class);
builderMethod.addParameter(ownedButNotRequired, contributionName);
builderMethod.body()
.addSnippet("// This module is declared, but not used in the component. "
+ "This method is a no-op")
.addSnippet(nullCheck(contributionName))
.addSnippet("return this;");
}
}

builderFields = builderFieldsBuilder.build();
buildMethod.body().addSnippet("return new %s(this);", name);
return builderFieldsBuilder.build();
}

private MethodWriter addBuilderMethodFromSpec(
Expand Down Expand Up @@ -966,7 +985,9 @@ private Snippet initializeFactoryForProductionBinding(ProductionBinding binding)
Lists.newArrayListWithCapacity(binding.dependencies().size() + 3);
// TODO(beder): Pass the actual ProductionComponentMonitor.
parameters.add(Snippet.format("null"));
parameters.add(getComponentContributionSnippet(binding.bindingTypeElement()));
if (!binding.bindingElement().getModifiers().contains(STATIC)) {
parameters.add(getComponentContributionSnippet(binding.bindingTypeElement()));
}
parameters.add(
getComponentContributionSnippet(
graph.componentDescriptor().executorDependency().get()));
Expand Down
45 changes: 40 additions & 5 deletions compiler/src/main/java/dagger/internal/codegen/BindingGraph.java
View file
Expand Up @@ -18,6 +18,7 @@
import com.google.auto.common.MoreTypes;
import com.google.auto.value.AutoValue;
import com.google.common.base.Equivalence;
import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
Expand All @@ -29,6 +30,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.collect.TreeTraverser;
import dagger.Component;
import dagger.Subcomponent;
import dagger.internal.codegen.ComponentDescriptor.ComponentMethodDescriptor;
Expand All @@ -51,13 +53,16 @@
import javax.lang.model.util.Elements;

import static com.google.auto.common.MoreElements.getAnnotationMirror;
import static com.google.common.base.Predicates.in;
import static com.google.common.base.Verify.verify;
import static dagger.internal.codegen.BindingKey.Kind.CONTRIBUTION;
import static dagger.internal.codegen.ComponentDescriptor.isComponentContributionMethod;
import static dagger.internal.codegen.ComponentDescriptor.isComponentProductionMethod;
import static dagger.internal.codegen.ComponentDescriptor.Kind.PRODUCTION_COMPONENT;
import static dagger.internal.codegen.ConfigurationAnnotations.getComponentDependencies;
import static dagger.internal.codegen.MembersInjectionBinding.Strategy.INJECT_MEMBERS;
import static dagger.internal.codegen.MembersInjectionBinding.Strategy.NO_OP;
import static javax.lang.model.element.Modifier.STATIC;

/**
* The canonical representation of a full-resolved graph.
Expand Down Expand Up @@ -86,17 +91,47 @@ ImmutableSet<TypeElement> ownedModuleTypes() {
.toSet();
}

private static final TreeTraverser<BindingGraph> SUBGRAPH_TRAVERSER =
new TreeTraverser<BindingGraph>() {
@Override
public Iterable<BindingGraph> children(BindingGraph node) {
return node.subgraphs().values();
}
};

/**
* Returns the set of types necessary to implement the component, but are not part of the injected
* graph. This includes modules, component dependencies and an {@link Executor} in the case of
* {@link ProductionComponent}.
*/
ImmutableSet<TypeElement> componentRequirements() {
return new ImmutableSet.Builder<TypeElement>()
.addAll(ownedModuleTypes())
.addAll(componentDescriptor().dependencies())
.addAll(componentDescriptor().executorDependency().asSet())
.build();
return SUBGRAPH_TRAVERSER.preOrderTraversal(this)
.transformAndConcat(new Function<BindingGraph, Iterable<ResolvedBindings>>() {
@Override
public Iterable<ResolvedBindings> apply(BindingGraph input) {
return input.resolvedBindings().values();
}
})
.transformAndConcat(new Function<ResolvedBindings, Set<? extends ContributionBinding>>() {
@Override
public Set<? extends ContributionBinding> apply(ResolvedBindings input) {
return (input.bindingKey().kind().equals(CONTRIBUTION))
? input.contributionBindings()
: ImmutableSet.<ContributionBinding>of();
}
})
.transformAndConcat(new Function<ContributionBinding, Set<TypeElement>>() {
@Override
public Set<TypeElement> apply(ContributionBinding input) {
return input.bindingElement().getModifiers().contains(STATIC)
? ImmutableSet.<TypeElement>of()
: input.contributedBy().asSet();
}
})
.filter(in(ownedModuleTypes()))
.append(componentDescriptor().dependencies())
.append(componentDescriptor().executorDependency().asSet())
.toSet();
}

ImmutableSet<TypeElement> availableDependencies() {
Expand Down

0 comments on commit 9b81e10

Please sign in to comment.