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

Dagger generates code with wrong parameter names and fails to compile #3401

Closed
kozaxinan opened this issue May 17, 2022 · 18 comments
Closed

Comments

@kozaxinan
Copy link

kozaxinan commented May 17, 2022

We recently updated Dagger from 2.37 to 2.42. Dagger time to time generates not compiling code.
We have a component that uses other component.

Our simplified component;

@Subcomponent
interface ParentComponent {

    fun inject(foo: Foo)

    fun plus(module: FooModule): FooComponent
}

Generated code;

    @Override
    public FooComponent plus(FooModule module) {
      Preconditions.checkNotNull(arg0);
      return new FooComponentImpl(applicationComponentImpl, parentComponentImp, arg0);
    }

Generated code has module parameter but code expects arg0 name. This fails to compile.
If i change something in generated code and compile again, old generated code invalided and dagger generate correct code. But issue appears again, I couldnt find a common use case.

As workaround we changed module parameter name in ParentComponent to arg0 and it seems to prevents the issue up until now.

@bcorso
Copy link

bcorso commented May 17, 2022

@kozaxinan thanks for reporting this. I think this may be fixed at HEAD. Would it be possible for you to try using the HEAD-snapshot to verify?

@kozaxinan
Copy link
Author

kozaxinan commented May 17, 2022

I checked new version but with snapshot, our build throws new exception during build before coming to original issue.

Caused by: com.sun.tools.javac.processing.AnnotationProcessingError: java.lang.IllegalStateException: No property named value was found in annotation QualifierMetadata
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:997)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.discoverAndRunProcs(JavacProcessingEnvironment.java:901)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.run(JavacProcessingEnvironment.java:1227)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(JavacProcessingEnvironment.java:1340)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.processAnnotations(JavaCompiler.java:1258)
        ... 38 more
Caused by: java.lang.IllegalStateException: No property named value was found in annotation QualifierMetadata
        at dagger.spi.shaded.androidx.room.compiler.processing.XAnnotation.getAnnotationValue(XAnnotation.kt:134)
        at dagger.spi.shaded.androidx.room.compiler.processing.XAnnotation.getAsStringList(XAnnotation.kt:86)
        at dagger.internal.codegen.binding.InjectionAnnotations.getQualifiersFromQualifierMetadata(InjectionAnnotations.java:246)
        at dagger.internal.codegen.binding.InjectionAnnotations.getQualifiers(InjectionAnnotations.java:206)
        at dagger.internal.codegen.validation.DependencyRequestValidator$Validator.<init>(DependencyRequestValidator.java:124)
        at dagger.internal.codegen.validation.DependencyRequestValidator.validateDependencyRequest(DependencyRequestValidator.java:89)
        at dagger.internal.codegen.validation.InjectValidator.validateDependencyRequest(InjectValidator.java:339)
        at dagger.internal.codegen.validation.InjectValidator.validateField(InjectValidator.java:290)
        at dagger.internal.codegen.validation.InjectValidator.validateForMembersInjectionInternalUncached(InjectValidator.java:363)
        at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
        at dagger.internal.codegen.validation.InjectValidator.validateForMembersInjectionInternal(InjectValidator.java:350)
        at dagger.internal.codegen.validation.InjectValidator.validateForMembersInjection(InjectValidator.java:346)
        at dagger.internal.codegen.validation.InjectValidator.lambda$validateForMembersInjectionInternalUncached$1(InjectValidator.java:389)
        at dagger.internal.codegen.validation.InjectValidator.validateForMembersInjectionInternalUncached(InjectValidator.java:386)
        at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
        at dagger.internal.codegen.validation.InjectValidator.validateForMembersInjectionInternal(InjectValidator.java:350)
        at dagger.internal.codegen.validation.InjectValidator.validateUncached(InjectValidator.java:134)
        at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
        at dagger.internal.codegen.validation.InjectValidator.validate(InjectValidator.java:129)
        at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterConstructor(InjectBindingRegistryImpl.java:252)
        at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterInjectConstructor(InjectBindingRegistryImpl.java:241)
        at dagger.internal.codegen.processingstep.InjectProcessingStep.process(InjectProcessingStep.java:65)
        at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.lambda$process$0(TypeCheckingProcessingStep.java:79)
        at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:196)
        at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:69)
        at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:47)
        at dagger.spi.shaded.androidx.room.compiler.processing.XProcessingStep.process(XProcessingStep.kt:59)
        at dagger.spi.shaded.androidx.room.compiler.processing.CommonProcessorDelegate.processRound(XBasicAnnotationProcessor.kt:123)
        at dagger.spi.shaded.androidx.room.compiler.processing.javac.JavacBasicAnnotationProcessor.process(JavacBasicAnnotationProcessor.kt:71)
        at org.jetbrains.kotlin.kapt3.base.incremental.IncrementalProcessor.process(incrementalProcessors.kt:90)
        at org.jetbrains.kotlin.kapt3.base.ProcessorWrapper.process(annotationProcessing.kt:188)
        at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:985)
        ... 42 more

@bcorso
Copy link

bcorso commented May 17, 2022

Hmm, that sounds like version skew between the different artifacts, e.g. maybe the com.google.dagger:dagger-compiler artifact is at HEAD-SNAPSHOT but the com.google.dagger:dagger artifact is not or one of your Gradle libraries is using a non-HEAD-SNAPSHOT version?

If you haven't already, you should add the resolutionStrategy suggested in the docs:

configurations.all {
    resolutionStrategy.eachDependency { DependencyResolveDetails details ->
        if (details.requested.group == 'com.google.dagger') {
            details.useVersion "HEAD-SNAPSHOT"
        }
    }
}

@kozaxinan kozaxinan changed the title Dagger generate code with wrong parameter names and fail to compile Dagger generates code with wrong parameter names and fails to compile May 17, 2022
@kozaxinan
Copy link
Author

kozaxinan commented May 17, 2022

Okay, my mistake. I run the build with Snapshot. I couldn't reproduce the issue. But now I cannot reproduce the issue with old version as well. Is there a way to trigger this error more often to reproduce and test?
Until next release we changed name of the parameters to arg0.

@bcorso
Copy link

bcorso commented May 17, 2022

It may be related to incremental processing, e.g. you can try first doing a clean build then changing something on the @Component source to get the component to reprocess without changing the @Subcomponent ParentComponent source.

@uKL
Copy link

uKL commented May 19, 2022

I'm having the same issue after updating from 2.27 to 2.42. Clean build does not help.

@uKL
Copy link

uKL commented May 19, 2022

Exactly like in the comment above, changing argument name in the component's method to arg0 has helped.

@bcorso
Copy link

bcorso commented May 19, 2022

@uKL can you try out the Dagger HEAD-SNAPSHOT artifacts to verify if it fixes your issue (without changing the method param to arg0).

@jonathan-whitaker
Copy link

jonathan-whitaker commented May 31, 2022

I was able to reproduce this on accident while switching to a new machine. I had the wrong asdf shim set and did an initial build with java 17 and switched back to java 11 and had this error. I was able to get rid of the error, by nuking the ~/.gradle dir and doing a fresh build, but I'm sure that was overkill and someone could just remove the incremental builds to get it working.

@rogovalex
Copy link

rogovalex commented Jun 16, 2022

I'm having the same issue after updating from 2.40.1 to 2.42. I've noticed the problem occurs in multi-module project with components from non-main modules.

Снимок экрана 2022-06-16 в 13 01 00

Using HEAD-SNAPSHOT artifacts causes multiple other issues with dagger modules from non-main modules.

Снимок экрана 2022-06-16 в 12 55 26

Version 2.41 does not have this issue.

Kotlin version 1.6.21/1.6.10.

@vRallev
Copy link

vRallev commented Jun 24, 2022

@bcorso I tried the snapshot as well and still can reproduce the issue. Please let me know if there's anything else I could do. I ensured with the :dependencies task that the correct version is being used:

