From 6eb79e6a876124141ecef2e6df2a98dd1e119fc0 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Fri, 2 Dec 2022 22:39:35 -0500 Subject: [PATCH 01/10] Support for interceptors --- bom/pom.xml | 6 +- builder/README.md | 16 ++- .../io/helidon/builder/AttributeVisitor.java | 4 +- .../main/java/io/helidon/builder/Builder.java | 16 +++ .../java/io/helidon/builder/Interceptor.java | 49 +++++++ .../builder/RequiredAttributeVisitor.java | 8 +- .../builder/processor/tools/BodyContext.java | 28 ++++ .../tools/DefaultBuilderCreatorProvider.java | 135 ++++++++++++------ .../tools/GenerateInterceptorSupport.java | 53 +++++++ .../tools/GenerateVisitorSupport.java | 1 + builder/tests/builder/pom.xml | 19 +-- .../testsubjects/BeanBuilderInterceptor.java | 40 ++++++ .../test/testsubjects/InterceptedBean.java | 45 ++++++ .../builder/test/InterceptedBeanTest.java | 43 ++++++ builder/tests/nodeps/pom.xml | 107 ++++++++++++++ .../test/NoDepsInterceptorBeanTest.java | 26 ++++ builder/tests/pom.xml | 3 +- pico/builder-config/README.md | 23 +++ .../builder/config/spi/ConfigBeanBase.java | 1 + .../config/spi/DefaultConfigResolver.java | 2 +- .../io/helidon/pico/InjectionPointInfo.java | 2 +- .../pico/ServiceInjectionPlanBinder.java | 4 +- 22 files changed, 563 insertions(+), 68 deletions(-) create mode 100644 builder/builder/src/main/java/io/helidon/builder/Interceptor.java create mode 100644 builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java create mode 100644 builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java create mode 100644 builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java create mode 100644 builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java create mode 100644 builder/tests/nodeps/pom.xml create mode 100644 builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java create mode 100644 pico/builder-config/README.md diff --git a/bom/pom.xml b/bom/pom.xml index 937d59f9c02..35125db948f 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -1432,6 +1432,11 @@ helidon-builder-processor ${helidon.version} + + io.helidon.builder.tests + helidon-builder-test-builder + ${helidon.version} + @@ -1457,7 +1462,6 @@ ${helidon.version} - diff --git a/builder/README.md b/builder/README.md index 35f94258158..62884107602 100644 --- a/builder/README.md +++ b/builder/README.md @@ -13,6 +13,12 @@ The Helidon Builder provides compile-time code generation for fluent buil Supported annotation types (see [builder](./builder/src/main/java/io/helidon/builder) for further details): * Builder - similar to Lombok's SuperBuilder. * Singular - similar to Lombok's Singular. +* NonNull - accomplished alternatively via Helidon's ConfiguredOption#required. +* Default - accomplished alternatively via Helidon's ConfiguredOption#value. + +Explicitly unsupported (i.e., these are just a few of the types that do not have a counterpart from Helidon's Builder): +* NoArgsConstructor - must instead use one of the toBuilder() methods +* AllArgsConstructor - must instead use one of the toBuilder() methods Any and all types are supported by the Builder, with special handling for List, Map, Set, and Optional types. The target interface, however, should only contain getter like methods (i.e., has a non-void return and takes no arguments). All static and default methods @@ -40,21 +46,19 @@ The result of this will create (under ./target/generated-sources/annotations): * Support for toBuilder(). * Support for streams (see javadoc for [Builder](./builder/src/main/java/io/helidon/builder/Builder.java)). * Support for attribute visitors (see [test-builder](./tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java)). -* Support for attribute validation (see ConfiguredOption#required() and [builder](./tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java)). - -The implementation of the processor also allows for a provider-based extensibility mechanism. +* Support for attribute validation (see ConfiguredOption#required() and [test-builder](./tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java)). +* Support for builder interception (i.e., including decoration or mutation). (see [test-builder](./tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java)). ## Modules * [builder](./builder) - provides the compile-time annotations, as well as optional runtime supporting types. * [processor-spi](./processor-spi) - defines the Builder Processor SPI runtime definitions used by builder tooling. This module is only needed at compile time. * [processor-tools](./processor-tools) - provides the concrete creators & code generators. This module is only needed at compile time. * [processor](./processor) - the annotation processor which delegates to the processor-tools module for the main processing logic. This module is only needed at compile time. -* [tests/builder](./tests/builder) - internal tests that can also serve as examples on usage. +* [tests/builder](./tests/builder) - tests that can also serve as examples for usage. ## Customizations To implement your own custom Builder: -* Write an implementation of BuilderCreator having a higher-than-default Weighted value as compared to DefaultBuilderCreator. -* Include your module with this creator in your annotation processing path. +* See [pico/builder-config](../pico/builder-config) for an example. ## Usage See [tests/builder](./tests/builder) for usage examples. diff --git a/builder/builder/src/main/java/io/helidon/builder/AttributeVisitor.java b/builder/builder/src/main/java/io/helidon/builder/AttributeVisitor.java index 91e0d9d7433..ff9bdf88623 100644 --- a/builder/builder/src/main/java/io/helidon/builder/AttributeVisitor.java +++ b/builder/builder/src/main/java/io/helidon/builder/AttributeVisitor.java @@ -22,11 +22,13 @@ /** * A functional interface that can be used to visit all attributes of this type. *

