Skip to content

Commit

Permalink
Fixes mismatched parameter names in subcomponent factory methods.
Browse files Browse the repository at this point in the history
Fixes #3401

This issue occurs when the original factory method is in a separate build unit from the component that implements it. This issue happens when building with Gradle (couldn't repro with Bazel) and when the original factory method is a kotlin library.

This CL changes `createSubcomponentFactoryMethod()` to use the "overriding" factory method's parameters instead of the parameter names from the original factory method.

RELNOTES=N/A
PiperOrigin-RevId: 460532209
  • Loading branch information
bcorso authored and Dagger Team committed Jul 12, 2022
1 parent a7f8dac commit 1763aa3
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 8 deletions.
Expand Up @@ -56,7 +56,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Sets;
import com.squareup.javapoet.ClassName;
Expand Down Expand Up @@ -84,13 +83,11 @@
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.javapoet.TypeSpecs;
import dagger.internal.codegen.langmodel.Accessibility;
import dagger.internal.codegen.xprocessing.JavaPoetExt;
import dagger.internal.codegen.xprocessing.XTypeElements;
import dagger.spi.model.BindingGraph.Node;
import dagger.spi.model.Key;
import dagger.spi.model.RequestKind;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -845,12 +842,12 @@ private boolean canInstantiateAllRequirements() {

private void createSubcomponentFactoryMethod(XMethodElement factoryMethod) {
checkState(parent.isPresent());
Collection<ParameterSpec> params =
Maps.transformValues(graph.factoryMethodParameters(), JavaPoetExt::getParameterSpec)
.values();
XType parentType = parent.get().graph().componentTypeElement().getType();
MethodSpec.Builder method = overriding(factoryMethod, parentType);
params.forEach(
// Use the parameter names from the overriding method, which may be different from the
// parameter names at the declaration site if it is pulled in as a class dependency from a
// separate build unit (see https://github.com/google/dagger/issues/3401).
method.parameters.forEach(
param -> method.addStatement("$T.checkNotNull($N)", Preconditions.class, param));
method.addStatement(
"return new $T($L)",
Expand All @@ -861,7 +858,7 @@ private void createSubcomponentFactoryMethod(XMethodElement factoryMethod) {
creatorComponentFields().stream()
.map(field -> ParameterSpec.builder(field.type, field.name).build())
.collect(toImmutableList()))
.addAll(params)
.addAll(method.parameters)
.build()));

parent.get().getComponentShard().addMethod(COMPONENT_METHOD, method.build());
Expand Down
1 change: 1 addition & 0 deletions javatests/artifacts/dagger/build-tests/build.gradle
Expand Up @@ -22,6 +22,7 @@ plugins {
// Set the versions in a system property so that tests can access it.
test {
systemProperty 'dagger_version', "$dagger_version"
systemProperty 'kotlin_version', "$kotlin_version"
}

dependencies {
Expand Down
@@ -0,0 +1,135 @@
/*
* Copyright (C) 2022 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 buildtests;

import static com.google.common.truth.Truth.assertThat;
import static org.gradle.testkit.runner.TaskOutcome.SUCCESS;

import java.io.File;
import java.io.IOException;
import org.gradle.testkit.runner.BuildResult;
import org.gradle.testkit.runner.GradleRunner;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

// This is a regression test for https://github.com/google/dagger/issues/3401
// This issues occurs specifically when the subcomponent factory method is defined in a separate
// kotlin library from the component that implements the subcomponent factory method.
@RunWith(JUnit4.class)
public class TransitiveSubcomponentTest {
@Rule public TemporaryFolder folder = new TemporaryFolder();

@Test
public void testBuild() throws IOException {
BuildResult result = setupRunner().build();
assertThat(result.task(":app:assemble").getOutcome()).isEqualTo(SUCCESS);
}

private GradleRunner setupRunner() throws IOException {
File projectDir = folder.getRoot();
GradleModule.create(projectDir)
.addSettingsFile(
"include 'app'",
"include 'library1'")
.addBuildFile(
"buildscript {",
" ext {",
String.format("dagger_version = \"%s\"", System.getProperty("dagger_version")),
String.format("kotlin_version = \"%s\"", System.getProperty("kotlin_version")),
" }",
"}",
"",
"allprojects {",
" repositories {",
" mavenCentral()",
" mavenLocal()",
" }",
"}");

GradleModule.create(projectDir, "app")
.addBuildFile(
"plugins {",
" id 'java'",
" id 'application'",
"}",
"dependencies {",
" implementation project(':library1')",
" implementation \"com.google.dagger:dagger:$dagger_version\"",
" annotationProcessor \"com.google.dagger:dagger-compiler:$dagger_version\"",
"}")
.addSrcFile(
"MyComponent.java",
"package app;",
"",
"import dagger.Component;",
"import library1.MySubcomponent1;",
"",
"@Component",
"interface MyComponent {",
" MySubcomponent1 subcomponent1();",
"}");

GradleModule.create(projectDir, "library1")
.addBuildFile(
"plugins {",
" id 'org.jetbrains.kotlin.jvm' version \"$kotlin_version\"",
" id 'org.jetbrains.kotlin.kapt' version \"$kotlin_version\"",
"}",
"dependencies {",
" implementation \"com.google.dagger:dagger:$dagger_version\"",
" annotationProcessor \"com.google.dagger:dagger-compiler:$dagger_version\"",
"}")
.addSrcFile(
"MyModule.kt",
"package library1",
"",
"import dagger.Module",
"import dagger.Provides",
"",
"@Module",
"public class MyModule(private val int: Int) {",
" @Provides public fun provideInt(): Int = int",
"}")
.addSrcFile(
"MySubcomponent1.kt",
"package library1",
"",
"import dagger.Subcomponent",
"",
"@Subcomponent",
"public interface MySubcomponent1 {",
" public fun subcomponent2(myModule: MyModule): MySubcomponent2",
"}")
.addSrcFile(
"MySubcomponent2.kt",
"package library1",
"",
"import dagger.Subcomponent",
"",
"@Subcomponent(modules = [MyModule::class])",
"public interface MySubcomponent2 {",
" public fun integer(): Int",
"}");

return GradleRunner.create()
.withArguments("--stacktrace", "build")
.withProjectDir(projectDir);
}
}

0 comments on commit 1763aa3

Please sign in to comment.