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 16aa198b..f39ccd73 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 @@ -111,11 +111,12 @@ public BuilderScopeResolver(ProcessingContext context) { * the classpath and returned if it satisfies the builder contract: an instantiation path * for an empty builder (no-arg constructor or static factory like {@code create()}), an * instantiation path seeded with the value (constructor accepting the referenced type or - * static factory like {@code create(T)}/{@code of(T)}), and a no-arg {@code 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, external - * tools, or different suffixes are supported. The referenced type must not be opted out - * with {@code @Ignore4BuilderGeneration}. + * static factory like {@code create(T)}/{@code of(T)}), and an accessible, parameterless + * method returning it (the build method, regardless of name) - 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 @@ -248,12 +249,12 @@ private Optional resolve(TypeElement referencedType) { * Looks up the candidate builder type on the classpath and verifies it satisfies the builder * contract: a way to create an empty instance (a no-arg constructor or a static parameterless * factory like {@code create()}), a way to create an instance seeded with a value (a constructor - * accepting the referenced type or a static factory like {@code create(T)}/{@code of(T)}), and a - * no-arg {@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}. + * accepting the referenced type or a static factory like {@code create(T)}/{@code of(T)}), and an + * accessible, parameterless method returning it (the build method, regardless of name) - 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 referenced type the builder must accept and return @@ -270,13 +271,18 @@ private Optional resolveByBuilderContract( resolveFuncForEmptyBuilder(builderTypeElement); Optional funcForPrefilledBuilder = resolveFuncForPrefilledBuilder(builderTypeElement, expectedType); + Optional buildMethod = findBuildMethod(builderTypeElement, expectedType); if (funcForEmptyBuilder.isEmpty() || funcForPrefilledBuilder.isEmpty() - || !JavaLangAnalyser.hasBuildMethodReturning(builderTypeElement, expectedType, context)) { + || buildMethod.isEmpty()) { return Optional.empty(); } return Optional.of( - new ResolvedBuilder(candidate, funcForEmptyBuilder.get(), funcForPrefilledBuilder.get())); + new ResolvedBuilder( + candidate, + funcForEmptyBuilder.get(), + funcForPrefilledBuilder.get(), + buildMethod.get().getSimpleName().toString())); } /** @@ -397,9 +403,8 @@ private Optional resolveAnchoredBuilder( for (ExecutableElement factory : factories) { TypeElement builderElement = (TypeElement) ((DeclaredType) factory.getReturnType()).asElement(); - if (!context.isMemberAccessibleFromBuilderPackage(builderElement) - || !JavaLangAnalyser.hasBuildMethodReturning( - builderElement, referencedTypeName, context)) { + Optional buildMethod = findBuildMethod(builderElement, referencedTypeName); + if (!context.isMemberAccessibleFromBuilderPackage(builderElement) || buildMethod.isEmpty()) { continue; } Optional funcForPrefilledBuilder = @@ -410,7 +415,8 @@ private Optional resolveAnchoredBuilder( JavaLangMapper.mapToTypeName(builderElement, context), new BuilderInstantiation.AnchorFactoryCall( referencedTypeName, factory.getSimpleName().toString()), - funcForPrefilledBuilder.get())); + funcForPrefilledBuilder.get(), + buildMethod.get().getSimpleName().toString())); } } return Optional.empty(); @@ -491,6 +497,31 @@ private static int preferredFactoryNameRank(String name) { return index < 0 ? PREFERRED_FACTORY_NAMES.size() : index; } + /** + * Finds the build method on the builder type: an accessible, non-static, parameterless method + * returning the referenced type, regardless of its name. When several candidates exist, {@code + * build} is preferred, then the alphabetically first name. Methods whose signature matches a + * {@link java.lang.Object} method (e.g. {@code toString()}) are excluded so types like {@code + * StringBuilder} do not satisfy the contract for {@code String}. + * + * @param builderType the candidate builder type to inspect + * @param expectedType the referenced type the build method must return + * @return the selected build method, or empty if no matching method exists + */ + private Optional findBuildMethod( + TypeElement builderType, TypeName expectedType) { + return JavaLangAnalyser.findMethods(builderType, List.of(), expectedType, context).stream() + .filter( + method -> + JavaLangAnalyser.isNotStatic(method) + && JavaLangAnalyser.isNoMethodOfObjectClass(method, context)) + .min( + Comparator.comparingInt( + (ExecutableElement method) -> + "build".contentEquals(method.getSimpleName()) ? 0 : 1) + .thenComparing(method -> method.getSimpleName().toString())); + } + private void refreshForConfigurationIfNeeded() { BuilderConfiguration configuration = context.getConfiguration(); ResolutionInputs inputs = new ResolutionInputs(configuration, context.getBuilderPackageName()); 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 2d6d9fb8..a7f8caa8 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 @@ -58,15 +58,23 @@ public final class JavaLangAnalyser { private JavaLangAnalyser() {} /** - * Helper function to filter methods from {@code java.lang.Object}. + * Helper function to filter methods from {@code java.lang.Object}. The comparison is + * signature-based, so overrides like {@code StringBuilder#toString} count as Object methods too. * * @param mth ExecutableElement to be validated - * @return {@code true}, if it is no method from java.lang.Object + * @param context the processing context, used to resolve {@link java.lang.Object}'s methods + * @return {@code true}, if it is no method with the signature of a java.lang.Object method */ - public static boolean isNoMethodOfObjectClass(ExecutableElement mth) { - String simpleNameOfParent = mth.getEnclosingElement().getSimpleName().toString(); - return !(Strings.CS.equals("java.lang.Object", simpleNameOfParent) - || Strings.CS.equals("Object", simpleNameOfParent)); + public static boolean isNoMethodOfObjectClass(ExecutableElement mth, ProcessingContext context) { + TypeElement objectElement = context.getTypeElement(Object.class.getCanonicalName()); + if (objectElement == null) { + return true; + } + return ElementFilter.methodsIn(objectElement.getEnclosedElements()).stream() + .noneMatch( + objectMethod -> + objectMethod.getSimpleName().equals(mth.getSimpleName()) + && objectMethod.getParameters().size() == mth.getParameters().size()); } /** @@ -80,7 +88,7 @@ public static boolean isNoMethodOfObjectClass(ExecutableElement mth) { public static List findAllPossibleSettersOfClass( TypeElement typeElement, ProcessingContext context) { return ElementFilter.methodsIn(context.getAllMembers(typeElement)).stream() - .filter(JavaLangAnalyser::isNoMethodOfObjectClass) + .filter(mth -> isNoMethodOfObjectClass(mth, context)) .filter(JavaLangAnalyser::isSetterForField) .sorted( (m1, m2) -> { @@ -243,24 +251,6 @@ public static Optional findAnnotationAttribute( 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 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, TypeName expectedReturnType, ProcessingContext context) { - return findMethods(builderType, List.of(), expectedReturnType, context).stream() - .anyMatch(method -> method.getSimpleName().contentEquals("build")); - } - /** * Single-parameter variant of {@link #findMethodsStatic(TypeElement, List, TypeName, * ProcessingContext)}. diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/generators/util/MethodGeneratorUtil.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/generators/util/MethodGeneratorUtil.java index 6fb7a55f..8c7b6bfd 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/generators/util/MethodGeneratorUtil.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/generators/util/MethodGeneratorUtil.java @@ -232,7 +232,8 @@ public static BuilderMethodDto createFieldConsumerWithBuilder( createBuilderMethod(field.getOriginalFieldName(), parentBuilderType, context); methodDto.addParameter(parameter); - String buildExpression = calculateBuildExpression(field.getFieldType()); + String buildExpression = + wrapConcreteCollectionType(field.getFieldType(), fieldBuilder.buildCall("builder")); methodDto.setCode( """ @@ -281,16 +282,6 @@ public static TypeNameGeneric createConsumerType(TypeName builderType) { return new TypeNameGeneric(JavaLangMapper.map2TypeName(Consumer.class), builderType); } - /** - * Calculates the build expression for a field type, wrapping concrete collections if needed. - * - * @param fieldType the field type - * @return the build expression - */ - private static String calculateBuildExpression(TypeName fieldType) { - return wrapConcreteCollectionType(fieldType, "builder.build()"); - } - /** * Wraps an expression with a concrete collection constructor if needed. * diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/type/ResolvedBuilder.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/type/ResolvedBuilder.java index b96fdeda..ebac36e5 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/model/type/ResolvedBuilder.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/model/type/ResolvedBuilder.java @@ -27,14 +27,17 @@ * {@code B.create()}) * @param funcForPrefilledBuilder how to obtain a builder instance seeded with a value (e.g. {@code * new B(value)} or {@code B.of(value)}) + * @param buildMethodName the name of the parameterless method returning the built value (e.g. + * {@code build}) */ public record ResolvedBuilder( TypeName typeName, BuilderInstantiation funcForEmptyBuilder, - BuilderInstantiation funcForPrefilledBuilder) { + BuilderInstantiation funcForPrefilledBuilder, + String buildMethodName) { /** - * A resolution instantiated exclusively through constructors. + * A resolution instantiated exclusively through constructors, building with {@code build()}. * * @param typeName the builder type to reference */ @@ -44,4 +47,30 @@ public ResolvedBuilder(TypeName typeName) { new BuilderInstantiation.ConstructorCall(), new BuilderInstantiation.ConstructorCall()); } + + /** + * A resolution whose terminal build method is named {@code build}, as on builders generated by + * this processor. + * + * @param typeName the builder type to reference + * @param funcForEmptyBuilder how to obtain an empty builder instance + * @param funcForPrefilledBuilder how to obtain a builder instance seeded with a value + */ + public ResolvedBuilder( + TypeName typeName, + BuilderInstantiation funcForEmptyBuilder, + BuilderInstantiation funcForPrefilledBuilder) { + this(typeName, funcForEmptyBuilder, funcForPrefilledBuilder, "build"); + } + + /** + * Renders the terminal call producing the built value: {@code + * builderExpression.buildMethodName()}. + * + * @param builderExpression expression evaluating to the builder instance (e.g. {@code builder}) + * @return the build call code + */ + public String buildCall(String builderExpression) { + return builderExpression + "." + buildMethodName + "()"; + } } 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 a6664bd9..2e2bbf70 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 @@ -620,6 +620,47 @@ public class LibHelperBuilder { LibHelper build() { return new LibHelper(); } } """, + probe(() -> ResolverProbeProcessor.usageWithoutAnnotation)), + // Neither void build() nor build(String) is a parameterless method returning the + // referenced type + Arguments.argumentSet( + "NoParameterlessMethodReturningTarget", + """ + package lib; + public class LibHelperBuilder { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + public void build() {} + public LibHelper build(String name) { return new LibHelper(); } + } + """, + probe(() -> ResolverProbeProcessor.usageWithoutAnnotation)), + // A static build() is a factory, not the terminal method; a private one is not + // accessible + Arguments.argumentSet( + "StaticOrPrivateBuildMethods", + """ + package lib; + public class LibHelperBuilder { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + public static LibHelper build() { return new LibHelper(); } + private LibHelper toTarget() { return new LibHelper(); } + } + """, + probe(() -> ResolverProbeProcessor.usageWithoutAnnotation)), + // The terminal method is package-private: not accessible from the generated builder's + // package (builderPackage unset), even though the constructors are public + Arguments.argumentSet( + "PackagePrivateBuildMethod_OtherPackage", + """ + package lib; + public class LibHelperBuilder { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + LibHelper toTarget() { return new LibHelper(); } + } + """, probe(() -> ResolverProbeProcessor.usageWithoutAnnotation))); } @@ -740,7 +781,6 @@ private static Compilation compileWithBuilder(String builderSource) { return compileWithSources(LIB_HELPER, builderSource); } - private static Supplier> probe( Supplier> field) { return field; @@ -756,6 +796,146 @@ private static void assertFactoryMethod( } } + @Test + void resolverUsageScope_ResolvesBuilderWhenReturningMethodNotNamedBuild() { + 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 toTarget() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // The build method is detected by signature, not by name: a public, parameterless method + // returning the referenced type satisfies the contract whatever it is called + assertEquals( + "lib.LibHelperBuilder", + ResolverProbeProcessor.usageWithoutAnnotation.get().typeName().getFullQualifiedName()); + assertEquals("toTarget", ResolverProbeProcessor.usageWithoutAnnotation.get().buildMethodName()); + } + + @Test + void resolverUsageScope_PrefersBuildNamedMethodAmongCandidates() { + 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 assemble() { return new LibHelper(); } + public LibHelper build() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // `build` wins over other signature-matching methods even if they sort earlier by name + assertEquals("build", ResolverProbeProcessor.usageWithoutAnnotation.get().buildMethodName()); + } + + @Test + void resolverUsageScope_PicksAlphabeticallyFirstMethodWhenNoBuildExists() { + 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 toTarget() { return new LibHelper(); } + public LibHelper assemble() { return new LibHelper(); } + } + """)); + + assertThat(compilation).succeeded(); + // Without a `build` method, the pick is deterministic: the alphabetically first name + assertEquals("assemble", ResolverProbeProcessor.usageWithoutAnnotation.get().buildMethodName()); + } + + @Test + void resolverUsageScope_ResolvesBuilderWithInheritedBuildMethod() { + 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 LibHelperBase { + public LibHelper toTarget() { return new LibHelper(); } + } + """), + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelperBuilder extends LibHelperBase { + public LibHelperBuilder() {} + public LibHelperBuilder(LibHelper value) {} + } + """)); + + assertThat(compilation).succeeded(); + // Inherited members count for the contract: the build method declared on the superclass is + // selected and its name recorded + assertEquals("toTarget", ResolverProbeProcessor.usageWithoutAnnotation.get().buildMethodName()); + } + + @Test + void resolverUsageScope_RejectsStringBuilderForString() { + ResolverProbeProcessor.reset(); + Compilation compilation = + Compiler.javac() + .withProcessors(new ResolverProbeProcessor()) + .compile( + ProcessorTestUtils.forSource( + """ + package lib; + public class LibHelper { public LibHelper() {} } + """)); + + assertThat(compilation).succeeded(); + // StringBuilder satisfies the instantiation contract for String, but its only parameterless + // method returning String is toString() - an Object signature, not a build method + assertEquals(Optional.empty(), ResolverProbeProcessor.contractResultString); + } + private static final class ResolverProbeProcessor extends AbstractProcessor { private static Optional first; private static Optional second; @@ -770,6 +950,7 @@ private static final class ResolverProbeProcessor extends AbstractProcessor { private static Optional usagePackagePrivate; private static Optional usageWithExistingDisabled; private static Optional usageGeneratedWithExistingDisabled; + private static Optional contractResultString; private boolean captured; @@ -787,6 +968,7 @@ static void reset() { usagePackagePrivate = null; usageWithExistingDisabled = null; usageGeneratedWithExistingDisabled = null; + contractResultString = null; } @Override @@ -857,6 +1039,13 @@ public boolean process(Set annotations, RoundEnvironment resolver.registerGeneratedBuilder( new TypeName("lib", "LibHelper"), new TypeName("lib", "LibHelperBuilder")); usageGeneratedWithExistingDisabled = resolver.resolveUsableBuilderType(helper); + // Builder-contract probe for java.lang.String: StringBuilder must not resolve because its + // only parameterless method returning String is toString(), an Object signature + context.initProcessingTarget(new ProcessingTarget(usageOnlyConfiguration("java.lang"), "")); + resolver.resetGeneratedBuilders(); + TypeElement stringElement = + processingEnv.getElementUtils().getTypeElement("java.lang.String"); + contractResultString = resolver.resolveUsableBuilderType(stringElement); captured = true; return false; }