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..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 @@ -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,8 +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 ResolutionInputs cachedResolutionInputs; private PackageScopes usagePackages = PackageScopes.unscoped(); private final Map> resolvedBuilderTypes = new HashMap<>(); private final GeneratedBuilders generatedBuilders = new GeneratedBuilders(); @@ -87,9 +93,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 +207,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 +237,8 @@ private Optional resolveByBuilderContract(TypeName candidate, String e private void refreshForConfigurationIfNeeded() { BuilderConfiguration configuration = context.getConfiguration(); - if (Objects.equals(cachedConfiguration, configuration)) { + ResolutionInputs inputs = new ResolutionInputs(configuration, context.getBuilderPackageName()); + if (inputs.equals(cachedResolutionInputs)) { return; } // The effective usage scope combines builderUsagePackages and builderGenerationPackages, @@ -242,7 +251,7 @@ private void refreshForConfigurationIfNeeded() { configuration == null ? PackageScopes.unscoped() : configuration.builderUsagePackages(); usagePackages = PackageScopes.merge(generation, usage); resolvedBuilderTypes.clear(); - cachedConfiguration = configuration; + cachedResolutionInputs = inputs; } 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; }