diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilder.java b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilder.java index abc4d56b..488bcb10 100644 --- a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilder.java +++ b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilder.java @@ -69,9 +69,8 @@ * carried {@code @SimpleBuilder} for the purpose of triggering builder generation, unless it is * explicitly excluded via {@link Ignore4BuilderGeneration}. The {@link Template} meta-annotation is * {@link Inherited} as well, so custom template annotations that are themselves {@code @Inherited} - * propagate to subclasses in the same way. Note that configuration options declared on the parent's - * {@code @SimpleBuilder(options = ...)} or template are not yet applied to inherited subclass - * builders; subclasses currently use default options (see issue #248). + * propagate to subclasses in the same way. Configuration options declared on the parent's + * {@code @SimpleBuilder(options = ...)} or template are also inherited by subclass builders. * *
Related annotations: * @@ -727,9 +726,9 @@ * {@code @SimpleBuilder.Template} meta-annotation itself; for a custom template annotation to * propagate to unannotated subclasses, the custom annotation must additionally be declared with * {@code @Inherited}. Without {@code @Inherited} on the custom annotation, only the exact type - * carrying it gets a builder. As with {@link SimpleBuilder}, configuration options declared on the - * template are not yet applied to inherited subclass builders; subclasses currently use default - * options (see issue #248). + * carrying it gets a builder. As with {@link SimpleBuilder}, configuration options declared on + * the template are also applied to inherited subclass builders, as long as the custom template + * annotation is itself {@code @Inherited} and no direct annotation overrides it. * *
Example: * diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f3dc39e2..0579421b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -141,7 +141,7 @@ public class ParentDto { ... } public class ChildDto extends ParentDto { ... } ``` -> **Note:** Inherited subclasses currently get a builder with **default options**, not the options declared on the parent's `@SimpleBuilder(options = ...)` or template. Options inheritance is tracked separately (see #248). +> **Note:** Inherited subclasses get a builder that also uses the options declared on the parent's `@SimpleBuilder(options = ...)` or template. If the subclass declares its own `@SimpleBuilder` or template annotation, those options take precedence. ## Excluding Types from Builder Generation @@ -1158,7 +1158,7 @@ Or in compiler options: 3. **Retention and Target**: Add `@Retention(RetentionPolicy.CLASS)` and `@Target(ElementType.TYPE)` 4. **Don't combine**: Don't use `@SimpleBuilder` when using a template annotation 5. **Subclasses not getting a builder**: Add `@Inherited` to the custom template annotation so it propagates to unannotated subclasses (see [Template Annotations](#template-annotations) above). Without `@Inherited`, only the exact type carrying the annotation gets a builder. -6. **Subclass builder has wrong options**: Inherited subclass builders currently use default options, not the parent's `@SimpleBuilder.Options` or template options. This is a known limitation (see #248). +6. **Subclass builder has wrong options**: Ensure the custom template annotation is `@Inherited` and that the parent annotation declares the desired `@SimpleBuilder.Options`. Inherited options are applied to subclass builders; a subclass's own annotation overrides the inherited options. ### Builder Not Generated - Access Modifier Errors diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/BuilderConfigurationReader.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/BuilderConfigurationReader.java index 74f96044..1711d768 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/BuilderConfigurationReader.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/BuilderConfigurationReader.java @@ -24,7 +24,11 @@ package org.javahelpers.simple.builders.processor.processing; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; @@ -39,19 +43,23 @@ /** * Reads builder configuration from annotated elements. * - *
This class analyzes {@link SimpleBuilder.Options} and {@link SimpleBuilder.Template} - * annotations on an element and extracts the raw configuration values without merging. + *
This class analyzes {@link SimpleBuilder.Options} and custom template annotations (annotations + * meta-annotated with {@link SimpleBuilder.Template}) on an element. * - *
Priority order: + *
Priority order (highest to lowest): * *
Note: If {@code @SimpleBuilder} is present, custom template annotations are ignored. + *
Custom template annotations are annotations that are themselves meta-annotated with + * {@code @SimpleBuilder.Template}; they are not placed directly on the class. Within each + * inheritance scope, {@code @SimpleBuilder} options take precedence over template options. */ public class BuilderConfigurationReader { private static final String SIMPLE_BUILDER_ANNOTATION = @@ -91,19 +99,30 @@ public BuilderConfiguration getGlobalConfiguration() { /** * Reads builder configuration from {@code @SimpleBuilder(options = ...)} inline options. * + *
Directly declared options take precedence over inherited options from superclasses. + * *
Returns null if the element has no {@code @SimpleBuilder} annotation. * * @param element the annotated element to analyze * @return configuration from the inline options, or null if not present */ public BuilderConfiguration readFromInlineOptions(Element element) { + BuilderConfiguration direct = readFromInlineOptions(element, AnnotationScope.DIRECT); + if (direct != null) { + return direct; + } + return readFromInlineOptions(element, AnnotationScope.INHERITED); + } + + private BuilderConfiguration readFromInlineOptions(Element element, AnnotationScope scope) { AnnotationMirror simpleBuilderMirror = - extractAnnotationMirror(element, SIMPLE_BUILDER_ANNOTATION); + extractAnnotationMirror(element, SIMPLE_BUILDER_ANNOTATION, scope); return extractOptionsFromAnnotationMirror(simpleBuilderMirror); } - private AnnotationMirror extractAnnotationMirror(Element element, String annotationName) { - for (AnnotationMirror mirror : element.getAnnotationMirrors()) { + private AnnotationMirror extractAnnotationMirror( + Element element, String annotationName, AnnotationScope scope) { + for (AnnotationMirror mirror : getAnnotationMirrors(element, scope)) { if (mirror.getAnnotationType().toString().equals(annotationName)) { return mirror; } @@ -227,9 +246,9 @@ private String extractEnumName(Object value) { /** * Reads builder configuration from a custom template annotation on the element. * - *
Only checks for custom template annotations if {@code @SimpleBuilder} is NOT present. Looks - * for any custom annotation on the element that is itself annotated with - * {@code @SimpleBuilder.Template}. + *
Directly declared template annotations take precedence over inherited template annotations. + * If a {@code @SimpleBuilder} annotation is present in the same scope, template annotations in + * that scope are ignored. * *
Returns null if no template annotation is found or if {@code @SimpleBuilder} is present.
*
@@ -237,15 +256,25 @@ private String extractEnumName(Object value) {
* @return configuration from the template annotation, or null if not present
*/
public BuilderConfiguration readFromTemplate(Element element) {
- // If @SimpleBuilder is present, ignore template annotations
- if (hasSimpleBuilderAnnotation(element)) {
+ BuilderConfiguration direct = readFromTemplate(element, AnnotationScope.DIRECT);
+ if (direct != null) {
+ return direct;
+ }
+ return readFromTemplate(element, AnnotationScope.INHERITED);
+ }
+
+ private BuilderConfiguration readFromTemplate(Element element, AnnotationScope scope) {
+ List extends AnnotationMirror> mirrors = getAnnotationMirrors(element, scope);
+
+ // If @SimpleBuilder is present in this scope, ignore template annotations in the same scope
+ if (containsSimpleBuilder(mirrors)) {
logger.debug(
- "Template annotations ignored because @SimpleBuilder present", element.getSimpleName());
+ "Template annotations ignored because @SimpleBuilder is present in %s scope", scope);
return null;
}
- // Check all annotations on the element to find one annotated with @SimpleBuilder.Template
- for (AnnotationMirror mirror : element.getAnnotationMirrors()) {
+ // Check all annotations in this scope to find one annotated with @SimpleBuilder.Template
+ for (AnnotationMirror mirror : mirrors) {
BuilderConfiguration templateConfig = checkForTemplateAnnotation(mirror, element);
if (templateConfig != null) {
logger.debug("Annotation based Configuration: %s", templateConfig.toString());
@@ -256,14 +285,33 @@ public BuilderConfiguration readFromTemplate(Element element) {
return null;
}
- /**
- * Checks if the element has a direct @SimpleBuilder annotation.
- *
- * @param element the element to check
- * @return true if @SimpleBuilder is present
- */
- private boolean hasSimpleBuilderAnnotation(Element element) {
+ private enum AnnotationScope {
+ DIRECT,
+ INHERITED
+ }
+
+ private List extends AnnotationMirror> getAnnotationMirrors(
+ Element element, AnnotationScope scope) {
+ if (scope == AnnotationScope.DIRECT) {
+ return element.getAnnotationMirrors();
+ }
+
+ Set Priority chain (highest to lowest):
*
* Note: If {@code @SimpleBuilder} is present, custom template annotations are completely
- * ignored. The merge chain ensures that for each field: inline options override compiler args,
- * which override defaults.
+ * Custom template annotations are annotations that are themselves meta-annotated with
+ * {@code @SimpleBuilder.Template} and are placed directly on the class; the
+ * {@code @SimpleBuilder.Template} meta-annotation is not placed on the class itself. Within each
+ * inheritance scope, {@code @SimpleBuilder} options take precedence over template options. Direct
+ * annotations always override inherited annotations.
*
* @param element the annotated element to resolve configuration for
* @return the fully resolved configuration with all sources merged
@@ -362,14 +414,21 @@ public BuilderConfiguration resolveConfiguration(Element element) throws Builder
String elementName = element.getSimpleName().toString();
logger.debugStartOperation("Resolving configuration for element: %s", elementName);
- BuilderConfiguration templateConfig = readFromTemplate(element);
- BuilderConfiguration inlineConfig = readFromInlineOptions(element);
+ BuilderConfiguration inheritedTemplateConfig =
+ readFromTemplate(element, AnnotationScope.INHERITED);
+ BuilderConfiguration inheritedInlineConfig =
+ readFromInlineOptions(element, AnnotationScope.INHERITED);
+ BuilderConfiguration directTemplateConfig = readFromTemplate(element, AnnotationScope.DIRECT);
+ BuilderConfiguration directInlineConfig =
+ readFromInlineOptions(element, AnnotationScope.DIRECT);
BuilderConfiguration result =
BuilderConfiguration.DEFAULT
.merge(globalConfiguration)
- .merge(templateConfig)
- .merge(inlineConfig);
+ .merge(inheritedTemplateConfig)
+ .merge(inheritedInlineConfig)
+ .merge(directTemplateConfig)
+ .merge(directInlineConfig);
// Validate access modifiers and warn about problematic configurations
validateAccessModifiers(element, result);
diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderAnnotationInheritanceTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderAnnotationInheritanceTest.java
index 663924ca..33e4ef44 100644
--- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderAnnotationInheritanceTest.java
+++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderAnnotationInheritanceTest.java
@@ -28,6 +28,7 @@
import static org.javahelpers.simple.builders.processor.testing.ProcessorAsserts.assertContaining;
import static org.javahelpers.simple.builders.processor.testing.ProcessorAsserts.assertGenerationSucceeded;
import static org.javahelpers.simple.builders.processor.testing.ProcessorAsserts.assertNoBuilderGenerated;
+import static org.javahelpers.simple.builders.processor.testing.ProcessorAsserts.assertNotContaining;
import static org.javahelpers.simple.builders.processor.testing.ProcessorTestUtils.loadGeneratedSource;
import com.google.testing.compile.Compilation;
@@ -277,4 +278,318 @@ public class ChildDto extends MiddleDto {
assertGenerationSucceeded(
compilation, "ChildDtoBuilder", loadGeneratedSource(compilation, "ChildDtoBuilder"));
}
+
+ /**
+ * An unannotated subclass must inherit not only the {@code @SimpleBuilder} trigger but also the
+ * configured {@code options} from the parent's annotation. If options are not inherited the child
+ * builder is generated with defaults instead.
+ */
+ @Test
+ void unannotatedSubclassInheritsSimpleBuilderOptions() {
+ JavaFileObject parentSource =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ import org.javahelpers.simple.builders.core.annotations.SimpleBuilder;
+ import org.javahelpers.simple.builders.core.enums.OptionState;
+
+ @SimpleBuilder(options = @SimpleBuilder.Options(
+ generateFieldSupplier = OptionState.DISABLED
+ ))
+ public class ParentDto {
+ private String name;
+
+ public String getName() { return name; }
+ public void setName(String name) { this.name = name; }
+ }
+ """);
+
+ JavaFileObject childSource =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ public class ChildDto extends ParentDto {
+ private int age;
+
+ public int getAge() { return age; }
+ public void setAge(int age) { this.age = age; }
+ }
+ """);
+
+ Compilation compilation = compile(parentSource, childSource);
+
+ assertThat(compilation).succeededWithoutWarnings();
+
+ String childBuilder = loadGeneratedSource(compilation, "ChildDtoBuilder");
+ assertGenerationSucceeded(compilation, "ChildDtoBuilder", childBuilder);
+ assertContaining(childBuilder, "public ChildDtoBuilder name(String name)");
+ assertContaining(childBuilder, "public ChildDtoBuilder age(int age)");
+ assertNotContaining(childBuilder, "Supplier<");
+ }
+
+ /**
+ * An unannotated subclass must inherit the options declared on a parent's custom
+ * {@code @Inherited} template annotation as well, not just the default template options.
+ */
+ @Test
+ void unannotatedSubclassInheritsTemplateOptions() {
+ JavaFileObject templateAnnotation =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ import java.lang.annotation.ElementType;
+ import java.lang.annotation.Inherited;
+ import java.lang.annotation.Retention;
+ import java.lang.annotation.RetentionPolicy;
+ import java.lang.annotation.Target;
+ import org.javahelpers.simple.builders.core.annotations.SimpleBuilder;
+ import org.javahelpers.simple.builders.core.enums.OptionState;
+
+ @SimpleBuilder.Template(options = @SimpleBuilder.Options(
+ generateFieldSupplier = OptionState.DISABLED
+ ))
+ @Inherited
+ @Retention(RetentionPolicy.CLASS)
+ @Target(ElementType.TYPE)
+ public @interface NoSupplierTemplate {}
+ """);
+
+ JavaFileObject parentSource =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ @NoSupplierTemplate
+ public class ParentDto {
+ private String name;
+
+ public String getName() { return name; }
+ public void setName(String name) { this.name = name; }
+ }
+ """);
+
+ JavaFileObject childSource =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ public class ChildDto extends ParentDto {
+ private int age;
+
+ public int getAge() { return age; }
+ public void setAge(int age) { this.age = age; }
+ }
+ """);
+
+ Compilation compilation = compile(templateAnnotation, parentSource, childSource);
+
+ assertThat(compilation).succeededWithoutWarnings();
+
+ String childBuilder = loadGeneratedSource(compilation, "ChildDtoBuilder");
+ assertGenerationSucceeded(compilation, "ChildDtoBuilder", childBuilder);
+ assertContaining(childBuilder, "public ChildDtoBuilder name(String name)");
+ assertContaining(childBuilder, "public ChildDtoBuilder age(int age)");
+ assertNotContaining(childBuilder, "Supplier<");
+ }
+
+ /**
+ * A subclass with its own template annotation must use that template's options, not the
+ * {@code @SimpleBuilder(options = ...)} inherited from the parent.
+ */
+ @Test
+ void subclassTemplateOverridesInheritedSimpleBuilderOptions() {
+ JavaFileObject childTemplate =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ import java.lang.annotation.ElementType;
+ import java.lang.annotation.Inherited;
+ import java.lang.annotation.Retention;
+ import java.lang.annotation.RetentionPolicy;
+ import java.lang.annotation.Target;
+ import org.javahelpers.simple.builders.core.annotations.SimpleBuilder;
+ import org.javahelpers.simple.builders.core.enums.OptionState;
+
+ @SimpleBuilder.Template(options = @SimpleBuilder.Options(
+ generateFieldConsumer = OptionState.DISABLED,
+ generateBuilderConsumer = OptionState.DISABLED,
+ generateWithInterface = OptionState.DISABLED,
+ generateConditionalHelper = OptionState.DISABLED,
+ builderSuffix = "ChildBuilder"
+ ))
+ @Inherited
+ @Retention(RetentionPolicy.CLASS)
+ @Target(ElementType.TYPE)
+ @interface ChildOwnTemplate {}
+ """);
+
+ JavaFileObject parentSource =
+ ProcessorTestUtils.forSource(
+ """
+ package test;
+
+ import org.javahelpers.simple.builders.core.annotations.SimpleBuilder;
+ import org.javahelpers.simple.builders.core.enums.OptionState;
+
+ @SimpleBuilder(options = @SimpleBuilder.Options(
+ generateFieldSupplier = OptionState.DISABLED,
+ builderSuffix = "ParentBuilder"
+ ))
+ public class ParentDto {
+ private String name;
+ private java.util.List
- *
*
- *