From fff0ff1c13ca3b8f451da72c11002be6ec029aee Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 15 Aug 2026 10:57:46 +0200 Subject: [PATCH 01/51] Make @SimpleBuilder @Inherited to match documentation (#244) The class-level Javadoc and docs/CONFIGURATION.md implied that @SimpleBuilder is inherited by subclasses, but the annotation was not meta-annotated with @Inherited. As a result BuilderProcessor, which collects types via RoundEnvironment.getElementsAnnotatedWith(...), only produced builders for the exact type carrying @SimpleBuilder and not for unannotated subclasses. Add @Inherited to @SimpleBuilder so subclasses are treated as if they also carried the annotation, mirroring the existing behaviour of @SimpleBuilder.Template (which is already @Inherited). Update the Javadoc to document the inheritance explicitly and clarify the CONFIGURATION.md wording. @Ignore4BuilderGeneration still suppresses generation for the exact type it is placed on, so opt-outs continue to work as before. Add SimpleBuilderInheritanceTest covering direct inheritance, the opt-out interaction, and multi-level (grandchild) inheritance. Closes #244 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../core/annotations/SimpleBuilder.java | 7 + docs/CONFIGURATION.md | 2 +- .../SimpleBuilderInheritanceTest.java | 222 ++++++++++++++++++ 3 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 processor/src/test/java/org/javahelpers/simple/builders/processor/SimpleBuilderInheritanceTest.java 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 46bbf140..271409ba 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 @@ -54,6 +54,12 @@ * *

Use {@link Template} to create reusable configuration presets. * + *

This annotation is {@link Inherited}: a subclass of an annotated type is treated as if it also + * carried {@code @SimpleBuilder}, 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. + * *

Related annotations: * *

@@ -708,21 +709,22 @@ /** * Comma-separated list of packages whose builders may be used as helper methods for other DTOs. *
- * Subpackages are included automatically and matching ignores case. A type in this scope but - * not in {@link #builderGenerationPackages()} must have its compiled builder verified (via type - * search) before a builder reference is emitted. If the builder type cannot be resolved, the - * field falls back to a plain setter. + * Subpackages are included automatically and matching ignores case. The processor constructs + * the candidate builder name using {@link #builderUsageSuffix()} (or {@link #builderSuffix()} + * if not configured) and verifies that a class with that name exists on the classpath or was + * generated in the current processing round. This allows referencing builders generated with + * custom template annotations or different suffixes. If the builder type cannot be resolved, + * the field falls back to a plain setter. * *

Packages in {@link #builderGenerationPackages()} are automatically part of the usage scope - * and never need to be repeated here. When both options are empty, builders from any package - * may be referenced; once a generation scope is configured, usage is limited to the packages - * listed in the two options. + * and never need to be repeated here. When this option is empty, builders from any package may + * be referenced; once set, only packages listed here (and generation-scope types) may provide + * builder helpers. * *

Example: {@code "com.example.library, com.example.external"} * - *

Default: "" (empty - all {@code @SimpleBuilder}-annotated types may be referenced as - * builders; the builder type is not verified to exist on the classpath, preserving - * backward-compatible behavior)
+ *

Default: "" (empty - all types may be referenced as builders; the builder type is not + * verified to exist on the classpath, preserving backward-compatible behavior)
* Compiler option: -Asimplebuilder.builderUsagePackages * * @return the packages whose builders may be used as helpers @@ -752,6 +754,35 @@ */ String builderSuffix() default "Builder"; + /** + * Suffix to append to the DTO name when looking up a builder from the usage scope.
+ * The processor constructs the candidate builder name using this suffix and verifies that a + * class with that name exists on the classpath. If empty, the value of {@link #builderSuffix()} + * is used. This allows referencing builders that were generated with a different suffix (e.g. + * by another module using "Factory" as suffix) without changing the suffix used for own builder + * generation. + * + *

Example: + * + * 

{@code
+     * @SimpleBuilder(options = @SimpleBuilder.Options(
+     *     builderSuffix = "Builder",
+     *     builderUsageSuffix = "Factory"
+     * ))
+     * public class OwnerDto {
+     *     // Own builder: OwnerDtoBuilder
+     *     // Referenced builders from usage scope: looked up as *Factory
+     * }
+     * }
+ * + * Default: "" (empty - falls back to {@link #builderSuffix()})
+ * Compiler option: -Asimplebuilder.builderUsageSuffix + * + * @return the suffix for usage-scope builder class names, or empty to use {@link + * #builderSuffix()} + */ + String builderUsageSuffix() default ""; + /** * Suffix to append to setter method names in the generated builder.
* For example, with suffix "with", a field named "name" will generate "withName()".
diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c756af8b..568e4fc0 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -691,19 +691,24 @@ use `@Ignore4BuilderGeneration` instead. The scope also applies to types that ca Controls which packages may provide builders as nested builder helpers. Packages are comma-separated, each listed package includes all of its subpackages, and matching ignores -case. For a type in the usage scope but outside the generation scope, the processor verifies -that its builder actually exists on the classpath before emitting a builder reference. If it -cannot be resolved, the field falls back to a plain setter. +case. -Packages listed in `builderGenerationPackages` are automatically part of the usage scope and -never need to be repeated here: their builders are generated in the same compilation, so they -are trusted without a type-existence search. +The processor constructs the candidate builder name using `builderUsageSuffix` +(or `builderSuffix` if not configured) and verifies the builder contract: a +constructor accepting the referenced type and a no-arg `build()` method returning +it. Any class with the expected name and a matching contract qualifies, allowing +references to builders generated with custom template annotations, external tools, +or different suffixes. If the candidate builder cannot be found, the field falls +back to a plain setter. -When both options are empty, builders from any package may be referenced (the behavior before -scoping existed). Once you configure `builderGenerationPackages`, usage is limited to the -packages listed in the two options — so setting a generation scope while leaving -`builderUsagePackages` empty means only generation-scope builders are used as helpers, and -every other field falls back to a plain setter. +Packages listed in `builderGenerationPackages` are automatically included in the usage +scope — their builders are generated in the same compilation and don't need to be listed +here. + +When `builderUsagePackages` is empty, builders from any package may be referenced (the +behavior before scoping existed). When set, only packages listed in `builderUsagePackages` +(and generation-scope types) may provide builder helpers — types outside both scopes fall +back to a plain setter. The `example` module contains a runnable demo in package `org.javahelpers.simple.builders.example.scoping` ([`ScopedOwnerDto.java`](../example/src/main/java/org/javahelpers/simple/builders/example/scoping/ScopedOwnerDto.java)). @@ -957,6 +962,39 @@ public class PersonDto { } --- +#### `builderUsageSuffix` + +**Default**: `""` (empty — falls back to `builderSuffix`) | **Compiler Option**: +`-Asimplebuilder.builderUsageSuffix=CustomSuffix` + +Customizes the suffix used when looking up builders from the usage scope. The processor +constructs the candidate builder name as `referencedType.getSimpleName() + builderUsageSuffix`. +If empty, `builderSuffix` is used instead. This allows referencing builders that were generated +with a different suffix (e.g. by another module using `"Factory"` as suffix) without changing +the suffix used for own builder generation. + +The candidate class must provide a constructor accepting the referenced type and a no-arg +`build()` method returning it. The contract check is annotation-agnostic, so builders +generated with custom template annotations or external tools are supported. If the +candidate class does not exist or does not satisfy this contract, the field falls back +to a plain setter. + +**Example**: +```java +@SimpleBuilder.Options( + builderSuffix = "Builder", // own builders: *Builder + builderUsageSuffix = "Factory" // usage-scope builders: *Factory +) +public class OwnerDto { + private ExternalDto external; +} + +// Own builder: OwnerDtoBuilder +// External builder looked up as: ExternalDtoFactory (not ExternalDtoBuilder) +``` + +--- + #### `setterSuffix` **Default**: `""` (empty) | **Compiler Option**: `-Asimplebuilder.setterSuffix=customPrefix` diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/BuilderScopeResolver.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/BuilderScopeResolver.java index 65189328..0f9bc265 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/BuilderScopeResolver.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/BuilderScopeResolver.java @@ -33,7 +33,6 @@ import javax.lang.model.element.Element; import javax.lang.model.element.TypeElement; import org.javahelpers.simple.builders.core.annotations.Ignore4BuilderGeneration; -import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; import org.javahelpers.simple.builders.processor.model.core.BuilderConfiguration; import org.javahelpers.simple.builders.processor.model.core.PackageScopes; import org.javahelpers.simple.builders.processor.model.type.TypeName; @@ -43,8 +42,9 @@ * Central resolver that decides whether a builder may be referenced for a given type. * *

This resolver is independent of generator/enhancer code; it only relies on the configured - * {@code builderGenerationPackages} and {@code builderUsagePackages} scopes, registered generated - * types, and the availability of builder types on the classpath. + * {@code builderUsagePackages} scope (which includes {@code builderGenerationPackages} + * automatically), registered generated types, and the availability of builder types on the + * classpath. * *

The resolver is constructed once per processing context and refreshes its parsed package * scopes and per-type results when the target configuration changes. Types whose builders are @@ -54,7 +54,6 @@ public final class BuilderScopeResolver { private final ProcessingContext context; private BuilderConfiguration cachedConfiguration; - private PackageScopes generationPackages = PackageScopes.unscoped(); private PackageScopes usagePackages = PackageScopes.unscoped(); private Set generatedTypeNames = Set.of(); private final Map> resolvedBuilderTypes = new HashMap<>(); @@ -81,16 +80,19 @@ public BuilderScopeResolver(ProcessingContext context) { *

The decision follows these rules: * *

    - *
  1. If the referenced type is opted out with {@code @Ignore4BuilderGeneration}, or is not - * annotated with {@code @SimpleBuilder}, no builder may be used. - *
  2. If both scopes are empty/unset, the candidate builder is returned for full backward - * compatibility (current behavior, no type search). - *
  3. If the referenced type's package is in {@code builderGenerationPackages}, the candidate - * builder is returned without a type-existence search. - *
  4. If the referenced type's package is in {@code builderUsagePackages} (but not in the - * generation scope), the candidate builder is returned if its builder is generated in the - * current processing round or can be resolved on the classpath. - *
  5. Otherwise no builder may be referenced. + *
  6. If the usage scope is set and the referenced type's package is not in it, no builder may + * be referenced. The usage scope includes generation-scope packages automatically. When the + * scope is empty, any package is allowed (backward compatibility). + *
  7. If the referenced type's builder is generated in the current processing round (registered + * via {@link #registerGeneratedTypes}), the candidate builder (using {@code builderSuffix}) + * is returned immediately — trusted without a classpath lookup or contract check. + *
  8. Otherwise, the candidate builder name is constructed using {@code builderUsageSuffix} + * (falling back to {@code builderSuffix} if not configured). The candidate is looked up on + * the classpath and returned if it satisfies the builder contract: a constructor accepting + * the referenced type and a no-arg {@code build()} method returning it. The contract check + * is annotation-agnostic, so builders generated with custom template annotations, external + * tools, or different suffixes are supported. The referenced type must not be opted out + * with {@code @Ignore4BuilderGeneration}. *
* * @param referencedType the type element being referenced as a field or collection element @@ -153,33 +155,63 @@ public void registerGeneratedTypes(Collection generatedTy } private Optional resolve(TypeElement referencedType) { - if (!hasSimpleBuilderAnnotation(referencedType) - || isIgnoredForBuilderGeneration(referencedType)) { + if (referencedType == null || isIgnoredForBuilderGeneration(referencedType)) { return Optional.empty(); } - TypeName candidate = JavaLangMapper.createBuilderTypeName(referencedType, context); + String referencedTypeFqn = referencedType.getQualifiedName().toString(); String packageName = context.getPackageName(referencedType); - // Both scopes unset → full backward compatibility, no type search. - if (generationPackages.isEmpty() && usagePackages.isEmpty()) { - return Optional.of(candidate); + // The usage scope determines whether a type is eligible to be referenced as a builder + // helper. When empty, any package is allowed (backward compatibility). When set, only + // packages in the scope qualify. The scope already includes generation-scope packages. + if (!usagePackages.isEmpty() && !usagePackages.includes(packageName)) { + return Optional.empty(); } - // Generation scope: trusted types whose builders are generated in this compilation. - if (generationPackages.includes(packageName)) { + // Types whose builders are generated in the current processing round are trusted + // immediately — our own generators always produce the builder contract, so no + // classpath lookup or contract check is needed. The candidate uses builderSuffix + // because that is what our own generators produce. + if (generatedTypeNames.contains(referencedTypeFqn)) { + TypeName candidate = + JavaLangMapper.createBuilderTypeName( + referencedType, context, context.getConfiguration().getBuilderSuffix()); return Optional.of(candidate); } - // Usage scope: types whose builders may be generated now or already compiled. - if (usagePackages.includes(packageName)) { - boolean builderAvailable = - generatedTypeNames.contains(referencedType.getQualifiedName().toString()) - || context.getTypeElement(candidate.getFullQualifiedName()) != null; - return builderAvailable ? Optional.of(candidate) : Optional.empty(); - } + // For types not generated in this round, look up the candidate on the classpath using + // builderUsageSuffix (which falls back to builderSuffix if not configured) and verify + // the builder contract. + String suffix = context.getConfiguration().getBuilderUsageSuffix(); + TypeName candidate = JavaLangMapper.createBuilderTypeName(referencedType, context, suffix); + return resolveByBuilderContract(candidate, referencedTypeFqn); + } - return Optional.empty(); + /** + * Looks up the candidate builder type on the classpath and verifies it satisfies the builder + * contract: a constructor accepting the referenced type and a no-arg {@code build()} method + * returning it. The contract check is annotation-agnostic, so builders generated with custom + * template annotations or from external sources are supported as long as they follow the builder + * contract. It also avoids false positives like {@code String} → {@code StringBuilder}. + * + * @param candidate the candidate builder type name to look up + * @param expectedType the qualified name of the referenced type the builder must accept and + * return + * @return the candidate if a matching builder class exists on the classpath, empty otherwise + */ + private Optional resolveByBuilderContract(TypeName candidate, String expectedType) { + TypeElement builderTypeElement = context.getTypeElement(candidate.getFullQualifiedName()); + if (builderTypeElement == null) { + return Optional.empty(); + } + if (!JavaLangAnalyser.hasConstructorAccepting(builderTypeElement, expectedType, context)) { + return Optional.empty(); + } + if (!JavaLangAnalyser.hasBuildMethodReturning(builderTypeElement, expectedType, context)) { + return Optional.empty(); + } + return Optional.of(candidate); } private void refreshForConfigurationIfNeeded() { @@ -187,23 +219,19 @@ private void refreshForConfigurationIfNeeded() { if (Objects.equals(cachedConfiguration, configuration)) { return; } - generationPackages = + // The effective usage scope combines builderUsagePackages and builderGenerationPackages, + // since generation-scope packages are automatically included in the usage scope. + PackageScopes generation = configuration == null ? PackageScopes.unscoped() : configuration.builderGenerationPackages(); - usagePackages = + PackageScopes usage = configuration == null ? PackageScopes.unscoped() : configuration.builderUsagePackages(); + usagePackages = PackageScopes.merge(generation, usage); resolvedBuilderTypes.clear(); cachedConfiguration = configuration; } - private static boolean hasSimpleBuilderAnnotation(TypeElement typeElement) { - if (typeElement == null) { - return false; - } - return JavaLangAnalyser.findAnnotation(typeElement, SimpleBuilder.class).isPresent(); - } - private static boolean isIgnoredForBuilderGeneration(TypeElement typeElement) { if (typeElement == null) { return false; diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangAnalyser.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangAnalyser.java index edf270c0..9da8610c 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangAnalyser.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangAnalyser.java @@ -219,6 +219,53 @@ public static Optional findAnnotation( return Optional.empty(); } + /** + * Checks whether the given type has a no-arg {@code build()} method returning the expected type. + * This is part of the builder contract used when the built value is retrieved. + * + *

The check is annotation-agnostic and avoids false positives like {@code StringBuilder} for + * {@code String}. + * + * @param builderType the candidate builder type element to check + * @param expectedReturnType the qualified name of the type that {@code build()} must return + * @param context the processing context, used to access all members + * @return {@code true} if the type declares or inherits a matching {@code build()} method + */ + public static boolean hasBuildMethodReturning( + TypeElement builderType, String expectedReturnType, ProcessingContext context) { + if (builderType == null) { + return false; + } + return ElementFilter.methodsIn(context.getAllMembers(builderType)).stream() + .anyMatch( + method -> + method.getSimpleName().contentEquals("build") + && method.getParameters().isEmpty() + && method.getReturnType().getKind() != VOID + && method.getReturnType().toString().equals(expectedReturnType)); + } + + /** + * Checks whether the given type has a constructor accepting the expected type. This is part of + * the builder contract used when the field already has a value that is passed to the builder. + * + * @param builderType the candidate builder type element to check + * @param expectedType the qualified name of the type the constructor must accept + * @param context the processing context, used to access all members + * @return {@code true} if the type declares or inherits a matching constructor + */ + public static boolean hasConstructorAccepting( + TypeElement builderType, String expectedType, ProcessingContext context) { + if (builderType == null) { + return false; + } + return ElementFilter.constructorsIn(context.getAllMembers(builderType)).stream() + .anyMatch( + constructor -> + constructor.getParameters().size() == 1 + && constructor.getParameters().get(0).asType().toString().equals(expectedType)); + } + /** * Determines whether a given type element is a functional interface. * diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangMapper.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangMapper.java index a0179f2e..ab54646a 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangMapper.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/analysis/JavaLangMapper.java @@ -245,15 +245,28 @@ private static TypeElement retrieveTypeElementIfExists( } /** - * Creates a TypeName for the builder of a given TypeElement. + * Creates a TypeName for the builder of a given TypeElement using the configured builder suffix. * * @param typeElement the type element to create builder name for * @param context the processing context * @return the TypeName for the builder */ public static TypeName createBuilderTypeName(TypeElement typeElement, ProcessingContext context) { - String builderClassName = - typeElement.getSimpleName().toString() + context.getConfiguration().getBuilderSuffix(); + return createBuilderTypeName( + typeElement, context, context.getConfiguration().getBuilderSuffix()); + } + + /** + * Creates a TypeName for the builder of a given TypeElement using an explicit suffix. + * + * @param typeElement the type element to create builder name for + * @param context the processing context + * @param suffix the builder class name suffix to append + * @return the TypeName for the builder + */ + public static TypeName createBuilderTypeName( + TypeElement typeElement, ProcessingContext context, String suffix) { + String builderClassName = typeElement.getSimpleName().toString() + suffix; String packageName = context.getPackageName(typeElement); return new TypeName(packageName, builderClassName); } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java index 5a0d6a6f..ef08c70e 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java @@ -71,6 +71,8 @@ * all packages if builderGenerationPackages is unscoped as well, otherwise only the generation * scope is usable) * @param builderSuffix Suffix for builder class name + * @param builderUsageSuffix Suffix for builder class name when referencing builders from the usage + * scope; null means fall back to {@code builderSuffix} * @param setterSuffix Suffix for setter method names * @param formattingMode Formatting mode for generated source code (null = inherit from compiler * arg) @@ -105,6 +107,7 @@ public record BuilderConfiguration( PackageScopes builderGenerationPackages, PackageScopes builderUsagePackages, String builderSuffix, + String builderUsageSuffix, String setterSuffix, String formattingMode, OptionState strict) { @@ -295,6 +298,17 @@ public String getBuilderSuffix() { return builderSuffix; } + /** + * Returns the suffix used when referencing builders from the usage scope. + * + * @return the usage-scope builder suffix, or {@link #getBuilderSuffix()} if not configured + */ + public String getBuilderUsageSuffix() { + return builderUsageSuffix != null && !builderUsageSuffix.isEmpty() + ? builderUsageSuffix + : builderSuffix; + } + public String getSetterSuffix() { return setterSuffix; } @@ -376,6 +390,7 @@ public BuilderConfiguration merge(BuilderConfiguration other) { mergeScopes(other.builderGenerationPackages, this.builderGenerationPackages)) .builderUsagePackages(mergeScopes(other.builderUsagePackages, this.builderUsagePackages)) .builderSuffix(mergeString(other.builderSuffix, this.builderSuffix)) + .builderUsageSuffix(mergeString(other.builderUsageSuffix, this.builderUsageSuffix)) .setterSuffix(mergeString(other.setterSuffix, this.setterSuffix)) .formattingMode(mergeString(other.formattingMode, this.formattingMode)) .strict(mergeOptionState(other.strict, this.strict)) @@ -458,6 +473,7 @@ public String toString() { .appendIfNotEmpty("builderGenerationPackages", builderGenerationPackages.toString()) .appendIfNotEmpty("builderUsagePackages", builderUsagePackages.toString()) .appendIfNotEmpty("builderSuffix", builderSuffix) + .appendIfNotEmpty("builderUsageSuffix", builderUsageSuffix) .appendIfNotEmpty("setterSuffix", setterSuffix) .appendIfNotEmpty("formattingMode", formattingMode) .appendValueIfSet("strict", strict) @@ -540,6 +556,7 @@ public static class Builder { // === Naming === private String builderSuffix = null; + private String builderUsageSuffix = null; private String setterSuffix = null; // === Formatting === @@ -823,6 +840,11 @@ public Builder builderSuffix(String value) { return this; } + public Builder builderUsageSuffix(String value) { + this.builderUsageSuffix = value == null ? null : value.trim(); + return this; + } + public Builder setterSuffix(String value) { this.setterSuffix = value == null ? null : value.trim(); return this; @@ -873,6 +895,7 @@ public BuilderConfiguration build() { builderGenerationPackages, builderUsagePackages, builderSuffix, + builderUsageSuffix, setterSuffix, formattingMode, strict); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/CompilerArgumentsEnum.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/CompilerArgumentsEnum.java index a864c129..43970a46 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/CompilerArgumentsEnum.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/processing/CompilerArgumentsEnum.java @@ -153,6 +153,9 @@ public enum CompilerArgumentsEnum { /** Option for builder class name suffix. */ BUILDER_SUFFIX("builderSuffix", string(Builder::builderSuffix)), + /** Option for builder class name suffix when referencing usage-scope builders. */ + BUILDER_USAGE_SUFFIX("builderUsageSuffix", string(Builder::builderUsageSuffix)), + /** Option for setter method name suffix. */ SETTER_SUFFIX("setterSuffix", string(Builder::setterSuffix)), diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeProcessingTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeProcessingTest.java index 3636bb9e..39957866 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeProcessingTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeProcessingTest.java @@ -183,6 +183,166 @@ void optOutTakesPrecedenceOverScopes() { unannotated, "UnannotatedFieldDtoBuilder", "UnannotatedDto", "UnannotatedDtoBuilder"); } + @Test + void usageScope_ReferencesBuilderWithoutSimpleBuilderAnnotation() { + JavaFileObject dto = dto("test", "UnannotatedUsageDto", "LibraryDto", "lib"); + JavaFileObject libraryDto = unannotatedDto("lib", "LibraryDto"); + JavaFileObject libraryDtoBuilder = manualBuilder("lib", "LibraryDtoBuilder", "LibraryDto"); + + Compilation compilation = + ProcessorTestUtils.createCompiler() + .withOptions( + "-Asimplebuilder.builderGenerationPackages=test", + "-Asimplebuilder.builderUsagePackages=lib") + .compile(dto, libraryDto, libraryDtoBuilder); + + assertThat(compilation).succeeded(); + String generated = + ProcessorTestUtils.loadGeneratedSource(compilation, "UnannotatedUsageDtoBuilder"); + ProcessorAsserts.assertContaining( + generated, "public UnannotatedUsageDtoBuilder referenced(LibraryDto referenced)"); + ProcessorAsserts.assertContaining(generated, "referencedBuilderConsumer"); + ProcessorAsserts.assertContaining(generated, "LibraryDtoBuilder"); + ProcessorAsserts.assertNoBuilderGenerated( + compilation, "LibraryDto", "The library type must not be generated in this compilation"); + } + + @Test + void usageScope_UsesBuilderUsageSuffixForLookup() { + JavaFileObject dto = dto("test", "SuffixUsageDto", "LibraryDto", "lib"); + JavaFileObject libraryDto = unannotatedDto("lib", "LibraryDto"); + JavaFileObject libraryDtoFactory = manualBuilder("lib", "LibraryDtoFactory", "LibraryDto"); + + Compilation compilation = + ProcessorTestUtils.createCompiler() + .withOptions( + "-Asimplebuilder.builderGenerationPackages=test", + "-Asimplebuilder.builderUsagePackages=lib", + "-Asimplebuilder.builderUsageSuffix=Factory") + .compile(dto, libraryDto, libraryDtoFactory); + + assertThat(compilation).succeeded(); + String generated = ProcessorTestUtils.loadGeneratedSource(compilation, "SuffixUsageDtoBuilder"); + ProcessorAsserts.assertContaining( + generated, "public SuffixUsageDtoBuilder referenced(LibraryDto referenced)"); + ProcessorAsserts.assertContaining(generated, "referencedBuilderConsumer"); + ProcessorAsserts.assertContaining(generated, "LibraryDtoFactory"); + ProcessorAsserts.assertNotContaining(generated, "LibraryDtoBuilder"); + ProcessorAsserts.assertNoBuilderGenerated( + compilation, "LibraryDto", "The library type must not be generated in this compilation"); + } + + @Test + void usageScope_FallsBackToBuilderSuffixWhenUsageSuffixNotConfigured() { + JavaFileObject dto = dto("test", "FallbackSuffixDto", "LibraryDto", "lib"); + JavaFileObject libraryDto = unannotatedDto("lib", "LibraryDto"); + JavaFileObject libraryDtoBuilder = manualBuilder("lib", "LibraryDtoBuilder", "LibraryDto"); + + Compilation compilation = + ProcessorTestUtils.createCompiler() + .withOptions( + "-Asimplebuilder.builderGenerationPackages=test", + "-Asimplebuilder.builderUsagePackages=lib") + .compile(dto, libraryDto, libraryDtoBuilder); + + assertThat(compilation).succeeded(); + String generated = + ProcessorTestUtils.loadGeneratedSource(compilation, "FallbackSuffixDtoBuilder"); + ProcessorAsserts.assertContaining(generated, "referencedBuilderConsumer"); + ProcessorAsserts.assertContaining(generated, "LibraryDtoBuilder"); + } + + @Test + void usageScope_DoesNotReferenceBuilderWhenClassNotFoundWithUsageSuffix() { + JavaFileObject dto = dto("test", "MissingBuilderDto", "LibraryDto", "lib"); + JavaFileObject libraryDto = unannotatedDto("lib", "LibraryDto"); + JavaFileObject libraryDtoBuilder = manualBuilder("lib", "LibraryDtoBuilder", "LibraryDto"); + + Compilation compilation = + ProcessorTestUtils.createCompiler() + .withOptions( + "-Asimplebuilder.builderGenerationPackages=test", + "-Asimplebuilder.builderUsagePackages=lib", + "-Asimplebuilder.builderUsageSuffix=Factory") + .compile(dto, libraryDto, libraryDtoBuilder); + + assertThat(compilation).succeeded(); + String generated = + ProcessorTestUtils.loadGeneratedSource(compilation, "MissingBuilderDtoBuilder"); + ProcessorAsserts.assertContaining( + generated, "public MissingBuilderDtoBuilder referenced(LibraryDto referenced)"); + ProcessorAsserts.assertNotContaining( + generated, "referencedBuilderConsumer", "LibraryDtoFactory"); + } + + @Test + void inlineOptions_ConfigureBuilderUsageSuffix() { + JavaFileObject dto = + ProcessorTestUtils.forSource( + """ + package test; + import lib.LibraryDto; + import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; + + @SimpleBuilder(options = @SimpleBuilder.Options( + builderGenerationPackages = "test", + builderUsagePackages = "lib", + builderUsageSuffix = "Factory" + )) + public class InlineSuffixDto { + private LibraryDto referenced; + public LibraryDto getReferenced() { return referenced; } + public void setReferenced(LibraryDto referenced) { this.referenced = referenced; } + } + """); + JavaFileObject libraryDto = unannotatedDto("lib", "LibraryDto"); + JavaFileObject libraryDtoFactory = manualBuilder("lib", "LibraryDtoFactory", "LibraryDto"); + + Compilation compilation = + ProcessorTestUtils.createCompiler().compile(dto, libraryDto, libraryDtoFactory); + + assertThat(compilation).succeeded(); + String generated = + ProcessorTestUtils.loadGeneratedSource(compilation, "InlineSuffixDtoBuilder"); + ProcessorAsserts.assertContaining(generated, "referencedBuilderConsumer"); + ProcessorAsserts.assertContaining(generated, "LibraryDtoFactory"); + } + + private static JavaFileObject unannotatedDto(String packageName, String className) { + return ProcessorTestUtils.forSource( + """ + package %s; + public class %s { public %s() {} } + """ + .formatted(packageName, className, className)); + } + + private static JavaFileObject manualBuilder( + String packageName, String builderName, String dtoName) { + return ProcessorTestUtils.forSource( + """ + package %s; + public class %s { + private %s value; + public %s() {} + public %s(%s value) { this.value = value; } + public static %s create() { return new %s(); } + public %s build() { return value != null ? value : new %s(); } + } + """ + .formatted( + packageName, + builderName, + dtoName, + builderName, + builderName, + dtoName, + builderName, + builderName, + dtoName, + dtoName)); + } + private static JavaFileObject dto(String packageName, String className, String fieldType) { return dto(packageName, className, fieldType, packageName); } diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeResolverTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeResolverTest.java index a5f796f3..c7bcd27c 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeResolverTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderScopeResolverTest.java @@ -70,9 +70,10 @@ public class LibHelper { public LibHelper() {} } """)); assertThat(compilation).succeeded(); - // First resolution with generation scope "lib" → builder found + // First resolution with generation scope "lib" and registered → builder found assertEquals("lib.LibHelperBuilder", ResolverProbeProcessor.first.get().getFullQualifiedName()); - // After changing config to exclude "lib", the resolver returns empty + // After clearing registration and changing config to exclude "lib", the resolver returns + // empty (not registered, not in scope) assertEquals(Optional.empty(), ResolverProbeProcessor.afterConfigurationChange); // Cache was cleared by the config change, so a new Optional instance is returned assertNotSame(ResolverProbeProcessor.first, ResolverProbeProcessor.afterConfigurationChange); @@ -114,11 +115,11 @@ public class LibHelper { public LibHelper() {} } """)); assertThat(compilation).succeeded(); - // With usage scope "other" (not "lib"), type is not in scope → empty + // With usage scope "other" (not "lib") and no registration → empty assertEquals(Optional.empty(), ResolverProbeProcessor.beforeRegistration); - // Registration alone doesn't help — type is still not in usage scope + // Registration alone is not enough — the type must also be in the usage scope assertEquals(Optional.empty(), ResolverProbeProcessor.afterRegistration); - // With usage scope "lib" but no registration → empty (builder not yet generated/verified) + // With usage scope "lib" but no registration → empty (builder not on classpath) assertEquals(Optional.empty(), ResolverProbeProcessor.usageBeforeRegistration); // With usage scope "lib" AND registration → builder resolved assertEquals( @@ -126,6 +127,95 @@ public class LibHelper { public LibHelper() {} } ResolverProbeProcessor.usageAfterRegistration.get().getFullQualifiedName()); } + @Test + void resolverUsageScope_ResolvesBuilderWithoutSimpleBuilderAnnotation() { + ResolverProbeProcessor.reset(); + Compilation compilation = + Compiler.javac() + .withProcessors(new ResolverProbeProcessor()) + .compile( + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelper { public LibHelper() {} } + """), + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelperBuilder { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + public LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // Usage scope without @SimpleBuilder annotation — builder resolved by contract check + assertEquals( + "lib.LibHelperBuilder", + ResolverProbeProcessor.usageWithoutAnnotation.get().getFullQualifiedName()); + } + + @Test + void resolverUsageScope_UsesBuilderUsageSuffixWhenConfigured() { + ResolverProbeProcessor.reset(); + Compilation compilation = + Compiler.javac() + .withProcessors(new ResolverProbeProcessor()) + .compile( + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelper { public LibHelper() {} } + """), + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelperFactory { + public LibHelperFactory() {} + public LibHelperFactory(LibHelper value) {} + public LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // With builderUsageSuffix="Factory", the candidate name uses "Factory" + assertEquals( + "lib.LibHelperFactory", + ResolverProbeProcessor.usageWithSuffix.get().getFullQualifiedName()); + } + + @Test + void resolverUsageScope_FallsBackToBuilderSuffixWhenUsageSuffixNotSet() { + ResolverProbeProcessor.reset(); + Compilation compilation = + Compiler.javac() + .withProcessors(new ResolverProbeProcessor()) + .compile( + ProcessorTestUtils.forSource( + """ + package lib; + import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; + @SimpleBuilder + public class LibHelper { public LibHelper() {} } + """), + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelperBuilder { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + public LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // Without builderUsageSuffix, the candidate name uses builderSuffix ("Builder") + assertEquals( + "lib.LibHelperBuilder", + ResolverProbeProcessor.usageDefaultSuffix.get().getFullQualifiedName()); + } + private static final class ResolverProbeProcessor extends AbstractProcessor { private static Optional first; private static Optional second; @@ -134,6 +224,9 @@ private static final class ResolverProbeProcessor extends AbstractProcessor { private static Optional afterRegistration; private static Optional usageBeforeRegistration; private static Optional usageAfterRegistration; + private static Optional usageWithoutAnnotation; + private static Optional usageWithSuffix; + private static Optional usageDefaultSuffix; private boolean captured; @@ -145,6 +238,9 @@ static void reset() { afterRegistration = null; usageBeforeRegistration = null; usageAfterRegistration = null; + usageWithoutAnnotation = null; + usageWithSuffix = null; + usageDefaultSuffix = null; } @Override @@ -168,19 +264,36 @@ public boolean process(Set annotations, RoundEnvironment new ProcessingLogger(processingEnv), BuilderConfiguration.DEFAULT, processingEnv); context.initConfigurationForProcessingTarget(configuration("lib", "Builder")); BuilderScopeResolver resolver = context.getBuilderScopeResolver(); + // Register the type as generated, mirroring the real processor which calls + // registerGeneratedTypes before any resolution happens. + resolver.registerGeneratedTypes(List.of(helper)); first = resolver.resolveUsableBuilderType(helper); second = resolver.resolveUsableBuilderType(helper); + // Clear registration before testing scope-only behavior + resolver.registerGeneratedTypes(List.of()); context.initConfigurationForProcessingTarget(configuration("other", "OtherBuilder")); afterConfigurationChange = resolver.resolveUsableBuilderType(helper); context.initConfigurationForProcessingTarget(usageOnlyConfiguration("other")); beforeRegistration = resolver.resolveUsableBuilderType(helper); + // Registration alone is not enough — the type must be in scope resolver.registerGeneratedTypes(List.of(helper)); afterRegistration = resolver.resolveUsableBuilderType(helper); + // Clear registration for usage-scope classpath lookup tests context.initConfigurationForProcessingTarget(usageOnlyConfiguration("lib")); resolver.registerGeneratedTypes(List.of()); usageBeforeRegistration = resolver.resolveUsableBuilderType(helper); resolver.registerGeneratedTypes(List.of(helper)); usageAfterRegistration = resolver.resolveUsableBuilderType(helper); + // Usage scope without @SimpleBuilder annotation — type existence check only + context.initConfigurationForProcessingTarget(usageOnlyConfiguration("lib")); + resolver.registerGeneratedTypes(List.of()); + usageWithoutAnnotation = resolver.resolveUsableBuilderType(helper); + // Usage scope with builderUsageSuffix="Factory" + context.initConfigurationForProcessingTarget(usageWithSuffixConfiguration("lib", "Factory")); + usageWithSuffix = resolver.resolveUsableBuilderType(helper); + // Usage scope with default suffix (no builderUsageSuffix configured) + context.initConfigurationForProcessingTarget(usageOnlyConfiguration("lib")); + usageDefaultSuffix = resolver.resolveUsableBuilderType(helper); captured = true; return false; } @@ -197,5 +310,14 @@ private static BuilderConfiguration usageOnlyConfiguration(String packageName) { return BuilderConfiguration.DEFAULT.merge( BuilderConfiguration.builder().builderUsagePackages(packageName).build()); } + + private static BuilderConfiguration usageWithSuffixConfiguration( + String packageName, String usageSuffix) { + return BuilderConfiguration.DEFAULT.merge( + BuilderConfiguration.builder() + .builderUsagePackages(packageName) + .builderUsageSuffix(usageSuffix) + .build()); + } } } From 1dee3d091ca3efbb2493d8b77339d62616982c5f Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Mon, 14 Sep 2026 00:26:22 +0200 Subject: [PATCH 49/51] Code and test improvements --- .../model/core/BuilderConfiguration.java | 16 ++- .../processor/model/core/PackageScopes.java | 22 +++ .../model/core/PackageScopesTest.java | 129 ++++++++++++++++++ 3 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java index ef08c70e..16711555 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java @@ -387,8 +387,8 @@ public BuilderConfiguration merge(BuilderConfiguration other) { .generateJavaDoc(mergeOptionState(other.generateJavaDoc, this.generateJavaDoc)) .jacksonModulePackage(mergeString(other.jacksonModulePackage, this.jacksonModulePackage)) .builderGenerationPackages( - mergeScopes(other.builderGenerationPackages, this.builderGenerationPackages)) - .builderUsagePackages(mergeScopes(other.builderUsagePackages, this.builderUsagePackages)) + overrideScopes(other.builderGenerationPackages, this.builderGenerationPackages)) + .builderUsagePackages(overrideScopes(other.builderUsagePackages, this.builderUsagePackages)) .builderSuffix(mergeString(other.builderSuffix, this.builderSuffix)) .builderUsageSuffix(mergeString(other.builderUsageSuffix, this.builderUsageSuffix)) .setterSuffix(mergeString(other.setterSuffix, this.setterSuffix)) @@ -435,14 +435,18 @@ private static String mergeString(String other, String thisValue) { } /** - * Merges two package scope values: the other configuration takes priority when it is scoped; - * unscoped means unset and falls back to this configuration's value. + * Resolves a package scope value by override: the other configuration takes priority when it is + * scoped; unscoped means unset and falls back to this configuration's value. + * + *

This is an override, not a union — when the other configuration specifies any packages, this + * configuration's packages are completely replaced. This matches the configuration layering + * semantics: annotation-level options override compiler-argument-level options when set. * * @param other the other scopes (higher priority) * @param thisValue the current scopes (lower priority) - * @return the merged scopes + * @return the resolved scopes */ - private static PackageScopes mergeScopes(PackageScopes other, PackageScopes thisValue) { + private static PackageScopes overrideScopes(PackageScopes other, PackageScopes thisValue) { return other.isEmpty() ? thisValue : other; } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java index 1740d42a..d414ce90 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java @@ -105,6 +105,28 @@ public boolean includes(String packageName) { return false; } + /** + * Merges two package scopes into a combined scope containing all packages from both. + * + *

If either scope is empty, the other is returned unchanged. If both are empty, the unscoped + * instance is returned. + * + * @param a the first scope, may be unscoped + * @param b the second scope, may be unscoped + * @return the merged scope + */ + public static PackageScopes merge(PackageScopes a, PackageScopes b) { + if (a.isEmpty()) { + return b; + } + if (b.isEmpty()) { + return a; + } + Set merged = new LinkedHashSet<>(a.packages()); + merged.addAll(b.packages()); + return new PackageScopes(Collections.unmodifiableSet(merged)); + } + /** * Returns the configured package names in declaration order. * diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java new file mode 100644 index 00000000..cb504d08 --- /dev/null +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java @@ -0,0 +1,129 @@ +/* + * MIT License + * + * Copyright (c) 2026 Andreas Igel + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package org.javahelpers.simple.builders.processor.model.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** Unit tests for {@link PackageScopes}. */ +class PackageScopesTest { + + @Test + void unscoped_isEmptyAndMatchesNothing() { + PackageScopes scopes = PackageScopes.unscoped(); + assertTrue(scopes.isEmpty()); + assertFalse(scopes.includes("com.example")); + } + + @Test + void parse_emptyOrBlank_returnsUnscoped() { + assertSame(PackageScopes.unscoped(), PackageScopes.parse(null)); + assertSame(PackageScopes.unscoped(), PackageScopes.parse("")); + assertSame(PackageScopes.unscoped(), PackageScopes.parse(" ")); + } + + @Test + void parse_trimsAndFiltersBlankEntries() { + PackageScopes scopes = PackageScopes.parse(" com.example , , com.other "); + assertFalse(scopes.isEmpty()); + assertTrue(scopes.includes("com.example")); + assertTrue(scopes.includes("com.other")); + } + + @Test + void includes_matchesExactPackage() { + PackageScopes scopes = PackageScopes.parse("com.example"); + assertTrue(scopes.includes("com.example")); + } + + @Test + void includes_matchesSubpackage() { + PackageScopes scopes = PackageScopes.parse("com.example"); + assertTrue(scopes.includes("com.example.sub")); + assertTrue(scopes.includes("com.example.deep.nested")); + } + + @Test + void includes_rejectsPackageWithSamePrefixButDifferentSegment() { + // This is the critical test: "com.examplefoo" starts with "com.example" as a string, + // but is NOT a subpackage. The "." separator in the scope check prevents this false + // positive. Removing the "." from scope + "." would break this test. + PackageScopes scopes = PackageScopes.parse("com.example"); + assertFalse(scopes.includes("com.examplefoo")); + assertFalse(scopes.includes("com.examples")); + assertFalse(scopes.includes("com.exampl")); + } + + @Test + void includes_isCaseInsensitive() { + PackageScopes scopes = PackageScopes.parse("com.Example"); + assertTrue(scopes.includes("com.example")); + assertTrue(scopes.includes("COM.EXAMPLE")); + assertTrue(scopes.includes("com.Example.Sub")); + } + + @Test + void includes_rejectsUnrelatedPackage() { + PackageScopes scopes = PackageScopes.parse("com.example"); + assertFalse(scopes.includes("org.other")); + assertFalse(scopes.includes("com")); + } + + @Test + void merge_withEmptyReturnsOther() { + PackageScopes a = PackageScopes.parse("com.example"); + PackageScopes b = PackageScopes.unscoped(); + assertSame(a, PackageScopes.merge(a, b)); + assertSame(a, PackageScopes.merge(b, a)); + } + + @Test + void merge_bothEmptyReturnsUnscoped() { + assertSame( + PackageScopes.unscoped(), + PackageScopes.merge(PackageScopes.unscoped(), PackageScopes.unscoped())); + } + + @Test + void merge_bothNonEmptyCombinesBoth() { + PackageScopes a = PackageScopes.parse("com.example"); + PackageScopes b = PackageScopes.parse("org.other"); + PackageScopes merged = PackageScopes.merge(a, b); + assertTrue(merged.includes("com.example")); + assertTrue(merged.includes("com.example.sub")); + assertTrue(merged.includes("org.other")); + assertTrue(merged.includes("org.other.deep")); + } + + @Test + void packages_returnsConfiguredInDeclarationOrder() { + PackageScopes scopes = PackageScopes.parse("com.first, com.second"); + assertEquals(2, scopes.packages().size()); + } +} From 9fb17d54713d7cc0c972e83886c445871416df27 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 19 Sep 2026 16:45:59 +0200 Subject: [PATCH 50/51] refactor: centralize package scope behavior Normalize package scopes for stable output and keep parsing and matching coverage with the PackageScopes abstraction. Remove redundant configuration scope helpers and their duplicate tests. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../model/core/BuilderConfiguration.java | 24 +---- .../processor/model/core/PackageScopes.java | 13 ++- .../processor/BuilderConfigurationTest.java | 94 ------------------- .../model/core/PackageScopesTest.java | 5 +- 4 files changed, 12 insertions(+), 124 deletions(-) delete mode 100644 processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderConfigurationTest.java diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java index 16711555..e2fe9d5e 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/BuilderConfiguration.java @@ -274,26 +274,6 @@ public Set getBuilderUsagePackagesSet() { return builderUsagePackages.packages(); } - /** - * Checks whether the given package is within the builder generation scope. - * - * @param packageName the package to check - * @return true if the package equals or is a subpackage of a configured generation package - */ - public boolean isInGenerationScope(String packageName) { - return builderGenerationPackages.includes(packageName); - } - - /** - * Checks whether the given package is within the builder usage scope. - * - * @param packageName the package to check - * @return true if the package equals or is a subpackage of a configured usage package - */ - public boolean isInUsageScope(String packageName) { - return builderUsagePackages.includes(packageName); - } - public String getBuilderSuffix() { return builderSuffix; } @@ -304,9 +284,7 @@ public String getBuilderSuffix() { * @return the usage-scope builder suffix, or {@link #getBuilderSuffix()} if not configured */ public String getBuilderUsageSuffix() { - return builderUsageSuffix != null && !builderUsageSuffix.isEmpty() - ? builderUsageSuffix - : builderSuffix; + return StringUtils.defaultIfBlank(builderUsageSuffix, builderSuffix); } public String getSetterSuffix() { diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java index d414ce90..fd27c90f 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/core/PackageScopes.java @@ -27,8 +27,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashSet; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; @@ -68,13 +70,14 @@ public static PackageScopes parse(String value) { if (StringUtils.isBlank(value)) { return UNSCOPED; } - return new PackageScopes( - Arrays.stream(StringUtils.split(value, ",")) + Set packages = + Stream.ofNullable(StringUtils.split(value, ',')) + .flatMap(Arrays::stream) .map(String::trim) .filter(StringUtils::isNotBlank) - .collect( - Collectors.collectingAndThen( - Collectors.toCollection(LinkedHashSet::new), Collections::unmodifiableSet))); + .map(packageName -> packageName.toLowerCase(Locale.ROOT)) + .collect(Collectors.toCollection(LinkedHashSet::new)); + return packages.isEmpty() ? UNSCOPED : new PackageScopes(Collections.unmodifiableSet(packages)); } /** diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderConfigurationTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderConfigurationTest.java deleted file mode 100644 index 77c5f68f..00000000 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderConfigurationTest.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2026 Andreas Igel - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -package org.javahelpers.simple.builders.processor; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.javahelpers.simple.builders.processor.model.core.BuilderConfiguration; -import org.junit.jupiter.api.Test; - -/** Unit tests for package scope configuration parsing and matching. */ -class BuilderConfigurationTest { - - @Test - void packageScopes_SplitAndTrimValues() { - BuilderConfiguration config = - BuilderConfiguration.builder() - .builderGenerationPackages("a.b , c.d") - .builderUsagePackages("x.y, z.w") - .build(); - - assertEquals(java.util.Set.of("a.b", "c.d"), config.getBuilderGenerationPackagesSet()); - assertEquals(java.util.Set.of("x.y", "z.w"), config.getBuilderUsagePackagesSet()); - } - - @Test - void blankScopesAreUnscoped() { - BuilderConfiguration config = - BuilderConfiguration.builder() - .builderGenerationPackages(" ") - .builderUsagePackages((String) null) - .build(); - - assertTrue(config.getBuilderGenerationPackagesSet().isEmpty()); - assertTrue(config.getBuilderUsagePackagesSet().isEmpty()); - assertFalse(config.isInGenerationScope("anything")); - assertFalse(config.isInUsageScope("anything")); - } - - @Test - void packageScopesMatchExactAndSubpackagesOnly() { - BuilderConfiguration config = - BuilderConfiguration.builder() - .builderGenerationPackages("a.b") - .builderUsagePackages("c.d") - .build(); - - assertTrue(config.isInGenerationScope("a.b")); - assertTrue(config.isInGenerationScope("a.b.child")); - assertFalse(config.isInGenerationScope("a.bc")); - assertFalse(config.isInGenerationScope("c.d")); - assertTrue(config.isInUsageScope("c.d")); - assertTrue(config.isInUsageScope("c.d.child")); - assertFalse(config.isInUsageScope("c.de")); - } - - @Test - void packageScopesMatchIgnoringCase() { - BuilderConfiguration config = - BuilderConfiguration.builder() - .builderGenerationPackages("com.Example.Dto") - .builderUsagePackages("com.library") - .build(); - - assertTrue(config.isInGenerationScope("com.example.dto")); - assertTrue(config.isInGenerationScope("COM.EXAMPLE.DTO.child")); - assertFalse(config.isInGenerationScope("com.example.dtos")); - assertTrue(config.isInUsageScope("COM.Library")); - assertTrue(config.isInUsageScope("com.LIBRARY.sub")); - } -} diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java index cb504d08..27610eb6 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/model/core/PackageScopesTest.java @@ -49,11 +49,12 @@ void parse_emptyOrBlank_returnsUnscoped() { } @Test - void parse_trimsAndFiltersBlankEntries() { - PackageScopes scopes = PackageScopes.parse(" com.example , , com.other "); + void parse_trimsNormalizesCaseAndFiltersBlankEntries() { + PackageScopes scopes = PackageScopes.parse(" com.Example , , COM.other "); assertFalse(scopes.isEmpty()); assertTrue(scopes.includes("com.example")); assertTrue(scopes.includes("com.other")); + assertEquals("com.example, com.other", scopes.toString()); } @Test From 84edecf5d42a1fc491c2d195e1baf74f2bf0d8a7 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 19 Sep 2026 16:46:38 +0200 Subject: [PATCH 51/51] test: reuse shared processing logger capture Replace formatter-specific logging stubs with CapturingProcessingLogger and remove the obsolete note assertion helper now superseded by ordered checks. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../roaster/RoasterSourceFormatterTest.java | 142 +++--------------- .../processor/testing/ProcessorAsserts.java | 13 -- 2 files changed, 23 insertions(+), 132 deletions(-) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterSourceFormatterTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterSourceFormatterTest.java index c129ebfc..bb5f84fb 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterSourceFormatterTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterSourceFormatterTest.java @@ -28,24 +28,9 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Locale; -import java.util.Map; import java.util.stream.Stream; -import javax.annotation.processing.Filer; -import javax.annotation.processing.Messager; -import javax.annotation.processing.ProcessingEnvironment; -import javax.lang.model.SourceVersion; -import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.AnnotationValue; -import javax.lang.model.element.Element; -import javax.lang.model.util.Elements; -import javax.lang.model.util.Types; -import javax.tools.Diagnostic; import org.javahelpers.simple.builders.core.enums.FormattingMode; -import org.javahelpers.simple.builders.processor.processing.logging.ProcessingLogger; +import org.javahelpers.simple.builders.processor.testing.CapturingProcessingLogger; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -65,87 +50,8 @@ */ class RoasterSourceFormatterTest { - /** A minimal ProcessingEnvironment stub that provides a no-op Messager. */ - private static final class TestProcessingEnv implements ProcessingEnvironment { - final TestMessager messager = new TestMessager(); - - @Override - public Messager getMessager() { - return messager; - } - - @Override - public Map getOptions() { - return Collections.emptyMap(); - } - - @Override - public Elements getElementUtils() { - return null; - } - - @Override - public Types getTypeUtils() { - return null; - } - - @Override - public Filer getFiler() { - return null; - } - - @Override - public SourceVersion getSourceVersion() { - return SourceVersion.RELEASE_17; - } - - @Override - public Locale getLocale() { - return Locale.getDefault(); - } - } - - /** A minimal Messager that captures warnings. */ - private static final class TestMessager implements Messager { - final List warnings = new ArrayList<>(); - - @Override - public void printMessage(Diagnostic.Kind kind, CharSequence msg) { - if (kind == Diagnostic.Kind.WARNING) { - warnings.add(msg.toString()); - } - } - - @Override - public void printMessage(Diagnostic.Kind kind, CharSequence msg, Element e) { - if (kind == Diagnostic.Kind.WARNING) { - warnings.add(msg.toString()); - } - } - - @Override - public void printMessage( - Diagnostic.Kind kind, CharSequence msg, Element e, AnnotationMirror a) { - if (kind == Diagnostic.Kind.WARNING) { - warnings.add(msg.toString()); - } - } - - @Override - public void printMessage( - Diagnostic.Kind kind, CharSequence msg, Element e, AnnotationMirror a, AnnotationValue v) { - if (kind == Diagnostic.Kind.WARNING) { - warnings.add(msg.toString()); - } - } - } - - private TestProcessingEnv createProcessingEnv() { - return new TestProcessingEnv(); - } - private RoasterSourceFormatter createFormatter(FormattingMode mode) { - return new RoasterSourceFormatter(new ProcessingLogger(createProcessingEnv()), mode); + return new RoasterSourceFormatter(CapturingProcessingLogger.create().logger(), mode); } @Test @@ -623,9 +529,9 @@ void format_noneMode_doesNotConvertTabs() { @Test void format_jdtMode_withProfile_producesFormattedOutput() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); - RoasterSourceFormatter formatter = new RoasterSourceFormatter(logger, FormattingMode.JDT); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); + RoasterSourceFormatter formatter = + new RoasterSourceFormatter(capturing.logger(), FormattingMode.JDT); String input = """ package test; @@ -635,48 +541,46 @@ void format_jdtMode_withProfile_producesFormattedOutput() { String result = formatter.format(input); assertNotNull(result, "Format should always return a non-null string"); assertTrue( - env.messager.warnings.stream().noneMatch(w -> w.contains("JDT formatting requested")), + capturing.messages().stream().noneMatch(w -> w.contains("JDT formatting requested")), "No fallback warning should be logged when formatter profile is available on classpath"); } @Test void constructor_jdtMode_missingProfile_logsFallbackWarning() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); - new RoasterSourceFormatter(logger, FormattingMode.JDT, "nonexistent-profile.xml"); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); + new RoasterSourceFormatter(capturing.logger(), FormattingMode.JDT, "nonexistent-profile.xml"); assertTrue( - env.messager.warnings.stream() + capturing.messages().stream() .anyMatch(w -> w.contains("JDT formatting requested") && w.contains("unavailable")), "JDT mode with missing profile should log fallback warning"); } @Test void constructor_lightweightMode_missingProfile_noFallbackWarning() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); - new RoasterSourceFormatter(logger, FormattingMode.LIGHTWEIGHT, "nonexistent-profile.xml"); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); + new RoasterSourceFormatter( + capturing.logger(), FormattingMode.LIGHTWEIGHT, "nonexistent-profile.xml"); assertTrue( - env.messager.warnings.stream().noneMatch(w -> w.contains("JDT formatting requested")), + capturing.messages().stream().noneMatch(w -> w.contains("JDT formatting requested")), "LIGHTWEIGHT mode should not log JDT fallback warning even if profile is missing"); } @Test void constructor_missingProfile_logsProfileNotFoundWarning() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); - new RoasterSourceFormatter(logger, FormattingMode.JDT, "nonexistent-profile.xml"); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); + new RoasterSourceFormatter(capturing.logger(), FormattingMode.JDT, "nonexistent-profile.xml"); assertTrue( - env.messager.warnings.stream() + capturing.messages().stream() .anyMatch(w -> w.contains("not found") && w.contains("nonexistent-profile.xml")), "Missing formatter profile should log 'not found' warning with resource name"); } @Test void format_jdtMode_missingProfile_fallsBackToLightweight() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); RoasterSourceFormatter formatter = - new RoasterSourceFormatter(logger, FormattingMode.JDT, "nonexistent-profile.xml"); + new RoasterSourceFormatter( + capturing.logger(), FormattingMode.JDT, "nonexistent-profile.xml"); String input = """ package test; @@ -692,11 +596,11 @@ void format_jdtMode_missingProfile_fallsBackToLightweight() { @Test void constructor_malformedProfile_logsLoadFailureWarning() { - TestProcessingEnv env = createProcessingEnv(); - ProcessingLogger logger = new ProcessingLogger(env); - new RoasterSourceFormatter(logger, FormattingMode.JDT, "eclipse-java-format-malformed.xml"); + CapturingProcessingLogger capturing = CapturingProcessingLogger.create(); + new RoasterSourceFormatter( + capturing.logger(), FormattingMode.JDT, "eclipse-java-format-malformed.xml"); assertTrue( - env.messager.warnings.stream() + capturing.messages().stream() .anyMatch( w -> w.contains("Failed to load") diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorAsserts.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorAsserts.java index 1dee5180..5c39bffb 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorAsserts.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorAsserts.java @@ -106,19 +106,6 @@ public static void assertContaining(String generatedCode, String... searches) { assertingResult(generatedCode, checks); } - /** - * Asserts that the compilation contains all the specified note messages. Useful for verifying - * debug or info logging output. - * - * @param compilation the compilation result - * @param noteMessages the note messages to check for - */ - public static void assertHadNoteContaining(Compilation compilation, String... noteMessages) { - for (String noteMessage : noteMessages) { - assertThat(compilation).hadNoteContaining(noteMessage); - } - } - /** * Asserts that the compilation's notes match the expected substrings in order and count. *