From 0d9bcf0e2f615840e3a1b4b2c941b8d66d963696 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:22:18 +0000 Subject: [PATCH 1/4] Make referenced-builder contract checks visibility-aware (#313) The contract checks in resolveByBuilderContract verified member presence only: a private or cross-package package-private ctor(T), B() or build() passed the contract but produced uncompilable generated code. The three JavaLangAnalyser helpers now filter members through isMemberAccessibleFromBuilderPackage before matching, consistent with the member checks used elsewhere. The resolver's refresh now also tracks builderPackage, not just the configuration: resolution results are cached per referenced type, and the accessible-member set depends on the generated builder's package, which may differ between targets sharing a configuration. Co-Authored-By: Andreas Igel --- docs/CONFIGURATION.md | 6 +- .../analysis/BuilderScopeResolver.java | 24 ++++--- .../processor/analysis/JavaLangAnalyser.java | 11 +++- .../processor/BuilderScopeResolverTest.java | 66 +++++++++++++++++++ 4 files changed, 94 insertions(+), 13 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 858f157b..96cd2291 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -746,7 +746,8 @@ case. 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, a no-arg constructor, and a no-arg -`build()` method returning it. Any class with the expected name and a matching contract qualifies, allowing +`build()` method returning it — each accessible from the generated builder's +package. 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. @@ -1024,7 +1025,8 @@ with a different suffix (e.g. by another module using `"Factory"` as suffix) wit the suffix used for own builder generation. The candidate class must provide a constructor accepting the referenced type, a no-arg -constructor, and a no-arg `build()` method returning it. The contract check is annotation-agnostic, so builders +constructor, and a no-arg `build()` method returning it — each accessible from the generated +builder's package. 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. 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 f8efc32c..9dc9c6a8 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 @@ -51,6 +51,7 @@ public final class BuilderScopeResolver { private final ProcessingContext context; private BuilderConfiguration cachedConfiguration; + private String cachedBuilderPackage; private PackageScopes usagePackages = PackageScopes.unscoped(); private final Map> resolvedBuilderTypes = new HashMap<>(); private final GeneratedBuilders generatedBuilders = new GeneratedBuilders(); @@ -87,9 +88,10 @@ public BuilderScopeResolver(ProcessingContext context) { * (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, a no-arg constructor, 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}. + * it - each accessible from the generated builder's package. 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 @@ -200,10 +202,11 @@ private Optional resolve(TypeElement referencedType) { /** * Looks up the candidate builder type on the classpath and verifies it satisfies the builder * contract: a constructor accepting the referenced type, a no-arg constructor, 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}. + * {@code build()} method returning it - each accessible from the generated builder's package, + * since the generated code calls them from there. 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 @@ -229,7 +232,9 @@ private Optional resolveByBuilderContract(TypeName candidate, String e private void refreshForConfigurationIfNeeded() { BuilderConfiguration configuration = context.getConfiguration(); - if (Objects.equals(cachedConfiguration, configuration)) { + String builderPackage = context.getBuilderPackageName(); + if (Objects.equals(cachedConfiguration, configuration) + && Objects.equals(cachedBuilderPackage, builderPackage)) { return; } // The effective usage scope combines builderUsagePackages and builderGenerationPackages, @@ -241,8 +246,11 @@ private void refreshForConfigurationIfNeeded() { PackageScopes usage = configuration == null ? PackageScopes.unscoped() : configuration.builderUsagePackages(); usagePackages = PackageScopes.merge(generation, usage); + // The resolution cache must be dropped on builderPackage changes too: contract member + // accessibility is checked relative to the generated builder's package. resolvedBuilderTypes.clear(); cachedConfiguration = configuration; + cachedBuilderPackage = builderPackage; } private static boolean isIgnoredForBuilderGeneration(TypeElement typeElement) { 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 86ab7957..fced6e0f 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 @@ -183,16 +183,19 @@ public static boolean isSetterForField(ExecutableElement mth) { } /** - * Check if the class (TypeElement) has an empty constructor. + * Check if the class (TypeElement) has an empty constructor accessible from the package the + * builder is generated into. * * @param typeElement the type element to check * @param context processing context - * @return {@code true}, if the element has an empty constructor + * @return {@code true}, if the element has an accessible empty constructor */ public static boolean hasEmptyConstructor(TypeElement typeElement, ProcessingContext context) { List constructors = ElementFilter.constructorsIn(context.getAllMembers(typeElement)); - return constructors.stream().anyMatch(c -> c.getParameters().isEmpty()); + return constructors.stream() + .filter(context::isMemberAccessibleFromBuilderPackage) + .anyMatch(c -> c.getParameters().isEmpty()); } /** @@ -258,6 +261,7 @@ public static boolean hasBuildMethodReturning( return false; } return ElementFilter.methodsIn(context.getAllMembers(builderType)).stream() + .filter(context::isMemberAccessibleFromBuilderPackage) .anyMatch( method -> method.getSimpleName().contentEquals("build") @@ -281,6 +285,7 @@ public static boolean hasConstructorAccepting( return false; } return ElementFilter.constructorsIn(context.getAllMembers(builderType)).stream() + .filter(context::isMemberAccessibleFromBuilderPackage) .anyMatch( constructor -> constructor.getParameters().size() == 1 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 9d24179c..ddd28c1c 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 @@ -183,6 +183,66 @@ public LibHelperBuilder(LibHelper value) {} assertEquals(Optional.empty(), ResolverProbeProcessor.usageWithoutAnnotation); } + @Test + void resolverUsageScope_RejectsBuilderWithPrivateConstructor() { + 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 { + private LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + public LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // The no-arg ctor exists but is private: generated code could not call it, so the builder + // must not qualify + assertEquals(Optional.empty(), ResolverProbeProcessor.usageWithoutAnnotation); + } + + @Test + void resolverUsageScope_PackagePrivateMembers_AccessibleOnlyFromSamePackage() { + 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 { + LibHelperBuilder() {} + LibHelperBuilder(LibHelper value) {} + LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // All contract members are package-private: accessible when the generated builder is in the + // same package (builderPackage "lib")... + assertEquals( + "lib.LibHelperBuilder", + ResolverProbeProcessor.usagePackagePrivate.get().getFullQualifiedName()); + // ...but not from a different one (builderPackage unset) + assertEquals(Optional.empty(), ResolverProbeProcessor.usageWithoutAnnotation); + } + @Test void resolverUsageScope_UsesBuilderUsageSuffixWhenConfigured() { ResolverProbeProcessor.reset(); @@ -254,6 +314,7 @@ private static final class ResolverProbeProcessor extends AbstractProcessor { private static Optional usageWithoutAnnotation; private static Optional usageWithSuffix; private static Optional usageDefaultSuffix; + private static Optional usagePackagePrivate; private boolean captured; @@ -268,6 +329,7 @@ static void reset() { usageWithoutAnnotation = null; usageWithSuffix = null; usageDefaultSuffix = null; + usagePackagePrivate = null; } @Override @@ -326,6 +388,10 @@ public boolean process(Set annotations, RoundEnvironment // Usage scope with default suffix (no builderUsageSuffix configured) context.initProcessingTarget(new ProcessingTarget(usageOnlyConfiguration("lib"), "")); usageDefaultSuffix = resolver.resolveUsableBuilderType(helper); + // Generated builder in the same package as the referenced builder: package-private + // contract members are accessible + context.initProcessingTarget(new ProcessingTarget(usageOnlyConfiguration("lib"), "lib")); + usagePackagePrivate = resolver.resolveUsableBuilderType(helper); captured = true; return false; } From 5f8a5498af142ec4a38e361d82f6d6e8bdb4d8e6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:53:59 +0000 Subject: [PATCH 2/4] Group resolution cache inputs into a ResolutionInputs record Replace the two loosely related cache markers (cachedConfiguration, cachedBuilderPackage) with a record compared as a unit, so the invalidation condition reads as 'the inputs are unchanged'. Co-Authored-By: Andreas Igel --- .../analysis/BuilderScopeResolver.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) 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 9dc9c6a8..c1dd21db 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 @@ -25,7 +25,6 @@ import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Optional; import javax.lang.model.element.Element; import javax.lang.model.element.TypeElement; @@ -49,9 +48,15 @@ */ public final class BuilderScopeResolver { + /** + * The inputs the resolution cache was built under. Cached resolutions are valid only while both + * components are unchanged: the configuration determines the usage scope and the builder package + * determines which contract members are accessible. + */ + private record ResolutionInputs(BuilderConfiguration configuration, String builderPackage) {} + private final ProcessingContext context; - private BuilderConfiguration cachedConfiguration; - private String cachedBuilderPackage; + private ResolutionInputs resolutionInputs; private PackageScopes usagePackages = PackageScopes.unscoped(); private final Map> resolvedBuilderTypes = new HashMap<>(); private final GeneratedBuilders generatedBuilders = new GeneratedBuilders(); @@ -231,12 +236,12 @@ private Optional resolveByBuilderContract(TypeName candidate, String e } private void refreshForConfigurationIfNeeded() { - BuilderConfiguration configuration = context.getConfiguration(); - String builderPackage = context.getBuilderPackageName(); - if (Objects.equals(cachedConfiguration, configuration) - && Objects.equals(cachedBuilderPackage, builderPackage)) { + ResolutionInputs inputs = + new ResolutionInputs(context.getConfiguration(), context.getBuilderPackageName()); + if (inputs.equals(resolutionInputs)) { return; } + BuilderConfiguration configuration = inputs.configuration(); // The effective usage scope combines builderUsagePackages and builderGenerationPackages, // since generation-scope packages are automatically included in the usage scope. PackageScopes generation = @@ -246,11 +251,8 @@ private void refreshForConfigurationIfNeeded() { PackageScopes usage = configuration == null ? PackageScopes.unscoped() : configuration.builderUsagePackages(); usagePackages = PackageScopes.merge(generation, usage); - // The resolution cache must be dropped on builderPackage changes too: contract member - // accessibility is checked relative to the generated builder's package. resolvedBuilderTypes.clear(); - cachedConfiguration = configuration; - cachedBuilderPackage = builderPackage; + resolutionInputs = inputs; } private static boolean isIgnoredForBuilderGeneration(TypeElement typeElement) { From dda62a24c74484d86217c700106b14aae637d5b1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:56:52 +0000 Subject: [PATCH 3/4] Read the configuration into a local before building ResolutionInputs Co-Authored-By: Andreas Igel --- .../builders/processor/analysis/BuilderScopeResolver.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 c1dd21db..ccfdfe1d 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 @@ -236,12 +236,11 @@ private Optional resolveByBuilderContract(TypeName candidate, String e } private void refreshForConfigurationIfNeeded() { - ResolutionInputs inputs = - new ResolutionInputs(context.getConfiguration(), context.getBuilderPackageName()); + BuilderConfiguration configuration = context.getConfiguration(); + ResolutionInputs inputs = new ResolutionInputs(configuration, context.getBuilderPackageName()); if (inputs.equals(resolutionInputs)) { return; } - BuilderConfiguration configuration = inputs.configuration(); // The effective usage scope combines builderUsagePackages and builderGenerationPackages, // since generation-scope packages are automatically included in the usage scope. PackageScopes generation = From b71333855d9fe78e8db0a1688df6b2e73b7fbec8 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:57:46 +0000 Subject: [PATCH 4/4] Rename resolutionInputs to cachedResolutionInputs The field holds the inputs the resolution cache was built under, so the cached marker belongs in the name. Co-Authored-By: Andreas Igel --- .../builders/processor/analysis/BuilderScopeResolver.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 ccfdfe1d..e46bda11 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 @@ -56,7 +56,7 @@ public final class BuilderScopeResolver { private record ResolutionInputs(BuilderConfiguration configuration, String builderPackage) {} private final ProcessingContext context; - private ResolutionInputs resolutionInputs; + private ResolutionInputs cachedResolutionInputs; private PackageScopes usagePackages = PackageScopes.unscoped(); private final Map> resolvedBuilderTypes = new HashMap<>(); private final GeneratedBuilders generatedBuilders = new GeneratedBuilders(); @@ -238,7 +238,7 @@ private Optional resolveByBuilderContract(TypeName candidate, String e private void refreshForConfigurationIfNeeded() { BuilderConfiguration configuration = context.getConfiguration(); ResolutionInputs inputs = new ResolutionInputs(configuration, context.getBuilderPackageName()); - if (inputs.equals(resolutionInputs)) { + if (inputs.equals(cachedResolutionInputs)) { return; } // The effective usage scope combines builderUsagePackages and builderGenerationPackages, @@ -251,7 +251,7 @@ private void refreshForConfigurationIfNeeded() { configuration == null ? PackageScopes.unscoped() : configuration.builderUsagePackages(); usagePackages = PackageScopes.merge(generation, usage); resolvedBuilderTypes.clear(); - resolutionInputs = inputs; + cachedResolutionInputs = inputs; } private static boolean isIgnoredForBuilderGeneration(TypeElement typeElement) {