- * This type is used when {@link Builder#requireLibraryDependencies()} is used. + * This type is used when {@link Builder#requireLibraryDependencies()} is used. When it is turned off, however, an equivalent + * type will be code-generated into each generated bean. * * @param type of the user defined context this attribute visitor supports */ @FunctionalInterface +// important note: this class is also code generated - please keep this in synch with generated code public interface AttributeVisitor { /** diff --git a/builder/builder/src/main/java/io/helidon/builder/Builder.java b/builder/builder/src/main/java/io/helidon/builder/Builder.java index 72d145431df..4fd4802f433 100644 --- a/builder/builder/src/main/java/io/helidon/builder/Builder.java +++ b/builder/builder/src/main/java/io/helidon/builder/Builder.java @@ -180,6 +180,22 @@ */ boolean allowNulls() default DEFAULT_ALLOW_NULLS; + /** + * The interceptor implementation type. See {@link Interceptor} for further details. Any interceptor applied will be called + * prior to validation. + * + * @return the interceptor implementation class + */ + Class interceptor() default Void.class; + + /** + * The (static) interceptor method to call on the {@link #interceptor()} implementation type. If left undefined then the new + * operator will be called on the type. This attribute is ignored if the {@link #interceptor()} class type is left undefined. + * + * @return the interceptor create method + */ + String interceptorCreateMethod() default ""; + /** * The list implementation type to apply, defaulting to {@link #DEFAULT_LIST_TYPE}. * diff --git a/builder/builder/src/main/java/io/helidon/builder/Interceptor.java b/builder/builder/src/main/java/io/helidon/builder/Interceptor.java new file mode 100644 index 00000000000..47e9fd96fe9 --- /dev/null +++ b/builder/builder/src/main/java/io/helidon/builder/Interceptor.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder; + +/** + * Provides a contract by which the {@link Builder}-annotated builder type can be intercepted (i.e., including decoration or + * mutation). + *

+ * This type is used when {@link Builder#requireLibraryDependencies} is used. When it is turned off, however, an equivalent + * type will be code-generated into each generated bean. + * Note also that in this situation your interceptor implementation does not need to implement this interface contract, + * but instead must adhere to the following: + *

+ * + * @param the type of the bean builder to intercept + * + * @see io.helidon.builder.Builder#interceptor() + */ +@FunctionalInterface +// important note: this class is also code generated - please keep this in synch with generated code +public interface Interceptor { + + /** + * Provides the ability to intercept (i.e., including decoration or mutation) the target. + * + * @param target the target being intercepted + * @return the mutated or replaced target (must not be null) + */ + T intercept(T target); + +} diff --git a/builder/builder/src/main/java/io/helidon/builder/RequiredAttributeVisitor.java b/builder/builder/src/main/java/io/helidon/builder/RequiredAttributeVisitor.java index aaee24ed362..d3787ae8a8b 100644 --- a/builder/builder/src/main/java/io/helidon/builder/RequiredAttributeVisitor.java +++ b/builder/builder/src/main/java/io/helidon/builder/RequiredAttributeVisitor.java @@ -43,7 +43,7 @@ public class RequiredAttributeVisitor implements AttributeVisitor { /** * Default constructor. */ - // important note: this needs to remain public since it will be new'ed from code-generated builder processing ... + // important note: this class is also code generated - please keep this in synch with generated code public RequiredAttributeVisitor() { this(Builder.DEFAULT_ALLOW_NULLS); } @@ -53,13 +53,13 @@ public RequiredAttributeVisitor() { * * @param allowNullsByDefault true if nulls should be allowed */ - // important note: this needs to remain public since it will be new'ed from code-generated builder processing ... + // important note: this class is also code generated - please keep this in synch with generated code public RequiredAttributeVisitor(boolean allowNullsByDefault) { this.allowNullsByDefault = allowNullsByDefault; } @Override - // important note: this needs to remain public since it will be new'ed from code-generated builder processing ... + // important note: this class is also code generated - please keep this in synch with generated code public void visit(String attrName, Supplier valueSupplier, Map meta, @@ -90,7 +90,7 @@ public void visit(String attrName, * * @throws java.lang.IllegalStateException when any attributes are in violation with the validation policy */ - // important note: this needs to remain public since it will be new'ed from code-generated builder processing ... + // important note: this class is also code generated - please keep this in synch with generated code public void validate() { if (!errors.isEmpty()) { throw new IllegalStateException(String.join(", ", errors)); diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 19783dd34b1..0f4d3c6367a 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -28,6 +28,7 @@ import io.helidon.builder.processor.spi.TypeInfo; import io.helidon.pico.types.AnnotationAndValue; import io.helidon.pico.types.DefaultAnnotationAndValue; +import io.helidon.pico.types.DefaultTypeName; import io.helidon.pico.types.TypeName; import io.helidon.pico.types.TypedElementName; @@ -70,6 +71,8 @@ public class BodyContext { private final String genericBuilderClassDecl; private final String genericBuilderAliasDecl; private final String genericBuilderAcceptAliasDecl; + private final TypeName interceptorTypeName; + private final String interceptorCreateMethod; /** * Constructor. @@ -106,6 +109,11 @@ public class BodyContext { this.genericBuilderClassDecl = "Builder"; this.genericBuilderAliasDecl = ("B".equals(typeInfo.typeName().className())) ? "BU" : "B"; this.genericBuilderAcceptAliasDecl = ("T".equals(typeInfo.typeName().className())) ? "TY" : "T"; + String interceptorType = searchForBuilderAnnotation("interceptor", builderTriggerAnnotation, typeInfo); + this.interceptorTypeName = (Void.class.getName().equals(interceptorType) || Objects.isNull(interceptorType)) + ? null : DefaultTypeName.createFromTypeName(interceptorType); + this.interceptorCreateMethod = Objects.requireNonNull( + searchForBuilderAnnotation("interceptorCreateMethod", builderTriggerAnnotation, typeInfo)); } /** @@ -323,6 +331,26 @@ protected String genericBuilderAcceptAliasDecl() { return genericBuilderAcceptAliasDecl; } + /** + * Returns the interceptor implementation type name. + * See {@link io.helidon.builder.Builder#interceptor()}. + * + * @return the interceptor type name + */ + public Optional interceptorTypeName() { + return Optional.ofNullable(interceptorTypeName); + } + + /** + * Returns the interceptor create method name. + * See {@link io.helidon.builder.Builder#interceptorCreateMethod()}. + * + * @return the interceptor create method + */ + public String interceptorCreateMethod() { + return interceptorCreateMethod; + } + /** * returns the bean attribute name of a particular method. * diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index e6cc3f58c38..b25c00a2cab 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -33,6 +33,7 @@ import io.helidon.builder.Annotated; import io.helidon.builder.AttributeVisitor; import io.helidon.builder.Builder; +import io.helidon.builder.Interceptor; import io.helidon.builder.RequiredAttributeVisitor; import io.helidon.builder.Singular; import io.helidon.builder.processor.spi.BuilderCreatorProvider; @@ -193,7 +194,7 @@ protected String toBody(BodyContext ctx) { appendFields(builder, ctx); appendCtor(builder, ctx); appendExtraPostCtorCode(builder, ctx); - appendInterfaceBasedGetters(builder, ctx); + appendInterfaceBasedGetters(builder, ctx, "", true); appendBasicGetters(builder, ctx); appendMetaAttributes(builder, ctx); appendToStringMethod(builder, ctx); @@ -219,18 +220,44 @@ protected void appendFooter(StringBuilder builder, builder.append("}\n"); } + /** + * Appends any interceptor on the builder. + * + * @param builder the builder + * @param ctx the context + * @param builderTag the tag (variable name) used for the builder arg + */ + protected void maybeAppendInterceptor(StringBuilder builder, + BodyContext ctx, + String builderTag) { + assert (!builderTag.equals("interceptor")); + if (ctx.interceptorTypeName().isPresent()) { + String impl = ctx.interceptorTypeName().get().name(); + builder.append("\t\t\t").append(impl).append(" interceptor = "); + if (ctx.interceptorCreateMethod().isBlank()) { + builder.append("new ").append(impl).append("();\n"); + } else { + builder.append(ctx.interceptorTypeName().get()).append(".").append(ctx.interceptorCreateMethod()).append("();\n"); + } + builder.append("\t\t\t").append(builderTag).append(" = interceptor.intercept(").append(builderTag).append(");\n"); + } + } + /** * Appends the simple {@link io.helidon.config.metadata.ConfiguredOption#required()} validation inside the build() method. * - * @param builder the builder - * @param ctx the context + * @param builder the builder + * @param ctx the context + * @param builderTag the tag (variable name) used for the builder arg */ - protected void appendRequiredValidator(StringBuilder builder, - BodyContext ctx) { + protected void appendRequiredVisitor(StringBuilder builder, + BodyContext ctx, + String builderTag) { + assert (!builderTag.equals("visitor")); if (ctx.includeMetaAttributes()) { builder.append("\t\t\tRequiredAttributeVisitor visitor = new RequiredAttributeVisitor(") .append(ctx.allowNulls()).append(");\n" - + "\t\t\tvisitAttributes(visitor, null);\n" + + "\t\t\t").append(builderTag).append(".visitAttributes(visitor, null);\n" + "\t\t\tvisitor.validate();\n"); } } @@ -247,13 +274,6 @@ protected void appendBasicGetters(StringBuilder builder, return; } - if (Objects.nonNull(ctx.parentAnnotationType().get())) { - builder.append("\t@Override\n"); - builder.append("\tpublic Class annotationType() {\n"); - builder.append("\t\treturn ").append(ctx.typeInfo().superTypeInfo().get().typeName()).append(".class;\n"); - builder.append("\t}\n\n"); - } - if (!ctx.hasParent() && ctx.hasStreamSupportOnImpl()) { builder.append("\t@Override\n" + "\tpublic T get() {\n" @@ -432,6 +452,7 @@ protected void appendExtraImports(StringBuilder builder, if (ctx.doingConcreteType()) { builder.append("import ").append(RequiredAttributeVisitor.class.getName()).append(";\n"); } + builder.append("import ").append(Interceptor.class.getName()).append(";\n"); builder.append("\n"); } } @@ -486,8 +507,8 @@ protected void appendExtraMethods(StringBuilder builder, } /** - * Adds extra inner classes to write on the builder. This default implementation will write the AttributeVisitor inner class - * if this is the root interface (i.e., hasParent is false), as well as the RequiredAttributeVisitor. + * Adds extra inner classes to write on the builder. This default implementation will write the {@code AttributeVisitor}, + * {@code RequiredAttributeVisitor} and {@code Interceptor} inner classes on the base abstract parent (ie, hasParent is false). * * @param builder the builder * @param ctx the context @@ -495,6 +516,7 @@ protected void appendExtraMethods(StringBuilder builder, protected void appendExtraInnerClasses(StringBuilder builder, BodyContext ctx) { GenerateVisitorSupport.appendExtraInnerClasses(builder, ctx); + GenerateInterceptorSupport.appendExtraInnerClasses(builder, ctx); } /** @@ -631,13 +653,17 @@ protected void appendExtraBuilderFields(StringBuilder builder, } /** - * Adds extra builder pre-steps. + * Adds extra builder build() method pre-steps prior to the builder being built into the target. * * @param builder the builder * @param ctx the context + * @param builderTag the tag (variable name) used for the builder arg */ protected void appendBuilderBuildPreSteps(StringBuilder builder, - BodyContext ctx) { + BodyContext ctx, + String builderTag) { + maybeAppendInterceptor(builder, ctx, builderTag); + appendRequiredVisitor(builder, ctx, builderTag); } /** @@ -1025,7 +1051,7 @@ private void appendBuilder(StringBuilder builder, appendExtraBuilderFields(builder, ctx); appendBuilderBody(builder, ctx); -// appendExtraBuilderMethods(builder, ctx); + appendInterfaceBasedGetters(builder, ctx, "\t", true); if (ctx.doingConcreteType()) { if (ctx.hasParent()) { @@ -1034,9 +1060,9 @@ private void appendBuilder(StringBuilder builder, GenerateJavadoc.buildMethod(builder); } builder.append("\t\tpublic ").append(ctx.implTypeName()).append(" build() {\n"); - appendRequiredValidator(builder, ctx); - appendBuilderBuildPreSteps(builder, ctx); - builder.append("\t\t\treturn new ").append(ctx.implTypeName().className()).append("(this);\n"); + builder.append("\t\t\tBuilder b = this;\n"); + appendBuilderBuildPreSteps(builder, ctx, "b"); + builder.append("\t\t\treturn new ").append(ctx.implTypeName().className()).append("(b);\n"); builder.append("\t\t}\n"); } else { int i = 0; @@ -1173,7 +1199,7 @@ private void appendBuilderBody(StringBuilder builder, if (typeName.isList() || typeName.isMap() || typeName.isSet()) { continue; } - addField(builder, method, typeName, beanAttributeName); + addBuilderField(builder, method, typeName, beanAttributeName); } builder.append("\n"); } @@ -1192,13 +1218,12 @@ private void appendBuilderBody(StringBuilder builder, builder.append("\t\t}\n\n"); } - private void addField(StringBuilder builder, - TypedElementName method, - TypeName type, - String beanAttributeName) { - + private void addBuilderField(StringBuilder builder, + TypedElementName method, + TypeName type, + String beanAttributeName) { GenerateJavadoc.builderField(builder, method); - builder.append("\t\tprotected ").append(type.array() ? type.fqName() : type.name()).append(" ") + builder.append("\t\tprivate ").append(type.array() ? type.fqName() : type.name()).append(" ") .append(beanAttributeName); Optional defaultVal = toConfiguredOptionValue(method, true, true); if (defaultVal.isPresent()) { @@ -1272,15 +1297,18 @@ private void appendBuilderHeader(StringBuilder builder, } else { Optional baseExtendsTypeName = baseExtendsBuilderTypeName(ctx); if (baseExtendsTypeName.isPresent()) { - builder.append("\n\t\t\t\t\t\t\t\t\t\t" - + "extends ") + builder.append("\n\t\t\t\t\t\t\t\t\t\textends ") .append(baseExtendsTypeName.get().fqName()) .append("\n\t\t\t\t\t\t\t\t\t\t"); } } + if (ctx.hasStreamSupportOnBuilder() || !ctx.hasParent()) { + builder.append("implements ").append(ctx.typeInfo().typeName().name()); + } + if (ctx.hasStreamSupportOnBuilder()) { - builder.append("implements Supplier<").append(ctx.genericBuilderAcceptAliasDecl()).append(">"); + builder.append(", Supplier<").append(ctx.genericBuilderAcceptAliasDecl()).append(">"); } if (!ctx.hasParent()) { @@ -1311,7 +1339,9 @@ private void appendToBuilderMethods(StringBuilder builder, } private void appendInterfaceBasedGetters(StringBuilder builder, - BodyContext ctx) { + BodyContext ctx, + String prefix, + boolean isOverrride) { if (ctx.doingConcreteType()) { return; } @@ -1320,13 +1350,30 @@ private void appendInterfaceBasedGetters(StringBuilder builder, for (String beanAttributeName : ctx.allAttributeNames()) { TypedElementName method = ctx.allTypeInfos().get(i); appendAnnotations(builder, method.annotations(), "\t"); - builder.append("\t@Override\n"); - builder.append("\tpublic ").append(toGenerics(method, false)).append(" ").append(method.elementName()) + if (isOverrride) { + builder.append(prefix) + .append("\t@Override\n"); + } + builder.append(prefix) + .append("\tpublic ").append(toGenerics(method, false)).append(" ").append(method.elementName()) .append("() {\n"); - builder.append("\t\treturn ").append(beanAttributeName).append(";\n"); - builder.append("\t}\n\n"); + builder.append(prefix) + .append("\t\treturn ").append(beanAttributeName).append(";\n"); + builder.append(prefix) + .append("\t}\n\n"); i++; } + + if (Objects.nonNull(ctx.parentAnnotationType().get())) { + builder.append(prefix) + .append("\t@Override\n"); + builder.append(prefix) + .append("\tpublic Class annotationType() {\n"); + builder.append(prefix) + .append("\t\treturn ").append(ctx.typeInfo().superTypeInfo().get().typeName()).append(".class;\n"); + builder.append(prefix) + .append("\t}\n\n"); + } } private void appendCtor(StringBuilder builder, @@ -1345,7 +1392,6 @@ private void appendCtor(StringBuilder builder, builder.append(" b) {\n"); appendExtraCtorCode(builder, ctx, "b"); appendCtorCodeBody(builder, ctx, "b"); -// builder.append("\t}\n"); } builder.append("\t}\n\n"); @@ -1356,13 +1402,13 @@ private void appendCtor(StringBuilder builder, * * @param builder the builder * @param ctx the context - * @param builderTag the builder tag + * @param builderTag the tag (variable name) used for the builder arg */ protected void appendCtorCodeBody(StringBuilder builder, BodyContext ctx, String builderTag) { if (ctx.hasParent()) { - builder.append("\t\tsuper(b);\n"); + builder.append("\t\tsuper(").append(builderTag).append(");\n"); } int i = 0; for (String beanAttributeName : ctx.allAttributeNames()) { @@ -1371,15 +1417,18 @@ protected void appendCtorCodeBody(StringBuilder builder, if (method.typeName().isList()) { builder.append("Collections.unmodifiableList(new ") - .append(ctx.listType()).append("<>(b.").append(beanAttributeName).append("));\n"); + .append(ctx.listType()).append("<>(").append(builderTag).append(".") + .append(beanAttributeName).append("));\n"); } else if (method.typeName().isMap()) { builder.append("Collections.unmodifiableMap(new ") - .append(ctx.mapType()).append("<>(b.").append(beanAttributeName).append("));\n"); + .append(ctx.mapType()).append("<>(").append(builderTag).append(".") + .append(beanAttributeName).append("));\n"); } else if (method.typeName().isSet()) { builder.append("Collections.unmodifiableSet(new ") - .append(ctx.setType()).append("<>(b.").append(beanAttributeName).append("));\n"); + .append(ctx.setType()).append("<>(").append(builderTag).append(".") + .append(beanAttributeName).append("));\n"); } else { - builder.append("b.").append(beanAttributeName).append(";\n"); + builder.append(builderTag).append(".").append(beanAttributeName).append(";\n"); } } } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java new file mode 100644 index 00000000000..2f323b81c3b --- /dev/null +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.processor.tools; + +/** + * See {@link io.helidon.builder.Interceptor} for the prototypical output for this generated class. + */ +final class GenerateInterceptorSupport { + + private GenerateInterceptorSupport() { + } + + static void appendExtraInnerClasses(StringBuilder builder, + BodyContext ctx) { + if (ctx.doingConcreteType()) { + return; + } + + if (!ctx.hasParent() + && !ctx.requireLibraryDependencies()) { + builder.append("\n\n\t/**\n" + + "\t * A functional interface that can be used to intercept the target type.\n" + + "\t *\n" + + "\t * @param the type to intercept" + + "\t */\n"); + builder.append("\t@FunctionalInterface\n" + + "\tpublic static interface Interceptor {\n" + + "\t\t /**\n" + + "\t\t * Provides the ability to intercept the target.\n" + + "\t\t *\n" + + "\t\t * @param target the target being intercepted\n" + + "\t\t * @return the mutated or replaced target (must not be null)\n" + + "\t\t */\n" + + "\t\tT intercept(T target);\n" + + "\t}\n"); + } + } + +} diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateVisitorSupport.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateVisitorSupport.java index 9756d164652..b7427e75375 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateVisitorSupport.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateVisitorSupport.java @@ -34,6 +34,7 @@ static void appendExtraInnerClasses(StringBuilder builder, && !ctx.requireLibraryDependencies()) { builder.append("\n\n\t/**\n" + "\t * A functional interface that can be used to visit all attributes of this type.\n" + + "\t *\n" + "\t * @param type of user defined context" + "\t */\n"); builder.append("\t@FunctionalInterface\n" diff --git a/builder/tests/builder/pom.xml b/builder/tests/builder/pom.xml index 9444fa1709d..e53a57a3ffd 100644 --- a/builder/tests/builder/pom.xml +++ b/builder/tests/builder/pom.xml @@ -31,14 +31,6 @@ helidon-builder-test-builder Helidon Builder Tests - - true - true - true - true - true - - io.helidon.builder @@ -102,6 +94,17 @@ + + org.apache.maven.plugins + maven-jar-plugin + + + + test-jar + + + + diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java new file mode 100644 index 00000000000..eea3ab3f19b --- /dev/null +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.testsubjects; + +/** + * See {@link InterceptedBean}. Notice how the Builder annotation on {@link InterceptedBean} sets the + * {@link io.helidon.builder.Builder#requireLibraryDependencies()} attribute to {@code false}, which is why this class does not + * need to implement {@link io.helidon.builder.Interceptor}. + * + * @see InterceptedBean + */ +class BeanBuilderInterceptor /* implements Interceptor */ { + + private BeanBuilderInterceptor() { + } + + static BeanBuilderInterceptor create() { + return new BeanBuilderInterceptor(); + } + +// @Override + DefaultInterceptedBean.Builder intercept(DefaultInterceptedBean.Builder target) { + return target.helloMessage("Hello " + target.name()); + } + +} diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java new file mode 100644 index 00000000000..d17a3ba8b4e --- /dev/null +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.testsubjects; + +import io.helidon.builder.Builder; +import io.helidon.config.metadata.ConfiguredOption; + +/** + * Demonstrates interception of builders. + */ +@Builder(requireLibraryDependencies = false, + interceptor = BeanBuilderInterceptor.class, + interceptorCreateMethod = "create") +public interface InterceptedBean { + + /** + * The name to say hello to. + * + * @return the name + */ + @ConfiguredOption(required = true) + String name(); + + /** + * The hello message will be explicitly overridden to say "hello {@link #name()}". + * + * @return the hello message + */ + String helloMessage(); + +} diff --git a/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java b/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java new file mode 100644 index 00000000000..94c43554e89 --- /dev/null +++ b/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test; + +import io.helidon.builder.test.testsubjects.DefaultInterceptedBean; +import io.helidon.builder.test.testsubjects.InterceptedBean; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +public class InterceptedBeanTest { + + @Test + protected void testMutation() { + InterceptedBean val = DefaultInterceptedBean.builder() + .name("Larry") + .build(); + assertThat(val.name(), equalTo("Larry")); + assertThat(val.helloMessage(), equalTo("Hello Larry")); + + InterceptedBean val2 = DefaultInterceptedBean.builder() + .name("Larry") + .build(); + assertThat(val, equalTo(val2)); + } + +} diff --git a/builder/tests/nodeps/pom.xml b/builder/tests/nodeps/pom.xml new file mode 100644 index 00000000000..9b1325a878d --- /dev/null +++ b/builder/tests/nodeps/pom.xml @@ -0,0 +1,107 @@ + + + + + + io.helidon.builder.tests + helidon-builder-tests-project + 4.0.0-SNAPSHOT + ../pom.xml + + 4.0.0 + + helidon-builder-test-nodeps + Helidon Builder No Dependencies Tests + A test that excludes all Helidon dependencies to ensure that builder works in such conditions + + + + io.helidon.builder.tests + helidon-builder-test-builder + ${helidon.version} + test + + + + io.helidon.builder + helidon-builder + + + io.helidon.common + helidon-common + + + + + io.helidon.builder.tests + helidon-builder-test-builder + ${helidon.version} + test-jar + test + + + + io.helidon.builder + helidon-builder + + + io.helidon.common + helidon-common + + + + + org.hamcrest + hamcrest-all + test + + + org.junit.jupiter + junit-jupiter-api + test + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -XprintProcessorInfo + -XprintRounds + -verbose + + true + + + io.helidon.builder + helidon-builder-processor + ${helidon.version} + + + + + + + + diff --git a/builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java b/builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java new file mode 100644 index 00000000000..207d65b0d39 --- /dev/null +++ b/builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.nodeps.test; + +import io.helidon.builder.test.InterceptedBeanTest; + +/** + * Will re-execute tests from the base... but here without Helidon library dependencies... + */ +public class NoDepsInterceptorBeanTest extends InterceptedBeanTest { + +} diff --git a/builder/tests/pom.xml b/builder/tests/pom.xml index ae271e397dc..bf2a7364288 100644 --- a/builder/tests/pom.xml +++ b/builder/tests/pom.xml @@ -39,7 +39,7 @@ true true - + true true true true @@ -47,6 +47,7 @@ builder + nodeps diff --git a/pico/builder-config/README.md b/pico/builder-config/README.md new file mode 100644 index 00000000000..eb5083914e0 --- /dev/null +++ b/pico/builder-config/README.md @@ -0,0 +1,23 @@ +# pico-builder-config + +This is a specialization of the [builder](../builder) that extends the builder to support additional integration with Helidon's configuration sub-system. It adds support for the [@ConfigBean](builder-config/src/main/java/io/helidon/pico/builder/config/ConfigBean.java) annotation. When applied to a target interface it will map that interface to configuration via a new toBuilder method generated on the implementation as follows: + +```java + ... + + public static Builder toBuilder(io.helidon.common.config.Config cfg) { + ... + } + + ... +``` + +There are a few additional caveats to understand about ConfigBean and its supporting infrastructure. + +* @Builder can be used in conjunction with @ConfigBean. All attributed will be honored exception one... +* Builder.requireLibraryDependencies is not supported. All generated configuration beans and builders will minimally require a compile-time and runtime dependency on Helidon's common-config module. But for full fidelity support of Helidon's config one should instead use the full config module. + +## Modules +* [builder-config](builder-config) - annotations and other SPI types. +* [processor](processor) - the annotation processor that should be used when using ConfigBeans. +* [tests](tests) - tests that can also serve as examples for usage. diff --git a/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/ConfigBeanBase.java b/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/ConfigBeanBase.java index bfe60a7b04f..0014605078d 100644 --- a/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/ConfigBeanBase.java +++ b/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/ConfigBeanBase.java @@ -31,6 +31,7 @@ public abstract class ConfigBeanBase implements ConfigBeanCommon { /** * Protected constructor for initializing the generated config bean instance variables. + * * @param b the builder * @param instanceId the instance id */ diff --git a/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/DefaultConfigResolver.java b/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/DefaultConfigResolver.java index 6fbe11c79d2..f9cbaa80c1d 100644 --- a/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/DefaultConfigResolver.java +++ b/pico/builder-config/builder-config/src/main/java/io/helidon/pico/builder/config/spi/DefaultConfigResolver.java @@ -31,7 +31,7 @@ * The default implementation of {@link ConfigResolver} simply resolves against {@link io.helidon.common.config.Config} directly. */ @Singleton -@Weight(Weighted.DEFAULT_WEIGHT - 1) +@Weight(Weighted.DEFAULT_WEIGHT - 1) // allow all other creators to take precedence over us... public class DefaultConfigResolver implements ConfigResolver, ConfigResolverProvider { /** diff --git a/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java b/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java index 5df97f2b3d8..7bc4b364cf3 100644 --- a/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java +++ b/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java @@ -34,7 +34,7 @@ public interface InjectionPointInfo extends ElementInfo { * * @return the unique identity */ - String identity(); + String ipIdentity(); /** * The base identifying name for this injection point. If the element represents a function, then the function arguments diff --git a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java index 75ecfefaca6..af19946473f 100644 --- a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java +++ b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java @@ -38,7 +38,7 @@ public interface ServiceInjectionPlanBinder { interface Binder { /** - * Binds a single service provider to the injection point identified by {@link InjectionPointInfo#identity()}. + * Binds a single service provider to the injection point identified by {@link InjectionPointInfo#ipIdentity()}. * It is assumed that the caller of this is aware of the proper cardinality for each injection point. * * @param ipIdentity the injection point identity @@ -49,7 +49,7 @@ interface Binder { Binder bind(String ipIdentity, ServiceProvider serviceProvider); /** - * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#identity()}. + * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#ipIdentity(). * It is assumed that the caller of this is aware of the proper cardinality for each injection point. * * @param ipIdentity the injection point identity From 42190051e3bf58b41b77809048e2e2e752b2aa53 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Sat, 3 Dec 2022 05:36:24 -0500 Subject: [PATCH 02/10] fix build --- bom/pom.xml | 5 -- .../java/io/helidon/builder/Interceptor.java | 2 +- .../builder/processor/tools/BodyContext.java | 12 +++-- .../tools/DefaultBuilderCreatorProvider.java | 10 ++-- .../tools/GenerateInterceptorSupport.java | 53 ------------------- builder/tests/builder/pom.xml | 11 ---- .../testsubjects/BeanBuilderInterceptor.java | 18 +++---- .../test/testsubjects/InterceptedBean.java | 4 +- .../builder/test/InterceptedBeanTest.java | 4 +- builder/tests/nodeps/pom.xml | 26 +++------ .../nodeps/NoDepsBeanBuilderInterceptor.java | 38 +++++++++++++ .../test/nodeps/NoDepsInterceptedBean.java} | 25 +++++++-- .../nodeps/NoDepsInterceptorBeanTest.java | 40 ++++++++++++++ 13 files changed, 127 insertions(+), 121 deletions(-) delete mode 100644 builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java create mode 100644 builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java rename builder/tests/nodeps/src/{test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java => main/java/io/helidon/builder/test/nodeps/NoDepsInterceptedBean.java} (52%) create mode 100644 builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java diff --git a/bom/pom.xml b/bom/pom.xml index 35125db948f..c15580801ad 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -1432,11 +1432,6 @@ helidon-builder-processor ${helidon.version} - - io.helidon.builder.tests - helidon-builder-test-builder - ${helidon.version} - diff --git a/builder/builder/src/main/java/io/helidon/builder/Interceptor.java b/builder/builder/src/main/java/io/helidon/builder/Interceptor.java index 47e9fd96fe9..122fb52be40 100644 --- a/builder/builder/src/main/java/io/helidon/builder/Interceptor.java +++ b/builder/builder/src/main/java/io/helidon/builder/Interceptor.java @@ -28,6 +28,7 @@ *
  • The implementation class type must provide a no-arg accessible constructor available to the generated class, unless * the {@link io.helidon.builder.Builder#interceptorCreateMethod()} is used. *
  • The implementation class type must provide a method-compatible (lambda) signature to the {@link #intercept} method. + *
  • Any exceptions that might be thrown from the {@link #intercept} method must be an unchecked exception type. * * * @param the type of the bean builder to intercept @@ -35,7 +36,6 @@ * @see io.helidon.builder.Builder#interceptor() */ @FunctionalInterface -// important note: this class is also code generated - please keep this in synch with generated code public interface Interceptor { /** diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 0f4d3c6367a..43bb9414886 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -112,8 +112,10 @@ public class BodyContext { String interceptorType = searchForBuilderAnnotation("interceptor", builderTriggerAnnotation, typeInfo); this.interceptorTypeName = (Void.class.getName().equals(interceptorType) || Objects.isNull(interceptorType)) ? null : DefaultTypeName.createFromTypeName(interceptorType); - this.interceptorCreateMethod = Objects.requireNonNull( - searchForBuilderAnnotation("interceptorCreateMethod", builderTriggerAnnotation, typeInfo)); + String interceptorCreateMethod = + searchForBuilderAnnotation("interceptorCreateMethod", builderTriggerAnnotation, typeInfo); + this.interceptorCreateMethod = ("".equals(interceptorCreateMethod) || Objects.isNull(interceptorCreateMethod)) + ? null : interceptorCreateMethod; } /** @@ -345,10 +347,10 @@ public Optional interceptorTypeName() { * Returns the interceptor create method name. * See {@link io.helidon.builder.Builder#interceptorCreateMethod()}. * - * @return the interceptor create method + * @return the interceptor create method name */ - public String interceptorCreateMethod() { - return interceptorCreateMethod; + public Optional interceptorCreateMethod() { + return Optional.ofNullable(interceptorCreateMethod); } /** diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index b25c00a2cab..4131f35e096 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -234,10 +234,11 @@ protected void maybeAppendInterceptor(StringBuilder builder, if (ctx.interceptorTypeName().isPresent()) { String impl = ctx.interceptorTypeName().get().name(); builder.append("\t\t\t").append(impl).append(" interceptor = "); - if (ctx.interceptorCreateMethod().isBlank()) { + if (ctx.interceptorCreateMethod().isEmpty()) { builder.append("new ").append(impl).append("();\n"); } else { - builder.append(ctx.interceptorTypeName().get()).append(".").append(ctx.interceptorCreateMethod()).append("();\n"); + builder.append(ctx.interceptorTypeName().get()) + .append(".").append(ctx.interceptorCreateMethod().get()).append("();\n"); } builder.append("\t\t\t").append(builderTag).append(" = interceptor.intercept(").append(builderTag).append(");\n"); } @@ -507,8 +508,8 @@ protected void appendExtraMethods(StringBuilder builder, } /** - * Adds extra inner classes to write on the builder. This default implementation will write the {@code AttributeVisitor}, - * {@code RequiredAttributeVisitor} and {@code Interceptor} inner classes on the base abstract parent (ie, hasParent is false). + * Adds extra inner classes to write on the builder. This default implementation will write the {@code AttributeVisitor} and + * {@code RequiredAttributeVisitor} inner classes on the base abstract parent (ie, hasParent is false). * * @param builder the builder * @param ctx the context @@ -516,7 +517,6 @@ protected void appendExtraMethods(StringBuilder builder, protected void appendExtraInnerClasses(StringBuilder builder, BodyContext ctx) { GenerateVisitorSupport.appendExtraInnerClasses(builder, ctx); - GenerateInterceptorSupport.appendExtraInnerClasses(builder, ctx); } /** diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java deleted file mode 100644 index 2f323b81c3b..00000000000 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/GenerateInterceptorSupport.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright (c) 2022 Oracle and/or its affiliates. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.helidon.builder.processor.tools; - -/** - * See {@link io.helidon.builder.Interceptor} for the prototypical output for this generated class. - */ -final class GenerateInterceptorSupport { - - private GenerateInterceptorSupport() { - } - - static void appendExtraInnerClasses(StringBuilder builder, - BodyContext ctx) { - if (ctx.doingConcreteType()) { - return; - } - - if (!ctx.hasParent() - && !ctx.requireLibraryDependencies()) { - builder.append("\n\n\t/**\n" - + "\t * A functional interface that can be used to intercept the target type.\n" - + "\t *\n" - + "\t * @param the type to intercept" - + "\t */\n"); - builder.append("\t@FunctionalInterface\n" - + "\tpublic static interface Interceptor {\n" - + "\t\t /**\n" - + "\t\t * Provides the ability to intercept the target.\n" - + "\t\t *\n" - + "\t\t * @param target the target being intercepted\n" - + "\t\t * @return the mutated or replaced target (must not be null)\n" - + "\t\t */\n" - + "\t\tT intercept(T target);\n" - + "\t}\n"); - } - } - -} diff --git a/builder/tests/builder/pom.xml b/builder/tests/builder/pom.xml index e53a57a3ffd..c82616d4352 100644 --- a/builder/tests/builder/pom.xml +++ b/builder/tests/builder/pom.xml @@ -94,17 +94,6 @@ - - org.apache.maven.plugins - maven-jar-plugin - - - - test-jar - - - - diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java index eea3ab3f19b..9c604ab7da0 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java @@ -16,24 +16,18 @@ package io.helidon.builder.test.testsubjects; +import io.helidon.builder.Interceptor; + /** - * See {@link InterceptedBean}. Notice how the Builder annotation on {@link InterceptedBean} sets the - * {@link io.helidon.builder.Builder#requireLibraryDependencies()} attribute to {@code false}, which is why this class does not - * need to implement {@link io.helidon.builder.Interceptor}. - * - * @see InterceptedBean + * See {@link InterceptedBean}. */ -class BeanBuilderInterceptor /* implements Interceptor */ { +class BeanBuilderInterceptor implements Interceptor { private BeanBuilderInterceptor() { } - static BeanBuilderInterceptor create() { - return new BeanBuilderInterceptor(); - } - -// @Override - DefaultInterceptedBean.Builder intercept(DefaultInterceptedBean.Builder target) { + @Override + public DefaultInterceptedBean.Builder intercept(DefaultInterceptedBean.Builder target) { return target.helloMessage("Hello " + target.name()); } diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java index d17a3ba8b4e..e0878f07aef 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java @@ -22,9 +22,7 @@ /** * Demonstrates interception of builders. */ -@Builder(requireLibraryDependencies = false, - interceptor = BeanBuilderInterceptor.class, - interceptorCreateMethod = "create") +@Builder public interface InterceptedBean { /** diff --git a/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java b/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java index 94c43554e89..7ae779c376a 100644 --- a/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java +++ b/builder/tests/builder/src/test/java/io/helidon/builder/test/InterceptedBeanTest.java @@ -24,10 +24,10 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -public class InterceptedBeanTest { +class InterceptedBeanTest { @Test - protected void testMutation() { + void testMutation() { InterceptedBean val = DefaultInterceptedBean.builder() .name("Larry") .build(); diff --git a/builder/tests/nodeps/pom.xml b/builder/tests/nodeps/pom.xml index 9b1325a878d..20ca76bf744 100644 --- a/builder/tests/nodeps/pom.xml +++ b/builder/tests/nodeps/pom.xml @@ -34,11 +34,10 @@ - io.helidon.builder.tests - helidon-builder-test-builder + io.helidon.builder + helidon-builder ${helidon.version} - test - + provided io.helidon.builder @@ -51,22 +50,9 @@ - io.helidon.builder.tests - helidon-builder-test-builder - ${helidon.version} - test-jar - test - - - - io.helidon.builder - helidon-builder - - - io.helidon.common - helidon-common - - + io.helidon.builder + helidon-builder-processor + provided org.hamcrest diff --git a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java new file mode 100644 index 00000000000..faf8cdf6963 --- /dev/null +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.nodeps; + +/** + * See {@link NoDepsInterceptedBean}. Notice how the Builder annotation on {@link NoDepsInterceptedBean} sets the + * {@link io.helidon.builder.Builder#requireLibraryDependencies()} attribute to {@code false}, which is why this class does not + * need to implement {@link io.helidon.builder.Interceptor}. + */ +class NoDepsBeanBuilderInterceptor /* implements Interceptor */ { + + private NoDepsBeanBuilderInterceptor() { + } + + static NoDepsBeanBuilderInterceptor create() { + return new NoDepsBeanBuilderInterceptor(); + } + +// @Override + DefaultNoDepsInterceptedBean.Builder intercept(DefaultNoDepsInterceptedBean.Builder target) { + return target.helloMessage("Hello " + target.name()); + } + +} diff --git a/builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsInterceptedBean.java similarity index 52% rename from builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java rename to builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsInterceptedBean.java index 207d65b0d39..4ee82189d1e 100644 --- a/builder/tests/nodeps/src/test/java/io/helidon/builder/nodeps/test/NoDepsInterceptorBeanTest.java +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsInterceptedBean.java @@ -14,13 +14,30 @@ * limitations under the License. */ -package io.helidon.builder.nodeps.test; +package io.helidon.builder.test.nodeps; -import io.helidon.builder.test.InterceptedBeanTest; +import io.helidon.builder.Builder; /** - * Will re-execute tests from the base... but here without Helidon library dependencies... + * Demonstrates interception of builders. */ -public class NoDepsInterceptorBeanTest extends InterceptedBeanTest { +@Builder(requireLibraryDependencies = false, + interceptor = NoDepsBeanBuilderInterceptor.class, + interceptorCreateMethod = "create") +public interface NoDepsInterceptedBean { + + /** + * The name to say hello to. + * + * @return the name + */ + String name(); + + /** + * The hello message will be explicitly overridden to say "hello {@link #name()}". + * + * @return the hello message + */ + String helloMessage(); } diff --git a/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java b/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java new file mode 100644 index 00000000000..dab0d9225f2 --- /dev/null +++ b/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.nodeps; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +class NoDepsInterceptorBeanTest { + + @Test + void testMutation() { + NoDepsInterceptedBean val = DefaultNoDepsInterceptedBean.builder() + .name("Larry") + .build(); + assertThat(val.name(), equalTo("Larry")); + assertThat(val.helloMessage(), equalTo("Hello Larry")); + + NoDepsInterceptedBean val2 = DefaultNoDepsInterceptedBean.builder() + .name("Larry") + .build(); + assertThat(val, equalTo(val2)); + } + +} From c0f374c1ffd1f7af516fbdf1167c761f2b80d31a Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Sat, 3 Dec 2022 05:43:15 -0500 Subject: [PATCH 03/10] checkstyles --- .../test/testsubjects/package-info.java | 2 +- .../builder/test/nodeps/package-info.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/package-info.java diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java index 245b94dfffb..51a236d59d9 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/package-info.java @@ -15,6 +15,6 @@ */ /** - * Test subjects for the Pico Builder. + * Test subjects for the Builder. */ package io.helidon.builder.test.testsubjects; diff --git a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/package-info.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/package-info.java new file mode 100644 index 00000000000..72424eef580 --- /dev/null +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Test subjects for the Builder. + */ +package io.helidon.builder.test.nodeps; From 45644910c88f31e372e31fd3eed3b8367b283a45 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Sat, 3 Dec 2022 06:33:42 -0500 Subject: [PATCH 04/10] javadoc --- .../main/java/io/helidon/pico/ServiceInjectionPlanBinder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java index af19946473f..ed45c65e098 100644 --- a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java +++ b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java @@ -49,7 +49,7 @@ interface Binder { Binder bind(String ipIdentity, ServiceProvider serviceProvider); /** - * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#ipIdentity(). + * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#ipIdentity()}. * It is assumed that the caller of this is aware of the proper cardinality for each injection point. * * @param ipIdentity the injection point identity From 8937f6c974b94e8e8e7d7c36b771f8460c4a3984 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Sat, 3 Dec 2022 18:45:11 -0500 Subject: [PATCH 05/10] handle case where common Builder methods interfere with target bean methods --- .../builder/processor/tools/BodyContext.java | 20 +++++++ .../tools/DefaultBuilderCreatorProvider.java | 59 +++++++++++-------- .../testsubjects/BeanBuilderInterceptor.java | 8 +-- .../test/testsubjects/InterceptedBean.java | 2 +- .../nodeps/NoDepsBeanBuilderInterceptor.java | 5 +- .../io/helidon/pico/InjectionPointInfo.java | 2 +- .../pico/ServiceInjectionPlanBinder.java | 4 +- 7 files changed, 66 insertions(+), 34 deletions(-) diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 43bb9414886..34ebd01834c 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -67,6 +67,7 @@ public class BodyContext { private final String mapType; private final String setType; private final boolean hasParent; + private final boolean hasAnyBuilderClashingMethodNames; private final TypeName ctorBuilderAcceptTypeName; private final String genericBuilderClassDecl; private final String genericBuilderAliasDecl; @@ -102,6 +103,7 @@ public class BodyContext { gatherAllAttributeNames(this, typeInfo); assert (allTypeInfos.size() == allAttributeNames.size()); this.hasParent = Objects.nonNull(parentTypeName.get()); + this.hasAnyBuilderClashingMethodNames = determineIfHasAnyClashingMethodNames(); this.ctorBuilderAcceptTypeName = (hasParent) ? typeInfo.typeName() : (Objects.nonNull(parentAnnotationType.get()) && typeInfo.elementInfo().isEmpty() @@ -297,6 +299,15 @@ public boolean hasParent() { return hasParent; } + /** + * Returns true if any getter methods from the target clash with any builder method name. + * + * @return true if there is a clash + */ + public boolean hasAnyBuilderClashingMethodNames() { + return hasAnyBuilderClashingMethodNames; + } + /** * Returns the streamable accept type of the builder and constructor. * @@ -557,4 +568,13 @@ private static void populateMap(Map map, } } + private boolean determineIfHasAnyClashingMethodNames() { + return allAttributeNames().stream().anyMatch(this::isBuilderClashingMethodName); + } + + private boolean isBuilderClashingMethodName(String beanAttributeName) { + return beanAttributeName.equals("identity") + || beanAttributeName.equals("get"); + } + } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index 4131f35e096..f215c3a3934 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -194,7 +194,7 @@ protected String toBody(BodyContext ctx) { appendFields(builder, ctx); appendCtor(builder, ctx); appendExtraPostCtorCode(builder, ctx); - appendInterfaceBasedGetters(builder, ctx, "", true); + appendInterfaceBasedGetters(builder, ctx, "", false); appendBasicGetters(builder, ctx); appendMetaAttributes(builder, ctx); appendToStringMethod(builder, ctx); @@ -1051,7 +1051,12 @@ private void appendBuilder(StringBuilder builder, appendExtraBuilderFields(builder, ctx); appendBuilderBody(builder, ctx); - appendInterfaceBasedGetters(builder, ctx, "\t", true); + if (ctx.hasAnyBuilderClashingMethodNames()) { + builder.append("\t\t// *** IMPORTANT NOTE: There are getter methods that clash with the base Builder methods ***\n"); + appendInterfaceBasedGetters(builder, ctx, "\t//", true); + } else { + appendInterfaceBasedGetters(builder, ctx, "\t", true); + } if (ctx.doingConcreteType()) { if (ctx.hasParent()) { @@ -1199,7 +1204,7 @@ private void appendBuilderBody(StringBuilder builder, if (typeName.isList() || typeName.isMap() || typeName.isSet()) { continue; } - addBuilderField(builder, method, typeName, beanAttributeName); + addBuilderField(builder, ctx, method, typeName, beanAttributeName); } builder.append("\n"); } @@ -1219,11 +1224,13 @@ private void appendBuilderBody(StringBuilder builder, } private void addBuilderField(StringBuilder builder, + BodyContext ctx, TypedElementName method, TypeName type, String beanAttributeName) { GenerateJavadoc.builderField(builder, method); - builder.append("\t\tprivate ").append(type.array() ? type.fqName() : type.name()).append(" ") + builder.append("\t\tprivate "); + builder.append(type.array() ? type.fqName() : type.name()).append(" ") .append(beanAttributeName); Optional defaultVal = toConfiguredOptionValue(method, true, true); if (defaultVal.isPresent()) { @@ -1303,27 +1310,30 @@ private void appendBuilderHeader(StringBuilder builder, } } + LinkedList impls = new LinkedList<>(); if (ctx.hasStreamSupportOnBuilder() || !ctx.hasParent()) { - builder.append("implements ").append(ctx.typeInfo().typeName().name()); + if (!ctx.hasAnyBuilderClashingMethodNames()) { + impls.add(ctx.typeInfo().typeName().name()); + } } - if (ctx.hasStreamSupportOnBuilder()) { - builder.append(", Supplier<").append(ctx.genericBuilderAcceptAliasDecl()).append(">"); + if (ctx.hasStreamSupportOnBuilder() && !ctx.requireLibraryDependencies()) { + impls.add("Supplier<" + ctx.genericBuilderAcceptAliasDecl() + ">"); } if (!ctx.hasParent()) { if (ctx.requireLibraryDependencies()) { - builder.append(", io.helidon.common.Builder<").append(ctx.genericBuilderAliasDecl()) - .append(", ").append(ctx.genericBuilderAcceptAliasDecl()).append(">"); - } else { - builder.append("/*, io.helidon.common.Builder<").append(ctx.genericBuilderAliasDecl()) - .append(", ").append(ctx.genericBuilderAcceptAliasDecl()).append("> */"); + impls.add(io.helidon.common.Builder.class.getName() + + "<" + ctx.genericBuilderAliasDecl() + ", " + ctx.genericBuilderAcceptAliasDecl() + ">"); } } List extraImplementBuilderContracts = extraImplementedBuilderContracts(ctx); - extraImplementBuilderContracts.forEach(t -> builder.append(",\n\t\t\t\t\t\t\t\t\t\t\t").append(t.fqName())); + extraImplementBuilderContracts.forEach(t -> impls.add(t.fqName())); + if (!impls.isEmpty()) { + builder.append(" implements ").append(String.join(", ", impls)); + } builder.append(" {\n"); } } @@ -1341,7 +1351,7 @@ private void appendToBuilderMethods(StringBuilder builder, private void appendInterfaceBasedGetters(StringBuilder builder, BodyContext ctx, String prefix, - boolean isOverrride) { + boolean isBuilder) { if (ctx.doingConcreteType()) { return; } @@ -1349,18 +1359,17 @@ private void appendInterfaceBasedGetters(StringBuilder builder, int i = 0; for (String beanAttributeName : ctx.allAttributeNames()) { TypedElementName method = ctx.allTypeInfos().get(i); - appendAnnotations(builder, method.annotations(), "\t"); - if (isOverrride) { - builder.append(prefix) - .append("\t@Override\n"); - } - builder.append(prefix) - .append("\tpublic ").append(toGenerics(method, false)).append(" ").append(method.elementName()) + String extraPrefix = prefix + "\t"; + appendAnnotations(builder, method.annotations(), extraPrefix); + builder.append(extraPrefix) + .append("@Override\n"); + builder.append(extraPrefix) + .append("public ").append(toGenerics(method, false)).append(" ").append(method.elementName()) .append("() {\n"); - builder.append(prefix) - .append("\t\treturn ").append(beanAttributeName).append(";\n"); - builder.append(prefix) - .append("\t}\n\n"); + builder.append(extraPrefix) + .append("\treturn ").append(beanAttributeName).append(";\n"); + builder.append(extraPrefix) + .append("}\n\n"); i++; } diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java index 9c604ab7da0..61994513903 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java @@ -22,13 +22,13 @@ * See {@link InterceptedBean}. */ class BeanBuilderInterceptor implements Interceptor { - - private BeanBuilderInterceptor() { - } + int callCount; @Override public DefaultInterceptedBean.Builder intercept(DefaultInterceptedBean.Builder target) { + if (callCount++ > 0) { + throw new AssertionError(); + } return target.helloMessage("Hello " + target.name()); } - } diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java index e0878f07aef..bcc798333a5 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java @@ -22,7 +22,7 @@ /** * Demonstrates interception of builders. */ -@Builder +@Builder(interceptor = BeanBuilderInterceptor.class) public interface InterceptedBean { /** diff --git a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java index faf8cdf6963..2275943a153 100644 --- a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java @@ -22,6 +22,7 @@ * need to implement {@link io.helidon.builder.Interceptor}. */ class NoDepsBeanBuilderInterceptor /* implements Interceptor */ { + int callCount; private NoDepsBeanBuilderInterceptor() { } @@ -32,7 +33,9 @@ static NoDepsBeanBuilderInterceptor create() { // @Override DefaultNoDepsInterceptedBean.Builder intercept(DefaultNoDepsInterceptedBean.Builder target) { + if (callCount++ > 0) { + throw new AssertionError(); + } return target.helloMessage("Hello " + target.name()); } - } diff --git a/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java b/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java index 7bc4b364cf3..5df97f2b3d8 100644 --- a/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java +++ b/pico/pico/src/main/java/io/helidon/pico/InjectionPointInfo.java @@ -34,7 +34,7 @@ public interface InjectionPointInfo extends ElementInfo { * * @return the unique identity */ - String ipIdentity(); + String identity(); /** * The base identifying name for this injection point. If the element represents a function, then the function arguments diff --git a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java index ed45c65e098..75ecfefaca6 100644 --- a/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java +++ b/pico/pico/src/main/java/io/helidon/pico/ServiceInjectionPlanBinder.java @@ -38,7 +38,7 @@ public interface ServiceInjectionPlanBinder { interface Binder { /** - * Binds a single service provider to the injection point identified by {@link InjectionPointInfo#ipIdentity()}. + * Binds a single service provider to the injection point identified by {@link InjectionPointInfo#identity()}. * It is assumed that the caller of this is aware of the proper cardinality for each injection point. * * @param ipIdentity the injection point identity @@ -49,7 +49,7 @@ interface Binder { Binder bind(String ipIdentity, ServiceProvider serviceProvider); /** - * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#ipIdentity()}. + * Binds a list of service providers to the injection point identified by {@link InjectionPointInfo#identity()}. * It is assumed that the caller of this is aware of the proper cardinality for each injection point. * * @param ipIdentity the injection point identity From f3385125e293b87707525cfe948be7b7ac1a414c Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Sun, 4 Dec 2022 09:51:27 -0500 Subject: [PATCH 06/10] more checkstyles --- .../builder/test/testsubjects/BeanBuilderInterceptor.java | 2 +- .../builder/test/nodeps/NoDepsBeanBuilderInterceptor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java index 61994513903..54387ea7fa1 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java @@ -22,7 +22,7 @@ * See {@link InterceptedBean}. */ class BeanBuilderInterceptor implements Interceptor { - int callCount; + private int callCount; @Override public DefaultInterceptedBean.Builder intercept(DefaultInterceptedBean.Builder target) { diff --git a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java index 2275943a153..d6393ad4d18 100644 --- a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java @@ -22,7 +22,7 @@ * need to implement {@link io.helidon.builder.Interceptor}. */ class NoDepsBeanBuilderInterceptor /* implements Interceptor */ { - int callCount; + private int callCount; private NoDepsBeanBuilderInterceptor() { } From 1a4c6b700de672a7d54f5e0304323c28b7a34098 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Tue, 6 Dec 2022 13:54:36 -0500 Subject: [PATCH 07/10] Fix for issue #5608 --- .../processor/spi/DefaultTypeInfo.java | 35 ++- .../builder/processor/spi/TypeInfo.java | 14 +- .../builder/processor/tools/BodyContext.java | 42 +++- .../processor/tools/BuilderTypeTools.java | 57 +++-- .../tools/DefaultBuilderCreatorProvider.java | 200 +++++++++--------- .../builder/processor/BuilderProcessor.java | 6 +- .../AbstractWithCustomMethods.java | 49 +++++ .../test/testsubjects/GeneralInterceptor.java | 52 +++++ .../test/AbstractWithCustomMethodsTest.java | 46 ++++ 9 files changed, 381 insertions(+), 120 deletions(-) create mode 100644 builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java create mode 100644 builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/GeneralInterceptor.java create mode 100644 builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java diff --git a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java index 9dd65c6b32f..377f607fee1 100644 --- a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java +++ b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java @@ -36,6 +36,7 @@ public class DefaultTypeInfo implements TypeInfo { private final String typeKind; private final List annotations; private final List elementInfo; + private final List otherElementInfo; private final TypeInfo superTypeInfo; /** @@ -49,6 +50,7 @@ protected DefaultTypeInfo(Builder b) { this.typeKind = b.typeKind; this.annotations = Collections.unmodifiableList(new LinkedList<>(b.annotations)); this.elementInfo = Collections.unmodifiableList(new LinkedList<>(b.elementInfo)); + this.otherElementInfo = Collections.unmodifiableList(new LinkedList<>(b.otherElementInfo)); this.superTypeInfo = b.superTypeInfo; } @@ -81,6 +83,11 @@ public List elementInfo() { return elementInfo; } + @Override + public List otherElementInfo() { + return otherElementInfo; + } + @Override public Optional superTypeInfo() { return Optional.ofNullable(superTypeInfo); @@ -99,6 +106,7 @@ public String toString() { protected String toStringInner() { return "typeName=" + typeName() + ", elementInfo=" + elementInfo() + + ", annotations=" + annotations() + ", superTypeInfo=" + superTypeInfo(); } @@ -108,7 +116,7 @@ protected String toStringInner() { public static class Builder implements io.helidon.common.Builder { private final List annotations = new ArrayList<>(); private final List elementInfo = new ArrayList<>(); - + private final List otherElementInfo = new ArrayList<>(); private TypeName typeName; private String typeKind; @@ -202,6 +210,31 @@ public Builder addElementInfo(TypedElementName val) { return this; } + /** + * Sets the otherElementInfo to val. + * + * @param val the value + * @return this fluent builder + */ + public Builder otherElementInfo(Collection val) { + Objects.requireNonNull(val); + this.otherElementInfo.clear(); + this.otherElementInfo.addAll(val); + return this; + } + + /** + * Adds a single otherElementInfo val. + * + * @param val the value + * @return this fluent builder + */ + public Builder addOtherElementInfo(TypedElementName val) { + Objects.requireNonNull(val); + otherElementInfo.add(Objects.requireNonNull(val)); + return this; + } + /** * Sets the superTypeInfo to val. * diff --git a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java index e269c37874f..5035206c0d0 100644 --- a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java +++ b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java @@ -24,7 +24,8 @@ import io.helidon.pico.types.TypedElementName; /** - * Represents the model object for an interface type (e.g., one that was annotated with {@link io.helidon.builder.Builder}). + * Represents the model object for an interface or an abstract type (i.e., one that was annotated with + * {@link io.helidon.builder.Builder}). */ public interface TypeInfo { @@ -50,12 +51,19 @@ public interface TypeInfo { List annotations(); /** - * The elements that make up the type. + * The elements that make up the type that are "relevant" for processing. * - * @return the elements that make up the type + * @return the elements that make up the type that are relevant for processing */ List elementInfo(); + /** + * The elements that make up this type that are considered "other", or being skipped because they are irrelevant to processing. + * + * @return the elements that still make up the type, but are otherwise deemed irrelevant for processing + */ + List otherElementInfo(); + /** * The parent/super class for this type info. * diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 34ebd01834c..142da344258 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -68,6 +68,7 @@ public class BodyContext { private final String setType; private final boolean hasParent; private final boolean hasAnyBuilderClashingMethodNames; + private final boolean isExtendingAnAbstractClass; private final TypeName ctorBuilderAcceptTypeName; private final String genericBuilderClassDecl; private final String genericBuilderAliasDecl; @@ -100,10 +101,15 @@ public class BodyContext { this.listType = toListImplType(builderTriggerAnnotation, typeInfo); this.mapType = toMapImplType(builderTriggerAnnotation, typeInfo); this.setType = toSetImplType(builderTriggerAnnotation, typeInfo); - gatherAllAttributeNames(this, typeInfo); + try { + gatherAllAttributeNames(this, typeInfo); + } catch (Exception e) { + throw new IllegalStateException("Failed while processing: " + typeInfo.typeName(), e); + } assert (allTypeInfos.size() == allAttributeNames.size()); this.hasParent = Objects.nonNull(parentTypeName.get()); this.hasAnyBuilderClashingMethodNames = determineIfHasAnyClashingMethodNames(); + this.isExtendingAnAbstractClass = typeInfo.typeKind().equals("CLASS"); this.ctorBuilderAcceptTypeName = (hasParent) ? typeInfo.typeName() : (Objects.nonNull(parentAnnotationType.get()) && typeInfo.elementInfo().isEmpty() @@ -308,6 +314,15 @@ public boolean hasAnyBuilderClashingMethodNames() { return hasAnyBuilderClashingMethodNames; } + /** + * Returns true if this builder is extending an abstract class as a target. + * + * @return true if the target is an abstract class + */ + public boolean isExtendingAnAbstractClass() { + return isExtendingAnAbstractClass; + } + /** * Returns the streamable accept type of the builder and constructor. * @@ -364,6 +379,28 @@ public Optional interceptorCreateMethod() { return Optional.ofNullable(interceptorCreateMethod); } + /** + * Checks whether there is an "other" method that matches the signature. + * + * @param name the method name + * @param typeInfo the type info to check, which will look through the parent chain + * @return true if there is any matches + */ + public boolean hasOtherMethod(String name, + TypeInfo typeInfo) { + for (TypedElementName elem : typeInfo.otherElementInfo()) { + if (elem.elementName().equals(name)) { + return true; + } + } + + if (typeInfo.superTypeInfo().isPresent()) { + return hasOtherMethod(name, typeInfo.superTypeInfo().get()); + } + + return false; + } + /** * returns the bean attribute name of a particular method. * @@ -574,7 +611,8 @@ private boolean determineIfHasAnyClashingMethodNames() { private boolean isBuilderClashingMethodName(String beanAttributeName) { return beanAttributeName.equals("identity") - || beanAttributeName.equals("get"); + || beanAttributeName.equals("get") + || beanAttributeName.equals("toStringInner"); } } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java index 7c4a000f561..7767ea205e6 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java @@ -63,6 +63,8 @@ @Weight(Weighted.DEFAULT_WEIGHT - 1) public class BuilderTypeTools implements TypeInfoCreatorProvider { + private static final boolean ACCEPT_ABSTRACT_CLASS_TARGETS = true; + /** * Default constructor. */ @@ -84,8 +86,8 @@ public Optional createTypeInfo(AnnotationAndValue annotation, return Optional.empty(); } - if (element.getKind() != ElementKind.INTERFACE && element.getKind() != ElementKind.ANNOTATION_TYPE) { - String msg = annotation.typeName() + " is intended to be used on interfaces only: " + element; + if (!isAcceptableBuilderTarget(element)) { + String msg = annotation.typeName() + " is not intended to be targeted to this type: " + element; processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, msg); throw new IllegalStateException(msg); } @@ -97,12 +99,13 @@ public Optional createTypeInfo(AnnotationAndValue annotation, .filter(it -> !it.getParameters().isEmpty()) .collect(Collectors.toList()); if (!problems.isEmpty()) { - String msg = "only simple getters with 0 args are supported: " + element + ": " + problems; + String msg = "only simple getters with no arguments are supported: " + element + ": " + problems; processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, msg); throw new IllegalStateException(msg); } - Collection elementInfo = toElementInfo(element, processingEnv); + Collection elementInfo = toElementInfo(element, processingEnv, true); + Collection otherElementInfo = toElementInfo(element, processingEnv, false); return Optional.of(DefaultTypeInfo.builder() .typeName(typeName) .typeKind(String.valueOf(element.getKind())) @@ -110,35 +113,63 @@ public Optional createTypeInfo(AnnotationAndValue annotation, .createAnnotationAndValueListFromElement(element, processingEnv.getElementUtils())) .elementInfo(elementInfo) + .otherElementInfo(otherElementInfo) .update(it -> toTypeInfo(annotation, element, processingEnv).ifPresent(it::superTypeInfo)) .build()); } + /** + * Determines if the target element with the {@link io.helidon.builder.Builder} annotation is an acceptable element type. + * If it is not acceptable then the caller is expected to throw an exception or log an error, etc. + * + * @param element the element + * @return true if the element is acceptable + */ + protected boolean isAcceptableBuilderTarget(Element element) { + final ElementKind kind = element.getKind(); + final Set modifiers = element.getModifiers(); + boolean isAcceptable = (kind == ElementKind.INTERFACE + || kind == ElementKind.ANNOTATION_TYPE + || (ACCEPT_ABSTRACT_CLASS_TARGETS + && (kind == ElementKind.CLASS && modifiers.contains(Modifier.ABSTRACT)))); + return isAcceptable; + } + /** * Translation the arguments to a collection of {@link io.helidon.pico.types.TypedElementName}'s. * - * @param element the typed element (i.e., class) - * @param processingEnv the processing env + * @param element the typed element (i.e., class) + * @param processingEnv the processing env + * @param wantWhatWeCanAccept pass true to get the elements we can accept to process, false for the other ones * @return the collection of typed elements */ - protected Collection toElementInfo(TypeElement element, ProcessingEnvironment processingEnv) { + protected Collection toElementInfo(TypeElement element, + ProcessingEnvironment processingEnv, + boolean wantWhatWeCanAccept) { return element.getEnclosedElements().stream() .filter(it -> it.getKind() == ElementKind.METHOD) .map(ExecutableElement.class::cast) - .filter(this::canAccept) + .filter(it -> (wantWhatWeCanAccept == canAccept(it))) .map(it -> createTypedElementNameFromElement(it, processingEnv.getElementUtils())) .collect(Collectors.toList()); } /** - * Returns true if the executable element passed is acceptable for processing (i.e., not a static and not a default method). + * Returns true if the executable element passed is acceptable for processing (i.e., not a static and not a default method + * on interfaces, and abstract methods on abstract classes). * - * @param ee the executable element - * @return true if not default and not static + * @param ee the executable element + * @return true if not able to accept */ protected boolean canAccept(ExecutableElement ee) { Set mods = ee.getModifiers(); - return !mods.contains(Modifier.DEFAULT) && !mods.contains(Modifier.STATIC); + if (mods.contains(Modifier.ABSTRACT)) { + return true; + } +// if (mods.contains(Modifier.DEFAULT) || mods.contains(Modifier.STATIC)) { +// return false; +// } + return false; } private Optional toTypeInfo(AnnotationAndValue annotation, @@ -147,7 +178,7 @@ private Optional toTypeInfo(AnnotationAndValue annotation, List ifaces = element.getInterfaces(); if (ifaces.size() > 1) { processingEnv.getMessager() - .printMessage(Diagnostic.Kind.ERROR, "currently only supports one parent interface: " + element); + .printMessage(Diagnostic.Kind.ERROR, "only supports one parent interface: " + element); } else if (ifaces.isEmpty()) { return Optional.empty(); } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index f215c3a3934..950dc9bbded 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -86,25 +86,18 @@ public Set> supportedAnnotationTypes() { @Override public List create(TypeInfo typeInfo, AnnotationAndValue builderAnnotation) { try { - Optional abstractImplTypeName = toAbstractImplTypeName(typeInfo.typeName(), builderAnnotation); - Optional implTypeName = toImplTypeName(typeInfo.typeName(), builderAnnotation); - if (implTypeName.isEmpty()) { - return Collections.emptyList(); - } - preValidate(implTypeName.get(), typeInfo, builderAnnotation); + TypeName abstractImplTypeName = toAbstractImplTypeName(typeInfo.typeName(), builderAnnotation); + TypeName implTypeName = toImplTypeName(typeInfo.typeName(), builderAnnotation); + preValidate(implTypeName, typeInfo, builderAnnotation); LinkedList builds = new LinkedList<>(); - if (abstractImplTypeName.isPresent()) { - TypeName typeName = abstractImplTypeName.get(); - builds.add(DefaultTypeAndBody.builder() - .typeName(typeName) - .body(toBody(createBodyContext(false, typeName, typeInfo, builderAnnotation))) - .build()); - } - TypeName typeName = implTypeName.get(); builds.add(DefaultTypeAndBody.builder() - .typeName(typeName) - .body(toBody(createBodyContext(true, typeName, typeInfo, builderAnnotation))) + .typeName(abstractImplTypeName) + .body(toBody(createBodyContext(false, abstractImplTypeName, typeInfo, builderAnnotation))) + .build()); + builds.add(DefaultTypeAndBody.builder() + .typeName(implTypeName) + .body(toBody(createBodyContext(true, implTypeName, typeInfo, builderAnnotation))) .build()); return postValidate(builds); @@ -140,14 +133,14 @@ protected List postValidate(List builds) { * * @param typeName the target interface that the builder applies to * @param builderAnnotation the builder annotation triggering the build - * @return the abstract type name of the implementation, or empty if this should not be code generated + * @return the abstract type name of the implementation */ - protected Optional toAbstractImplTypeName(TypeName typeName, - AnnotationAndValue builderAnnotation) { + protected TypeName toAbstractImplTypeName(TypeName typeName, + AnnotationAndValue builderAnnotation) { String toPackageName = toPackageName(typeName.packageName(), builderAnnotation); String prefix = toAbstractImplTypePrefix(builderAnnotation); String suffix = toImplTypeSuffix(builderAnnotation); - return Optional.of(DefaultTypeName.create(toPackageName, prefix + typeName.className() + suffix)); + return DefaultTypeName.create(toPackageName, prefix + typeName.className() + suffix); } /** @@ -155,14 +148,14 @@ protected Optional toAbstractImplTypeName(TypeName typeName, * * @param typeName the target interface that the builder applies to * @param builderAnnotation the builder annotation triggering the build - * @return the type name of the implementation, or empty if this should not be code generated + * @return the type name of the implementation */ - protected Optional toImplTypeName(TypeName typeName, - AnnotationAndValue builderAnnotation) { + protected TypeName toImplTypeName(TypeName typeName, + AnnotationAndValue builderAnnotation) { String toPackageName = toPackageName(typeName.packageName(), builderAnnotation); String prefix = toImplTypePrefix(builderAnnotation); String suffix = toImplTypeSuffix(builderAnnotation); - return Optional.of(DefaultTypeName.create(toPackageName, prefix + typeName.className() + suffix)); + return DefaultTypeName.create(toPackageName, prefix + typeName.className() + suffix); } /** @@ -240,7 +233,8 @@ protected void maybeAppendInterceptor(StringBuilder builder, builder.append(ctx.interceptorTypeName().get()) .append(".").append(ctx.interceptorCreateMethod().get()).append("();\n"); } - builder.append("\t\t\t").append(builderTag).append(" = interceptor.intercept(").append(builderTag).append(");\n"); + builder.append("\t\t\t").append(builderTag) + .append(" = (Builder) interceptor.intercept(").append(builderTag).append(");\n"); } } @@ -361,34 +355,36 @@ protected void appendHeader(StringBuilder builder, } builder.append("class ").append(ctx.implTypeName().className()); - if (ctx.hasParent() || ctx.doingConcreteType()) { + Optional baseExtendsTypeName = baseExtendsTypeName(ctx); + if (baseExtendsTypeName.isEmpty() && ctx.isExtendingAnAbstractClass()) { + baseExtendsTypeName = Optional.of(ctx.typeInfo().typeName()); + } + if (ctx.hasParent() || ctx.doingConcreteType() || baseExtendsTypeName.isPresent()) { builder.append(" extends "); } if (ctx.doingConcreteType()) { - builder.append(toAbstractImplTypeName(ctx.typeInfo().typeName(), ctx.builderTriggerAnnotation()).get()); + builder.append(toAbstractImplTypeName(ctx.typeInfo().typeName(), ctx.builderTriggerAnnotation())); } else { if (ctx.hasParent()) { - builder.append(toAbstractImplTypeName(ctx.parentTypeName().get(), ctx.builderTriggerAnnotation()).get()); - } else { - Optional baseExtendsTypeName = baseExtendsTypeName(ctx); - if (baseExtendsTypeName.isPresent()) { - builder.append(" extends ").append(baseExtendsTypeName.get().fqName()).append("\n\t\t\t\t\t\t\t\t\t\t"); - } + builder.append(toAbstractImplTypeName(ctx.parentTypeName().get(), ctx.builderTriggerAnnotation())); + } else if (baseExtendsTypeName.isPresent()) { + builder.append(baseExtendsTypeName.get().fqName()); } - if (!ctx.hasParent() && ctx.hasStreamSupportOnImpl()) { - builder.append("<").append(ctx.genericBuilderAcceptAliasDecl()).append(" extends ") - .append(ctx.implTypeName().className()).append(">"); + LinkedList impls = new LinkedList<>(); + if (!ctx.isExtendingAnAbstractClass()) { + impls.add(ctx.typeInfo().typeName().fqName()); } - - builder.append(" implements ").append(ctx.typeInfo().typeName()); if (!ctx.hasParent() && ctx.hasStreamSupportOnImpl()) { - builder.append(", Supplier<").append(ctx.genericBuilderAcceptAliasDecl()).append(">"); + impls.add("Supplier<" + ctx.genericBuilderAcceptAliasDecl() + ">"); } - List extraImplementContracts = extraImplementedTypeNames(ctx); - extraImplementContracts.forEach(t -> builder.append(",\n\t\t\t\t\t\t\t\t\t\t\t").append(t.fqName())); + extraImplementContracts.forEach(t -> impls.add(t.fqName())); + + if (!impls.isEmpty()) { + builder.append(" implements ").append(String.join(", ", impls)); + } } builder.append(" {\n"); @@ -470,17 +466,19 @@ protected void appendToStringMethod(StringBuilder builder, return; } - builder.append("\t@Override\n"); - builder.append("\tpublic String toString() {\n"); - builder.append("\t\treturn ").append(ctx.typeInfo().typeName()); - builder.append(".class.getSimpleName() + "); + if (!ctx.hasOtherMethod("toString", ctx.typeInfo())) { + builder.append("\t@Override\n"); + builder.append("\tpublic String toString() {\n"); + builder.append("\t\treturn ").append(ctx.typeInfo().typeName()); + builder.append(".class.getSimpleName() + "); - String instanceIdRef = instanceIdRef(ctx); - if (!instanceIdRef.isBlank()) { - builder.append("\"{\" + ").append(instanceIdRef).append(" + \"}\" + "); + String instanceIdRef = instanceIdRef(ctx); + if (!instanceIdRef.isBlank()) { + builder.append("\"{\" + ").append(instanceIdRef).append(" + \"}\" + "); + } + builder.append("\"(\" + toStringInner() + \")\";\n"); + builder.append("\t}\n\n"); } - builder.append("\"(\" + toStringInner() + \")\";\n"); - builder.append("\t}\n\n"); } /** @@ -1284,7 +1282,7 @@ private void appendBuilderHeader(StringBuilder builder, if (ctx.doingConcreteType()) { builder.append(" extends "); - builder.append(toAbstractImplTypeName(ctx.typeInfo().typeName(), ctx.builderTriggerAnnotation()).get()); + builder.append(toAbstractImplTypeName(ctx.typeInfo().typeName(), ctx.builderTriggerAnnotation())); builder.append(".").append(ctx.genericBuilderClassDecl()); builder.append("<").append(ctx.genericBuilderClassDecl()).append(", ").append(ctx.ctorBuilderAcceptTypeName()) .append("> {\n"); @@ -1293,47 +1291,47 @@ private void appendBuilderHeader(StringBuilder builder, builder.append("<").append(ctx.genericBuilderAliasDecl()).append(", "); builder.append(ctx.genericBuilderAcceptAliasDecl()).append(">, ").append(ctx.genericBuilderAcceptAliasDecl()) .append(" extends "); - builder.append(ctx.ctorBuilderAcceptTypeName()).append("> "); + builder.append(ctx.ctorBuilderAcceptTypeName()).append(">"); if (ctx.hasParent()) { - builder.append("extends ") - .append(toAbstractImplTypeName(ctx.parentTypeName().get(), ctx.builderTriggerAnnotation()).get()) + builder.append(" extends ") + .append(toAbstractImplTypeName(ctx.parentTypeName().get(), ctx.builderTriggerAnnotation())) .append(".").append(ctx.genericBuilderClassDecl()); builder.append("<").append(ctx.genericBuilderAliasDecl()) .append(", ").append(ctx.genericBuilderAcceptAliasDecl()); builder.append(">"); } else { Optional baseExtendsTypeName = baseExtendsBuilderTypeName(ctx); + if (baseExtendsTypeName.isEmpty() && ctx.isExtendingAnAbstractClass()) { + baseExtendsTypeName = Optional.of(ctx.typeInfo().typeName()); + } if (baseExtendsTypeName.isPresent()) { builder.append("\n\t\t\t\t\t\t\t\t\t\textends ") - .append(baseExtendsTypeName.get().fqName()) - .append("\n\t\t\t\t\t\t\t\t\t\t"); + .append(baseExtendsTypeName.get().fqName()); } } LinkedList impls = new LinkedList<>(); - if (ctx.hasStreamSupportOnBuilder() || !ctx.hasParent()) { - if (!ctx.hasAnyBuilderClashingMethodNames()) { - impls.add(ctx.typeInfo().typeName().name()); - } - } - - if (ctx.hasStreamSupportOnBuilder() && !ctx.requireLibraryDependencies()) { - impls.add("Supplier<" + ctx.genericBuilderAcceptAliasDecl() + ">"); + if (!ctx.isExtendingAnAbstractClass()) { + impls.add(ctx.typeInfo().typeName().name()); } - if (!ctx.hasParent()) { + if (ctx.hasStreamSupportOnBuilder() && !ctx.requireLibraryDependencies()) { + impls.add("Supplier<" + ctx.genericBuilderAcceptAliasDecl() + ">"); + } + if (ctx.requireLibraryDependencies()) { impls.add(io.helidon.common.Builder.class.getName() + "<" + ctx.genericBuilderAliasDecl() + ", " + ctx.genericBuilderAcceptAliasDecl() + ">"); } - } - List extraImplementBuilderContracts = extraImplementedBuilderContracts(ctx); - extraImplementBuilderContracts.forEach(t -> impls.add(t.fqName())); + List extraImplementBuilderContracts = extraImplementedBuilderContracts(ctx); + extraImplementBuilderContracts.forEach(t -> impls.add(t.fqName())); + } if (!impls.isEmpty()) { builder.append(" implements ").append(String.join(", ", impls)); } + builder.append(" {\n"); } } @@ -1448,40 +1446,44 @@ private void appendHashCodeAndEquals(StringBuilder builder, return; } - builder.append("\t@Override\n"); - builder.append("\tpublic int hashCode() {\n"); - if (ctx.hasParent()) { - builder.append("\t\tint hashCode = super.hashCode();\n"); - } else { - builder.append("\t\tint hashCode = 1;\n"); - } - List methods = new ArrayList<>(); - for (TypedElementName method : ctx.allTypeInfos()) { - methods.add(method.elementName() + "()"); + if (!ctx.hasOtherMethod("hashCode", ctx.typeInfo())) { + builder.append("\t@Override\n"); + builder.append("\tpublic int hashCode() {\n"); + if (ctx.hasParent()) { + builder.append("\t\tint hashCode = super.hashCode();\n"); + } else { + builder.append("\t\tint hashCode = 1;\n"); + } + List methods = new ArrayList<>(); + for (TypedElementName method : ctx.allTypeInfos()) { + methods.add(method.elementName() + "()"); + } + builder.append("\t\thashCode = 31 * hashCode + Objects.hash(").append(String.join(", ", methods)).append(");\n"); + builder.append("\t\treturn hashCode;\n"); + builder.append("\t}\n\n"); } - builder.append("\t\thashCode = 31 * hashCode + Objects.hash(").append(String.join(", ", methods)).append(");\n"); - builder.append("\t\treturn hashCode;\n"); - builder.append("\t}\n\n"); - builder.append("\t@Override\n"); - builder.append("\tpublic boolean equals(Object another) {\n"); - builder.append("\t\tif (this == another) {\n\t\t\treturn true;\n\t\t}\n"); - builder.append("\t\tif (!(another instanceof ").append(ctx.typeInfo().typeName()).append(")) {\n"); - builder.append("\t\t\treturn false;\n"); - builder.append("\t\t}\n"); - builder.append("\t\t").append(ctx.typeInfo().typeName()).append(" other = (") - .append(ctx.typeInfo().typeName()).append(") another;\n"); - if (ctx.hasParent()) { - builder.append("\t\tboolean equals = super.equals(other);\n"); - } else { - builder.append("\t\tboolean equals = true;\n"); - } - for (TypedElementName method : ctx.allTypeInfos()) { - builder.append("\t\tequals &= Objects.equals(").append(method.elementName()).append("(), other.") - .append(method.elementName()).append("());\n"); + if (!ctx.hasOtherMethod("equals", ctx.typeInfo())) { + builder.append("\t@Override\n"); + builder.append("\tpublic boolean equals(Object another) {\n"); + builder.append("\t\tif (this == another) {\n\t\t\treturn true;\n\t\t}\n"); + builder.append("\t\tif (!(another instanceof ").append(ctx.typeInfo().typeName()).append(")) {\n"); + builder.append("\t\t\treturn false;\n"); + builder.append("\t\t}\n"); + builder.append("\t\t").append(ctx.typeInfo().typeName()).append(" other = (") + .append(ctx.typeInfo().typeName()).append(") another;\n"); + if (ctx.hasParent()) { + builder.append("\t\tboolean equals = super.equals(other);\n"); + } else { + builder.append("\t\tboolean equals = true;\n"); + } + for (TypedElementName method : ctx.allTypeInfos()) { + builder.append("\t\tequals &= Objects.equals(").append(method.elementName()).append("(), other.") + .append(method.elementName()).append("());\n"); + } + builder.append("\t\treturn equals;\n"); + builder.append("\t}\n\n"); } - builder.append("\t\treturn equals;\n"); - builder.append("\t}\n\n"); } private void appendInnerToStringMethod(StringBuilder builder, diff --git a/builder/processor/src/main/java/io/helidon/builder/processor/BuilderProcessor.java b/builder/processor/src/main/java/io/helidon/builder/processor/BuilderProcessor.java index d1a67b8891b..19977c8d4ff 100644 --- a/builder/processor/src/main/java/io/helidon/builder/processor/BuilderProcessor.java +++ b/builder/processor/src/main/java/io/helidon/builder/processor/BuilderProcessor.java @@ -109,7 +109,8 @@ public void init(ProcessingEnvironment processingEnv) { } @Override - public boolean process(Set annotations, RoundEnvironment roundEnv) { + public boolean process(Set annotations, + RoundEnvironment roundEnv) { if (roundEnv.processingOver() || tools == null || PRODUCERS.isEmpty()) { elementsProcessed.clear(); return false; @@ -146,7 +147,8 @@ public boolean process(Set annotations, RoundEnvironment * @param element the element being processed * @throws IOException if unable to write the generated class */ - protected void process(Class annoType, Element element) throws IOException { + protected void process(Class annoType, + Element element) throws IOException { AnnotationMirror am = BuilderTypeTools.findAnnotationMirror(annoType.getName(), element.getAnnotationMirrors()) .orElseThrow(() -> new IllegalArgumentException("Cannot find annotation mirror for " + annoType diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java new file mode 100644 index 00000000000..82d3d0cd943 --- /dev/null +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.testsubjects; + +import io.helidon.builder.Builder; + +/** + * Usage of an abstract class target, as well as overriding {@link #toString()}, {@link #hashCode()}, and {@link #equals(Object)}. + */ +@Builder(interceptor = GeneralInterceptor.class) +public abstract class AbstractWithCustomMethods { + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract String name(); + + @Override + public String toString() { + return name(); + } + + @Override + public int hashCode() { + return 1; + } + + @Override + public boolean equals(Object another) { + return true; + } + +} diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/GeneralInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/GeneralInterceptor.java new file mode 100644 index 00000000000..847869c0844 --- /dev/null +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/GeneralInterceptor.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test.testsubjects; + +import java.util.LinkedList; +import java.util.List; + +import io.helidon.common.Builder; + +/** + * Here for testing. + */ +@Deprecated +@SuppressWarnings("unchecked") +public class GeneralInterceptor { + private static final List INTERCEPT_CALLS = new LinkedList<>(); + + /** + * Generic interceptor. + * + * @param builder generic builder + * @return the builder + */ + public Object intercept(Builder builder) { + INTERCEPT_CALLS.add(builder); + return builder; + } + + /** + * Gets all interceptor calls ever made. + * + * @return all interceptor calls + */ + public static List getInterceptCalls() { + return INTERCEPT_CALLS; + } + +} diff --git a/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java b/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java new file mode 100644 index 00000000000..b05c4d7e42e --- /dev/null +++ b/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.builder.test; + +import io.helidon.builder.test.testsubjects.AbstractWithCustomMethods; +import io.helidon.builder.test.testsubjects.DefaultAbstractWithCustomMethods; +import io.helidon.builder.test.testsubjects.GeneralInterceptor; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +class AbstractWithCustomMethodsTest { + + @Test + void testIt() { + AbstractWithCustomMethods val = DefaultAbstractWithCustomMethods.builder() + .name("test") + .build(); + assertThat(val.toString(), + equalTo("test")); + assertThat(val.hashCode(), + equalTo(1)); + assertThat(val.equals(this), + equalTo(true)); + + assertThat(GeneralInterceptor.getInterceptCalls().size(), + equalTo(1)); + } + +} From 40e58b63b59aa85e18ca72987a775ab433992090 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Tue, 6 Dec 2022 14:53:37 -0500 Subject: [PATCH 08/10] Fix for issue #5609 --- .../builder/processor/tools/BeanUtils.java | 26 ++++++-- .../builder/processor/tools/BodyContext.java | 3 +- .../processor/tools/BuilderTypeTools.java | 3 - .../tools/DefaultBuilderCreatorProvider.java | 6 +- .../AbstractWithCustomMethods.java | 65 +++++++++++++++++++ .../test/AbstractWithCustomMethodsTest.java | 6 ++ 6 files changed, 97 insertions(+), 12 deletions(-) diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BeanUtils.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BeanUtils.java index 36a94a657c3..93b4786757b 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BeanUtils.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BeanUtils.java @@ -100,21 +100,36 @@ public static boolean validateAndParseMethodName(String methodName, return invalidMethod(methodName, throwIfInvalid, "invalid method name (must start with get or is)"); } - return validMethod(methodName.substring(3), attributeNameRef, throwIfInvalid); + return validMethod(methodName, attributeNameRef, throwIfInvalid); + } + + /** + * Returns true if the word provided is considered to be a reserved word and should otherwise be avoided from generation. + * + * @param word the word + * @return true if it appears to be a reserved word + */ + public static boolean isReservedWord(String word) { + word = word.toLowerCase(); + return word.equals("class") || word.equals("interface") || word.equals("package") || word.equals("static") + || word.equals("final") || word.equals("public") || word.equals("protected") || word.equals("private") + || word.equals("abstract"); } private static boolean validMethod(String name, AtomicReference>> attributeNameRef, boolean throwIfInvalid) { assert (name.trim().equals(name)); - char c = name.charAt(0); + String attrName = name.substring(3); + char c = attrName.charAt(0); - if (!validMethodCase(name, c, throwIfInvalid)) { + if (!validMethodCase(attrName, c, throwIfInvalid)) { return false; } c = Character.toLowerCase(c); - attributeNameRef.set(Optional.of(Collections.singletonList("" + c + name.substring(1)))); + String altName = "" + c + attrName.substring(1); + attributeNameRef.set(Optional.of(Collections.singletonList(isReservedWord(altName) ? name : altName))); return true; } @@ -130,7 +145,8 @@ private static boolean validBooleanIsMethod(String name, } c = Character.toLowerCase(c); - attributeNameRef.set(Optional.of(List.of("" + c + name.substring(3), name))); + String altName = "" + c + name.substring(3); + attributeNameRef.set(Optional.of(isReservedWord(altName) ? List.of(name) : List.of(altName, name))); return true; } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 142da344258..985b25376b2 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -550,7 +550,8 @@ private static void gatherAllAttributeNames(BodyContext ctx, Optional alternateName = alternateNames.get().orElse(Collections.emptyList()).stream() .filter(it -> !it.equals(currentAttrName)) .findFirst(); - if (alternateName.isPresent() && !ctx.map.containsKey(alternateName.get())) { + if (alternateName.isPresent() && !ctx.map.containsKey(alternateName.get()) + && !BeanUtils.isReservedWord(alternateName.get())) { beanAttributeName = alternateName.get(); existing = ctx.map.get(beanAttributeName); } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java index 7767ea205e6..4be6a98b33c 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java @@ -166,9 +166,6 @@ protected boolean canAccept(ExecutableElement ee) { if (mods.contains(Modifier.ABSTRACT)) { return true; } -// if (mods.contains(Modifier.DEFAULT) || mods.contains(Modifier.STATIC)) { -// return false; -// } return false; } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index 950dc9bbded..0cd2d40d46a 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -1084,7 +1084,8 @@ private void appendBuilder(StringBuilder builder, String basicAttributeName = "" + Character.toLowerCase(beanAttributeName.charAt(2)) + beanAttributeName.substring(3); - if (!ctx.allAttributeNames().contains(basicAttributeName)) { + if (!BeanUtils.isReservedWord(basicAttributeName) + && !ctx.allAttributeNames().contains(basicAttributeName)) { appendSetter(builder, ctx, beanAttributeName, basicAttributeName, method); } } @@ -1311,7 +1312,7 @@ private void appendBuilderHeader(StringBuilder builder, } LinkedList impls = new LinkedList<>(); - if (!ctx.isExtendingAnAbstractClass()) { + if (!ctx.isExtendingAnAbstractClass() && !ctx.hasAnyBuilderClashingMethodNames()) { impls.add(ctx.typeInfo().typeName().name()); } if (!ctx.hasParent()) { @@ -1587,7 +1588,6 @@ private void appendDefaultValueAssignment(StringBuilder builder, private void appendOverridesOfDefaultValues(StringBuilder builder, BodyContext ctx) { - boolean first = true; for (TypedElementName method : ctx.typeInfo().elementInfo()) { String beanAttributeName = toBeanAttributeName(method, ctx.beanStyleRequired()); if (!ctx.allAttributeNames().contains(beanAttributeName)) { diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java index 82d3d0cd943..e9d3f12fa5a 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/AbstractWithCustomMethods.java @@ -16,6 +16,8 @@ package io.helidon.builder.test.testsubjects; +import java.util.Optional; + import io.helidon.builder.Builder; /** @@ -31,6 +33,69 @@ public abstract class AbstractWithCustomMethods { */ public abstract String name(); + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isStatic(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isClass(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isInterface(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isAbstract(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isFinal(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isPublic(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isProtected(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract Optional getAbstract(); + + /** + * Used for testing. + * + * @return ignored, here for testing purposes only + */ + public abstract boolean isPrivate(); + @Override public String toString() { return name(); diff --git a/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java b/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java index b05c4d7e42e..87acea1dc7b 100644 --- a/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java +++ b/builder/tests/builder/src/test/java/io/helidon/builder/test/AbstractWithCustomMethodsTest.java @@ -27,10 +27,16 @@ class AbstractWithCustomMethodsTest { + /** + * Testing more corner cases.. + */ @Test void testIt() { AbstractWithCustomMethods val = DefaultAbstractWithCustomMethods.builder() .name("test") + .isStatic(true) + .isClass(false) + .getAbstract("x") .build(); assertThat(val.toString(), equalTo("test")); From 3f45f57eb36bbf70cebe673ec4845eb1f463a960 Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Wed, 14 Dec 2022 10:33:21 -0500 Subject: [PATCH 09/10] address review comments --- bom/pom.xml | 1 + .../src/main/java/io/helidon/builder/Builder.java | 2 +- .../{Interceptor.java => BuilderInterceptor.java} | 2 +- .../helidon/builder/processor/spi/DefaultTypeInfo.java | 9 ++++----- .../java/io/helidon/builder/processor/spi/TypeInfo.java | 2 +- .../io/helidon/builder/processor/tools/BodyContext.java | 4 ++-- .../builder/processor/tools/BuilderTypeTools.java | 7 ++----- .../processor/tools/DefaultBuilderCreatorProvider.java | 4 ++-- ...terceptor.java => BeanBuilderBuilderInterceptor.java} | 4 ++-- .../builder/test/testsubjects/InterceptedBean.java | 2 +- .../test/nodeps/NoDepsBeanBuilderInterceptor.java | 2 +- ...anTest.java => NoDepsBuilderInterceptorBeanTest.java} | 2 +- 12 files changed, 19 insertions(+), 22 deletions(-) rename builder/builder/src/main/java/io/helidon/builder/{Interceptor.java => BuilderInterceptor.java} (98%) rename builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/{BeanBuilderInterceptor.java => BeanBuilderBuilderInterceptor.java} (87%) rename builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/{NoDepsInterceptorBeanTest.java => NoDepsBuilderInterceptorBeanTest.java} (96%) diff --git a/bom/pom.xml b/bom/pom.xml index c15580801ad..937d59f9c02 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -1457,6 +1457,7 @@ ${helidon.version} + diff --git a/builder/builder/src/main/java/io/helidon/builder/Builder.java b/builder/builder/src/main/java/io/helidon/builder/Builder.java index 4fd4802f433..f3a3e09dcd6 100644 --- a/builder/builder/src/main/java/io/helidon/builder/Builder.java +++ b/builder/builder/src/main/java/io/helidon/builder/Builder.java @@ -181,7 +181,7 @@ boolean allowNulls() default DEFAULT_ALLOW_NULLS; /** - * The interceptor implementation type. See {@link Interceptor} for further details. Any interceptor applied will be called + * The interceptor implementation type. See {@link BuilderInterceptor} for further details. Any interceptor applied will be called * prior to validation. * * @return the interceptor implementation class diff --git a/builder/builder/src/main/java/io/helidon/builder/Interceptor.java b/builder/builder/src/main/java/io/helidon/builder/BuilderInterceptor.java similarity index 98% rename from builder/builder/src/main/java/io/helidon/builder/Interceptor.java rename to builder/builder/src/main/java/io/helidon/builder/BuilderInterceptor.java index 122fb52be40..289e846e192 100644 --- a/builder/builder/src/main/java/io/helidon/builder/Interceptor.java +++ b/builder/builder/src/main/java/io/helidon/builder/BuilderInterceptor.java @@ -36,7 +36,7 @@ * @see io.helidon.builder.Builder#interceptor() */ @FunctionalInterface -public interface Interceptor { +public interface BuilderInterceptor { /** * Provides the ability to intercept (i.e., including decoration or mutation) the target. diff --git a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java index 377f607fee1..6d9c08b3b0b 100644 --- a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java +++ b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java @@ -19,7 +19,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedList; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -48,9 +47,9 @@ public class DefaultTypeInfo implements TypeInfo { protected DefaultTypeInfo(Builder b) { this.typeName = b.typeName; this.typeKind = b.typeKind; - this.annotations = Collections.unmodifiableList(new LinkedList<>(b.annotations)); - this.elementInfo = Collections.unmodifiableList(new LinkedList<>(b.elementInfo)); - this.otherElementInfo = Collections.unmodifiableList(new LinkedList<>(b.otherElementInfo)); + this.annotations = Collections.unmodifiableList(new ArrayList<>(b.annotations)); + this.elementInfo = Collections.unmodifiableList(new ArrayList<>(b.elementInfo)); + this.otherElementInfo = Collections.unmodifiableList(new ArrayList<>(b.otherElementInfo)); this.superTypeInfo = b.superTypeInfo; } @@ -206,7 +205,7 @@ public Builder elementInfo(Collection val) { */ public Builder addElementInfo(TypedElementName val) { Objects.requireNonNull(val); - elementInfo.add(Objects.requireNonNull(val)); + elementInfo.add(val); return this; } diff --git a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java index 5035206c0d0..73eadde6c37 100644 --- a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java +++ b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/TypeInfo.java @@ -51,7 +51,7 @@ public interface TypeInfo { List annotations(); /** - * The elements that make up the type that are "relevant" for processing. + * The elements that make up the type that are relevant for processing. * * @return the elements that make up the type that are relevant for processing */ diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java index 985b25376b2..d2af1a286a3 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BodyContext.java @@ -118,11 +118,11 @@ public class BodyContext { this.genericBuilderAliasDecl = ("B".equals(typeInfo.typeName().className())) ? "BU" : "B"; this.genericBuilderAcceptAliasDecl = ("T".equals(typeInfo.typeName().className())) ? "TY" : "T"; String interceptorType = searchForBuilderAnnotation("interceptor", builderTriggerAnnotation, typeInfo); - this.interceptorTypeName = (Void.class.getName().equals(interceptorType) || Objects.isNull(interceptorType)) + this.interceptorTypeName = (interceptorType == null || Void.class.getName().equals(interceptorType)) ? null : DefaultTypeName.createFromTypeName(interceptorType); String interceptorCreateMethod = searchForBuilderAnnotation("interceptorCreateMethod", builderTriggerAnnotation, typeInfo); - this.interceptorCreateMethod = ("".equals(interceptorCreateMethod) || Objects.isNull(interceptorCreateMethod)) + this.interceptorCreateMethod = (interceptorCreateMethod == null || interceptorCreateMethod.isEmpty()) ? null : interceptorCreateMethod; } diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java index 4be6a98b33c..5b178e8b91f 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/BuilderTypeTools.java @@ -158,15 +158,12 @@ protected Collection toElementInfo(TypeElement element, * Returns true if the executable element passed is acceptable for processing (i.e., not a static and not a default method * on interfaces, and abstract methods on abstract classes). * - * @param ee the executable element + * @param ee the executable element * @return true if not able to accept */ protected boolean canAccept(ExecutableElement ee) { Set mods = ee.getModifiers(); - if (mods.contains(Modifier.ABSTRACT)) { - return true; - } - return false; + return mods.contains(Modifier.ABSTRACT); } private Optional toTypeInfo(AnnotationAndValue annotation, diff --git a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java index 0cd2d40d46a..965c992e5d3 100644 --- a/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java +++ b/builder/processor-tools/src/main/java/io/helidon/builder/processor/tools/DefaultBuilderCreatorProvider.java @@ -33,7 +33,7 @@ import io.helidon.builder.Annotated; import io.helidon.builder.AttributeVisitor; import io.helidon.builder.Builder; -import io.helidon.builder.Interceptor; +import io.helidon.builder.BuilderInterceptor; import io.helidon.builder.RequiredAttributeVisitor; import io.helidon.builder.Singular; import io.helidon.builder.processor.spi.BuilderCreatorProvider; @@ -449,7 +449,7 @@ protected void appendExtraImports(StringBuilder builder, if (ctx.doingConcreteType()) { builder.append("import ").append(RequiredAttributeVisitor.class.getName()).append(";\n"); } - builder.append("import ").append(Interceptor.class.getName()).append(";\n"); + builder.append("import ").append(BuilderInterceptor.class.getName()).append(";\n"); builder.append("\n"); } } diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java similarity index 87% rename from builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java rename to builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java index 54387ea7fa1..ac8c53370b3 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java @@ -16,12 +16,12 @@ package io.helidon.builder.test.testsubjects; -import io.helidon.builder.Interceptor; +import io.helidon.builder.BuilderInterceptor; /** * See {@link InterceptedBean}. */ -class BeanBuilderInterceptor implements Interceptor { +class BeanBuilderBuilderInterceptor implements BuilderInterceptor { private int callCount; @Override diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java index bcc798333a5..cc0d1b3d31b 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java @@ -22,7 +22,7 @@ /** * Demonstrates interception of builders. */ -@Builder(interceptor = BeanBuilderInterceptor.class) +@Builder(interceptor = BeanBuilderBuilderInterceptor.class) public interface InterceptedBean { /** diff --git a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java index d6393ad4d18..b9f4cfffdbd 100644 --- a/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java +++ b/builder/tests/nodeps/src/main/java/io/helidon/builder/test/nodeps/NoDepsBeanBuilderInterceptor.java @@ -19,7 +19,7 @@ /** * See {@link NoDepsInterceptedBean}. Notice how the Builder annotation on {@link NoDepsInterceptedBean} sets the * {@link io.helidon.builder.Builder#requireLibraryDependencies()} attribute to {@code false}, which is why this class does not - * need to implement {@link io.helidon.builder.Interceptor}. + * need to implement {@link io.helidon.builder.BuilderInterceptor}. */ class NoDepsBeanBuilderInterceptor /* implements Interceptor */ { private int callCount; diff --git a/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java b/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsBuilderInterceptorBeanTest.java similarity index 96% rename from builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java rename to builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsBuilderInterceptorBeanTest.java index dab0d9225f2..01ac458cae8 100644 --- a/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsInterceptorBeanTest.java +++ b/builder/tests/nodeps/src/test/java/io/helidon/builder/test/nodeps/NoDepsBuilderInterceptorBeanTest.java @@ -21,7 +21,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -class NoDepsInterceptorBeanTest { +class NoDepsBuilderInterceptorBeanTest { @Test void testMutation() { From 5991d58c557c56696282bf863197f035228497ca Mon Sep 17 00:00:00 2001 From: Jeff Trent Date: Thu, 15 Dec 2022 10:44:14 -0500 Subject: [PATCH 10/10] resolved the rest of the review comments --- .../main/java/io/helidon/builder/Builder.java | 36 +++++++++++++++++-- .../processor/spi/DefaultTypeInfo.java | 7 ++-- ...eptor.java => BeanBuilderInterceptor.java} | 2 +- .../test/testsubjects/InterceptedBean.java | 2 +- 4 files changed, 38 insertions(+), 9 deletions(-) rename builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/{BeanBuilderBuilderInterceptor.java => BeanBuilderInterceptor.java} (91%) diff --git a/builder/builder/src/main/java/io/helidon/builder/Builder.java b/builder/builder/src/main/java/io/helidon/builder/Builder.java index f3a3e09dcd6..2e9a23d4bf9 100644 --- a/builder/builder/src/main/java/io/helidon/builder/Builder.java +++ b/builder/builder/src/main/java/io/helidon/builder/Builder.java @@ -182,15 +182,45 @@ /** * The interceptor implementation type. See {@link BuilderInterceptor} for further details. Any interceptor applied will be called - * prior to validation. + * prior to validation. The interceptor implementation can be any lambda-like implementation for the {@link BuilderInterceptor} + * functional interface. This means that the implementation should declare a public method that matches the following: + * 
    {@code
+     *    Builder intercept(Builder builder);
+     * }
+     *
    + * Note that the method name must be named intercept. * * @return the interceptor implementation class */ Class interceptor() default Void.class; /** - * The (static) interceptor method to call on the {@link #interceptor()} implementation type. If left undefined then the new - * operator will be called on the type. This attribute is ignored if the {@link #interceptor()} class type is left undefined. + * The (static) interceptor method to call on the {@link #interceptor()} implementation type in order to create the interceptor. + * If left undefined then the {@code new} operator will be called on the type. If provided then the method must be public + * and take no arguments. Example (see the create() method): + * 
    {@code
+     *  public class CustomBuilderInterceptor { // implements BuilderInterceptor
+     *      public CustomBuilderInterceptor() {
+     *      }
+     *
+     *      public static CustomBuilderInterceptor create() {
+     *          ...
+     *      }
+     *
+     *      public Builder intercept(Builder builder) {
+     *          ...
+     *      }
+     *  }
+     * }
+     *
    + *

    + * This attribute is ignored if the {@link #interceptor()} class type is left undefined. + * Note that the method must return an instance of the Builder, and there must be a public method that matches the following: + * 

    {@code
+     *    public Builder intercept(Builder builder);
+     * }
+     *
    + * Note that the method name must be named intercept. * * @return the interceptor create method */ diff --git a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java index 6d9c08b3b0b..1548cb9cd9b 100644 --- a/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java +++ b/builder/processor-spi/src/main/java/io/helidon/builder/processor/spi/DefaultTypeInfo.java @@ -18,7 +18,6 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -47,9 +46,9 @@ public class DefaultTypeInfo implements TypeInfo { protected DefaultTypeInfo(Builder b) { this.typeName = b.typeName; this.typeKind = b.typeKind; - this.annotations = Collections.unmodifiableList(new ArrayList<>(b.annotations)); - this.elementInfo = Collections.unmodifiableList(new ArrayList<>(b.elementInfo)); - this.otherElementInfo = Collections.unmodifiableList(new ArrayList<>(b.otherElementInfo)); + this.annotations = List.copyOf(b.annotations); + this.elementInfo = List.copyOf(b.elementInfo); + this.otherElementInfo = List.copyOf(b.otherElementInfo); this.superTypeInfo = b.superTypeInfo; } diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java similarity index 91% rename from builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java rename to builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java index ac8c53370b3..87560c41347 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderBuilderInterceptor.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/BeanBuilderInterceptor.java @@ -21,7 +21,7 @@ /** * See {@link InterceptedBean}. */ -class BeanBuilderBuilderInterceptor implements BuilderInterceptor { +class BeanBuilderInterceptor implements BuilderInterceptor { private int callCount; @Override diff --git a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java index cc0d1b3d31b..bcc798333a5 100644 --- a/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java +++ b/builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/InterceptedBean.java @@ -22,7 +22,7 @@ /** * Demonstrates interception of builders. */ -@Builder(interceptor = BeanBuilderBuilderInterceptor.class) +@Builder(interceptor = BeanBuilderInterceptor.class) public interface InterceptedBean { /**