From 952c2504ae4ddd4f1fa7939ac101b27dd17f0b6c Mon Sep 17 00:00:00 2001 From: Eric Chang Date: Mon, 22 Jul 2024 16:36:59 -0700 Subject: [PATCH] Fix an issue where the parameter name "instance" could conflict with fields of the same name in the component. Fixes #4352. RELNOTES=Fixes #4352. Fix an issue where the parameter name "instance" could conflict with fields of the same name in the component. PiperOrigin-RevId: 654937603 --- .../writing/MembersInjectionMethods.java | 10 +++- .../MembersWithInstanceNameTest.java | 57 ++++++++++++++++++- ..._FAST_INIT_MODE_test.DaggerParentComponent | 6 +- 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/java/dagger/internal/codegen/writing/MembersInjectionMethods.java b/java/dagger/internal/codegen/writing/MembersInjectionMethods.java index d9934b53a8d..c698e2f12f8 100644 --- a/java/dagger/internal/codegen/writing/MembersInjectionMethods.java +++ b/java/dagger/internal/codegen/writing/MembersInjectionMethods.java @@ -106,7 +106,15 @@ private Expression injectMethodExpression(Binding binding) { // simple names Foo.Builder -> injectFooBuilder String methodName = shardImplementation.getUniqueMethodName("inject" + bindingTypeName); ParameterSpec parameter = - ParameterSpec.builder(membersInjectedType.getTypeName(), "instance").build(); + ParameterSpec.builder( + membersInjectedType.getTypeName(), + // Technically this usage only needs to be unique within this method, but this will + // allocate + // a unique name within the shard. We could optimize this by cloning the + // UniqueNameSet or + // using NameAllocator which has a clone method in the future. + shardImplementation.getUniqueFieldName("instance")) + .build(); MethodSpec.Builder methodBuilder = methodBuilder(methodName) .addModifiers(PRIVATE) diff --git a/javatests/dagger/functional/membersinject/MembersWithInstanceNameTest.java b/javatests/dagger/functional/membersinject/MembersWithInstanceNameTest.java index 7cd97d96b64..729059a939f 100644 --- a/javatests/dagger/functional/membersinject/MembersWithInstanceNameTest.java +++ b/javatests/dagger/functional/membersinject/MembersWithInstanceNameTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; +import dagger.BindsInstance; import dagger.Component; import dagger.Module; import dagger.Provides; @@ -36,6 +37,16 @@ static final class Foo { @Inject Foo() {} } + static final class Bar { + // Checks that member injection fields can use injecting a bound instance that was + // named "instance" when bound. Note that the field name here doesn't matter as of + // this writing, but name it "instance" anyway in case that changes. + // https://github.com/google/dagger/issues/4352 + @Inject BoundInstance instance; + + @Inject Bar() {} + } + @Module interface TestModule { @Provides @@ -44,16 +55,60 @@ static String provideString() { } } + public static final class BoundInstance {} + @Component(modules = TestModule.class) interface TestComponent { Foo foo(); + + Bar bar(); + + @Component.Builder + interface Builder { + // As of writing, the method name is the one that matters, but name the + // parameter the same anyway in case that changes. + Builder instance(@BindsInstance BoundInstance instance); + TestComponent build(); + } + } + + @Component(modules = TestModule.class) + interface TestComponentWithFactory { + Foo foo(); + + Bar bar(); + + @Component.Factory + interface Factory { + // As of writing, the parameter name is the one that matters, but name the + // method the same anyway in case that changes. + TestComponentWithFactory instance(@BindsInstance BoundInstance instance); + } } @Test public void testMemberWithInstanceName() { - TestComponent component = DaggerMembersWithInstanceNameTest_TestComponent.create(); + BoundInstance boundInstance = new BoundInstance(); + TestComponent component = DaggerMembersWithInstanceNameTest_TestComponent + .builder().instance(boundInstance).build(); + Foo foo = component.foo(); + assertThat(foo).isNotNull(); + assertThat(foo.instance).isEqualTo("test"); + Bar bar = component.bar(); + assertThat(bar).isNotNull(); + assertThat(bar.instance).isSameInstanceAs(boundInstance); + } + + @Test + public void testMemberWithInstanceNameUsingFactory() { + BoundInstance boundInstance = new BoundInstance(); + TestComponentWithFactory component = DaggerMembersWithInstanceNameTest_TestComponentWithFactory + .factory().instance(boundInstance); Foo foo = component.foo(); assertThat(foo).isNotNull(); assertThat(foo.instance).isEqualTo("test"); + Bar bar = component.bar(); + assertThat(bar).isNotNull(); + assertThat(bar.instance).isSameInstanceAs(boundInstance); } } diff --git a/javatests/dagger/internal/codegen/goldens/SubcomponentValidationTest_delegateFactoryNotCreatedForSubcomponentWhenProviderExistsInParent_FAST_INIT_MODE_test.DaggerParentComponent b/javatests/dagger/internal/codegen/goldens/SubcomponentValidationTest_delegateFactoryNotCreatedForSubcomponentWhenProviderExistsInParent_FAST_INIT_MODE_test.DaggerParentComponent index 67917e4571f..b058f021e9e 100644 --- a/javatests/dagger/internal/codegen/goldens/SubcomponentValidationTest_delegateFactoryNotCreatedForSubcomponentWhenProviderExistsInParent_FAST_INIT_MODE_test.DaggerParentComponent +++ b/javatests/dagger/internal/codegen/goldens/SubcomponentValidationTest_delegateFactoryNotCreatedForSubcomponentWhenProviderExistsInParent_FAST_INIT_MODE_test.DaggerParentComponent @@ -114,9 +114,9 @@ final class DaggerParentComponent { } @CanIgnoreReturnValue - private Dep2 injectDep2(Dep2 instance) { - Dep2_MembersInjector.injectDep2Method(instance); - return instance; + private Dep2 injectDep2(Dep2 instance2) { + Dep2_MembersInjector.injectDep2Method(instance2); + return instance2; } private static final class SwitchingProvider implements Provider {