Skip to content

Commit

Permalink
Reuse the Default root generated components for test roots in the sam…
Browse files Browse the repository at this point in the history
…e compilation unit that do not have test-specific bindings, rather than generating new components per-test. This can significantly reduce the amount of generated code for large projects with many test classes that do not provide test-specific bindings.

This is currently off-by-default behind a compiler flag. This may be turned on by default in a future release.

RELNOTES=Only generate and compile a single _HiltComponents class for test roots in the same compilation unit that do not have test-specific bindings, if flag enabled.
PiperOrigin-RevId: 366281911
  • Loading branch information
groakley authored and Dagger Team committed Apr 1, 2021
1 parent efcdbe1 commit faebc3c
Show file tree
Hide file tree
Showing 23 changed files with 973 additions and 40 deletions.
1 change: 1 addition & 0 deletions java/dagger/hilt/processor/internal/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ java_library(
"ComponentNames.java",
],
deps = [
":classnames",
":processors",
"@google_bazel_common//third_party/java/javapoet",
],
Expand Down
2 changes: 2 additions & 0 deletions java/dagger/hilt/processor/internal/ClassNames.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ public final class ClassNames {
get("dagger.hilt.android.internal.managers", "ComponentSupplier");
public static final ClassName APPLICATION_CONTEXT_MODULE =
get("dagger.hilt.android.internal.modules", "ApplicationContextModule");
public static final ClassName DEFAULT_ROOT =
ClassName.get("dagger.hilt.android.internal.testing.root", "Default");
public static final ClassName INTERNAL_TEST_ROOT =
get("dagger.hilt.android.internal.testing", "InternalTestRoot");
public static final ClassName TEST_INJECTOR =
Expand Down
6 changes: 6 additions & 0 deletions java/dagger/hilt/processor/internal/ComponentNames.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ public static ClassName generatedComponentDataHolder(ClassName testName) {
return Processors.append(Processors.getEnclosedClassName(testName), "_ComponentDataHolder");
}

/** Returns the name of components generated for the default root. */
public static ClassName defaultRootComponentName(ClassName component) {
return generatedComponentsWrapper(ClassNames.DEFAULT_ROOT)
.nestedClass(componentName(component));
}

/**
* Returns the shortened component name by replacing the ending "Component" with "C" if it exists.
*
Expand Down
12 changes: 11 additions & 1 deletion java/dagger/hilt/processor/internal/HiltCompilerOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,17 @@ public enum BooleanOption {
"android.internal.disableAndroidSuperclassValidation", false),

/** Flag that disables check on modules to be annotated with @InstallIn. */
DISABLE_MODULES_HAVE_INSTALL_IN_CHECK("disableModulesHaveInstallInCheck", false);
DISABLE_MODULES_HAVE_INSTALL_IN_CHECK("disableModulesHaveInstallInCheck", false),

/**
* Flag that enables unit tests to share a single generated Component, rather than using a
* separate generated Component per Hilt test root.
*
* <p>Tests that provide their own test bindings (e.g. using {@link
* dagger.hilt.android.testing.BindValue} or a test {@link dagger.Module}) cannot use the shared
* component. In these cases, a component will be generated for the test.
*/
SHARE_TEST_COMPONENTS("shareTestComponents", false);

private final String name;
private final boolean defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ public boolean hasEarlySingletonEntryPoints() {
entryPoint -> Processors.hasAnnotation(entryPoint, ClassNames.EARLY_ENTRY_POINT));
}

/**
* Returns {@code true} if the test binds or uninstalls test-specific bindings that would prevent
* it from sharing components with other test roots.
*/
public final boolean includesTestDeps(ClassName root) {
return modules().testDeps().keySet().stream().anyMatch((key) -> key.test().equals(root))
|| modules().uninstalledTestDeps().containsKey(root);
}

