Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid code generated when combining (sub)component factories and assisted inject #2570

Closed
ZacSweers opened this issue Apr 23, 2021 · 1 comment · Fixed by #2733
Closed

Comments

@ZacSweers
Copy link

ZacSweers commented Apr 23, 2021

We've run into a strange bug in Dagger when combining assisted inject and component factories.

Some context: It generates code that doesn't compile in a subcomponent. In particular, it passes the wrong parameter into an assisted injected view

Example:

private SearchResultsView searchResultsView(Context arg0, AttributeSet arg1) {
  return new SearchResultsView(arg0, arg1, DoubleCheck.lazy(TestOrgComponentImpl.this.forScopeFeatureFlagStoreProvider()), DoubleCheck.lazy(searchFileViewBinderProvider()), DoubleCheck.lazy(searchMessageViewBinderProvider()), DoubleCheck.lazy(viewHolderFactoryProvider()), timeFormatter(), searchModifierViewHolderFactory(), arg0, searchTrackerImpl(), DoubleCheck.lazy(DaggerTestAppComponent.this.toasterProvider()), DoubleCheck.lazy(DaggerTestAppComponent.this.networkInfoManagerProvider()), DaggerTestAppComponent.this.appBuildConfig);
}

Where arg0 is correct for those first two as assisted inputs, but then weirdly pops up later. That specific later position (right before searchTrackerImpl()) is of some interested - its type is LoggedInUser and it's the sole @BindsInstance argument of the subcomponent's factory method.

interface UserComponent {
  interface Factory<T : UserComponent> {
    fun create(@BindsInstance loggedInUser: LoggedInUser): T
  }
}

@Subcomponent(...)
interface TestUserComponent : UserComponent {
  @Subcomponent.Factory
  interface Factory : UserComponent.Factory<TestUserComponent>
}

When I look more closely, the problem is that the LoggedInUser argument passed into the factory is also called arg0.

    private final class TestUserComponentImpl implements TestUserComponent {
      private final LoggedInUser arg0;  // <--- This is what it's trying to use in searchResultsView() below

As a result, I think the fix is that it either needs to use NameAllocator or qualify the second arg0 call with this.arg0. Confirmed this is broken in both 2.34.1 and 2.35

@ZacSweers
Copy link
Author

Interestingly, we seem to avoid this when consuming old generated factories. Dagger generates a wildly different component when this version of an upstream library is used, suggesting some sort of compatibility mode maybe?

copybara-service bot pushed a commit that referenced this issue Jun 30, 2021
…licts.

Fixes #2570

RELNOTES=De-dupe assisted parameter names and field names to avoid naming conflicts. #2570
PiperOrigin-RevId: 378977454
copybara-service bot pushed a commit that referenced this issue Jul 1, 2021
…licts.

Fixes #2570

RELNOTES=De-dupe assisted parameter names and field names to avoid naming conflicts. #2570
PiperOrigin-RevId: 378977454
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants