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.

Sign up for GitHub

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

Jump to bottom

Scoped binding isn't scoped if scope isn't on classpath. #3136

Closed
bcorso opened this issue Dec 29, 2021 · 1 comment
Closed

Scoped binding isn't scoped if scope isn't on classpath. #3136

bcorso opened this issue Dec 29, 2021 · 1 comment
Assignees
@bcorso
Labels
area: dagger type: bug

Comments

@bcorso
Copy link

bcorso commented Dec 29, 2021

Dagger treats scoped bindings as unscoped if the scope isn't on the classpath.

This may be a rare case, but the situation can happen if you have a setup like :component -> :subcomponent -> :subcomponent-scope where :subcomponent-scope is an implementation dependency of :subcomponent and thus isn't on the classpath of :component.

The expected behavior is that the build would fail in this case, and ideally with an error message that tells the user that the scope cannot be resolved.

Normally, this would be handled by our use of SuperficialValidation. However, the fact that the build doesn't fail seems to be due to a bug in TypeVisitor which causes it to visit visitDeclared rather than visitError for an annotation's type even if it has TypeKind.ERROR. Thus, SuperficialValidation will report that the annotation's type is valid when it's actually not. Furthermore, getting the corresponding TypeElement for that annotation and checking Element#getAnnotationMirrors() will return an empty list, which fools Dagger into thinking that the annotation is not a scope (since it can't find the @Scope annotation).
@bcorso bcorso self-assigned this Dec 29, 2021
@copybara-service copybara-service bot mentioned this issue Dec 29, 2021
Closed
copybara-service bot pushed a commit that referenced this issue Dec 29, 2021
@bcorso
Add repro test for #3136.
e7a626a 
This CL adds some infrastructure to run multi-module Gradle build tests to javatests/artifacts/dagger, and creates a repro test for issue #3136.

RELNOTES=N/A
PiperOrigin-RevId: 418822365
copybara-service bot pushed a commit that referenced this issue Dec 29, 2021
@bcorso
Add repro test for #3136.
fa9b41a 
This CL adds some infrastructure to run multi-module Gradle build tests to javatests/artifacts/dagger, and creates a repro test for issue #3136.

RELNOTES=N/A
PiperOrigin-RevId: 418822365
copybara-service bot pushed a commit that referenced this issue Dec 29, 2021
@bcorso
Add repro test for #3136.
f37b9a1 
This CL adds some infrastructure to run multi-module Gradle build tests to javatests/artifacts/dagger, and creates a repro test for issue #3136.

RELNOTES=N/A
PiperOrigin-RevId: 418853078
@copybara-service copybara-service bot mentioned this issue Jan 10, 2022
Closed
copybara-service bot pushed a commit that referenced this issue Jan 10, 2022
@bcorso
Add repro test for #3136 for missing qualifiers in @Inject types.
5c24ed6 
This CL adds a test showing a missing qualifier if the annotation is missing on the classpath during processing.

RELNOTES=N/A
PiperOrigin-RevId: 420662003
copybara-service bot pushed a commit that referenced this issue Jan 10, 2022
@bcorso
Add repro test for #3136 for missing qualifiers in @Inject types.
08be250 
This CL adds a test showing a missing qualifier if the annotation is missing on the classpath during processing.

RELNOTES=N/A
PiperOrigin-RevId: 420810694
@copybara-service copybara-service bot mentioned this issue Jan 11, 2022
Closed
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add new @Scopemetadata to generated factories to store fqn of scope.
8a1f129 
This is useful so that we can skip superficial validation of annotations that are not scopes when validating inject types in subsequent compilation units.

RELNOTES=Fixes missing scopes described in #3136. However, this change only fixes one case of missing scopes on @Inject types -- there are a number of other cases that still need to be fixed for this bug to actually be closed.
PiperOrigin-RevId: 420166213
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add new @Scopemetadata to generated factories to store fqn of scope.
86b83d6 
This is useful so that we can skip superficial validation of annotations that are not scopes when validating inject types in subsequent compilation units.

RELNOTES=Fixes missing scopes described in #3136. However, this change only fixes one case of missing scopes on @Inject types -- there are a number of other cases that still need to be fixed for this bug to actually be closed.
PiperOrigin-RevId: 421042966
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
a014abc 
This CL adds a test for transitive scope and qualifier on a module's @provides method.

RELNOTES=N/A
PiperOrigin-RevId: 420943853
@copybara-service copybara-service bot mentioned this issue Jan 11, 2022
Closed
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
b77b93f 
This CL adds a test for transitive scope and qualifier on a module's @provides method.

RELNOTES=N/A
PiperOrigin-RevId: 420943853
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
006e59f 
This CL adds a test for transitive scope and qualifier on a module's @provides method.

RELNOTES=N/A
PiperOrigin-RevId: 421067911
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
cdde094 
This CL adds tests to show how Dagger handles unrelated transitive annotations on the module and inject classes.

RELNOTES=N/A
PiperOrigin-RevId: 420662469
@copybara-service copybara-service bot mentioned this issue Jan 11, 2022
Closed
copybara-service bot pushed a commit that referenced this issue Jan 11, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
5ed67ca 
This CL adds tests to show how Dagger handles unrelated transitive annotations on the module and inject classes.

RELNOTES=N/A
PiperOrigin-RevId: 421078020
@copybara-service copybara-service bot mentioned this issue Jan 18, 2022
Closed
copybara-service bot pushed a commit that referenced this issue Jan 18, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
a733f3e 
This CL adds tests for @BINDS usages with scopes and qualifiers.

RELNOTES=N/A
PiperOrigin-RevId: 422554664
copybara-service bot pushed a commit that referenced this issue Jan 18, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
b12d930 
This CL adds tests for @BINDS usages with scopes and qualifiers.

RELNOTES=N/A
PiperOrigin-RevId: 422554664
copybara-service bot pushed a commit that referenced this issue Jan 18, 2022
@bcorso
Add more Gradle test coverage for issue #3136.
505afed 
This CL adds tests for @BINDS usages with scopes and qualifiers.

RELNOTES=N/A
PiperOrigin-RevId: 422618557
@bcorso
Copy link
Author

bcorso commented Feb 15, 2022

This will be fixed in the next release (v2.41).

@bcorso bcorso closed this as completed Feb 15, 2022
@bcorso bcorso mentioned this issue Feb 17, 2022
Closed
@ParaskP7 ParaskP7 mentioned this issue Mar 14, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bcorso bcorso

Labels
area: dagger type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bcorso