Skip to content

Commit

Permalink
Fix an issue where the parameter name "instance" could conflict with …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
Chang-Eric authored and Dagger Team committed Jul 22, 2024
1 parent 917c005 commit 952c250
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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

import dagger.BindsInstance;
import dagger.Component;
import dagger.Module;
import dagger.Provides;
Expand All @@ -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
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> implements Provider<T> {
Expand Down

0 comments on commit 952c250

Please sign in to comment.