@AutoValue.Builder
abstract static class Builder {
abstract Dependencies.Builder modulesBuilder();
Expand Down
1 change: 1 addition & 0 deletions java/dagger/hilt/processor/internal/root/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ java_library(
deps = [
":root_type",
"//java/dagger/hilt/processor/internal:classnames",
"//java/dagger/hilt/processor/internal:compiler_options",
"//java/dagger/hilt/processor/internal:component_descriptor",
"//java/dagger/hilt/processor/internal:kotlin",
"//java/dagger/hilt/processor/internal:processor_errors",
Expand Down
21 changes: 10 additions & 11 deletions java/dagger/hilt/processor/internal/root/Root.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,30 @@
import com.google.auto.common.MoreElements;
import com.google.auto.value.AutoValue;
import com.squareup.javapoet.ClassName;
import dagger.hilt.processor.internal.ClassNames;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;

/** Metadata for a root element that can trigger the {@link RootProcessor}. */
@AutoValue
abstract class Root {
private static final ClassName DEFAULT_ROOT =
ClassName.get("dagger.hilt.android.internal.testing.root", "Default");

/**
* Creates the default root for this (test) build compilation.
*
* <p>A default root installs only the global {@code InstallIn} and {@code TestInstallIn}
* dependencies. Test-specific dependencies are not installed in the default root.
*
* <p>A note on the generated SingletonComponent: currently, the default root is only used for
* {@code EarlyEntryPoint}, so the generated SingletonComponent is limited to entry points
* annotated with {@code EarlyEntryPoint}. In the future, the default root may be used to
* generate a shared component for tests that only depend on global dependencies. In that case,
* we'll need to install all SingletonComponent entry points, in addition to entry points for each
* test injector.
* <p>The default root is used for two purposes:
*
* <ul>
* <li>To inject {@code EarlyEntryPoint} annotated interfaces.
* <li>To inject tests that only depend on global dependencies
* </ul>
*/
static Root createDefaultRoot(ProcessingEnvironment env) {
TypeElement rootElement = env.getElementUtils().getTypeElement(DEFAULT_ROOT.canonicalName());
TypeElement rootElement =
env.getElementUtils().getTypeElement(ClassNames.DEFAULT_ROOT.canonicalName());
return new AutoValue_Root(rootElement, /*isTestRoot=*/ true);
}

Expand Down Expand Up @@ -71,6 +70,6 @@ public final String toString() {

/** Returns {@code true} if this uses the default root. */
boolean isDefaultRoot() {
return classname().equals(DEFAULT_ROOT);
return classname().equals(ClassNames.DEFAULT_ROOT);
}
}
65 changes: 52 additions & 13 deletions java/dagger/hilt/processor/internal/root/RootMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@

import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Suppliers.memoize;
import static dagger.hilt.processor.internal.HiltCompilerOptions.BooleanOption.SHARE_TEST_COMPONENTS;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
import static javax.lang.model.element.Modifier.ABSTRACT;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.STATIC;

import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.squareup.javapoet.ClassName;
Expand Down Expand Up @@ -53,22 +55,42 @@ static RootMetadata create(
ComponentTree componentTree,
ComponentDependencies deps,
ProcessingEnvironment env) {
RootMetadata metadata = new RootMetadata(root, componentTree, deps, env);
metadata.validate();
return metadata;
return createInternal(root, ImmutableList.of(), componentTree, deps, env);
}

static RootMetadata copyWithNewTree(
RootMetadata other,
ComponentTree componentTree) {
return create(other.root, componentTree, other.deps, other.env);
static RootMetadata createForDefaultRoot(
Root root,
ImmutableList<RootMetadata> rootsUsingDefaultComponents,
ComponentTree componentTree,
ComponentDependencies deps,
ProcessingEnvironment env) {
checkState(root.isDefaultRoot());
return createInternal(root, rootsUsingDefaultComponents, componentTree, deps, env);
}

static RootMetadata copyWithNewTree(RootMetadata other, ComponentTree componentTree) {
return createInternal(
other.root, other.rootsUsingDefaultComponents, componentTree, other.deps, other.env);
}

private static RootMetadata createInternal(
Root root,
ImmutableList<RootMetadata> rootsUsingDefaultComponents,
ComponentTree componentTree,
ComponentDependencies deps,
ProcessingEnvironment env) {
RootMetadata metadata =
new RootMetadata(root, componentTree, deps, rootsUsingDefaultComponents, env);
metadata.validate();
return metadata;
}

private final Root root;
private final ProcessingEnvironment env;
private final Elements elements;
private final ComponentTree componentTree;
private final ComponentDependencies deps;
private final ImmutableList<RootMetadata> rootsUsingDefaultComponents;
private final Supplier<ImmutableSetMultimap<ClassName, ClassName>> scopesByComponent =
memoize(this::getScopesByComponentUncached);
private final Supplier<TestRootMetadata> testRootMetadata =
Expand All @@ -78,12 +100,14 @@ private RootMetadata(
Root root,
ComponentTree componentTree,
ComponentDependencies deps,
ImmutableList<RootMetadata> rootsUsingDefaultComponents,
ProcessingEnvironment env) {
this.root = root;
this.env = env;
this.elements = env.getElementUtils();
this.componentTree = componentTree;
this.deps = deps;
this.rootsUsingDefaultComponents = rootsUsingDefaultComponents;
}

public Root root() {
Expand All @@ -102,6 +126,18 @@ public ImmutableSet<TypeElement> modules(ClassName componentName) {
return deps.modules().get(componentName, root.classname(), root.isTestRoot());
}

/**
* Returns {@code true} if this is a test root that provides no test-specific dependencies or sets
* other options that would prevent it from sharing components with other test roots.
*/
// TODO(groakley): Allow more tests to share modules, e.g. tests that uninstall the same module.
// In that case, this might instead return which shared dep grouping should be used.
public boolean canShareTestComponents() {
return SHARE_TEST_COMPONENTS.get(env)
&& root.isTestRoot()
&& !deps.includesTestDeps(root.classname());
}

public ImmutableSet<TypeName> entryPoints(ClassName componentName) {
return ImmutableSet.<TypeName>builder()
.addAll(getUserDefinedEntryPoints(componentName))
Expand Down Expand Up @@ -177,12 +213,15 @@ private void validate() {

private ImmutableSet<TypeName> getUserDefinedEntryPoints(ClassName componentName) {
ImmutableSet.Builder<TypeName> entryPointSet = ImmutableSet.builder();
if (root.isDefaultRoot() && componentName.equals(ClassNames.SINGLETON_COMPONENT)) {
// Filter to only use the entry points annotated with @EarlyEntryPoint. We only do this
// for SingletonComponent because EarlyEntryPoints can only be installed in the
// SingletonComponent.
// TODO(bcorso): Once the DefaultComponent is used as a "shared" component across tests we'll
// only do this filtering if there are no tests that need to share it.
if (root.isDefaultRoot() && !rootsUsingDefaultComponents.isEmpty()) {
// Add entry points for shared component
rootsUsingDefaultComponents.stream()
.flatMap(metadata -> metadata.entryPoints(componentName).stream())
.forEach(entryPointSet::add);
} else if (root.isDefaultRoot() && componentName.equals(ClassNames.SINGLETON_COMPONENT)) {
// Filter to only use the entry points annotated with @EarlyEntryPoint. We only
// do this for SingletonComponent because EarlyEntryPoints can only be installed
// in the SingletonComponent.
deps.entryPoints().get(componentName, root.classname(), root.isTestRoot()).stream()
.filter(ep -> Processors.hasAnnotation(ep, ClassNames.EARLY_ENTRY_POINT))
.map(ClassName::get)
Expand Down
14 changes: 11 additions & 3 deletions java/dagger/hilt/processor/internal/root/RootProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,22 @@ public void postRoundProcess(RoundEnvironment roundEnv) throws Exception {

for (RootMetadata rootMetadata : rootMetadatas) {
setProcessingState(rootMetadata.root());
generateComponents(rootMetadata);
if (!rootMetadata.canShareTestComponents()) {
generateComponents(rootMetadata);
}
}

if (isTestEnv) {
ImmutableList<RootMetadata> rootsThatCanShareComponents =
rootMetadatas.stream()
.filter(RootMetadata::canShareTestComponents)
.collect(toImmutableList());
generateTestComponentData(rootMetadatas);
if (deps.hasEarlySingletonEntryPoints()) {
if (deps.hasEarlySingletonEntryPoints() || !rootsThatCanShareComponents.isEmpty()) {
Root defaultRoot = Root.createDefaultRoot(getProcessingEnv());
generateComponents(RootMetadata.create(defaultRoot, tree, deps, getProcessingEnv()));
generateComponents(
RootMetadata.createForDefaultRoot(
defaultRoot, rootsThatCanShareComponents, tree, deps, getProcessingEnv()));
EarlySingletonComponentCreatorGenerator.generate(getProcessingEnv());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,18 @@ public void generate() throws IOException {
Processors.addGeneratedAnnotation(
generator, processingEnv, ClassNames.ROOT_PROCESSOR.toString());

JavaFile.builder(rootMetadata.testRootMetadata().testName().packageName(), generator.build())
JavaFile.builder(name.packageName(), generator.build())
.build()
.writeTo(processingEnv.getFiler());
}

private MethodSpec getMethod() {
TypeElement testElement = rootMetadata.testRootMetadata().testElement();
ClassName component =
ComponentNames.generatedComponent(
ClassName.get(testElement), ClassNames.SINGLETON_COMPONENT);
rootMetadata.canShareTestComponents()
? ComponentNames.defaultRootComponentName(ClassNames.SINGLETON_COMPONENT)
: ComponentNames.generatedComponent(
ClassName.get(testElement), ClassNames.SINGLETON_COMPONENT);
ImmutableSet<TypeElement> daggerRequiredModules =
rootMetadata.modulesThatDaggerCannotConstruct(ClassNames.SINGLETON_COMPONENT);
ImmutableSet<TypeElement> hiltRequiredModules =
Expand Down Expand Up @@ -200,14 +202,14 @@ private MethodSpec getTestInjectInternalMethod() {
AnnotationSpec.builder(SuppressWarnings.class)
.addMember("value", "$S", "unchecked")
.build())
.addStatement("$L.injectTest(testInstance)", getInjector(testElement))
.addStatement(callInjectTest(testElement))
.build();
}

private static CodeBlock getInjector(TypeElement testElement) {
private CodeBlock callInjectTest(TypeElement testElement) {
return CodeBlock.of(
"(($T) (($T) $T.getApplicationContext()).generatedComponent())",
ClassNames.TEST_INJECTOR,
"(($T) (($T) $T.getApplicationContext()).generatedComponent()).injectTest(testInstance)",
rootMetadata.testRootMetadata().testInjectorName(),
ClassNames.GENERATED_COMPONENT_MANAGER,
ClassNames.APPLICATION_PROVIDER);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.JavaFile;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeSpec;
import dagger.hilt.processor.internal.ClassNames;
import dagger.hilt.processor.internal.Processors;
Expand All @@ -41,7 +40,9 @@ public final class TestInjectorGenerator {

// @GeneratedEntryPoint
// @InstallIn(SingletonComponent.class)
// public interface FooTest_GeneratedInjector extends TestInjector<FooTest> {}
// public interface FooTest_GeneratedInjector {
// void injectTest(FooTest fooTest);
// }
public void generate() throws IOException {
TypeSpec.Builder builder =
TypeSpec.interfaceBuilder(metadata.testInjectorName())
Expand All @@ -53,11 +54,8 @@ public void generate() throws IOException {
.addMember("value", "$T.class", installInComponent(metadata.testElement()))
.build())
.addModifiers(Modifier.PUBLIC)
.addSuperinterface(
ParameterizedTypeName.get(ClassNames.TEST_INJECTOR, metadata.testName()))
.addMethod(
MethodSpec.methodBuilder("injectTest")
.addAnnotation(Override.class)
.addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
.addParameter(
metadata.testName(),
Expand Down
6 changes: 6 additions & 0 deletions javatests/dagger/hilt/android/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
<activity
android:name=".QualifierInKotlinFieldsTest$TestActivity"
android:exported="false"/>
<activity
android:name=".UsesSharedComponent1Test$TestActivity"
android:exported="false"/>
<activity
android:name=".UsesSharedComponent2Test$TestActivity"
android:exported="false"/>
<activity
android:name=".ViewModelScopedTest$TestActivity"
android:exported="false"/>
Expand Down
Loading

0 comments on commit faebc3c

Please sign in to comment.