kaptAndroidTestDebug
\--- com.google.dagger:dagger-compiler:HEAD-SNAPSHOT
     +--- com.google.dagger:dagger:HEAD-SNAPSHOT
     |    \--- javax.inject:javax.inject:1
     +--- com.google.dagger:dagger-producers:HEAD-SNAPSHOT

@bcorso
Copy link

bcorso commented Jun 24, 2022

@vRallev if you have time to put together a sample project with repro steps that would help a lot. I've been short on time lately, so it would make it a lot easier for me to narrow down the issue and verify the fix.

@vRallev
Copy link

vRallev commented Jun 24, 2022

The sample is here: https://github.com/vRallev/anvil/tree/ralf/dagger-bug Notice that the branch is ralf/dagger-bug. You should be able to reproduce the bug by running ./gradlew test:

> Task :integration-tests:tests:compileTestJava FAILED
/Users/ralf/Development/projects/square/anvil/integration-tests/tests/build/generated/source/kapt/test/com/squareup/anvil/test/DaggerMyComponent.java:69: error: cannot find symbol
      Preconditions.checkNotNull(arg0);
                                 ^
  symbol:   variable arg0
  location: class MySubcomponent1Impl
/Users/ralf/Development/projects/square/anvil/integration-tests/tests/build/generated/source/kapt/test/com/squareup/anvil/test/DaggerMyComponent.java:70: error: cannot find symbol
      return new MySubcomponent2Impl(myComponentImpl, mySubcomponent1Impl, arg0);
                                                                           ^
  symbol:   variable arg0
  location: class MySubcomponent1Impl
2 errors

@bcorso
Copy link

bcorso commented Jun 29, 2022

Thanks @vRallev! I'll make some time to look into it next week.

@designedbyz
Copy link

designedbyz commented Jul 8, 2022

Seeing this as well, but interestingly, only in a test component that inherits from another component;

Our (simplified) usage in an Android app looks something like this:

Application component pretty standard android here; written in Kotlin

@Singleton
@Component(modules = [ApplicationModule::class])
interface AppComponent {
    @Component.Builder
    interface Builder {

        @BindsInstance
        fun application(application: Application): Builder

        fun build(): AppComponent
    }

    fun plusActivityComponent(activityModule: ActivityModule): ActivityComponent
...
}

TestComponent extends AppComponent for use in some connected android tests and is written in Kotlin

@Singleton
@Component(modules = [ApplicationModule::class])
interface TestComponent : AppComponent {

    @Component.Builder
    interface Builder {

        @BindsInstance
        fun application(application: Application): Builder

        fun build(): TestComponent
    }
...
}

ActivityComponent a pretty standard subcomponent written in Kotlin

@Subcomponent(modules = arrayOf(ActivityModule::class))
@ActivityScoped
interface ActivityComponent {
...
}

Interestingly when I roll back to Dagger 2.41 the function is declared with a parameter name of arg0 in DaggerTestComponent:

  @Override
  public ActivityComponent plusActivityComponent(ActivityModule arg0) {
    Preconditions.checkNotNull(arg0);
    return new ActivityComponentImpl(testComponent, arg0);
  }

but in Dagger 2.42 the function parameter name is not replaced with arg0, but it's uses remain arg0.

    @Override
    public ActivityComponent plusActivityComponent(ActivityModule activityModule) {
      Preconditions.checkNotNull(arg0);
      return new ActivityComponentImpl(testComponentImpl, arg0);
    }

Interestingly, this suggests a workaround:
Anytime you're doing something like this with a module, just name it arg0 🤷‍♀️

@bcorso
Copy link

bcorso commented Jul 12, 2022

Sorry, I was out last week.

Just got some time to look into this and should have a fix submitted soon.

copybara-service bot pushed a commit that referenced this issue Jul 12, 2022
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: 460473493
@designedbyz
Copy link

designedbyz commented Jul 12, 2022

Awesome, thanks for the fix!

@vRallev
Copy link

vRallev commented Jul 21, 2022

A similar issue still happens for member injection. I filed a separate issue #3476.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants