From 706158186edb936f3015cce07dae0cc4e6513a86 Mon Sep 17 00:00:00 2001 From: jbock Date: Sat, 2 Dec 2023 16:42:52 +0100 Subject: [PATCH 1/2] ISSUES-17 add BindingRegistry --- .../simple/processor/ContextComponent.java | 26 ++++++++++ .../processor/binding/ComponentElement.java | 34 ++++++++++--- .../processor/binding/InjectBinding.java | 20 ++++++++ .../binding/InjectBindingFactory.java | 25 ++++----- .../processor/step/BindingRegistry.java | 30 +++++++++++ .../simple/processor/step/ComponentStep.java | 19 +++---- .../simple/processor/step/InjectStep.java | 7 ++- .../simple/processor/step/ProvidesStep.java | 6 ++- .../validation/InjectBindingValidator.java | 48 ++++++++++------- .../processor/BindingValidationTest.java | 2 +- .../processor/DuplicateBindingTest.java | 51 +++++++++++++++++++ 11 files changed, 214 insertions(+), 54 deletions(-) create mode 100644 compiler/src/main/java/io/jbock/simple/processor/step/BindingRegistry.java diff --git a/compiler/src/main/java/io/jbock/simple/processor/ContextComponent.java b/compiler/src/main/java/io/jbock/simple/processor/ContextComponent.java index ed9fa9e..9dc2d98 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/ContextComponent.java +++ b/compiler/src/main/java/io/jbock/simple/processor/ContextComponent.java @@ -1,6 +1,7 @@ package io.jbock.simple.processor; import io.jbock.simple.Component; +import io.jbock.simple.Inject; import io.jbock.simple.processor.binding.ComponentElement; import io.jbock.simple.processor.binding.InjectBindingFactory; import io.jbock.simple.processor.binding.KeyFactory; @@ -40,4 +41,29 @@ static ContextComponent create( Generator generator(); TopologicalSorter topologicalSorter(); + + final class Factory { + private final TypeTool tool; + private final InjectBindingFactory injectBindingFactory; + private final KeyFactory keyFactory; + + @Inject + public Factory( + TypeTool tool, + InjectBindingFactory injectBindingFactory, + KeyFactory keyFactory) { + this.tool = tool; + this.injectBindingFactory = injectBindingFactory; + this.keyFactory = keyFactory; + } + + public ContextComponent create(ComponentElement component) { + return ContextComponent_Impl.builder() + .componentElement(component) + .tool(tool) + .injectBindingFactory(injectBindingFactory) + .keyFactory(keyFactory) + .build(); + } + } } diff --git a/compiler/src/main/java/io/jbock/simple/processor/binding/ComponentElement.java b/compiler/src/main/java/io/jbock/simple/processor/binding/ComponentElement.java index a229899..1f63361 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/binding/ComponentElement.java +++ b/compiler/src/main/java/io/jbock/simple/processor/binding/ComponentElement.java @@ -2,6 +2,7 @@ import io.jbock.javapoet.ClassName; import io.jbock.simple.Component; +import io.jbock.simple.Inject; import io.jbock.simple.Provides; import io.jbock.simple.processor.util.ValidationFailure; import io.jbock.simple.processor.util.Visitors; @@ -25,6 +26,7 @@ public final class ComponentElement { private final TypeElement element; private final KeyFactory keyFactory; + private final InjectBinding.Factory injectBindingFactory; private final Supplier generatedClass = memoize(() -> { ClassName className = ClassName.get(element()); @@ -72,7 +74,7 @@ public final class ComponentElement { continue; // ignore } Key key = keyFactory().getKey(method); - InjectBinding b = InjectBinding.create(keyFactory(), method); + InjectBinding b = injectBindingFactory().create(method); result.put(key, b); } return result; @@ -122,15 +124,11 @@ public final class ComponentElement { private ComponentElement( TypeElement element, - KeyFactory keyFactory) { + KeyFactory keyFactory, + InjectBinding.Factory injectBindingFactory) { this.element = element; this.keyFactory = keyFactory; - } - - public static ComponentElement create( - TypeElement element, - KeyFactory keyFactory) { - return new ComponentElement(element, keyFactory); + this.injectBindingFactory = injectBindingFactory; } public TypeElement element() { @@ -165,6 +163,10 @@ private KeyFactory keyFactory() { return keyFactory; } + private InjectBinding.Factory injectBindingFactory() { + return injectBindingFactory; + } + public Optional parameterBinding(Key key) { return Optional.ofNullable(parameterBindings.get().get(key)); } @@ -188,4 +190,20 @@ public boolean omitMockBuilder() { } return annotation.omitMockBuilder(); } + + public static final class Factory { + private final KeyFactory keyFactory; + private final InjectBinding.Factory injectBindingFactory; + + @Inject + public Factory(KeyFactory keyFactory, InjectBinding.Factory injectBindingFactory) { + this.keyFactory = keyFactory; + this.injectBindingFactory = injectBindingFactory; + } + + public ComponentElement create( + TypeElement element) { + return new ComponentElement(element, keyFactory, injectBindingFactory); + } + } } diff --git a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java index 349213d..7687f29 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java +++ b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java @@ -4,6 +4,7 @@ import io.jbock.javapoet.ParameterSpec; import io.jbock.javapoet.ParameterizedTypeName; import io.jbock.javapoet.TypeName; +import io.jbock.simple.Inject; import io.jbock.simple.Provides; import io.jbock.simple.processor.util.ValidationFailure; @@ -141,4 +142,23 @@ public String toString() { private KeyFactory keyFactory() { return keyFactory; } + + public static final class Factory { + private final KeyFactory keyFactory; + + @Inject + public Factory(KeyFactory keyFactory) { + this.keyFactory = keyFactory; + } + + InjectBinding create(ExecutableElement m) { + Key key = keyFactory.getKey(m); + if (m.getKind() == ElementKind.CONSTRUCTOR) { + if (key.qualifier().isPresent()) { + throw new ValidationFailure("Constructors can't have qualifiers", m); + } + } + return new InjectBinding(key, keyFactory, m); + } + } } diff --git a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBindingFactory.java b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBindingFactory.java index c91bf3a..0bb1abc 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBindingFactory.java +++ b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBindingFactory.java @@ -3,7 +3,6 @@ import io.jbock.simple.Inject; import io.jbock.simple.processor.util.ClearableCache; import io.jbock.simple.processor.util.TypeTool; -import io.jbock.simple.processor.util.ValidationFailure; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; @@ -15,8 +14,6 @@ import java.util.Optional; import java.util.stream.Collectors; -import static io.jbock.simple.processor.util.Printing.INDENT; -import static io.jbock.simple.processor.util.Printing.bindingElementToString; import static io.jbock.simple.processor.util.Visitors.EXECUTABLE_ELEMENT_VISITOR; import static io.jbock.simple.processor.util.Visitors.TYPE_ELEMENT_VISITOR; @@ -26,11 +23,16 @@ public final class InjectBindingFactory implements ClearableCache { private final TypeTool tool; private final KeyFactory keyFactory; + private final InjectBinding.Factory injectBindingFactory; @Inject - public InjectBindingFactory(TypeTool tool, KeyFactory keyFactory) { + public InjectBindingFactory( + TypeTool tool, + KeyFactory keyFactory, + InjectBinding.Factory injectBindingFactory) { this.tool = tool; this.keyFactory = keyFactory; + this.injectBindingFactory = injectBindingFactory; } public Map injectBindings(TypeElement typeElement) { @@ -47,18 +49,11 @@ public Map injectBindings(TypeElement typeElement) { return Map.of(); } result = new LinkedHashMap<>(); - for (ExecutableElement m : allMembers) { - InjectBinding b = InjectBinding.create(keyFactory, m); - InjectBinding previous = result.put(b.key(), b); - if (previous != null) { - throw new ValidationFailure("This binding clashes with:\n" - + INDENT - + bindingElementToString(previous.element()) - + ".\n" - + "Consider a (different) qualifier", b.element()); - } - injectBindingCache.put(typeElement, result); + for (ExecutableElement method : allMembers) { + InjectBinding b = injectBindingFactory.create(method); + result.put(b.key(), b); // duplicates handled elsewhere } + injectBindingCache.put(typeElement, result); return result; } diff --git a/compiler/src/main/java/io/jbock/simple/processor/step/BindingRegistry.java b/compiler/src/main/java/io/jbock/simple/processor/step/BindingRegistry.java new file mode 100644 index 0000000..7eeba18 --- /dev/null +++ b/compiler/src/main/java/io/jbock/simple/processor/step/BindingRegistry.java @@ -0,0 +1,30 @@ +package io.jbock.simple.processor.step; + +import io.jbock.simple.Inject; +import io.jbock.simple.processor.binding.Key; +import io.jbock.simple.processor.binding.KeyFactory; +import io.jbock.simple.processor.util.ValidationFailure; + +import javax.lang.model.element.Element; +import javax.lang.model.element.ExecutableElement; +import java.util.HashMap; +import java.util.Map; + +public class BindingRegistry { + + private final Map bindings = new HashMap<>(); + private final KeyFactory keyFactory; + + @Inject + public BindingRegistry(KeyFactory keyFactory) { + this.keyFactory = keyFactory; + } + + void register(ExecutableElement method) { + Key key = keyFactory.getKey(method); + Element previous = bindings.put(key, method); + if (previous != null && !previous.equals(method)) { + throw new ValidationFailure("Duplicate binding for " + key, method); + } + } +} diff --git a/compiler/src/main/java/io/jbock/simple/processor/step/ComponentStep.java b/compiler/src/main/java/io/jbock/simple/processor/step/ComponentStep.java index b1fba97..1c18f7b 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/step/ComponentStep.java +++ b/compiler/src/main/java/io/jbock/simple/processor/step/ComponentStep.java @@ -7,8 +7,6 @@ import io.jbock.simple.processor.ContextComponent; import io.jbock.simple.processor.binding.Binding; import io.jbock.simple.processor.binding.ComponentElement; -import io.jbock.simple.processor.binding.InjectBindingFactory; -import io.jbock.simple.processor.binding.KeyFactory; import io.jbock.simple.processor.util.SpecWriter; import io.jbock.simple.processor.util.TypeTool; import io.jbock.simple.processor.util.ValidationFailure; @@ -31,28 +29,28 @@ public class ComponentStep implements Step { private final Messager messager; private final TypeTool tool; - private final KeyFactory keyFactory; private final TypeElementValidator typeElementValidator; private final ExecutableElementValidator executableElementValidator; private final SpecWriter specWriter; - private final InjectBindingFactory injectBindingFactory; + private final ComponentElement.Factory componentElementFactory; + private final ContextComponent.Factory contextComponentFactory; @Inject public ComponentStep( Messager messager, TypeTool tool, - KeyFactory keyFactory, TypeElementValidator typeElementValidator, ExecutableElementValidator executableElementValidator, SpecWriter specWriter, - InjectBindingFactory injectBindingFactory) { + ComponentElement.Factory componentElementFactory, + ContextComponent.Factory contextComponentFactory) { this.messager = messager; this.tool = tool; - this.keyFactory = keyFactory; this.typeElementValidator = typeElementValidator; this.executableElementValidator = executableElementValidator; this.specWriter = specWriter; - this.injectBindingFactory = injectBindingFactory; + this.componentElementFactory = componentElementFactory; + this.contextComponentFactory = contextComponentFactory; } @Override @@ -76,7 +74,7 @@ public Set process(Map> elementsByAnnota private void process(TypeElement typeElement) { typeElementValidator.validate(typeElement); - ComponentElement component = ComponentElement.create(typeElement, keyFactory); + ComponentElement component = componentElementFactory.create(typeElement); component.factoryElement().ifPresent(factory -> { ExecutableElement method = factory.singleAbstractMethod(); if (!tool.types().isSameType(method.getReturnType(), typeElement.asType())) { @@ -88,8 +86,7 @@ private void process(TypeElement typeElement) { executableElementValidator.validate(m); } } - ContextComponent componentComponent = ContextComponent.create( - component, tool, injectBindingFactory, keyFactory); + ContextComponent componentComponent = contextComponentFactory.create(component); Generator generator = componentComponent.generator(); List bindings = componentComponent.topologicalSorter().sortedBindings(); TypeSpec typeSpec = generator.generate(bindings); diff --git a/compiler/src/main/java/io/jbock/simple/processor/step/InjectStep.java b/compiler/src/main/java/io/jbock/simple/processor/step/InjectStep.java index e3a050b..8894227 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/step/InjectStep.java +++ b/compiler/src/main/java/io/jbock/simple/processor/step/InjectStep.java @@ -25,15 +25,18 @@ public class InjectStep implements Step { private final InjectBindingValidator validator; private final ExecutableElementValidator executableElementValidator; private final Messager messager; + private final BindingRegistry bindingRegistry; @Inject public InjectStep( InjectBindingValidator validator, ExecutableElementValidator executableElementValidator, - Messager messager) { + Messager messager, + BindingRegistry bindingRegistry) { this.validator = validator; this.executableElementValidator = executableElementValidator; this.messager = messager; + this.bindingRegistry = bindingRegistry; } @Override @@ -55,10 +58,12 @@ public Set process(Map> elementsByAnnota for (ExecutableElement constructor : constructors) { executableElementValidator.validate(constructor); validator.validateConstructor(constructor); + bindingRegistry.register(constructor); } for (ExecutableElement method : methods) { executableElementValidator.validate(method); validator.validateStaticMethod(method); + bindingRegistry.register(method); } checkFields(elements); } catch (ValidationFailure f) { diff --git a/compiler/src/main/java/io/jbock/simple/processor/step/ProvidesStep.java b/compiler/src/main/java/io/jbock/simple/processor/step/ProvidesStep.java index 69cf19c..99ade03 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/step/ProvidesStep.java +++ b/compiler/src/main/java/io/jbock/simple/processor/step/ProvidesStep.java @@ -20,11 +20,14 @@ public class ProvidesStep implements Step { private final Messager messager; + private final BindingRegistry bindingRegistry; @Inject public ProvidesStep( - Messager messager) { + Messager messager, + BindingRegistry bindingRegistry) { this.messager = messager; + this.bindingRegistry = bindingRegistry; } @Override @@ -50,6 +53,7 @@ public Set process(Map> elementsByAnnota if (enclosing.getAnnotation(Component.class) == null) { throw new ValidationFailure("The @Provides method must be nested inside a @Component", m); } + bindingRegistry.register(m); } } catch (ValidationFailure f) { f.writeTo(messager); diff --git a/compiler/src/main/java/io/jbock/simple/processor/validation/InjectBindingValidator.java b/compiler/src/main/java/io/jbock/simple/processor/validation/InjectBindingValidator.java index e4e265f..b632edb 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/validation/InjectBindingValidator.java +++ b/compiler/src/main/java/io/jbock/simple/processor/validation/InjectBindingValidator.java @@ -8,7 +8,6 @@ import io.jbock.simple.processor.util.ValidationFailure; import io.jbock.simple.processor.util.Visitors; -import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; @@ -37,13 +36,39 @@ public void validateConstructor(ExecutableElement element) { validate(element); } - public void validateStaticMethod(ExecutableElement element) { - validate(element); - if (!tool.types().isSameType(element.getReturnType(), element.getEnclosingElement().asType())) { - throw new ValidationFailure("Static method binding must return the enclosing type", - element); + public void validateStaticMethod(ExecutableElement method) { + validate(method); + TypeElement typeElement = Visitors.TYPE_ELEMENT_VISITOR.visit(method.getEnclosingElement()); +// List hierarchyMethod = getEnclosingElements(typeElement); +// List hierarchyRt = tool.types().asElement(method.getReturnType()) +// .map(Visitors.TYPE_ELEMENT_VISITOR::visit) +// .map(this::getEnclosingElements) +// .orElse(List.of()); + if (!method.getModifiers().contains(Modifier.STATIC)) { + throw new ValidationFailure("The factory method must be static", method); + } + if (method.getReturnType().getKind() == TypeKind.VOID) { + throw new ValidationFailure("The factory method may not return void", method); + } + if (!tool.types().isSameType(method.getReturnType(), typeElement.asType())) { + throw new ValidationFailure("The factory method must return the type of its enclosing class", method); + } + } + +/* + private List getEnclosingElements(TypeElement typeElement) { + if (typeElement == null) { + return List.of(); } + List acc = new ArrayList<>(2); + acc.add(typeElement); + TypeElement el = typeElement; + if ((el = Visitors.TYPE_ELEMENT_VISITOR.visit(el.getEnclosingElement())) != null) { + acc.add(el); + } + return acc; } +*/ private void validate(ExecutableElement element) { TypeElement typeElement = Visitors.TYPE_ELEMENT_VISITOR.visit(element.getEnclosingElement()); @@ -52,17 +77,6 @@ private void validate(ExecutableElement element) { } Map m = injectBindingFactory.injectBindings(typeElement); for (InjectBinding b : m.values()) { - if (b.element().getKind() == ElementKind.METHOD) { - if (!b.element().getModifiers().contains(Modifier.STATIC)) { - throw new ValidationFailure("The factory method must be static", b.element()); - } - if (b.element().getReturnType().getKind() == TypeKind.VOID) { - throw new ValidationFailure("The factory method may not return void", b.element()); - } - if (!tool.types().isSameType(b.element().getReturnType(), typeElement.asType())) { - throw new ValidationFailure("The factory method must return the type of its enclosing class", b.element()); - } - } if (b.element().getAnnotationMirrors().stream().filter(mirror -> { DeclaredType annotationType = mirror.getAnnotationType(); return tool.isSameType(annotationType, JAVAX_INJECT) diff --git a/compiler/src/test/java/io/jbock/simple/processor/BindingValidationTest.java b/compiler/src/test/java/io/jbock/simple/processor/BindingValidationTest.java index b4f9105..040c457 100644 --- a/compiler/src/test/java/io/jbock/simple/processor/BindingValidationTest.java +++ b/compiler/src/test/java/io/jbock/simple/processor/BindingValidationTest.java @@ -26,7 +26,7 @@ void multipleStaticBindings() { "}"); Compilation compilation = simpleCompiler().compile(component); assertThat(compilation).failed(); - assertThat(compilation).hadErrorContaining("Consider a (different) qualifier"); + assertThat(compilation).hadErrorContaining("Duplicate binding for test.TestClass"); } @Test diff --git a/compiler/src/test/java/io/jbock/simple/processor/DuplicateBindingTest.java b/compiler/src/test/java/io/jbock/simple/processor/DuplicateBindingTest.java index 0acd9a3..671871c 100644 --- a/compiler/src/test/java/io/jbock/simple/processor/DuplicateBindingTest.java +++ b/compiler/src/test/java/io/jbock/simple/processor/DuplicateBindingTest.java @@ -79,4 +79,55 @@ void parameterBindingTakesPrecedence() { " }", "}"); } + + @Test + void staticMethodConflict() { + JavaFileObject component = forSourceLines("test.TestClass", + "package test;", + "", + "import io.jbock.simple.Component;", + "import io.jbock.simple.Inject;", + "import io.jbock.simple.Named;", + "", + "final class TestClass {", + "", + " static class A {", + " @Inject static A a() { return null; }", + " @Inject static A b() { return null; }", + " }", + "", + " @Component", + " interface AComponent {", + " A getA();", + " }", + "}"); + Compilation compilation = simpleCompiler().compile(component); + assertThat(compilation).failed(); + assertThat(compilation).hadErrorContaining("Duplicate binding for test.TestClass.A"); + } + + @Test + void staticMethodNoConflict() { + JavaFileObject component = forSourceLines("test.TestClass", + "package test;", + "", + "import io.jbock.simple.Component;", + "import io.jbock.simple.Inject;", + "import io.jbock.simple.Named;", + "", + "final class TestClass {", + "", + " static class A {", + " @Inject static A a() { return null; }", + " @Inject static @Named(\"b\") A b() { return null; }", + " }", + "", + " @Component", + " interface AComponent {", + " A getA();", + " }", + "}"); + Compilation compilation = simpleCompiler().compile(component); + assertThat(compilation).succeeded(); + } } \ No newline at end of file From 88689dba56bd03b09cb8cd975e07046966ef3ebc Mon Sep 17 00:00:00 2001 From: jbock Date: Sat, 2 Dec 2023 16:44:53 +0100 Subject: [PATCH 2/2] ISSUES-17 add BindingRegistry --- .../simple/processor/binding/InjectBinding.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java index 7687f29..47b81de 100644 --- a/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java +++ b/compiler/src/main/java/io/jbock/simple/processor/binding/InjectBinding.java @@ -95,18 +95,6 @@ private InjectBinding( this.keyFactory = keyFactory; } - static InjectBinding create( - KeyFactory keyFactory, - ExecutableElement m) { - Key key = keyFactory.getKey(m); - if (m.getKind() == ElementKind.CONSTRUCTOR) { - if (key.qualifier().isPresent()) { - throw new ValidationFailure("Constructors can't have qualifiers", m); - } - } - return new InjectBinding(key, keyFactory, m); - } - @Override public String suggestedVariableName() { return suggestedVariableName.get(); @@ -142,7 +130,7 @@ public String toString() { private KeyFactory keyFactory() { return keyFactory; } - + public static final class Factory { private final KeyFactory keyFactory;