From 1eeaec0528fa954bd005cacb0c8732a1c4cccc19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Sat, 3 Aug 2019 15:04:09 +0100 Subject: [PATCH 1/3] Fix "TODO implicit set binding duplicates explicit one.". --- .../java/com/example/IntegrationTest.java | 21 ++++++++++++ .../com/example/MultibindingSetConflict.java | 34 +++++++++++++++++++ .../src/main/java/dagger/reflect/Scope.java | 19 ++++++++++- 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 integration-tests/src/test/java/com/example/MultibindingSetConflict.java diff --git a/integration-tests/src/test/java/com/example/IntegrationTest.java b/integration-tests/src/test/java/com/example/IntegrationTest.java index 73fd798f..7ce0ed01 100644 --- a/integration-tests/src/test/java/com/example/IntegrationTest.java +++ b/integration-tests/src/test/java/com/example/IntegrationTest.java @@ -835,6 +835,27 @@ public void multibindingSetElements() { assertThat(component.values()).containsExactly("one", "two"); } + @Test + @IgnoreCodegen + public void multibindingSetConflictingExplicitAndImplicit() { + try { + backend.create(MultibindingSetConflict.class); + fail(); + } catch (IllegalStateException e) { + // Note: using only one of each IntoSet and ElementsIntoSet to make sure the test is stable, + // because Class.getDeclaredMethods() doesn't guarantee ordering. + assertThat(e) + .hasMessageThat() + .isEqualTo( + "java.util.Set has incompatible bindings or declarations:\n" + + " Set bindings and declarations:\n" + + " [single ] @Provides[com.example.MultibindingSetConflict$Module1.one(…)]\n" + + " [multiple] @Provides[com.example.MultibindingSetConflict$Module1.two(…)]\n" + + " Unique bindings and declarations:\n" + + " @Provides[com.example.MultibindingSetConflict$Module1.explicit(…)]\n"); + } + } + @Test public void multibindingSetPrimitive() { MultibindingSetPrimitive component = backend.create(MultibindingSetPrimitive.class); diff --git a/integration-tests/src/test/java/com/example/MultibindingSetConflict.java b/integration-tests/src/test/java/com/example/MultibindingSetConflict.java new file mode 100644 index 00000000..8c7bb66c --- /dev/null +++ b/integration-tests/src/test/java/com/example/MultibindingSetConflict.java @@ -0,0 +1,34 @@ +package com.example; + +import dagger.Component; +import dagger.Module; +import dagger.Provides; +import dagger.multibindings.ElementsIntoSet; +import dagger.multibindings.IntoSet; +import java.util.Collections; +import java.util.Set; + +@Component(modules = MultibindingSetConflict.Module1.class) +interface MultibindingSetConflict { + Set values(); + + @Module + abstract class Module1 { + @Provides + @IntoSet + static String one() { + return "one"; + } + + @Provides + @ElementsIntoSet + static Set two() { + return Collections.singleton("two"); + } + + @Provides + static Set explicit() { + return Collections.emptySet(); + } + } +} diff --git a/reflect/src/main/java/dagger/reflect/Scope.java b/reflect/src/main/java/dagger/reflect/Scope.java index 5e018355..81062416 100644 --- a/reflect/src/main/java/dagger/reflect/Scope.java +++ b/reflect/src/main/java/dagger/reflect/Scope.java @@ -334,7 +334,8 @@ Scope build() { Binding replaced = allBindings.put(key, new UnlinkedSetBinding(elementBindings, elementsBindings)); if (replaced != null) { - throw new IllegalStateException(); // TODO implicit set binding duplicates explicit one. + throw new IllegalStateException( + buildDuplicateSetBindingMessage(key, setBindings, replaced)); } } @@ -374,6 +375,22 @@ Scope build() { return new Scope(allBindings, jitLookupFactory, annotations, parent); } + private static String buildDuplicateSetBindingMessage( + Key key, SetBindings setBindings, Binding replaced) { + StringBuilder builder = new StringBuilder(); + builder.append(key).append(" has incompatible bindings or declarations:\n"); + builder.append(" Set bindings and declarations:\n"); + for (Binding elementBinding : setBindings.elementBindings) { + builder.append(" [single ] ").append(elementBinding).append("\n"); + } + for (Binding elementsBinding : setBindings.elementsBindings) { + builder.append(" [multiple] ").append(elementsBinding).append("\n"); + } + builder.append(" Unique bindings and declarations:\n"); + builder.append(" ").append(replaced).append("\n"); + return builder.toString(); + } + private static final class SetBindings { /** Bindings which produce a single element for the target set. */ final List elementBindings = new ArrayList<>(); From 010b9f0cc3dcbc7f66c44a52456c5fc3efba1592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Sat, 3 Aug 2019 15:26:20 +0100 Subject: [PATCH 2/3] Fix "TODO implicit map binding duplicates explicit one." for values. --- .../java/com/example/IntegrationTest.java | 20 +++++++++++++ .../com/example/MultibindingMapConflict.java | 29 +++++++++++++++++++ .../src/main/java/dagger/reflect/Scope.java | 21 +++++++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 integration-tests/src/test/java/com/example/MultibindingMapConflict.java diff --git a/integration-tests/src/test/java/com/example/IntegrationTest.java b/integration-tests/src/test/java/com/example/IntegrationTest.java index 7ce0ed01..9909cda7 100644 --- a/integration-tests/src/test/java/com/example/IntegrationTest.java +++ b/integration-tests/src/test/java/com/example/IntegrationTest.java @@ -916,6 +916,26 @@ public void multibindingMapNoUnwrap() { Annotations.tableKey(2, 3), "two"); } + @Test + @IgnoreCodegen + public void multibindingMapConflictingExplicitAndImplicit() { + try { + backend.create(MultibindingMapConflict.class); + fail(); + } catch (IllegalStateException e) { + // Note: using only one IntoMap to make sure the test is stable, + // because Class.getDeclaredMethods() doesn't guarantee ordering. + assertThat(e) + .hasMessageThat() + .isEqualTo( + "java.util.Map has incompatible bindings or declarations:\n" + + " Map bindings and declarations:\n" + + " first = @Provides[com.example.MultibindingMapConflict$Module1.one(…)]\n" + + " Unique bindings and declarations:\n" + + " @Provides[com.example.MultibindingMapConflict$Module1.explicit(…)]\n"); + } + } + @Test public void multibindingProviderMap() { MultibindingProviderMap component = backend.create(MultibindingProviderMap.class); diff --git a/integration-tests/src/test/java/com/example/MultibindingMapConflict.java b/integration-tests/src/test/java/com/example/MultibindingMapConflict.java new file mode 100644 index 00000000..8da3cd61 --- /dev/null +++ b/integration-tests/src/test/java/com/example/MultibindingMapConflict.java @@ -0,0 +1,29 @@ +package com.example; + +import dagger.Component; +import dagger.Module; +import dagger.Provides; +import dagger.multibindings.IntoMap; +import dagger.multibindings.StringKey; +import java.util.Collections; +import java.util.Map; + +@Component(modules = MultibindingMapConflict.Module1.class) +interface MultibindingMapConflict { + Map values(); + + @Module + abstract class Module1 { + @Provides + @IntoMap + @StringKey("first") + static Integer one() { + return 1; + } + + @Provides + static Map explicit() { + return Collections.emptyMap(); + } + } +} diff --git a/reflect/src/main/java/dagger/reflect/Scope.java b/reflect/src/main/java/dagger/reflect/Scope.java index 81062416..a289c24d 100644 --- a/reflect/src/main/java/dagger/reflect/Scope.java +++ b/reflect/src/main/java/dagger/reflect/Scope.java @@ -362,7 +362,8 @@ Scope build() { Binding replaced = allBindings.put(mapOfValueKey, new UnlinkedMapOfValueBinding(mapOfProviderKey)); if (replaced != null) { - throw new IllegalStateException(); // TODO implicit map binding duplicates explicit one. + throw new IllegalStateException( + buildDuplicateMapBindingMessage(mapOfValueKey, entryBindings, replaced)); } replaced = @@ -391,6 +392,24 @@ private static String buildDuplicateSetBindingMessage( return builder.toString(); } + private static String buildDuplicateMapBindingMessage( + Key key, Map entryBindings, Binding replaced) { + StringBuilder builder = new StringBuilder(); + builder.append(key).append(" has incompatible bindings or declarations:\n"); + builder.append(" Map bindings and declarations:\n"); + for (Map.Entry entry : entryBindings.entrySet()) { + builder + .append(" ") + .append(entry.getKey()) + .append(" = ") + .append(entry.getValue()) + .append("\n"); + } + builder.append(" Unique bindings and declarations:\n"); + builder.append(" ").append(replaced).append("\n"); + return builder.toString(); + } + private static final class SetBindings { /** Bindings which produce a single element for the target set. */ final List elementBindings = new ArrayList<>(); From db6b03c957bc3b9bcbf99feb7c75e9baf2891c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Papp=20=28TWiStErRob=29?= Date: Sat, 3 Aug 2019 15:41:12 +0100 Subject: [PATCH 3/3] Fix "TODO implicit map binding duplicates explicit one." for Provider. --- .../java/com/example/IntegrationTest.java | 20 +++++++++++++ .../MultibindingMapProviderConflict.java | 30 +++++++++++++++++++ .../src/main/java/dagger/reflect/Scope.java | 3 +- 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 integration-tests/src/test/java/com/example/MultibindingMapProviderConflict.java diff --git a/integration-tests/src/test/java/com/example/IntegrationTest.java b/integration-tests/src/test/java/com/example/IntegrationTest.java index 9909cda7..e256f09f 100644 --- a/integration-tests/src/test/java/com/example/IntegrationTest.java +++ b/integration-tests/src/test/java/com/example/IntegrationTest.java @@ -963,6 +963,26 @@ public void multibindingMapProvider() { assertThat(values.get("1").get()).isEqualTo("one"); } + @Test + @IgnoreCodegen + public void multibindingMapProviderConflictingExplicitAndImplicit() { + try { + backend.create(MultibindingMapProviderConflict.class); + fail(); + } catch (IllegalStateException e) { + // Note: using only one IntoMap to make sure the test is stable, + // because Class.getDeclaredMethods() doesn't guarantee ordering. + assertThat(e) + .hasMessageThat() + .isEqualTo( + "java.util.Map> has incompatible bindings or declarations:\n" + + " Map bindings and declarations:\n" + + " first = @Provides[com.example.MultibindingMapProviderConflict$Module1.one(…)]\n" + + " Unique bindings and declarations:\n" + + " @Provides[com.example.MultibindingMapProviderConflict$Module1.explicit(…)]\n"); + } + } + @Test public void moduleClass() { ModuleClass component = backend.create(ModuleClass.class); diff --git a/integration-tests/src/test/java/com/example/MultibindingMapProviderConflict.java b/integration-tests/src/test/java/com/example/MultibindingMapProviderConflict.java new file mode 100644 index 00000000..08033134 --- /dev/null +++ b/integration-tests/src/test/java/com/example/MultibindingMapProviderConflict.java @@ -0,0 +1,30 @@ +package com.example; + +import dagger.Component; +import dagger.Module; +import dagger.Provides; +import dagger.multibindings.IntoMap; +import dagger.multibindings.StringKey; +import java.util.Collections; +import java.util.Map; +import javax.inject.Provider; + +@Component(modules = MultibindingMapProviderConflict.Module1.class) +interface MultibindingMapProviderConflict { + Map> values(); + + @Module + abstract class Module1 { + @Provides + @IntoMap + @StringKey("first") + static Integer one() { + return 1; + } + + @Provides + static Map> explicit() { + return Collections.emptyMap(); + } + } +} diff --git a/reflect/src/main/java/dagger/reflect/Scope.java b/reflect/src/main/java/dagger/reflect/Scope.java index a289c24d..ea3f9d4c 100644 --- a/reflect/src/main/java/dagger/reflect/Scope.java +++ b/reflect/src/main/java/dagger/reflect/Scope.java @@ -369,7 +369,8 @@ Scope build() { replaced = allBindings.put(mapOfProviderKey, new UnlinkedMapOfProviderBinding(entryBindings)); if (replaced != null) { - throw new IllegalStateException(); // TODO implicit map binding duplicates explicit one. + throw new IllegalStateException( + buildDuplicateMapBindingMessage(mapOfProviderKey, entryBindings, replaced)); } }