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

Fix MissingBinding error message for similar bindings. #4454

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Fix MissingBinding error message for similar bindings. #4454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions java/dagger/internal/codegen/bindinggraphvalidation/MissingBindingValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import dagger.internal.codegen.model.BindingGraph;
import dagger.internal.codegen.model.BindingGraph.DependencyEdge;
import dagger.internal.codegen.model.BindingGraph.MissingBinding;
import dagger.internal.codegen.model.DaggerAnnotation;
import dagger.internal.codegen.model.DaggerType;
import dagger.internal.codegen.model.DiagnosticReporter;
import dagger.internal.codegen.model.Key;
import dagger.internal.codegen.validation.DiagnosticMessageGenerator;
Expand All @@ -53,8 +53,6 @@
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import javax.inject.Inject;

/** Reports errors for missing bindings. */
Expand Down Expand Up @@ -114,29 +112,33 @@ private void reportMissingBinding(

private static ImmutableSet<Binding> getSimilarTypeBindings(
BindingGraph graph, Key missingBindingKey) {
XType missingBindingType = missingBindingKey.type().xprocessing();
Optional<DaggerAnnotation> missingBindingQualifier = missingBindingKey.qualifier();
ImmutableList<TypeName> flatMissingBindingType = flattenBindingType(missingBindingType);
ImmutableList<TypeName> flatMissingBindingType = flattenBindingType(missingBindingKey.type());
if (flatMissingBindingType.size() <= 1) {
return ImmutableSet.of();
}
return graph.bindings().stream()
.filter(
binding ->
binding.key().qualifier().equals(missingBindingQualifier)
&& isSimilarType(binding.key().type().xprocessing(), flatMissingBindingType))
// Filter out multibinding contributions (users can't request these directly).
.filter(binding -> binding.key().multibindingContributionIdentifier().isEmpty())
// Filter out keys with the exact same type (those are reported elsewhere).
.filter(binding -> !binding.key().type().equals(missingBindingKey.type()))
// Filter out keys with different qualifiers.
// TODO(bcorso): We should consider allowing keys with different qualifiers here, as that
// could actually be helpful when users forget a qualifier annotation on the request.
.filter(binding -> binding.key().qualifier().equals(missingBindingKey.qualifier()))
// Filter out keys that don't have a similar type (i.e. same type if ignoring wildcards).
.filter(binding -> isSimilarType(binding.key().type(), flatMissingBindingType))
.collect(toImmutableSet());
}

/**
* Unwraps a parameterized type to a list of TypeNames. e.g. {@code Map<Foo, List<Bar>>} to {@code
* [Map, Foo, List, Bar]}.
*/
private static ImmutableList<TypeName> flattenBindingType(XType type) {
private static ImmutableList<TypeName> flattenBindingType(DaggerType type) {
return ImmutableList.copyOf(new TypeDfsIterator(type));
}

private static boolean isSimilarType(XType type, List<TypeName> flatTypeNames) {
private static boolean isSimilarType(DaggerType type, ImmutableList<TypeName> flatTypeNames) {
return Iterators.elementsEqual(flatTypeNames.iterator(), new TypeDfsIterator(type));
}

Expand Down Expand Up @@ -261,8 +263,8 @@ private boolean typeHasInjectionSites(Key key) {
private static class TypeDfsIterator implements Iterator<TypeName> {
final Deque<XType> stack = new ArrayDeque<>();

TypeDfsIterator(XType root) {
stack.push(root);
TypeDfsIterator(DaggerType root) {
stack.push(root.xprocessing());
}

@Override
Expand Down
12 changes: 0 additions & 12 deletions javatests/dagger/internal/codegen/MissingBindingValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1460,12 +1460,6 @@ public void requestWildcardTypeWithNonWildcardTypeBindingProvided_failsWithMissi
"Note: A similar binding is provided in the following other components:",
" Set<Bar> is provided at:",
" [MyComponent] Dagger-generated binding for Set<Bar>",
// TODO(b/360278200): Remove multibinding contributions from this list.
" Set<Bar> TestModule#provideBars is provided at:",
" [MyComponent] TestModule.provideBars()",
// TODO(b/360278200): Remove this as it's already listed above.
" Set<? extends Bar> is provided at:",
" [Child] ChildModule.provideBar()",
JVM_SUPPRESS_WILDCARDS_MESSAGE,
"",
"======================"))
Expand Down Expand Up @@ -1539,9 +1533,6 @@ public void requestWildcardTypeWithNonWildcardTypeBindingProvided_failsWithMissi
"Note: A similar binding is provided in the following other components:",
" Set<Bar> is provided at:",
" [MyComponent] Dagger-generated binding for Set<Bar>",
// TODO(b/360278200): Remove multibinding contributions from this list.
" Set<Bar> TestModule#provideBars is provided at:",
" [MyComponent] TestModule.provideBars()",
JVM_SUPPRESS_WILDCARDS_MESSAGE,
"",
"======================"))
Expand Down Expand Up @@ -1618,9 +1609,6 @@ public void injectWildcardTypeWithNonWildcardTypeBindingProvided_failsWithMissin
"Note: A similar binding is provided in the following other components:",
" Set<Bar> is provided at:",
" [MyComponent] Dagger-generated binding for Set<Bar>",
// TODO(b/360278200): Remove multibinding contributions from this list.
" Set<Bar> TestModule#provideBars is provided at:",
" [MyComponent] TestModule.provideBars()",
JVM_SUPPRESS_WILDCARDS_MESSAGE,
"",
"======================"))
Expand Down
Loading