From 37e85dfe68ece89d7ecfc84b3372e542e60facc2 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Fri, 9 May 2025 21:31:38 +0200 Subject: [PATCH 01/35] adding explicit setter property in builder definition --- .../simple/builders/processor/dtos/FieldDto.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index 8189de4d..e0c074b7 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -37,6 +37,9 @@ public class FieldDto { /** Type of field. Containing generic, name of package and class */ private TypeName fieldType; + + /** Name of setter in target object. Entry may be {@code null} if field is only been set via constructor */ + private final String setterName; /** List of all methods in builder, which change the field. */ private final List fieldSetterMethodsList = new ArrayList<>(); @@ -58,6 +61,14 @@ public String getFieldName() { public void setFieldName(String fieldName) { this.fieldName = fieldName; } + + public Optional getSetterName() { + return Optional.ofNullable(setterName); + } + + public void setSetterName(String setterName) { + this.setterName = setterName; + } /** * Getting type of field. From 209e76f6c06fd287337c12428c082ed81dfb8777 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Fri, 9 May 2025 21:32:41 +0200 Subject: [PATCH 02/35] Improving Javadoc --- .../simple/builders/processor/dtos/BuilderDefinitionDto.java | 2 +- .../javahelpers/simple/builders/processor/dtos/FieldDto.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index 9a5746bc..ef651671 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -42,7 +42,7 @@ public class BuilderDefinitionDto { /** * List of all fields of target DTO which are supported by builder. A field could be supported by - * several functions in builder. + * multiple functions in builder. */ private final LinkedList setterFieldsForBuilder = new LinkedList<>(); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index e0c074b7..b314a942 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -41,7 +41,7 @@ public class FieldDto { /** Name of setter in target object. Entry may be {@code null} if field is only been set via constructor */ private final String setterName; - /** List of all methods in builder, which change the field. */ + /** List of all methods in builder, which provide helpers to change the field. */ private final List fieldSetterMethodsList = new ArrayList<>(); /** From 6f4cccce4b5b5521cd08f2d224c8024267898c90 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Fri, 9 May 2025 21:34:00 +0200 Subject: [PATCH 03/35] Renaming fields in BuilderDefinition for better understanding --- .../processor/dtos/BuilderDefinitionDto.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index ef651671..62e679cb 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -38,13 +38,13 @@ public class BuilderDefinitionDto { * List of all methods of target DTO which are supported by builder and not targeting a specific * DTO field. */ - private final LinkedList methodsForBuilder = new LinkedList<>(); + private final LinkedList methods = new LinkedList<>(); /** * List of all fields of target DTO which are supported by builder. A field could be supported by * multiple functions in builder. */ - private final LinkedList setterFieldsForBuilder = new LinkedList<>(); + private final LinkedList fields = new LinkedList<>(); /** * Getting type of builder. @@ -90,7 +90,7 @@ public void setBuildingTargetTypeName(TypeName buildingTargetTypeName) { * @param member {@code org.javahelpers.simple.builders.internal.dtos.MethodDto} to be added */ public void addMethod(MethodDto member) { - methodsForBuilder.add(member); + methods.add(member); } /** @@ -100,7 +100,7 @@ public void addMethod(MethodDto member) { * org.javahelpers.simple.builders.internal.dtos.MethodDto} */ public LinkedList getMethodsForBuilder() { - return methodsForBuilder; + return methods; } /** @@ -109,7 +109,7 @@ public LinkedList getMethodsForBuilder() { * @param field {@code org.javahelpers.simple.builders.internal.dtos.FieldDto} to be added */ public void addField(FieldDto field) { - setterFieldsForBuilder.add(field); + fields.add(field); } /** @@ -118,6 +118,6 @@ public void addField(FieldDto field) { * @return list of fields with type {@code org.javahelpers.simple.builders.internal.dtos.FieldDto} */ public LinkedList getSetterFieldsForBuilder() { - return setterFieldsForBuilder; + return fields; } } From 65a90d8ffd7591a1a153a386b80357ac7478ad74 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Fri, 9 May 2025 21:40:15 +0200 Subject: [PATCH 04/35] Adding fields of constructor to BuilderDefintion too --- .../simple/builders/processor/dtos/BuilderDefinitionDto.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index 62e679cb..7bf304f6 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -33,6 +33,9 @@ public class BuilderDefinitionDto { /** Type of builder. Containing package and name of DTO. */ private TypeName builderTypeName; + + /** Fields in constructor. Containing all fields which need to be given for calling the constructor. Ordering in LinkedList is matching orderung of parameters in constructor call. Instances of fields are added in LinkedList of all fields too. */ + private final LinkedList fieldsInConstructor = new LinkedList<>(); /** * List of all methods of target DTO which are supported by builder and not targeting a specific From a8bdfa38b2147c46afc687bba43f9a64f22ff1e8 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Thu, 28 Aug 2025 16:35:33 +0200 Subject: [PATCH 05/35] Updating tests according to desired functionality --- .../processor/BuilderProcessorTest.java | 228 ++++++++---------- 1 file changed, 96 insertions(+), 132 deletions(-) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 6a96fba7..78c779e7 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -74,8 +74,7 @@ void shouldFailWhenAnnotationPlacedOnAbstractClass() { } @Test - @Disabled("Constructor-Parameters are not supported yet") - void shouldIgnoreConstructorParametersAndOnlyUseSetters() { + void shouldUseConstructorParameters() { // Given String packageName = "test"; String className = "CtorAndSetter"; @@ -86,7 +85,7 @@ void shouldIgnoreConstructorParametersAndOnlyUseSetters() { packageName, className, """ - private int a; // has no setter, only in constructor + private final int a; // has no setter, only in constructor private String name; // will be set via setter public CtorAndSetter() {} @@ -107,14 +106,15 @@ public CtorAndSetter() {} ProcessorAsserts.assertingResult( generatedCode, contains("public CtorAndSetterBuilder a(int a)"), - contains("instance.setA(a);"), + contains("this.a = a;"), contains("public CtorAndSetterBuilder name(String name)"), - contains("instance.setName(name);")); + contains("this.name = name;"), + contains("CtorAndSetter instance = new CtorAndSetter(this.a);"), + contains("instance.setName(this.name);")); } @Test - @Disabled("Records are not supported for builder generation yet") - void shouldHandleRecordWithoutGeneratingBuilder() { + void shouldHandleRecordWithBuilder() { // Given String packageName = "test"; String recordName = "PersonRecord"; @@ -145,9 +145,10 @@ public record PersonRecord(String name, int age) { ProcessorAsserts.assertingResult( generatedCode, contains("public PersonRecordBuilder name(String name)"), - contains("instance.setName(name);"), + contains("this.name = name;"), contains("public PersonRecordBuilder age(int age)"), - contains("instance.setAge(age);")); + contains("this.age = age;"), + contains("PersonRecord instance = new PersonRecord(this.name,this.age);")); } @Test @@ -189,18 +190,18 @@ void shouldGenerateCollectionSettersAndProviders() { contains("public WithCollectionsBuilder names(List names)"), contains("public WithCollectionsBuilder names(Supplier> namesSupplier)"), contains("public WithCollectionsBuilder names(String... names)"), - contains("instance.setNames(List.of(names));"), + contains("this.names = List.of(names);"), contains("public WithCollectionsBuilder tags(Set tags)"), contains("public WithCollectionsBuilder tags(Supplier> tagsSupplier)"), contains("public WithCollectionsBuilder tags(String... tags)"), - contains("instance.setTags(Set.of(tags));"), + contains("this.tags = Set.of(tags);"), contains("public WithCollectionsBuilder map(Map map)"), contains("public WithCollectionsBuilder map(Supplier> mapSupplier)"), - contains("instance.setMap(map);")); + contains("this.map = map;")); } @Test - void shouldDeclareInstanceFieldAsPrivateFinalWithJavadoc() { + void shouldDeclareFieldAsBuilderFieldsWithJavadoc() { // Given String packageName = "test"; String className = "FieldDoc"; @@ -211,8 +212,11 @@ void shouldDeclareInstanceFieldAsPrivateFinalWithJavadoc() { packageName, className, """ - private int x; public int getX(){return x;} public void setX(int x){this.x=x;} - """); + /** Field X is an example for having javadoc **/ + private int x; + public int getX(){return x;} + public void setX(int x){this.x=x;} + """); // When Compilation compilation = compile(sourceFile); @@ -223,8 +227,8 @@ void shouldDeclareInstanceFieldAsPrivateFinalWithJavadoc() { ProcessorAsserts.assertingResult( generatedCode, - contains("Inner instance of builder."), - contains("private final FieldDoc instance;")); + contains("Field X is an example for having javadoc"), + contains("private int x;")); } @Test @@ -239,8 +243,10 @@ void shouldGenerateCreateMethodWithModifiersAndJavadoc() { packageName, className, """ - private int x; public int getX(){return x;} public void setX(int x){this.x=x;} - """); + private int x; + public int getX(){return x;} + public void setX(int x){this.x=x;} + """); // When Compilation compilation = compile(sourceFile); @@ -252,8 +258,8 @@ void shouldGenerateCreateMethodWithModifiersAndJavadoc() { ProcessorAsserts.assertingResult( generatedCode, contains("public static CreateDocBuilder create()"), - contains("Creating a new builder for {"), - contains("@return builder for {")); + contains("Creating a new builder for {@code CreateDoc}"), + contains("@return builder for {@code CreateDoc}")); } @Test @@ -268,8 +274,10 @@ void shouldGenerateBuildOverrideAndReturnInstance() { packageName, className, """ - private int x; public int getX(){return x;} public void setX(int x){this.x=x;} - """); + private int x; + public int getX(){return x;} + public void setX(int x){this.x=x;} + """); // When Compilation compilation = compile(sourceFile); @@ -308,7 +316,10 @@ void shouldReturnThisFromFluentMethods() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertingResult( - generatedCode, contains("instance.setName(name);"), contains("return this;")); + generatedCode, + contains("public FluentReturnBuilder name(String name)"), + contains("this.name = name;"), + contains("return this;")); } @Test @@ -369,7 +380,7 @@ void shouldContainReturnJavadocForConsumerSupplierAndConsumerByBuilder() { } @Test - void shouldGenerateProxyWithMixedParamsAndArrayVarargs() { + void nonSetterMethodsShouldNotBeInBuilder() { // Given String packageName = "test"; String className = "MixedProxy"; @@ -380,6 +391,8 @@ void shouldGenerateProxyWithMixedParamsAndArrayVarargs() { packageName, className, """ + private String name; + public String getName(){return name;} public void setName(String name){this.name=name;} public void doMixed(int a, String b, int[] nums) { /* no-op */ } """); @@ -393,8 +406,9 @@ public void doMixed(int a, String b, int[] nums) { /* no-op */ } // Expect last parameter mapped to varargs in builder signature and proper call ProcessorAsserts.assertingResult( generatedCode, - contains("doMixed(int a, String b, int... nums)"), - contains("instance.doMixed(a,b,nums);")); + contains("public MixedProxyBuilder name(String name)"), + containsNot("doMixed(int a, String b, int... nums)"), + containsNot("instance.doMixed(a,b,nums);")); } @Test @@ -483,8 +497,7 @@ void shouldGenerateBothConstructorsWithJavadocs() { contains("Empty constructor of builder for"), contains("Initialisation of builder for"), contains("@param instance object instance for initialisiation"), - contains("this.instance = instance;"), - contains("this.instance = new CtorDoc();")); + contains("this.x = instance.getX();")); } @Test @@ -524,13 +537,10 @@ public void doSomething(int a, String b) { /* no-op */ } String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // PROXY method javadoc has @param for each parameter and @return + // PROXY method is not in builder ProcessorAsserts.assertingResult( generatedCode, - contains("Calling doSomething on dto-instance with parameters."), - contains("@param a value for a."), - contains("@param b value for b."), - contains("@return current instance of builder")); + containsNot("doSomething")); // SUPPLIER method javadoc and code ProcessorAsserts.assertingResult( @@ -586,7 +596,7 @@ void shouldGenerateConsumerByBuilderForAnnotatedField() { contains("helperBuilderConsumer"), contains("HelperAnnoBuilder builder = new HelperAnnoBuilder();"), contains("helperBuilderConsumer.accept(builder);"), - contains("instance.setHelper(builder.build());")); + contains("this.helper = builder.build();")); } @Test @@ -627,9 +637,9 @@ void shouldGenerateConsumerForCustomTypeWithEmptyConstructor() { // Expect consumer method with local var named 'consumer' of HelperPlain ProcessorAsserts.assertingResult( generatedCode, - contains("HelperPlain consumer = new HelperPlain();"), - contains("helperPlainConsumer.accept(consumer);"), - contains("instance.setHelperPlain(consumer);")); + contains("HelperPlain consumerInstance = new HelperPlain();"), + contains("helperPlainConsumer.accept(consumerInstance);"), + contains("this.helperPlain = consumerInstance;")); } @Test @@ -659,7 +669,7 @@ void shouldGenerateSupplierMethodForField() { // Expect supplier-based setter usage ProcessorAsserts.assertingResult( - generatedCode, contains("nameSupplier"), contains("instance.setName(nameSupplier.get());")); + generatedCode, contains("nameSupplier"), contains("this.name = nameSupplier.get();")); } protected Compilation compile(JavaFileObject... sourceFiles) { @@ -733,11 +743,18 @@ public void setAge(int age) { // Verify the generated code contains builder methods (build/create are checked centrally) ProcessorAsserts.assertingResult( generatedCode, + contains("private String name;"), + contains("private int age;"), contains("public PersonBuilder name(String name)"), - contains("public PersonBuilder age(int age)")); + contains("public PersonBuilder age(int age)"), + contains("public Person build()"), + contains("Person instance = new Person();"), + contains("instance.setName(this.name);"), + contains("instance.setAge(this.age);")); } @Test + //Todo: what should that test validate? void shouldGenerateSetterForSetOfStrings_whenImplemented() { // Given String packageName = "test"; @@ -766,6 +783,7 @@ void shouldGenerateSetterForSetOfStrings_whenImplemented() { } @Test + //Todo: what should that test validate? void shouldHandleSetOfStrings() { // Given String packageName = "test"; @@ -825,7 +843,8 @@ void shouldGenerateSetterForHelperInDifferentPackage_whenImplemented() { generatedCode, contains("public UsesOtherPackageHelperBuilder helper(Helper helper)")); } - @Test + @Test + //Todo: what should that test validate? void shouldHandleSetOfCustomType() { // Given String packageName = "test"; @@ -944,14 +963,15 @@ void shouldFilterOutNonRelevantMethods() { packageName, className, """ - // valid proxy candidate - public void ok() {} + // valid setter + public void setOk(int ok) {} // should be filtered - private void hidden() {} - public static void util() {} - public void risky() throws Exception {} + private void setHidden(int hidden) {} + public static void setUtil(int util) {} + public void setRisky(int risk) throws Exception {} public int returnsInt() { return 42; } + public int getJustGetter() { return 25; } """); // When @@ -963,15 +983,15 @@ public void risky() throws Exception {} // Expect only ok() proxy to be present ProcessorAsserts.assertingResult( generatedCode, - contains("public HasVariousMethodsBuilder ok()"), - contains("instance.ok();"), - contains("return this;")); + contains("public HasVariousMethodsBuilder ok(int ok)"), + contains("this.ok = ok;")); ProcessorAsserts.assertNotContaining( generatedCode, - ProcessorAsserts.notContains("hidden()"), - ProcessorAsserts.notContains("util()"), - ProcessorAsserts.notContains("risky()"), - ProcessorAsserts.notContains("returnsInt(")); + ProcessorAsserts.notContains("hidden"), + ProcessorAsserts.notContains("util"), + ProcessorAsserts.notContains("risky"), + ProcessorAsserts.notContains("returnsInt"), + ProcessorAsserts.notContains("justGetter")); } @Test @@ -1005,16 +1025,12 @@ void shouldNotGenerateConsumersForPrimitiveAndArrayFields() { // Expect simple setters and suppliers only; no consumer methods for primitive/array ProcessorAsserts.assertingResult( generatedCode, - contains("public PrimAndArrayBuilder count("), - contains("countSupplier)"), - contains("public PrimAndArrayBuilder names("), - contains("namesSupplier)")); + contains("public PrimAndArrayBuilder count(Supplier countSupplier"), + contains("public PrimAndArrayBuilder names(Supplier> namesSupplier")); ProcessorAsserts.assertNotContaining( generatedCode, - ProcessorAsserts.notContains("countConsumer"), - ProcessorAsserts.notContains("namesConsumer"), - ProcessorAsserts.notContains("countBuilderConsumer"), - ProcessorAsserts.notContains("namesBuilderConsumer")); + ProcessorAsserts.notContains("public PrimAndArrayBuilder count(Consumer"), + ProcessorAsserts.notContains("public PrimAndArrayBuilder names(Consumer")); } @Test @@ -1123,7 +1139,7 @@ void shouldGenerateAnnotationsAndCreateBuildMethods() { contains("@Generated("), contains("@BuilderImplementation("), contains("public static AnnoTargetBuilder create()"), - contains("return new AnnoTargetBuilder(instance);"), + contains("return new AnnoTargetBuilder();"), contains("public AnnoTarget build()"), contains("return instance;")); } @@ -1159,41 +1175,9 @@ void shouldGenerateVarargsConvenienceForCollections() { ProcessorAsserts.assertingResult( generatedCode, contains("public HasCollectionsConvenienceBuilder names(String... names)"), - contains("instance.setNames(List.of(names));"), + contains("this.names = List.of(names);"), contains("public HasCollectionsConvenienceBuilder tags(String... tags)"), - contains("instance.setTags(Set.of(tags));")); - } - - @Test - void shouldGenerateProxyMethodIncludingVarargsParameters() { - // Given - String packageName = "test"; - String className = "HasActionMethods"; - String builderClassName = className + "Builder"; - - JavaFileObject sourceFile = - ProcessorTestUtils.simpleBuilderClass( - packageName, - className, - """ - public void reset(int level, String[] notes) {} - public void clearAll() {} - """); - - // When - Compilation compilation = compile(sourceFile); - - // Then - String generatedCode = loadGeneratedSource(compilation, builderClassName); - assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // Method proxies return builder and forward to instance, last array parameter becomes varargs - ProcessorAsserts.assertingResult( - generatedCode, - contains("public HasActionMethodsBuilder reset(int level, String... notes)"), - contains("instance.reset(level,notes);"), - contains("public HasActionMethodsBuilder clearAll()"), - contains("instance.clearAll();"), - contains("return this;")); + contains("this.tags = Set.of(tags);")); } @Test @@ -1217,38 +1201,14 @@ void shouldHandleJavaTimeField() { // When Compilation compilation = compile(sourceFile); - // Then - String generatedCode = loadGeneratedSource(compilation, builderClassName); - // Currently no positive assertion; future expectation covered by @Disabled test below - assertGenerationSucceeded(compilation, builderClassName, generatedCode); - } - - @Test - void shouldGenerateSetterForJavaTime_whenImplemented() { - // Given - String packageName = "test"; - String className = "HasLocalDate"; - String builderClassName = className + "Builder"; - - JavaFileObject sourceFile = - ProcessorTestUtils.simpleBuilderClass( - packageName, - className, - """ - private java.time.LocalDate date; - - public java.time.LocalDate getDate() { return date; } - public void setDate(java.time.LocalDate date) { this.date = date; } - """); - - // When - Compilation compilation = compile(sourceFile); - // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertingResult( - generatedCode, contains("public HasLocalDateBuilder date(LocalDate date)")); + generatedCode, + contains("private LocalDate date;"), + contains("public HasLocalDateBuilder date(LocalDate date)"), + contsins("this.date = date;")); } @Test @@ -1291,7 +1251,7 @@ void shouldHandleMixedFields() { // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO add asserts here for fields and build method } @Test @@ -1318,7 +1278,7 @@ void shouldHandleObjectArrayField() { // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO: check test is needed? } @Test @@ -1347,6 +1307,7 @@ void shouldHandleClassWithOnlyGetters() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); // Even without setters, builder should exist (build/create checked centrally). No extra // asserts. + // TODO Without Setters or Constructors there is no need for a builder. we should have a warning in build } @Test @@ -1370,7 +1331,7 @@ void shouldHandleEmptyClass() { // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // No setters expected; build/create checked centrally. + // TODO Without Setters or Constructors there is no need for a builder. we should have a warning in build } @Test @@ -1424,10 +1385,11 @@ void shouldHandleListField() { // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO: add assterts } @Test + //TODO: class code does not match testname void shouldBoxPrimitiveTypeArgumentsInGenerics() { // Given String packageName = "test"; @@ -1494,7 +1456,7 @@ void shouldHandleNestedTypeWithEmptyConstructor() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO: add asserts } @Test @@ -1535,9 +1497,11 @@ void shouldHandleNestedTypeWithoutEmptyConstructor() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + //TODO: sdd asserts, because of missing empty constructor there could be no consumer } +//TODO adding list of custom types without empty constructor + @Test void shouldHandleListOfCustomType() { // Given @@ -1574,7 +1538,7 @@ void shouldHandleListOfCustomType() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO: Adding asserts } @Test @@ -1613,6 +1577,6 @@ void shouldHandleMapWithCustomValueType() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // build/create are checked centrally; no additional builder setter checks here + // TODO: Adding asserts } } From 4dd44017b07465f947da65bbf4fdff94442d2bb5 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 7 Sep 2025 21:51:09 +0200 Subject: [PATCH 06/35] Update FieldDto.java --- .../simple/builders/processor/dtos/FieldDto.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index 4b5a744a..28838c4a 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; /** * Definition of a field. Containing all information to generate methods in builder to set or modify @@ -37,9 +38,12 @@ public class FieldDto { /** Type of field. Containing generic, name of package and class */ private TypeName fieldType; - - /** Name of setter in target object. Entry may be {@code null} if field is only been set via constructor */ - private final String setterName; + + /** + * Name of setter in target object. Entry may be {@code null} if field is only been set via + * constructor + */ + private String setterName; /** List of all methods in builder, which provide helpers to change the field. */ private final List fieldSetterMethodsList = new ArrayList<>(); @@ -67,14 +71,14 @@ public String getFieldName() { public void setFieldName(String fieldName) { this.fieldName = fieldName; } - + public Optional getSetterName() { return Optional.ofNullable(setterName); } - + public void setSetterName(String setterName) { this.setterName = setterName; - } + } /** * Getting type of field. From 389565f451a73da69818424219724baee0027dfd Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Thu, 11 Sep 2025 23:02:08 +0200 Subject: [PATCH 07/35] Added annotation for selecting correct constructor --- .../annotations/SimpleBuilderConstructor.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java new file mode 100644 index 00000000..1f2aa304 --- /dev/null +++ b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java @@ -0,0 +1,35 @@ +/* + * MIT License + * + * Copyright (c) 2025 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.core.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Annotation to be placed on a constructor, if a specific constructor should be used for building the resulting DTO. */ +@Target({ElementType.METHOD}) +@Retention(RetentionPolicy.CLASS) +public @interface SimpleBuilderConstructor {} \ No newline at end of file From 95f12071f2e5a0e22c1a7fb17bef3f33923ee208 Mon Sep 17 00:00:00 2001 From: Andreas Igel <193066328+AndreasIgel@users.noreply.github.com> Date: Thu, 11 Sep 2025 23:03:01 +0200 Subject: [PATCH 08/35] Adding todos for open tasks --- .../builders/processor/util/BuilderDefinitionCreator.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 8cf50293..f4d0898d 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -79,6 +79,9 @@ public static BuilderDefinitionDto extractFromElement( result.setBuilderTypeName(new TypeName(packageName, simpleClassName + BUILDER_SUFFIX)); result.setBuildingTargetTypeName(new TypeName(packageName, simpleClassName)); + // Todo: extract Constructor and his fields. result needs to be filled im constructors and fields. + + // Todo: moving into helper function List allMembers = elementUtils.getAllMembers(annotatedType); List methods = ElementFilter.methodsIn(allMembers); @@ -86,6 +89,7 @@ public static BuilderDefinitionDto extractFromElement( // nur public if (isMethodRelevantForBuilder(mth)) { if (isSetterForField(mth)) { + // Todo: we need to check setter-fields for duplication with constructor fields createFieldDto(mth, elementUtils, typeUtils).ifPresent(result::addField); } else { createMethodDto(mth, elementUtils, typeUtils).ifPresent(result::addMethod); @@ -104,6 +108,7 @@ && isNotPrivate(mth) && isNotStatic(mth); } + // Todo: remove, should not be supported anymore private static Optional createMethodDto( ExecutableElement mth, Elements elementUtils, Types typeUtils) { String methodName = mth.getSimpleName().toString(); From 07e0f64a8a85f438d1ef489a8e81cb3ffd3df0ae Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 13 Sep 2025 17:26:47 +0200 Subject: [PATCH 09/35] Fixing codeformat --- .../core/annotations/SimpleBuilderConstructor.java | 7 +++++-- .../builders/processor/util/BuilderDefinitionCreator.java | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java index 1f2aa304..cdaa92dc 100644 --- a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java +++ b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java @@ -29,7 +29,10 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** Annotation to be placed on a constructor, if a specific constructor should be used for building the resulting DTO. */ +/** + * Annotation to be placed on a constructor, if a specific constructor should be used for building + * the resulting DTO. + */ @Target({ElementType.METHOD}) @Retention(RetentionPolicy.CLASS) -public @interface SimpleBuilderConstructor {} \ No newline at end of file +public @interface SimpleBuilderConstructor {} diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index d84db954..3f4bf023 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -79,7 +79,8 @@ public static BuilderDefinitionDto extractFromElement( JavaLangMapper.map2GenericParameterDtos(annotatedType, elementUtils, typeUtils) .forEach(result::addGeneric); - // Todo: extract Constructor and his fields. result needs to be filled im constructors and fields. + // Todo: extract Constructor and his fields. result needs to be filled im constructors and + // fields. // Todo: moving into helper function List allMembers = elementUtils.getAllMembers(annotatedType); From 26b1f1f0cf53d23dcba9b9787c341c8fd113cf60 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 13 Sep 2025 17:40:55 +0200 Subject: [PATCH 10/35] Removing methods without field-scope, because in this feature the inner instance in builder is to be removed --- .../processor/dtos/BuilderDefinitionDto.java | 25 ----------- .../util/BuilderDefinitionCreator.java | 35 --------------- .../processor/util/JavaCodeGenerator.java | 10 ----- .../processor/BuilderProcessorTest.java | 44 ++----------------- 4 files changed, 4 insertions(+), 110 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index 22df3433..a047edf4 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -45,12 +45,6 @@ public class BuilderDefinitionDto { /** Generic parameters declared on the target DTO (e.g., ). */ private final List generics = new LinkedList<>(); - /** - * List of all methods of target DTO which are supported by builder and not targeting a specific - * DTO field. - */ - private final LinkedList methods = new LinkedList<>(); - /** * List of all fields of target DTO which are supported by builder. A field could be supported by * multiple functions in builder. @@ -95,25 +89,6 @@ public void setBuildingTargetTypeName(TypeName buildingTargetTypeName) { this.buildingTargetTypeName = buildingTargetTypeName; } - /** - * Adding a method-definition to builder defintion. - * - * @param member {@code org.javahelpers.simple.builders.internal.dtos.MethodDto} to be added - */ - public void addMethod(MethodDto member) { - methods.add(member); - } - - /** - * Getting all definied methods in builder definition. - * - * @return list of methods with type {@code - * org.javahelpers.simple.builders.internal.dtos.MethodDto} - */ - public LinkedList getMethodsForBuilder() { - return methods; - } - /** * Adding a field-definition to builder defintion. * diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 3f4bf023..39c250af 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -29,11 +29,9 @@ import static org.javahelpers.simple.builders.processor.util.JavaLangAnalyser.*; import static org.javahelpers.simple.builders.processor.util.JavaLangMapper.map2MethodParameter; import static org.javahelpers.simple.builders.processor.util.JavaLangMapper.map2TypeName; -import static org.javahelpers.simple.builders.processor.util.JavaLangMapper.mapRelevantModifier; import static org.javahelpers.simple.builders.processor.util.TypeNameAnalyser.*; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; import java.util.function.Supplier; @@ -92,8 +90,6 @@ public static BuilderDefinitionDto extractFromElement( if (isSetterForField(mth)) { // Todo: we need to check setter-fields for duplication with constructor fields createFieldDto(mth, elementUtils, typeUtils).ifPresent(result::addField); - } else { - createMethodDto(mth, elementUtils, typeUtils).ifPresent(result::addMethod); } } } @@ -110,37 +106,6 @@ && isNotPrivate(mth) && isNotStatic(mth); } - // Todo: remove, should not be supported anymore - private static Optional createMethodDto( - ExecutableElement mth, Elements elementUtils, Types typeUtils) { - String methodName = mth.getSimpleName().toString(); - List parameters = mth.getParameters(); - - MethodDto result = new MethodDto(); - result.setMethodName(methodName); - result.setModifier(mapRelevantModifier(mth.getModifiers())); - parameters.stream() - .map(v -> map2MethodParameter(v, elementUtils, typeUtils)) - .forEach(result::addParameter); - if (result.getParameters().stream().anyMatch(Objects::isNull)) { - // TODO: Logging - return Optional.empty(); - } - - List paramList = - result.getParameters().stream().map(MethodParameterDto::getParameterName).toList(); - String paramListJoin = String.join(",", paramList); - result.setCode( - """ - instance.$dtoMethod:N($dtoMethodParams:N); - return this; - """); - result.addArgument("dtoMethod", methodName); - result.addArgument("dtoMethodParams", paramListJoin); - - return Optional.of(result); - } - private static void addAdditionalHelperMethodsForField( FieldDto result, String fieldName, TypeName fieldType) { List innerTypes; diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 8e14c365..440fd660 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -97,11 +97,6 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep classBuilder.addMethod(createConstructorWithInstance(dtoBaseClass, dtoTypeName)); classBuilder.addMethod(createEmptyConstructor(dtoBaseClass, builderDef.getGenerics())); - // Generate Methods in Builder without being setter or getter - for (MethodDto methodDto : builderDef.getMethodsForBuilder()) { - classBuilder.addMethod(createMethod(methodDto, builderTypeName)); - } - // Generate Fields and Fieldspecific funtions in Builder for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { List methodSpecs = createFieldMethods(fieldDto, builderTypeName); @@ -261,11 +256,6 @@ private List createFieldMethods( .toList(); } - private MethodSpec createMethod(MethodDto methodDto, com.palantir.javapoet.TypeName returnType) { - // TODO: Remove when switched to fields instead of instance in builder - return createMethod(methodDto, returnType, ""); - } - private MethodSpec createMethod( MethodDto methodDto, com.palantir.javapoet.TypeName returnType, diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index e5ca93f2..a3bcc1ee 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -364,33 +364,6 @@ public class HelperPlain { public HelperPlain() {} } "@return current instance of builder"); } - @Test - void shouldGenerateProxyWithMixedParamsAndArrayVarargs() { - // Given - String packageName = "test"; - String className = "MixedProxy"; - String builderClassName = className + "Builder"; - - JavaFileObject sourceFile = - ProcessorTestUtils.simpleBuilderClass( - packageName, - className, - """ - public void doMixed(int a, String b, int[] nums) { /* no-op */ } - """); - - // When - Compilation compilation = compile(sourceFile); - - // Then - String generatedCode = loadGeneratedSource(compilation, builderClassName); - assertGenerationSucceeded(compilation, builderClassName, generatedCode); - - // Expect last parameter mapped to varargs in builder signature and proper call - ProcessorAsserts.assertContaining( - generatedCode, "doMixed(int a, String b, int... nums)", "instance.doMixed(a,b,nums);"); - } - @Test void shouldIncludeNestedPackageInClassJavadoc() { // Given @@ -516,13 +489,7 @@ public HelperPlain() {} String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // PROXY method javadoc has @param for each parameter and @return - ProcessorAsserts.assertContaining( - generatedCode, - "Calling doSomething on dto-instance with parameters.", - "@param a", - "@param b", - "@return current instance of builder"); + // Note: Proxy methods are no longer supported; do not assert proxy method javadoc here. // Setter method javadoc and code (default values, when nothing is set) ProcessorAsserts.assertContaining(generatedCode, "@param name name"); @@ -987,13 +954,11 @@ void shouldFilterOutNonRelevantMethods() { packageName, className, """ - // valid proxy candidate - public void ok() {} - // valid setter public void setOk(int ok) {} // should be filtered + public void nonSetterMethods() {} private void setHidden(int hidden) {} public static void setUtil(int util) {} public void setRisky(int risk) throws Exception {} @@ -1007,13 +972,12 @@ public void setRisky(int risk) throws Exception {} // Then String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // Expect only ok() proxy to be present + // Expect only setter-derived methods to be present (no proxy methods) ProcessorAsserts.assertingResult( generatedCode, - contains("public HasVariousMethodsBuilder ok()"), - contains("instance.ok();"), contains("public HasVariousMethodsBuilder ok(int ok)"), contains("instance.setOk(ok);"), + notContains("nonSetterMethods"), notContains("hidden"), notContains("util(int util)"), notContains("risky(int risk)"), From 564a24d975719239929e8572964c40ffa024e6fd Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sat, 13 Sep 2025 17:41:51 +0200 Subject: [PATCH 11/35] adding a fallback in BuilderProcessor to handle cases, where simple-buildder:core dependency is missing --- .../builders/processor/BuilderProcessor.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/BuilderProcessor.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/BuilderProcessor.java index 669bcbff..941acebd 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/BuilderProcessor.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/BuilderProcessor.java @@ -41,7 +41,6 @@ import javax.lang.model.util.Elements; import javax.lang.model.util.Types; import javax.tools.Diagnostic; -import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; import org.javahelpers.simple.builders.processor.dtos.BuilderDefinitionDto; import org.javahelpers.simple.builders.processor.exceptions.BuilderException; import org.javahelpers.simple.builders.processor.util.JavaCodeGenerator; @@ -88,7 +87,18 @@ public boolean process(Set annotations, RoundEnvironment // Fail fast: we already emitted an error in init(); do not attempt any processing. return true; } - for (Element annotatedElement : roundEnv.getElementsAnnotatedWith(SimpleBuilder.class)) { + // Resolve annotation as TypeElement to support environments where the Class overload + // of getElementsAnnotatedWith is unavailable. + TypeElement simpleBuilderAnnotation = + elementUtils.getTypeElement( + org.javahelpers.simple.builders.core.annotations.SimpleBuilder.class + .getCanonicalName()); + if (simpleBuilderAnnotation == null) { + // TODO: Logging + // Annotation type not on classpath; nothing to do this round. + return true; + } + for (Element annotatedElement : roundEnv.getElementsAnnotatedWith(simpleBuilderAnnotation)) { try { process(annotatedElement); } catch (BuilderException ex) { From 6ced7efc96b4e92bdc1d03a9a568706f34087c28 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 17:05:17 +0200 Subject: [PATCH 12/35] Removing name for setter from FieldDto, this should be generated with "setFieldName" by default --- .../builders/processor/dtos/FieldDto.java | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index 28838c4a..0b683f68 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -26,7 +26,7 @@ import java.util.ArrayList; import java.util.List; -import java.util.Optional; +import org.apache.commons.lang3.StringUtils; /** * Definition of a field. Containing all information to generate methods in builder to set or modify @@ -39,12 +39,6 @@ public class FieldDto { /** Type of field. Containing generic, name of package and class */ private TypeName fieldType; - /** - * Name of setter in target object. Entry may be {@code null} if field is only been set via - * constructor - */ - private String setterName; - /** List of all methods in builder, which provide helpers to change the field. */ private final List fieldSetterMethodsList = new ArrayList<>(); @@ -72,12 +66,13 @@ public void setFieldName(String fieldName) { this.fieldName = fieldName; } - public Optional getSetterName() { - return Optional.ofNullable(setterName); - } - - public void setSetterName(String setterName) { - this.setterName = setterName; + /** + * Getting name of setter method. + * + * @return name of setter for field + */ + public String getSetterName() { + return "set" + StringUtils.capitalize(fieldName); } /** From d592a5382b71ee4378c9272e00b9b50a5c073a28 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 22:18:34 +0200 Subject: [PATCH 13/35] Switching from one instance of target-type in builder to having all fields in builder --- .../util/BuilderDefinitionCreator.java | 17 +- .../processor/util/JavaCodeGenerator.java | 159 +++++++++++------- .../processor/util/JavapoetMapper.java | 5 + .../processor/BuilderProcessorTest.java | 108 ++++++------ 4 files changed, 171 insertions(+), 118 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 39c250af..85b97d79 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -244,6 +244,7 @@ private static Optional createFieldDto( return Optional.empty(); } TypeName fieldType = fieldParameterDto.getParameterType(); + result.setFieldType(fieldType); // Extract generics declared on the setter itself (field-specific), e.g., @@ -287,10 +288,10 @@ private static MethodDto createFieldSetterWithTransform( } methodDto.setCode( """ - instance.$dtoMethod:N($dtoMethodParams:N); + this.$fieldName:N = $dtoMethodParams:N; return this; """); - methodDto.addArgument("dtoMethod", methodDto.createFieldSetterMethodName()); + methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParams", params); return methodDto; } @@ -309,10 +310,10 @@ private static MethodDto createFieldConsumer(String fieldName, TypeName fieldTyp """ $helperType:T consumer = new $helperType:T(); $dtoMethodParam:N.accept(consumer); - instance.$dtoMethod:N(consumer); + this.$fieldName:N = consumer; return this; """); - methodDto.addArgument("dtoMethod", methodDto.createFieldSetterMethodName()); + methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); methodDto.addArgument("helperType", fieldType); return methodDto; @@ -338,10 +339,10 @@ private static MethodDto createFieldConsumerWithBuilder(String fieldName, TypeNa """ $helperType:T builder = new $helperType:T(); $dtoMethodParam:N.accept(builder); - instance.$dtoMethod:N(builder.build()); + this.$fieldName:N = builder.build(); return this; """); - methodDto.addArgument("dtoMethod", methodDto.createFieldSetterMethodName()); + methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); methodDto.addArgument("helperType", builderType); return methodDto; @@ -361,10 +362,10 @@ private static MethodDto createFieldSupplier( methodDto.setMethodGenerics(fieldGenerics); methodDto.setCode( """ - instance.$dtoMethod:N($dtoMethodParam:N.get()); + this.$fieldName:N = $dtoMethodParam:N.get(); return this; """); - methodDto.addArgument("dtoMethod", methodDto.createFieldSetterMethodName()); + methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); return methodDto; } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 440fd660..e8755f76 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -89,13 +89,19 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep TypeSpec.classBuilder(builderBaseClass) .addTypeVariables(map2TypeVariables(builderDef.getGenerics())) .addJavadoc(createJavadocForClass(dtoBaseClass)) - .addSuperinterface(createInterfaceBuilderBase(dtoTypeName)) - .addField(createFieldDtoInstance(dtoTypeName)); + .addSuperinterface(createInterfaceBuilderBase(dtoTypeName)); - // TODO: Constructors sollten in der BuilderDefinitionDto definiert werden, nur für Klassen mit - // leerem Constructor ist auch ein Builder mit leerem Constructor möglich - classBuilder.addMethod(createConstructorWithInstance(dtoBaseClass, dtoTypeName)); - classBuilder.addMethod(createEmptyConstructor(dtoBaseClass, builderDef.getGenerics())); + // Adding Constructors for builder + classBuilder.addMethod( + createConstructorWithInstance( + dtoBaseClass, dtoTypeName, builderDef.getSetterFieldsForBuilder())); + classBuilder.addMethod(createEmptyConstructor(dtoBaseClass)); + + // Generate backing fields for each DTO field (boxed types to allow null/unset) + for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { + FieldSpec fieldSpec = createFieldMember(builderDef, fieldDto); + classBuilder.addField(fieldSpec); + } // Generate Fields and Fieldspecific funtions in Builder for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { @@ -104,14 +110,15 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep } // Adding builder-specific methods - classBuilder.addMethod(createMethodBuild(dtoTypeName)); classBuilder.addMethod( - createMethodStaticCreate( - builderBaseClass, - builderTypeName, + createMethodBuild( dtoBaseClass, dtoTypeName, + builderDef.getSetterFieldsForBuilder(), builderDef.getGenerics())); + classBuilder.addMethod( + createMethodStaticCreate( + builderBaseClass, builderTypeName, dtoBaseClass, builderDef.getGenerics())); // Adding annotations classBuilder.addAnnotation(createAnnotationGenerated()); @@ -151,14 +158,7 @@ private AnnotationSpec createAnnotationBuilderImplementation(ClassName dtoClass) .build(); } - private FieldSpec createFieldDtoInstance(com.palantir.javapoet.TypeName dtoTypeName) { - return FieldSpec.builder(dtoTypeName, "instance", Modifier.PRIVATE, Modifier.FINAL) - .addJavadoc("Inner instance of builder.") - .build(); - } - - private MethodSpec createEmptyConstructor( - ClassName dtoClass, List generics) { + private MethodSpec createEmptyConstructor(ClassName dtoClass) { MethodSpec.Builder constructorBuilder = MethodSpec.constructorBuilder() .addModifiers(Modifier.PUBLIC) @@ -168,49 +168,97 @@ private MethodSpec createEmptyConstructor( """, dtoClass.packageName(), dtoClass); - if (generics.isEmpty()) { - constructorBuilder.addStatement("this.instance = new $1T()", dtoClass); - } else { - // Use diamond operator only if the DTO is generic; otherwise, use raw constructor - constructorBuilder.addStatement("this.instance = new $1T<>()", dtoClass); - } return constructorBuilder.build(); } private MethodSpec createConstructorWithInstance( - ClassName dtoBaseClass, com.palantir.javapoet.TypeName dtoType) { - return MethodSpec.constructorBuilder() - .addModifiers(Modifier.PUBLIC) - .addParameter(dtoType, "instance") - .addStatement("this.instance = instance") - .addJavadoc( - """ + ClassName dtoBaseClass, com.palantir.javapoet.TypeName dtoType, List fields) { + MethodSpec.Builder cb = + MethodSpec.constructorBuilder() + .addModifiers(Modifier.PUBLIC) + .addParameter(dtoType, "instance") + .addJavadoc( + """ Initialisation of builder for {@code $1N.$2T} by a instance. @param instance object instance for initialisiation """, - dtoBaseClass.packageName(), - dtoBaseClass) - .build(); + dtoBaseClass.packageName(), + dtoBaseClass); + + for (FieldDto f : fields) { + String cap = org.apache.commons.lang3.StringUtils.capitalize(f.getFieldName()); + boolean isPrimitiveBoolean = + f.getFieldType() + instanceof org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive + && ((org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive) + f.getFieldType()) + .getType() + == org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive + .PrimitiveTypeEnum.BOOLEAN; + String getter = (isPrimitiveBoolean ? "is" : "get") + cap; + cb.addStatement("this.$N = instance.$N()", f.getFieldName(), getter); + } + return cb.build(); } - private MethodSpec createMethodBuild(com.palantir.javapoet.TypeName returnType) { - return MethodSpec.methodBuilder("build") - .addModifiers(PUBLIC) - .returns(returnType) - .addAnnotation(Override.class) - .addCode( - """ - return instance; - """) - .build(); + private FieldSpec createFieldMember(BuilderDefinitionDto builderDef, FieldDto fieldDto) { + com.palantir.javapoet.TypeName fieldType = map2ParameterType(fieldDto.getFieldType()); + // Checking if the type could be extractedfrom the field, even if it is generic + /* + if (fieldDto.getFieldType() + instanceof org.javahelpers.simple.builders.processor.dtos.TypeNameVariable varType) { + String varName = varType.getClassName(); + boolean declaredOnDto = + builderDef.getGenerics().stream().anyMatch(g -> g.getName().equals(varName)); + if (!declaredOnDto) { + // Because the Generics are created on base of existing classes, this should be possible + // TODO Add Lgging here + // Ensure havin no Builder-Generation error in this case, so downgrade to object + fieldType = ClassName.get("java.lang", "Object"); + } + }*/ + if (fieldType.isPrimitive()) { + fieldType = fieldType.box(); + } + if (!fieldDto.getFieldGenerics().isEmpty()) { + fieldType = map2GenericParameterDto(fieldDto.getFieldGenerics().getFirst().getUpperBounds().getFirst()); + } + FieldSpec.Builder fsb = FieldSpec.builder(fieldType, fieldDto.getFieldName(), Modifier.PRIVATE); + return fsb.build(); + } + + private MethodSpec createMethodBuild( + ClassName dtoBaseClass, + com.palantir.javapoet.TypeName returnType, + List fields, + List generics) { + MethodSpec.Builder mb = + MethodSpec.methodBuilder("build") + .addModifiers(PUBLIC) + .returns(returnType) + .addAnnotation(Override.class); + + if (generics.isEmpty()) { + mb.addStatement("$1T result = new $1T()", dtoBaseClass); + } else { + mb.addStatement("$1T result = new $2T<>()", returnType, dtoBaseClass); + } +/* + for (FieldDto f : fields) { + mb.beginControlFlow("if (this.$N != null)", f.getFieldName()) + .addStatement("result.$N(this.$N)", f.getSetterName(), f.getFieldName()) + .endControlFlow(); + } + */ + mb.addStatement("return result"); + return mb.build(); } private MethodSpec createMethodStaticCreate( com.palantir.javapoet.ClassName builderBaseClass, com.palantir.javapoet.TypeName builderType, com.palantir.javapoet.ClassName dtoBaseClass, - com.palantir.javapoet.TypeName dtoType, List generics) { MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder("create") @@ -224,27 +272,12 @@ private MethodSpec createMethodStaticCreate( dtoBaseClass.packageName(), dtoBaseClass); if (generics.isEmpty()) { - methodBuilder - .returns(builderBaseClass) - .addCode( - """ - $1T instance = new $1T(); - return new $2T(instance); - """, - dtoBaseClass, - builderBaseClass); + methodBuilder.returns(builderBaseClass).addCode("return new $1T();\n", builderBaseClass); } else { methodBuilder .returns(builderType) - .addCode( - """ - $1T instance = new $2T<>(); - return new $3T<>(instance); - """, - dtoType, - dtoBaseClass, - builderBaseClass) - .addTypeVariables(map2TypeVariables(generics)); + .addTypeVariables(map2TypeVariables(generics)) + .addCode("return new $1T<>();\n", builderBaseClass); } return methodBuilder.build(); } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavapoetMapper.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavapoetMapper.java index fd271988..9d1a9ae6 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavapoetMapper.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavapoetMapper.java @@ -66,6 +66,11 @@ public static TypeName[] map2TypeArgumentsArray( */ public static TypeName map2ParameterType( org.javahelpers.simple.builders.processor.dtos.TypeName parameterType) { + // Handle generic type variables (e.g., T, K, V) + if (parameterType + instanceof org.javahelpers.simple.builders.processor.dtos.TypeNameVariable typeVariable) { + return TypeVariableName.get(typeVariable.getClassName()); + } ClassName classNameParameter = ClassName.get(parameterType.getPackageName(), parameterType.getClassName()); if (parameterType instanceof TypeNameArray parameterTypeArray) { diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index a3bcc1ee..1d7f7cf4 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -110,9 +110,9 @@ public CtorAndSetter() {} ProcessorAsserts.assertContaining( generatedCode, "public CtorAndSetterBuilder a(int a)", - "instance.setA(a);", + "this.a = a;", "public CtorAndSetterBuilder name(String name)", - "instance.setName(name);"); + "this.name = name;"); } @Test @@ -145,9 +145,9 @@ public record PersonRecord(String name, int age) { ProcessorAsserts.assertContaining( generatedCode, "public PersonRecordBuilder name(String name)", - "instance.setName(name);", + "this.name = name;", "public PersonRecordBuilder age(int age)", - "instance.setAge(age);"); + "this.age = age;"); } @Test @@ -189,18 +189,18 @@ void shouldGenerateCollectionSettersAndProviders() { "public WithCollectionsBuilder names(List names)", "public WithCollectionsBuilder names(Supplier> namesSupplier)", "public WithCollectionsBuilder names(String... names)", - "instance.setNames(List.of(names));", + "this.names = List.of(names);", "public WithCollectionsBuilder tags(Set tags)", "public WithCollectionsBuilder tags(Supplier> tagsSupplier)", "public WithCollectionsBuilder tags(String... tags)", - "instance.setTags(Set.of(tags));", + "this.tags = Set.of(tags);", "public WithCollectionsBuilder map(Map map)", "public WithCollectionsBuilder map(Supplier> mapSupplier)", - "instance.setMap(map);"); + "this.map = map;"); } @Test - void shouldDeclareInstanceFieldAsPrivateFinalWithJavadoc() { + void shouldDeclarePerFieldBackingField() { // Given String packageName = "test"; String className = "FieldDoc"; @@ -221,8 +221,9 @@ void shouldDeclareInstanceFieldAsPrivateFinalWithJavadoc() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - ProcessorAsserts.assertContaining( - generatedCode, "Inner instance of builder.", "private final FieldDoc instance;"); + // Expect a private boxed backing field for primitive int setter + ProcessorAsserts.assertContaining(generatedCode, "private Integer x;"); + ProcessorAsserts.assertNotContaining(generatedCode, " FieldDoc instance;"); } @Test @@ -253,11 +254,12 @@ void shouldGenerateCreateMethodWithModifiersAndJavadoc() { generatedCode, "public static CreateDocBuilder create()", "Creating a new builder for {@code test.CreateDoc}", - "@return builder for {@code test.CreateDoc}"); + "@return builder for {@code test.CreateDoc}", + "return new CreateDocBuilder();"); } @Test - void shouldGenerateBuildOverrideAndReturnInstance() { + void shouldGenerateBuildOverrideAndReturnAnInstanceWithCalledSetters() { // Given String packageName = "test"; String className = "BuildDoc"; @@ -281,7 +283,17 @@ void shouldGenerateBuildOverrideAndReturnInstance() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( - generatedCode, "@Override", "public BuildDoc build()", "return instance;"); + generatedCode, + """ + @Override + public BuildDoc build() { + BuildDoc result = new BuildDoc(); + if (this.x != null) { + result.setX(this.x); + } + return result; + } + """); } @Test @@ -306,7 +318,11 @@ void shouldReturnThisFromFluentMethods() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - ProcessorAsserts.assertContaining(generatedCode, "instance.setName(name);", "return this;"); + ProcessorAsserts.assertContaining( + generatedCode, + "public FluentReturnBuilder name(String name)", + "this.name = name;", + "return this;"); } @Test @@ -326,7 +342,9 @@ void shouldContainReturnJavadocForConsumerSupplierAndConsumerByBuilder() { private HelperPlain helperPlain; public String getName() { return name; } public void setName(String name) { this.name = name; } + public HelperAnno getHelper() { return helper; } public void setHelper(HelperAnno helper) { this.helper = helper; } + public HelperPlain getHelperPlain() { return helperPlain; } public void setHelperPlain(HelperPlain helperPlain) { this.helperPlain = helperPlain; } """); @@ -449,8 +467,8 @@ void shouldGenerateBothConstructorsWithJavadocs() { "Empty constructor of builder for", "Initialisation of builder for", "@param instance object instance for initialisiation", - "this.instance = instance;", - "this.instance = new CtorDoc();"); + "public CtorDocBuilder()", + "public CtorDocBuilder(CtorDoc instance)"); } @Test @@ -469,6 +487,7 @@ void shouldGenerateMethodJavadocsForAllMethodTypes() { private HelperPlain helperPlain; public String getName() { return name; } public void setName(String name) { this.name = name; } + public HelperPlain getHelperPlain() { return helperPlain; } public void setHelperPlain(HelperPlain helperPlain) { this.helperPlain = helperPlain; } public void doSomething(int a, String b) { /* no-op */ } """); @@ -578,7 +597,7 @@ public HelperAnno() {} "helperBuilderConsumer", "HelperAnnoBuilder builder = new HelperAnnoBuilder();", "helperBuilderConsumer.accept(builder);", - "instance.setHelper(builder.build());"); + "this.helper = builder.build();"); } @Test @@ -619,8 +638,7 @@ public HelperPlain() {} ProcessorAsserts.assertContaining( generatedCode, "HelperPlain consumer = new HelperPlain();", - "helperPlainConsumer.accept(consumer);", - "instance.setHelperPlain(consumer);"); + "helperPlainConsumer.accept(consumer);"); } @Test @@ -650,7 +668,7 @@ void shouldGenerateSupplierMethodForField() { // Expect supplier-based setter usage ProcessorAsserts.assertContaining( - generatedCode, "nameSupplier", "instance.setName(nameSupplier.get());"); + generatedCode, "nameSupplier", "this.name = nameSupplier.get();"); } protected Compilation compile(JavaFileObject... sourceFiles) { @@ -852,13 +870,13 @@ void shouldHandleSetOfStrings() { ProcessorAsserts.assertContaining( generatedCode, "public HasSetStringBuilder tags(Set tags)", - "instance.setTags(tags);", + "this.tags = tags;", "public HasSetStringBuilder tags(Consumer> tagsBuilderConsumer)", - "instance.setTags(builder.build());", + "this.tags = builder.build();", "public HasSetStringBuilder tags(String... tags)", - "instance.setTags(Set.of(tags));", + "this.tags = Set.of(tags);", "public HasSetStringBuilder tags(Supplier> tagsSupplier)", - "instance.setTags(tagsSupplier.get());"); + "this.tags = tagsSupplier.get();"); } @Test @@ -897,13 +915,13 @@ public Helper() {} ProcessorAsserts.assertContaining( generatedCode, "public HasSetCustomBuilder helpers(Set helpers)", - "instance.setHelpers(helpers);", + "this.helpers = helpers;", "public HasSetCustomBuilder helpers(Consumer> helpersBuilderConsumer)", - "instance.setHelpers(builder.build());", + "this.helpers = builder.build();", "public HasSetCustomBuilder helpers(Helper... helpers)", - "instance.setHelpers(Set.of(helpers));", + "this.helpers = Set.of(helpers);", "public HasSetCustomBuilder helpers(Supplier> helpersSupplier)", - "instance.setHelpers(helpersSupplier.get());"); + "this.helpers = helpersSupplier.get();"); } @Test @@ -954,8 +972,9 @@ void shouldFilterOutNonRelevantMethods() { packageName, className, """ - // valid setter + // valid setter (having a getter) public void setOk(int ok) {} + public int getOk() { return 0; } // should be filtered public void nonSetterMethods() {} @@ -976,7 +995,7 @@ public void setRisky(int risk) throws Exception {} ProcessorAsserts.assertingResult( generatedCode, contains("public HasVariousMethodsBuilder ok(int ok)"), - contains("instance.setOk(ok);"), + contains("this.ok = ok;"), notContains("nonSetterMethods"), notContains("hidden"), notContains("util(int util)"), @@ -1119,12 +1138,10 @@ void shouldGenerateAnnotationsAndCreateBuildMethods() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( generatedCode, - "@Generated(", - "@BuilderImplementation(", - "public static AnnoTargetBuilder create()", - "return new AnnoTargetBuilder(instance);", - "public AnnoTarget build()", - "return instance;"); + "@Generated(\"Generated by org.javahelpers.simple.builders.processor.BuilderProcessor\")", + "@BuilderImplementation( forClass = AnnoTarget.class )", + "public static AnnoTargetBuilder create() { return new AnnoTargetBuilder(); }", + "@Override public AnnoTarget build()"); } @Test @@ -1157,10 +1174,8 @@ void shouldGenerateVarargsConvenienceForCollections() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( generatedCode, - "public HasCollectionsConvenienceBuilder names(String... names)", - "instance.setNames(List.of(names));", - "public HasCollectionsConvenienceBuilder tags(String... tags)", - "instance.setTags(Set.of(tags));"); + "public HasCollectionsConvenienceBuilder names(String... names) { this.names = List.of(names);", + "public HasCollectionsConvenienceBuilder tags(String... tags) { this.tags = Set.of(tags);"); } @Test @@ -1188,9 +1203,7 @@ void shouldHandleJavaTimeField() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( - generatedCode, - "public HasLocalDateBuilder date(LocalDate date)", - "instance.setDate(date);"); + generatedCode, "public HasLocalDateBuilder date(LocalDate date) { this.date = date;"); } @Test @@ -1342,10 +1355,11 @@ void shouldHandleEmptyClass() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); // There should be no builder generated for an empty class - ProcessorAsserts.assertingResult( + ProcessorAsserts.assertContaining( generatedCode, - contains("public EmptyClass build() { return instance; }"), - notContains("return this;")); + "public EmptyClass build()", + "EmptyClass result = new EmptyClass();", + "return result;"); } @Test @@ -1431,7 +1445,7 @@ void shouldBoxPrimitiveTypeArguments() { public void setLongValue(long longValue) { this.longValue = longValue; } public double getDoubleValue() { return doubleValue; } public void setDoubleValue(double doubleValue) { this.doubleValue = doubleValue; } - public boolean getBooleanValue() { return booleanValue; } + public boolean isBooleanValue() { return booleanValue; } public void setBooleanValue(boolean booleanValue) { this.booleanValue = booleanValue; } """); From 540ef7e75611510491cb4a3e7b1382ebfe6ebc2d Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 22:18:52 +0200 Subject: [PATCH 14/35] Removing codesmell --- .../builders/processor/util/BuilderDefinitionCreator.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 85b97d79..a4b44a75 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -86,11 +86,9 @@ public static BuilderDefinitionDto extractFromElement( for (ExecutableElement mth : methods) { // nur public - if (isMethodRelevantForBuilder(mth)) { - if (isSetterForField(mth)) { - // Todo: we need to check setter-fields for duplication with constructor fields - createFieldDto(mth, elementUtils, typeUtils).ifPresent(result::addField); - } + if (isMethodRelevantForBuilder(mth) && JavaLangAnalyser.isSetterForField(mth)) { + // Todo: we need to check setter-fields for duplication with constructor fields + createFieldDto(mth, elementUtils, typeUtils).ifPresent(result::addField); } } From 9ecadce15547dd9d4395c455e3102d82f60e16b4 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 22:44:56 +0200 Subject: [PATCH 15/35] Readding outcommented code --- .../simple/builders/processor/util/JavaCodeGenerator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index e8755f76..0eb6db16 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -205,7 +205,6 @@ private MethodSpec createConstructorWithInstance( private FieldSpec createFieldMember(BuilderDefinitionDto builderDef, FieldDto fieldDto) { com.palantir.javapoet.TypeName fieldType = map2ParameterType(fieldDto.getFieldType()); // Checking if the type could be extractedfrom the field, even if it is generic - /* if (fieldDto.getFieldType() instanceof org.javahelpers.simple.builders.processor.dtos.TypeNameVariable varType) { String varName = varType.getClassName(); @@ -217,7 +216,7 @@ private FieldSpec createFieldMember(BuilderDefinitionDto builderDef, FieldDto fi // Ensure havin no Builder-Generation error in this case, so downgrade to object fieldType = ClassName.get("java.lang", "Object"); } - }*/ + } if (fieldType.isPrimitive()) { fieldType = fieldType.box(); } From affaf65bf82f9973893df99514b5f75db09e81d7 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 22:47:33 +0200 Subject: [PATCH 16/35] Removing field specific generics, because they prohibit generation of member variables --- processor/pom.xml | 7 +++ .../builders/processor/dtos/MethodDto.java | 17 ------ .../util/BuilderDefinitionCreator.java | 55 +++++++++---------- .../processor/util/JavaCodeGenerator.java | 29 +--------- .../processor/BuilderProcessorTest.java | 43 --------------- 5 files changed, 37 insertions(+), 114 deletions(-) diff --git a/processor/pom.xml b/processor/pom.xml index 49da7dcf..625f84d1 100644 --- a/processor/pom.xml +++ b/processor/pom.xml @@ -56,6 +56,7 @@ 1.1.1 0.7.0 3.18.0 + 4.4 5.13.4 0.22.0 @@ -91,6 +92,12 @@ commons-lang3 ${commons-lang.version} + + + org.apache.commons + commons-collections4 + ${commons-collections.version} + com.google.auto.service diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/MethodDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/MethodDto.java index a9a45482..165e9f0b 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/MethodDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/MethodDto.java @@ -27,7 +27,6 @@ import static org.javahelpers.simple.builders.processor.dtos.MethodTypes.PROXY; import java.util.LinkedList; -import java.util.List; import java.util.Optional; import javax.lang.model.element.Modifier; import org.apache.commons.lang3.StringUtils; @@ -52,9 +51,6 @@ public class MethodDto { /** Definition of inner implementation for method. */ private final MethodCodeDto methodCodeDto = new MethodCodeDto(); - /** Method-level type parameters (e.g., ). */ - private final List methodGenerics = new LinkedList<>(); - /** * Getting type of method for source code generation. Defaults to {@code * org.javahelpers.simple.builders.internal.dtos.MethodTypes.PROXY}. @@ -163,19 +159,6 @@ public LinkedList getParameters() { return parameters; } - /** Replaces method-level generic parameter list with the provided entries. */ - public void setMethodGenerics(List generics) { - this.methodGenerics.clear(); - if (generics != null) { - this.methodGenerics.addAll(generics); - } - } - - /** Returns declared method-level generic parameters. */ - public List getMethodGenerics() { - return methodGenerics; - } - /** * Getting the access modifier for method. Optional for usage in stream-notation. * diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index a4b44a75..120a509e 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -40,6 +40,7 @@ import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.javahelpers.simple.builders.core.annotations.IgnoreInBuilder; import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; @@ -88,7 +89,8 @@ public static BuilderDefinitionDto extractFromElement( // nur public if (isMethodRelevantForBuilder(mth) && JavaLangAnalyser.isSetterForField(mth)) { // Todo: we need to check setter-fields for duplication with constructor fields - createFieldDto(mth, elementUtils, typeUtils).ifPresent(result::addField); + createFieldDto(mth, result.getGenerics(), elementUtils, typeUtils) + .ifPresent(result::addField); } } @@ -119,25 +121,18 @@ private static void addAdditionalHelperMethodsForField( if (isList(fieldType) && innerTypesCnt == 1) { result.addMethod( createFieldSetterWithTransform( - fieldName, - "List.of(%s)", - new TypeNameArray(innerTypes.get(0), false), - result.getFieldGenerics())); + fieldName, "List.of(%s)", new TypeNameArray(innerTypes.get(0), false))); } else if (isSet(fieldType) && innerTypesCnt == 1) { result.addMethod( createFieldSetterWithTransform( - fieldName, - "Set.of(%s)", - new TypeNameArray(innerTypes.get(0), true), - result.getFieldGenerics())); + fieldName, "Set.of(%s)", new TypeNameArray(innerTypes.get(0), true))); } else if (isMap(fieldType) && innerTypesCnt == 2) { TypeName mapEntryType = new TypeNameArray( new TypeNameGeneric("java.util", "Map.Entry", innerTypes.get(0), innerTypes.get(1)), false); result.addMethod( - createFieldSetterWithTransform( - fieldName, "Map.ofEntries(%s)", mapEntryType, result.getFieldGenerics())); + createFieldSetterWithTransform(fieldName, "Map.ofEntries(%s)", mapEntryType)); } } @@ -205,11 +200,14 @@ private static void addSupplierMethodsForField( if (isFunctionalInterface(fieldTypeElement)) { return; } - result.addMethod(createFieldSupplier(fieldName, fieldType, result.getFieldGenerics())); + result.addMethod(createFieldSupplier(fieldName, fieldType)); } private static Optional createFieldDto( - ExecutableElement mth, Elements elementUtils, Types typeUtils) { + ExecutableElement mth, + List dtoGenerics, + Elements elementUtils, + Types typeUtils) { String methodName = mth.getSimpleName().toString(); String fieldName = StringUtils.uncapitalize(StringUtils.removeStart(methodName, "set")); @@ -244,16 +242,23 @@ private static Optional createFieldDto( TypeName fieldType = fieldParameterDto.getParameterType(); result.setFieldType(fieldType); - // Extract generics declared on the setter itself (field-specific), e.g., - List fieldGenerics = - mth.getTypeParameters().stream() - .map(tp -> JavaLangMapper.map2GenericParameterDto(tp, elementUtils, typeUtils)) - .toList(); - result.setFieldGenerics(fieldGenerics); + if (CollectionUtils.isNotEmpty(mth.getTypeParameters())) { + List dtoGenericsNames = + dtoGenerics.stream().map(GenericParameterDto::getName).toList(); + List mthGenericsNames = + mth.getTypeParameters().stream().map(tp -> tp.getSimpleName().toString()).toList(); + // If there are field-specific generics, no field in builder could be generated for it, so it + // needs to be ignored + if (CollectionUtils.containsAll(dtoGenericsNames, mthGenericsNames)) { + // TODO: Logging + return Optional.empty(); + } + } // simple setter - result.addMethod(createFieldSetterWithTransform(fieldName, null, fieldType, fieldGenerics)); + result.addMethod(createFieldSetterWithTransform(fieldName, null, fieldType)); // add consumer/supplier generation via helpers addConsumerMethodsForField( @@ -265,10 +270,7 @@ private static Optional createFieldDto( } private static MethodDto createFieldSetterWithTransform( - String fieldName, - String transform, - TypeName fieldType, - List fieldGenerics) { + String fieldName, String transform, TypeName fieldType) { MethodParameterDto parameter = new MethodParameterDto(); parameter.setParameterName(fieldName); parameter.setParameterTypeName(fieldType); @@ -277,7 +279,6 @@ private static MethodDto createFieldSetterWithTransform( methodDto.addParameter(parameter); methodDto.setModifier(Modifier.PUBLIC); methodDto.setMethodType(MethodTypes.PROXY); - methodDto.setMethodGenerics(fieldGenerics); String params; if (StringUtils.isBlank(transform)) { params = parameter.getParameterName(); @@ -346,8 +347,7 @@ private static MethodDto createFieldConsumerWithBuilder(String fieldName, TypeNa return methodDto; } - private static MethodDto createFieldSupplier( - String fieldName, TypeName fieldType, List fieldGenerics) { + private static MethodDto createFieldSupplier(String fieldName, TypeName fieldType) { TypeNameGeneric supplierType = new TypeNameGeneric(map2TypeName(Supplier.class), fieldType); MethodParameterDto parameter = new MethodParameterDto(); parameter.setParameterName(fieldName + "Supplier"); @@ -357,7 +357,6 @@ private static MethodDto createFieldSupplier( methodDto.addParameter(parameter); methodDto.setModifier(Modifier.PUBLIC); methodDto.setMethodType(MethodTypes.SUPPLIER); - methodDto.setMethodGenerics(fieldGenerics); methodDto.setCode( """ this.$fieldName:N = $dtoMethodParam:N.get(); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 0eb6db16..d805dd17 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -99,7 +99,7 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep // Generate backing fields for each DTO field (boxed types to allow null/unset) for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { - FieldSpec fieldSpec = createFieldMember(builderDef, fieldDto); + FieldSpec fieldSpec = createFieldMember(fieldDto); classBuilder.addField(fieldSpec); } @@ -202,29 +202,12 @@ private MethodSpec createConstructorWithInstance( return cb.build(); } - private FieldSpec createFieldMember(BuilderDefinitionDto builderDef, FieldDto fieldDto) { + private FieldSpec createFieldMember(FieldDto fieldDto) { com.palantir.javapoet.TypeName fieldType = map2ParameterType(fieldDto.getFieldType()); - // Checking if the type could be extractedfrom the field, even if it is generic - if (fieldDto.getFieldType() - instanceof org.javahelpers.simple.builders.processor.dtos.TypeNameVariable varType) { - String varName = varType.getClassName(); - boolean declaredOnDto = - builderDef.getGenerics().stream().anyMatch(g -> g.getName().equals(varName)); - if (!declaredOnDto) { - // Because the Generics are created on base of existing classes, this should be possible - // TODO Add Lgging here - // Ensure havin no Builder-Generation error in this case, so downgrade to object - fieldType = ClassName.get("java.lang", "Object"); - } - } if (fieldType.isPrimitive()) { fieldType = fieldType.box(); } - if (!fieldDto.getFieldGenerics().isEmpty()) { - fieldType = map2GenericParameterDto(fieldDto.getFieldGenerics().getFirst().getUpperBounds().getFirst()); - } - FieldSpec.Builder fsb = FieldSpec.builder(fieldType, fieldDto.getFieldName(), Modifier.PRIVATE); - return fsb.build(); + return FieldSpec.builder(fieldType, fieldDto.getFieldName(), Modifier.PRIVATE).build(); } private MethodSpec createMethodBuild( @@ -243,13 +226,11 @@ private MethodSpec createMethodBuild( } else { mb.addStatement("$1T result = new $2T<>()", returnType, dtoBaseClass); } -/* for (FieldDto f : fields) { mb.beginControlFlow("if (this.$N != null)", f.getFieldName()) .addStatement("result.$N(this.$N)", f.getSetterName(), f.getFieldName()) .endControlFlow(); } - */ mb.addStatement("return result"); return mb.build(); } @@ -295,10 +276,6 @@ private MethodSpec createMethod( MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(methodDto.getMethodName()).returns(returnType); methodDto.getModifier().ifPresent(methodBuilder::addModifiers); - // add method-level generics if present (e.g., ) - if (!methodDto.getMethodGenerics().isEmpty()) { - methodBuilder.addTypeVariables(map2TypeVariables(methodDto.getMethodGenerics())); - } int maxIndexParameters = methodDto.getParameters().size() - 1; // Adding javadoc for method diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 1d7f7cf4..95d9987b 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -1882,49 +1882,6 @@ public class GenericDto { contains("public static GenericDtoBuilder create()")); } - @Test - void shouldGenerateBuilderRetainingFieldspecificTypeParameter() { - // Given - String builderClassName = "GenericFieldDtoBuilder"; - - JavaFileObject genericDtoSource = - ProcessorTestUtils.forSource( - """ - package test; - import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; - @SimpleBuilder - public class GenericFieldDto { - private java.io.Serializable value; - public java.io.Serializable getValue() { return value; } - public void setValue(T value) { this.value = value; } - } - """); - - // When - Compilation compilation = compile(genericDtoSource); - - // Then - String generatedCode = loadGeneratedSource(compilation, builderClassName); - assertGenerationSucceeded(compilation, builderClassName, generatedCode); - ProcessorAsserts.assertingResult( - generatedCode, - // builder preserves type parameter T - contains("class GenericFieldDtoBuilder"), - // new import for Serializable - contains("import java.io.Serializable;"), - // setter keeps T - contains("public GenericFieldDtoBuilder value(T value)"), - // supplier-based setter retains T - contains( - "public GenericFieldDtoBuilder value(Supplier valueSupplier)"), - // no direct consumer possible for unknown T (no empty ctor info) - notContains("GenericFieldDtoBuilder value(Consumer"), - // build returns GenericFieldDto - contains("public GenericFieldDto build()"), - // create() exposes no generic, because the Builder is not generic - contains("public static GenericFieldDtoBuilder create()")); - } - @Test void shouldNotGenerateBuilderWhenNestedBuilderInterfaceExists() { // Given: an annotated class that already declares a nested interface named like the would-be From 3ead6eeca9ce1511247d919c814ca7313560d84c Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 22:54:58 +0200 Subject: [PATCH 17/35] Adding a check for methods, which just start with "set" without being a setter --- .../builders/processor/util/BuilderDefinitionCreator.java | 5 +++-- .../simple/builders/processor/util/JavaLangAnalyser.java | 6 +++++- .../simple/builders/processor/BuilderProcessorTest.java | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 120a509e..39c7533e 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -87,7 +87,7 @@ public static BuilderDefinitionDto extractFromElement( for (ExecutableElement mth : methods) { // nur public - if (isMethodRelevantForBuilder(mth) && JavaLangAnalyser.isSetterForField(mth)) { + if (isMethodRelevantForBuilder(mth)) { // Todo: we need to check setter-fields for duplication with constructor fields createFieldDto(mth, result.getGenerics(), elementUtils, typeUtils) .ifPresent(result::addField); @@ -98,7 +98,8 @@ public static BuilderDefinitionDto extractFromElement( } private static boolean isMethodRelevantForBuilder(ExecutableElement mth) { - return isNoMethodOfObjectClass(mth) + return isSetterForField(mth) + && isNoMethodOfObjectClass(mth) && hasNoThrowablesDeclared(mth) && hasNoReturnValue(mth) && hasNotAnnotation(IgnoreInBuilder.class, mth) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java index da3aa81c..d0b32775 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java @@ -117,7 +117,11 @@ public static boolean isNotStatic(ExecutableElement mth) { * @return {@code true}, if the signature matches setters */ public static boolean isSetterForField(ExecutableElement mth) { - return StringUtils.startsWith(mth.getSimpleName(), "set") && mth.getParameters().size() == 1; + String name = mth.getSimpleName().toString(); + return StringUtils.length(name) > 3 + && Strings.CS.startsWith(name, "set") + && StringUtils.isAllUpperCase(StringUtils.substring(name, 3, 4)) + && mth.getParameters().size() == 1; } /** diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 95d9987b..4550c88f 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -978,6 +978,7 @@ public void setOk(int ok) {} // should be filtered public void nonSetterMethods() {} + public void settingChanged(String nonSetterValue) {} private void setHidden(int hidden) {} public static void setUtil(int util) {} public void setRisky(int risk) throws Exception {} @@ -997,6 +998,7 @@ public void setRisky(int risk) throws Exception {} contains("public HasVariousMethodsBuilder ok(int ok)"), contains("this.ok = ok;"), notContains("nonSetterMethods"), + notContains("settingChanged"), notContains("hidden"), notContains("util(int util)"), notContains("risky(int risk)"), From 37ad6138cfbc8a0061dbab0566dad80403799667 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Sun, 14 Sep 2025 23:03:23 +0200 Subject: [PATCH 18/35] Improving tests to check that member variable is generated too --- .../processor/BuilderProcessorTest.java | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 4550c88f..31f5a2a8 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -190,13 +190,16 @@ void shouldGenerateCollectionSettersAndProviders() { "public WithCollectionsBuilder names(Supplier> namesSupplier)", "public WithCollectionsBuilder names(String... names)", "this.names = List.of(names);", + "private List names;", "public WithCollectionsBuilder tags(Set tags)", "public WithCollectionsBuilder tags(Supplier> tagsSupplier)", "public WithCollectionsBuilder tags(String... tags)", "this.tags = Set.of(tags);", + "private Set tags;", "public WithCollectionsBuilder map(Map map)", "public WithCollectionsBuilder map(Supplier> mapSupplier)", - "this.map = map;"); + "this.map = map;", + "private Map map;"); } @Test @@ -468,7 +471,8 @@ void shouldGenerateBothConstructorsWithJavadocs() { "Initialisation of builder for", "@param instance object instance for initialisiation", "public CtorDocBuilder()", - "public CtorDocBuilder(CtorDoc instance)"); + "public CtorDocBuilder(CtorDoc instance)", + "private String x;"); } @Test @@ -519,6 +523,11 @@ public HelperPlain() {} // CONSUMER method javadoc and code ProcessorAsserts.assertContaining( generatedCode, "@param helperPlainConsumer consumer providing an instance of helperPlain"); + + // Backing fields present + // TODO: Add Javadoc to fields + ProcessorAsserts.assertContaining( + generatedCode, "private String name;", "private HelperPlain helperPlain;"); } @Test @@ -552,7 +561,8 @@ void shouldPropagateFieldJavadocToBuilderParameter() { ProcessorAsserts.assertContaining( generatedCode, "@param name the person name", - "public FieldWithDocBuilder name(String name)"); + "public FieldWithDocBuilder name(String name)", + "private String name;"); } @Test @@ -594,6 +604,7 @@ public HelperAnno() {} // Expect consumer-by-builder method using HelperAnnoBuilder and builder.build() ProcessorAsserts.assertContaining( generatedCode, + "private HelperAnno helper;", "helperBuilderConsumer", "HelperAnnoBuilder builder = new HelperAnnoBuilder();", "helperBuilderConsumer.accept(builder);", @@ -638,7 +649,8 @@ public HelperPlain() {} ProcessorAsserts.assertContaining( generatedCode, "HelperPlain consumer = new HelperPlain();", - "helperPlainConsumer.accept(consumer);"); + "helperPlainConsumer.accept(consumer);", + "private HelperPlain helperPlain;"); } @Test @@ -668,7 +680,7 @@ void shouldGenerateSupplierMethodForField() { // Expect supplier-based setter usage ProcessorAsserts.assertContaining( - generatedCode, "nameSupplier", "this.name = nameSupplier.get();"); + generatedCode, "nameSupplier", "private String name;", "this.name = nameSupplier.get();"); } protected Compilation compile(JavaFileObject... sourceFiles) { @@ -744,7 +756,9 @@ public void setAge(int age) { ProcessorAsserts.assertContaining( generatedCode, "public PersonBuilder name(String name)", - "public PersonBuilder age(int age)"); + "public PersonBuilder age(int age)", + "private String name;", + "private Integer age;"); } @Test @@ -799,6 +813,12 @@ public class OptionalWrapper { "name(String...)", "count(Integer...)", "date(LocalDate...)"); + // Backing fields present + ProcessorAsserts.assertContaining( + optionalBuilder, + "private Optional name;", + "private Optional count;", + "private Optional date;"); } @Test @@ -839,7 +859,9 @@ public Helper() {} String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( - generatedCode, "public UsesOtherPackageHelperBuilder helper(Helper helper)"); + generatedCode, + "public UsesOtherPackageHelperBuilder helper(Helper helper)", + "private Helper helper;"); } @Test From a805f8d70b5af8d9df87c3cd1a1a95232a7bfecf Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 24 Sep 2025 21:53:29 +0200 Subject: [PATCH 19/35] Extracting getterName and only using it in constructor if there is a getter --- .../builders/processor/dtos/FieldDto.java | 14 +++++++ .../util/BuilderDefinitionCreator.java | 6 +++ .../processor/util/JavaCodeGenerator.java | 14 ++----- .../processor/util/JavaLangAnalyser.java | 41 +++++++++++++++++++ 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index 0b683f68..0ad44452 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; import org.apache.commons.lang3.StringUtils; /** @@ -48,6 +49,9 @@ public class FieldDto { /** Method-level generics declared on the setter, to be reused in builder methods. */ private final List fieldGenerics = new ArrayList<>(); + /** The name of the getter method to call, because in case of boolean it does not need to start with get (e.g., getName or isActive). */ + private String getterName; + /** * Getting name of field. * @@ -142,4 +146,14 @@ public void setFieldGenerics(List generics) { this.fieldGenerics.addAll(generics); } } + + /** Returns the getter method name to use (without parentheses). */ + public Optional getGetterName() { + return Optional.ofNullable(getterName); + } + + /** Sets the getter method name to use (without parentheses). */ + public void setGetterName(String getterName) { + this.getterName = getterName; + } } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 39c7533e..e5e020a7 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -243,6 +243,12 @@ private static Optional createFieldDto( TypeName fieldType = fieldParameterDto.getParameterType(); result.setFieldType(fieldType); + // Determine if a corresponding getter exists on the DTO and record its name + TypeElement dtoType = (TypeElement) mth.getEnclosingElement(); + JavaLangAnalyser.findGetterForField( + dtoType, fieldName, fieldTypeMirror, elementUtils, typeUtils) + .ifPresent(getter -> result.setGetterName(getter.getSimpleName().toString())); + // Finding generics declared on the setter itself (field-specific), e.g., if (CollectionUtils.isNotEmpty(mth.getTypeParameters())) { diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index d805dd17..6ae03155 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -187,17 +187,9 @@ private MethodSpec createConstructorWithInstance( dtoBaseClass); for (FieldDto f : fields) { - String cap = org.apache.commons.lang3.StringUtils.capitalize(f.getFieldName()); - boolean isPrimitiveBoolean = - f.getFieldType() - instanceof org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive - && ((org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive) - f.getFieldType()) - .getType() - == org.javahelpers.simple.builders.processor.dtos.TypeNamePrimitive - .PrimitiveTypeEnum.BOOLEAN; - String getter = (isPrimitiveBoolean ? "is" : "get") + cap; - cb.addStatement("this.$N = instance.$N()", f.getFieldName(), getter); + f.getGetterName() + .ifPresent( + getter -> cb.addStatement("this.$N = instance.$N()", f.getFieldName(), getter)); } return cb.build(); } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java index d0b32775..5629e3fe 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java @@ -36,8 +36,10 @@ import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; import javax.lang.model.element.VariableElement; +import javax.lang.model.type.TypeMirror; import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Elements; +import javax.lang.model.util.Types; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; @@ -244,4 +246,43 @@ public static String extractParamJavaDoc(String javaDoc, VariableElement paramet String result = sb.toString().trim(); return result.isEmpty() ? null : result; } + + /** + * Finds the getter method for a given field on the specified DTO type. + * + *

Prefers boolean-style "isX" over "getX" when both are present. The getter must have no + * parameters and its return type must match the provided field type mirror. + * + * @param dtoType the enclosing DTO type element + * @param fieldName the field name (uncapitalized) + * @param fieldTypeMirror the expected return type of the getter + * @param elementUtils elements utility + * @param typeUtils types utility + * @return Optional containing the getter ExecutableElement if found + */ + public static Optional findGetterForField( + TypeElement dtoType, + String fieldName, + TypeMirror fieldTypeMirror, + Elements elementUtils, + Types typeUtils) { + if (dtoType == null || fieldName == null || fieldTypeMirror == null) { + return Optional.empty(); + } + String cap = StringUtils.capitalize(fieldName); + String getterCandidate = "get" + cap; + String booleanGetterCandidate = "is" + cap; + List classMethods = + ElementFilter.methodsIn(elementUtils.getAllMembers(dtoType)); + // Prefer boolean-style getter if present + for (ExecutableElement candidate : classMethods) { + String name = candidate.getSimpleName().toString(); + if ((name.equals(booleanGetterCandidate) || name.equals(getterCandidate)) + && candidate.getParameters().isEmpty() + && typeUtils.isSameType(candidate.getReturnType(), fieldTypeMirror)) { + return Optional.of(candidate); + } + } + return Optional.empty(); + } } From 94ca14fb395f69a2cfc45c33b33fe6200693ba10 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Thu, 25 Sep 2025 00:02:38 +0200 Subject: [PATCH 20/35] Implementation of constructor fields extraction --- .../processor/dtos/BuilderDefinitionDto.java | 12 +++- .../util/BuilderDefinitionCreator.java | 58 +++++++++++++++++-- .../processor/util/JavaCodeGenerator.java | 41 +++++++++---- .../processor/BuilderProcessorTest.java | 15 +++-- 4 files changed, 101 insertions(+), 25 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index a047edf4..2b322c43 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -40,7 +40,7 @@ public class BuilderDefinitionDto { * constructor. Ordering in LinkedList is matching orderung of parameters in constructor call. * Instances of fields are added in LinkedList of all fields too. */ - private final LinkedList fieldsInConstructor = new LinkedList<>(); + private final List fieldsInConstructor = new LinkedList<>(); /** Generic parameters declared on the target DTO (e.g., ). */ private final List generics = new LinkedList<>(); @@ -98,6 +98,11 @@ public void addField(FieldDto field) { fields.add(field); } + /** Adds a field definition that will be provided via the DTO constructor. */ + public void addFieldInConstructor(FieldDto field) { + fieldsInConstructor.add(field); + } + /** * Getting all definied fields in builder definition. * @@ -107,6 +112,11 @@ public LinkedList getSetterFieldsForBuilder() { return fields; } + /** Returns the ordered list of fields to be passed to the DTO constructor. */ + public List getConstructorFieldsForBuilder() { + return fieldsInConstructor; + } + /** Adds a generic parameter definition. */ public void addGeneric(GenericParameterDto generic) { generics.add(generic); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index e5e020a7..e6254752 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -78,8 +78,15 @@ public static BuilderDefinitionDto extractFromElement( JavaLangMapper.map2GenericParameterDtos(annotatedType, elementUtils, typeUtils) .forEach(result::addGeneric); - // Todo: extract Constructor and his fields. result needs to be filled im constructors and - // fields. + // Extract constructors and their parameters (treat as constructor fields) + Optional constructorOpt = findConstructorForBuilder(annotatedType, elementUtils); + if (constructorOpt.isPresent()) { + ExecutableElement ctor = constructorOpt.get(); + for (VariableElement param : ctor.getParameters()) { + Optional fieldFromCtor = createFieldFromConstructor(param, elementUtils, typeUtils); + fieldFromCtor.ifPresent(result::addFieldInConstructor); + } + } // Todo: moving into helper function List allMembers = elementUtils.getAllMembers(annotatedType); @@ -89,7 +96,7 @@ public static BuilderDefinitionDto extractFromElement( // nur public if (isMethodRelevantForBuilder(mth)) { // Todo: we need to check setter-fields for duplication with constructor fields - createFieldDto(mth, result.getGenerics(), elementUtils, typeUtils) + createFieldFromSetter(mth, result.getGenerics(), elementUtils, typeUtils) .ifPresent(result::addField); } } @@ -204,7 +211,7 @@ private static void addSupplierMethodsForField( result.addMethod(createFieldSupplier(fieldName, fieldType)); } - private static Optional createFieldDto( + private static Optional createFieldFromSetter( ExecutableElement mth, List dtoGenerics, Elements elementUtils, @@ -247,7 +254,7 @@ private static Optional createFieldDto( TypeElement dtoType = (TypeElement) mth.getEnclosingElement(); JavaLangAnalyser.findGetterForField( dtoType, fieldName, fieldTypeMirror, elementUtils, typeUtils) - .ifPresent(getter -> result.setGetterName(getter.getSimpleName().toString())); + .ifPresent(getterExecutable -> result.setGetterName(getterExecutable.getSimpleName().toString())); // Finding generics declared on the setter itself (field-specific), e.g., @@ -276,6 +283,47 @@ private static Optional createFieldDto( return Optional.of(result); } + /** + * Determines which constructor to use for builder initialization. Currently selects the + * constructor with the highest number of parameters. Returns empty if no constructor has + * parameters (i.e., only default constructor or none found). + */ + private static Optional findConstructorForBuilder( + TypeElement annotatedType, Elements elementUtils) { + List ctors = + ElementFilter.constructorsIn(elementUtils.getAllMembers(annotatedType)); + ExecutableElement selected = null; + int maxParams = -1; + for (ExecutableElement ctor : ctors) { + int p = ctor.getParameters().size(); + if (p > maxParams) { + maxParams = p; + selected = ctor; + } + } + return (selected != null && maxParams > 0) ? Optional.of(selected) : Optional.empty(); + } + + /** + * Creates a FieldDto from a constructor parameter, including a simple builder setter to supply + * the constructor argument. + */ + private static Optional createFieldFromConstructor( + VariableElement param, Elements elementUtils, Types typeUtils) { + MethodParameterDto paramDto = map2MethodParameter(param, elementUtils, typeUtils); + if (paramDto == null) { + return Optional.empty(); + } + String fieldName = param.getSimpleName().toString(); + TypeName fieldType = paramDto.getParameterType(); + FieldDto ctorField = new FieldDto(); + ctorField.setFieldName(fieldName); + ctorField.setFieldType(fieldType); + // Provide a simple setter in builder to set the constructor argument + ctorField.addMethod(createFieldSetterWithTransform(fieldName, null, fieldType)); + return Optional.of(ctorField); + } + private static MethodDto createFieldSetterWithTransform( String fieldName, String transform, TypeName fieldType) { MethodParameterDto parameter = new MethodParameterDto(); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 6ae03155..386ffca4 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -26,7 +26,6 @@ import static javax.lang.model.element.Modifier.PUBLIC; import static javax.lang.model.element.Modifier.STATIC; -import static org.javahelpers.simple.builders.processor.dtos.MethodTypes.*; import static org.javahelpers.simple.builders.processor.util.JavapoetMapper.*; import com.palantir.javapoet.AnnotationSpec; @@ -97,13 +96,21 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep dtoBaseClass, dtoTypeName, builderDef.getSetterFieldsForBuilder())); classBuilder.addMethod(createEmptyConstructor(dtoBaseClass)); - // Generate backing fields for each DTO field (boxed types to allow null/unset) + // Generate backing fields for each DTO field (constructor and setter fields) + for (FieldDto fieldDto : builderDef.getConstructorFieldsForBuilder()) { + FieldSpec fieldSpec = createFieldMember(fieldDto); + classBuilder.addField(fieldSpec); + } for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { FieldSpec fieldSpec = createFieldMember(fieldDto); classBuilder.addField(fieldSpec); } - // Generate Fields and Fieldspecific funtions in Builder + // Generate field-specific functions in Builder (constructor fields first, then setter fields) + for (FieldDto fieldDto : builderDef.getConstructorFieldsForBuilder()) { + List methodSpecs = createFieldMethods(fieldDto, builderTypeName); + classBuilder.addMethods(methodSpecs); + } for (FieldDto fieldDto : builderDef.getSetterFieldsForBuilder()) { List methodSpecs = createFieldMethods(fieldDto, builderTypeName); classBuilder.addMethods(methodSpecs); @@ -114,6 +121,7 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep createMethodBuild( dtoBaseClass, dtoTypeName, + builderDef.getConstructorFieldsForBuilder(), builderDef.getSetterFieldsForBuilder(), builderDef.getGenerics())); classBuilder.addMethod( @@ -205,7 +213,8 @@ private FieldSpec createFieldMember(FieldDto fieldDto) { private MethodSpec createMethodBuild( ClassName dtoBaseClass, com.palantir.javapoet.TypeName returnType, - List fields, + List constructorFields, + List setterFields, List generics) { MethodSpec.Builder mb = MethodSpec.methodBuilder("build") @@ -213,12 +222,22 @@ private MethodSpec createMethodBuild( .returns(returnType) .addAnnotation(Override.class); + // Build constructor argument list: use backing fields in declared order + String ctorArgs = + constructorFields.stream() + .map(FieldDto::getFieldName) + .map(n -> String.format("this.%s", n)) + .reduce((a, b) -> a + ", " + b) + .orElse(""); + if (generics.isEmpty()) { - mb.addStatement("$1T result = new $1T()", dtoBaseClass); + mb.addStatement("$1T result = new $1T($2L)", dtoBaseClass, ctorArgs); } else { - mb.addStatement("$1T result = new $2T<>()", returnType, dtoBaseClass); + mb.addStatement("$1T result = new $2T<>($3L)", returnType, dtoBaseClass, ctorArgs); } - for (FieldDto f : fields) { + + // Apply setter-based fields + for (FieldDto f : setterFields) { mb.beginControlFlow("if (this.$N != null)", f.getFieldName()) .addStatement("result.$N(this.$N)", f.getSetterName(), f.getFieldName()) .endControlFlow(); @@ -302,20 +321,20 @@ private MethodSpec createMethod( switch (methodDto.getMethodType()) { case PROXY -> methodBuilder.addJavadoc( - "\n@param $1N $2N", paramDto.getParameterName(), optionalFieldParamJavaDoc); + "\n@param $1N $2L", paramDto.getParameterName(), optionalFieldParamJavaDoc); case CONSUMER -> methodBuilder.addJavadoc( - "\n@param $1N consumer providing an instance of $2N", + "\n@param $1N consumer providing an instance of $2L", paramDto.getParameterName(), optionalFieldParamJavaDoc); case CONSUMER_BY_BUILDER -> methodBuilder.addJavadoc( - "\n@param $1N consumer providing an instance of a builder for $2N", + "\n@param $1N consumer providing an instance of a builder for $2L", paramDto.getParameterName(), optionalFieldParamJavaDoc); case SUPPLIER -> methodBuilder.addJavadoc( - "\n@param $1N supplier for $2N", + "\n@param $1N supplier for $2L", paramDto.getParameterName(), optionalFieldParamJavaDoc); } diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 31f5a2a8..c7e2b83c 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -77,8 +77,7 @@ public abstract class AbstractAnnotated { } } @Test - @Disabled("TODO: Constructor-Parameters are not supported yet") - void shouldIgnoreConstructorParametersAndOnlyUseSetters() { + void shouldHandleFieldsOfConstructorAndSetters() { // Given String packageName = "test"; String className = "CtorAndSetter"; @@ -89,10 +88,9 @@ void shouldIgnoreConstructorParametersAndOnlyUseSetters() { packageName, className, """ - private int a; // has no setter, only in constructor + private final int a; // has no setter, only in constructor private String name; // will be set via setter - public CtorAndSetter() {} public CtorAndSetter(int a) { this.a = a; } public String getName() { return name; } @@ -109,10 +107,11 @@ public CtorAndSetter() {} // Setter-based API should appear (name) and Constructor params (a) ProcessorAsserts.assertContaining( generatedCode, - "public CtorAndSetterBuilder a(int a)", - "this.a = a;", - "public CtorAndSetterBuilder name(String name)", - "this.name = name;"); + "public CtorAndSetterBuilder()", // empty constructor for builder + "public CtorAndSetterBuilder name(String name)", // helpermethod for setter + "public CtorAndSetterBuilder a(int a)", // helpermethod for constructor param + "CtorAndSetter result = new CtorAndSetter(this.a);", + "result.setName(this.name);"); } @Test From f9e15dbac8b6b78b4db0d39c53252edbb0d551b8 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Thu, 25 Sep 2025 00:02:57 +0200 Subject: [PATCH 21/35] Adding javadoc for FieldDto --- .../javahelpers/simple/builders/processor/dtos/FieldDto.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java index 0ad44452..7a76b82d 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/FieldDto.java @@ -49,7 +49,10 @@ public class FieldDto { /** Method-level generics declared on the setter, to be reused in builder methods. */ private final List fieldGenerics = new ArrayList<>(); - /** The name of the getter method to call, because in case of boolean it does not need to start with get (e.g., getName or isActive). */ + /** + * The name of the getter method to call, because in case of boolean it does not need to start + * with get (e.g., getName or isActive). + */ private String getterName; /** From 247e834cdd8e520514c76fe88506b3c070799f7b Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Thu, 25 Sep 2025 00:03:28 +0200 Subject: [PATCH 22/35] Fixing codeformat --- .../processor/util/BuilderDefinitionCreator.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index e6254752..b6961a15 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -79,11 +79,13 @@ public static BuilderDefinitionDto extractFromElement( .forEach(result::addGeneric); // Extract constructors and their parameters (treat as constructor fields) - Optional constructorOpt = findConstructorForBuilder(annotatedType, elementUtils); + Optional constructorOpt = + findConstructorForBuilder(annotatedType, elementUtils); if (constructorOpt.isPresent()) { ExecutableElement ctor = constructorOpt.get(); for (VariableElement param : ctor.getParameters()) { - Optional fieldFromCtor = createFieldFromConstructor(param, elementUtils, typeUtils); + Optional fieldFromCtor = + createFieldFromConstructor(param, elementUtils, typeUtils); fieldFromCtor.ifPresent(result::addFieldInConstructor); } } @@ -254,7 +256,8 @@ private static Optional createFieldFromSetter( TypeElement dtoType = (TypeElement) mth.getEnclosingElement(); JavaLangAnalyser.findGetterForField( dtoType, fieldName, fieldTypeMirror, elementUtils, typeUtils) - .ifPresent(getterExecutable -> result.setGetterName(getterExecutable.getSimpleName().toString())); + .ifPresent( + getterExecutable -> result.setGetterName(getterExecutable.getSimpleName().toString())); // Finding generics declared on the setter itself (field-specific), e.g., From 78fbf6c592c2b84eb8f0e51fe4adf197165686e9 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Thu, 25 Sep 2025 00:40:20 +0200 Subject: [PATCH 23/35] Checking behavior if constructor and setter is existing for a field --- .../util/BuilderDefinitionCreator.java | 27 +++++++++-- .../processor/util/JavaCodeGenerator.java | 7 +-- .../processor/BuilderProcessorTest.java | 48 +++++++++++++++++++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index b6961a15..2f33f9a5 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -24,6 +24,7 @@ package org.javahelpers.simple.builders.processor.util; +import static java.util.stream.Collectors.toSet; import static javax.lang.model.type.TypeKind.ARRAY; import static org.javahelpers.simple.builders.processor.util.AnnotationValidator.validateAnnotatedElement; import static org.javahelpers.simple.builders.processor.util.JavaLangAnalyser.*; @@ -33,6 +34,7 @@ import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Consumer; import java.util.function.Supplier; import javax.lang.model.element.*; @@ -85,7 +87,7 @@ public static BuilderDefinitionDto extractFromElement( ExecutableElement ctor = constructorOpt.get(); for (VariableElement param : ctor.getParameters()) { Optional fieldFromCtor = - createFieldFromConstructor(param, elementUtils, typeUtils); + createFieldFromConstructor(annotatedType, param, elementUtils, typeUtils); fieldFromCtor.ifPresent(result::addFieldInConstructor); } } @@ -94,12 +96,24 @@ public static BuilderDefinitionDto extractFromElement( List allMembers = elementUtils.getAllMembers(annotatedType); List methods = ElementFilter.methodsIn(allMembers); + // Build a set of constructor field names to avoid duplicates from setters + Set ctorFieldNames = + result.getConstructorFieldsForBuilder().stream() + .map(FieldDto::getFieldName) + .collect(toSet()); + for (ExecutableElement mth : methods) { // nur public if (isMethodRelevantForBuilder(mth)) { - // Todo: we need to check setter-fields for duplication with constructor fields - createFieldFromSetter(mth, result.getGenerics(), elementUtils, typeUtils) - .ifPresent(result::addField); + // Avoid adding setter-derived fields that duplicate constructor params + Optional maybeField = + createFieldFromSetter(mth, result.getGenerics(), elementUtils, typeUtils); + if (maybeField.isPresent()) { + FieldDto field = maybeField.get(); + if (!ctorFieldNames.contains(field.getFieldName())) { + result.addField(field); + } + } } } @@ -312,7 +326,7 @@ private static Optional findConstructorForBuilder( * the constructor argument. */ private static Optional createFieldFromConstructor( - VariableElement param, Elements elementUtils, Types typeUtils) { + TypeElement dtoType, VariableElement param, Elements elementUtils, Types typeUtils) { MethodParameterDto paramDto = map2MethodParameter(param, elementUtils, typeUtils); if (paramDto == null) { return Optional.empty(); @@ -322,6 +336,9 @@ private static Optional createFieldFromConstructor( FieldDto ctorField = new FieldDto(); ctorField.setFieldName(fieldName); ctorField.setFieldType(fieldType); + // Find matching getter on the DTO type + JavaLangAnalyser.findGetterForField(dtoType, fieldName, param.asType(), elementUtils, typeUtils) + .ifPresent(getter -> ctorField.setGetterName(getter.getSimpleName().toString())); // Provide a simple setter in builder to set the constructor argument ctorField.addMethod(createFieldSetterWithTransform(fieldName, null, fieldType)); return Optional.of(ctorField); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 386ffca4..7cd0605f 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -91,9 +91,10 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep .addSuperinterface(createInterfaceBuilderBase(dtoTypeName)); // Adding Constructors for builder - classBuilder.addMethod( - createConstructorWithInstance( - dtoBaseClass, dtoTypeName, builderDef.getSetterFieldsForBuilder())); + java.util.List initFields = new java.util.LinkedList<>(); + initFields.addAll(builderDef.getConstructorFieldsForBuilder()); + initFields.addAll(builderDef.getSetterFieldsForBuilder()); + classBuilder.addMethod(createConstructorWithInstance(dtoBaseClass, dtoTypeName, initFields)); classBuilder.addMethod(createEmptyConstructor(dtoBaseClass)); // Generate backing fields for each DTO field (constructor and setter fields) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index c7e2b83c..3c8e0a53 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -114,6 +114,54 @@ void shouldHandleFieldsOfConstructorAndSetters() { "result.setName(this.name);"); } + @Test + void shouldPreferConstructorOverSetterWhenBothExist() { + // Given + String packageName = "test"; + String className = "CtorAndSetterSameField"; + String builderClassName = className + "Builder"; + + JavaFileObject sourceFile = + ProcessorTestUtils.simpleBuilderClass( + packageName, + className, + """ + private int a; // could be set via constructor and setter + private String name; + + public CtorAndSetterSameField(int a) { this.a = a; } + + public int getA() { return a; } + public void setA(int a) { this.a = a; } + + public String getName() { return name; } + public void setName(String name) { this.name = name; } + """); + + // When + Compilation compilation = compile(sourceFile); + + // Then + String generatedCode = loadGeneratedSource(compilation, builderClassName); + assertGenerationSucceeded(compilation, builderClassName, generatedCode); + // Expect helper for constructor param and no duplicate setter application for 'a' + ProcessorAsserts.assertContaining( + generatedCode, + "public CtorAndSetterSameFieldBuilder()", + // Expectations on constructor of builder with instance + "public CtorAndSetterSameFieldBuilder(CtorAndSetterSameField instance)", + "this.name = instance.getName();", + "this.a = instance.getA();", + // Expectations on helper functions + "public CtorAndSetterSameFieldBuilder a(int a)", + "public CtorAndSetterSameFieldBuilder name(String name)", + // Expectations on build function + "CtorAndSetterSameField result = new CtorAndSetterSameField(this.a);", + "result.setName(this.name);"); + // expect no setter application for 'a' on build-function + ProcessorAsserts.assertNotContaining(generatedCode, "result.setA(this.a);"); + } + @Test @Disabled("TODO: Records are not supported for builder generation yet") void shouldHandleRecordWithoutGeneratingBuilder() { From 47d6b53d6241cd8876e8f7c30bbb56eba741385e Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Thu, 25 Sep 2025 00:42:54 +0200 Subject: [PATCH 24/35] Refactoring code with a helper function in BuilderDef, which provides all fields --- .../builders/processor/dtos/BuilderDefinitionDto.java | 11 +++++++++++ .../builders/processor/util/JavaCodeGenerator.java | 7 +++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index 2b322c43..d17c5639 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -117,6 +117,17 @@ public List getConstructorFieldsForBuilder() { return fieldsInConstructor; } + /** + * Returns all fields known to the builder in a single list. Constructor parameters appear first + * (in constructor order), followed by setter-derived fields. + */ + public List getAllFieldsForBuilder() { + LinkedList all = new LinkedList<>(); + all.addAll(fieldsInConstructor); + all.addAll(fields); + return all; + } + /** Adds a generic parameter definition. */ public void addGeneric(GenericParameterDto generic) { generics.add(generic); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index 7cd0605f..ef93ffc5 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -91,10 +91,9 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep .addSuperinterface(createInterfaceBuilderBase(dtoTypeName)); // Adding Constructors for builder - java.util.List initFields = new java.util.LinkedList<>(); - initFields.addAll(builderDef.getConstructorFieldsForBuilder()); - initFields.addAll(builderDef.getSetterFieldsForBuilder()); - classBuilder.addMethod(createConstructorWithInstance(dtoBaseClass, dtoTypeName, initFields)); + classBuilder.addMethod( + createConstructorWithInstance( + dtoBaseClass, dtoTypeName, builderDef.getAllFieldsForBuilder())); classBuilder.addMethod(createEmptyConstructor(dtoBaseClass)); // Generate backing fields for each DTO field (constructor and setter fields) From 9559df4cb42e3e3657c873fa8fa9971a3494794c Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Tue, 7 Oct 2025 22:10:30 +0200 Subject: [PATCH 25/35] Replacing rawType of ArrayList by generic one --- .../simple/builders/core/builders/ArrayListBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/builders/ArrayListBuilder.java b/core/src/main/java/org/javahelpers/simple/builders/core/builders/ArrayListBuilder.java index 6e394cc9..6b5d13d6 100644 --- a/core/src/main/java/org/javahelpers/simple/builders/core/builders/ArrayListBuilder.java +++ b/core/src/main/java/org/javahelpers/simple/builders/core/builders/ArrayListBuilder.java @@ -66,6 +66,6 @@ public ArrayListBuilder addAll(List elements) { * @return new ArrayList holding all elements which have been added before */ public List build() { - return new ArrayList(mList); + return new ArrayList<>(mList); } } From 12719f144daf037c5984d388a63068fb1ae9f673 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Tue, 7 Oct 2025 23:07:49 +0200 Subject: [PATCH 26/35] Wrapp all fields into a record to decide on changed and unchanged values --- .../builders/core/util/TrackedValue.java | 24 +++++ .../builders/processor/dtos/TypeName.java | 4 + .../util/BuilderDefinitionCreator.java | 13 ++- .../processor/util/JavaCodeGenerator.java | 34 +++++-- .../processor/BuilderProcessorTest.java | 93 ++++++++++--------- 5 files changed, 112 insertions(+), 56 deletions(-) create mode 100644 core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java b/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java new file mode 100644 index 00000000..70e728c9 --- /dev/null +++ b/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java @@ -0,0 +1,24 @@ +package org.javahelpers.simple.builders.core.util; + +import java.util.function.Consumer; + +public record TrackedValue(T value, boolean isChanged) { + + public void ifChanged(Consumer consumer) { + if (isChanged) { + consumer.accept(value); + } + } + + public static TrackedValue unsetValue() { + return new TrackedValue<>(null, false); + } + + public static TrackedValue initialValue(T value) { + return new TrackedValue<>(value, false); + } + + public static TrackedValue changedValue(T value) { + return new TrackedValue<>(value, true); + } +} diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java index a62c567a..676a5a06 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java @@ -79,4 +79,8 @@ public String getClassName() { public Optional getInnerType() { return Optional.empty(); } + + public static TypeName of(Class clazz) { + return new TypeName(clazz.getPackage().getName(), clazz.getSimpleName()); + } } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 2f33f9a5..c4f062b5 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -49,6 +49,7 @@ import org.javahelpers.simple.builders.core.builders.ArrayListBuilder; import org.javahelpers.simple.builders.core.builders.HashMapBuilder; import org.javahelpers.simple.builders.core.builders.HashSetBuilder; +import org.javahelpers.simple.builders.core.util.TrackedValue; import org.javahelpers.simple.builders.processor.dtos.*; import org.javahelpers.simple.builders.processor.exceptions.BuilderException; @@ -362,11 +363,12 @@ private static MethodDto createFieldSetterWithTransform( } methodDto.setCode( """ - this.$fieldName:N = $dtoMethodParams:N; + this.$fieldName:N = $builderFieldWrapper:T.changedValue($dtoMethodParams:N); return this; """); methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParams", params); + methodDto.addArgument("builderFieldWrapper", TypeName.of(TrackedValue.class)); return methodDto; } @@ -384,12 +386,13 @@ private static MethodDto createFieldConsumer(String fieldName, TypeName fieldTyp """ $helperType:T consumer = new $helperType:T(); $dtoMethodParam:N.accept(consumer); - this.$fieldName:N = consumer; + this.$fieldName:N = $builderFieldWrapper:T.changedValue(consumer); return this; """); methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); methodDto.addArgument("helperType", fieldType); + methodDto.addArgument("builderFieldWrapper", TypeName.of(TrackedValue.class)); return methodDto; } @@ -413,12 +416,13 @@ private static MethodDto createFieldConsumerWithBuilder(String fieldName, TypeNa """ $helperType:T builder = new $helperType:T(); $dtoMethodParam:N.accept(builder); - this.$fieldName:N = builder.build(); + this.$fieldName:N = $builderFieldWrapper:T.changedValue(builder.build()); return this; """); methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); methodDto.addArgument("helperType", builderType); + methodDto.addArgument("builderFieldWrapper", TypeName.of(TrackedValue.class)); return methodDto; } @@ -434,11 +438,12 @@ private static MethodDto createFieldSupplier(String fieldName, TypeName fieldTyp methodDto.setMethodType(MethodTypes.SUPPLIER); methodDto.setCode( """ - this.$fieldName:N = $dtoMethodParam:N.get(); + this.$fieldName:N = $builderFieldWrapper:T.changedValue($dtoMethodParam:N.get()); return this; """); methodDto.addArgument("fieldName", fieldName); methodDto.addArgument("dtoMethodParam", parameter.getParameterName()); + methodDto.addArgument("builderFieldWrapper", TypeName.of(TrackedValue.class)); return methodDto; } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index ef93ffc5..a182a01d 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -43,6 +43,7 @@ import javax.lang.model.element.Modifier; import org.javahelpers.simple.builders.core.annotations.BuilderImplementation; import org.javahelpers.simple.builders.core.interfaces.IBuilderBase; +import org.javahelpers.simple.builders.core.util.TrackedValue; import org.javahelpers.simple.builders.processor.dtos.*; import org.javahelpers.simple.builders.processor.exceptions.BuilderException; @@ -137,7 +138,13 @@ public void generateBuilder(BuilderDefinitionDto builderDef) throws BuilderExcep private void writeClassToFile(String packageName, TypeSpec typeSpec) throws BuilderException { try { - JavaFile.builder(packageName, typeSpec).skipJavaLangImports(true).build().writeTo(filer); + JavaFile.builder(packageName, typeSpec) + .skipJavaLangImports(true) + .addStaticImport(TrackedValue.class, "initialValue") + .addStaticImport(TrackedValue.class, "changedValue") + .addStaticImport(TrackedValue.class, "unsetValue") + .build() + .writeTo(filer); } catch (IOException ex) { throw new BuilderException(null, ex); } @@ -197,7 +204,12 @@ private MethodSpec createConstructorWithInstance( for (FieldDto f : fields) { f.getGetterName() .ifPresent( - getter -> cb.addStatement("this.$N = instance.$N()", f.getFieldName(), getter)); + getter -> + cb.addStatement( + "this.$N = $T.initialValue(instance.$N())", + f.getFieldName(), + ClassName.get(TrackedValue.class), + getter)); } return cb.build(); } @@ -207,7 +219,13 @@ private FieldSpec createFieldMember(FieldDto fieldDto) { if (fieldType.isPrimitive()) { fieldType = fieldType.box(); } - return FieldSpec.builder(fieldType, fieldDto.getFieldName(), Modifier.PRIVATE).build(); + // Wrap all fields in TrackedValue + ClassName builderFieldWrapper = ClassName.get(TrackedValue.class); + ParameterizedTypeName wrappedFieldType = + ParameterizedTypeName.get(builderFieldWrapper, fieldType); + return FieldSpec.builder(wrappedFieldType, fieldDto.getFieldName(), Modifier.PRIVATE) + .initializer("$T.unsetValue()", builderFieldWrapper) + .build(); } private MethodSpec createMethodBuild( @@ -222,11 +240,11 @@ private MethodSpec createMethodBuild( .returns(returnType) .addAnnotation(Override.class); - // Build constructor argument list: use backing fields in declared order + // Build constructor argument list: use backing fields' values in declared order String ctorArgs = constructorFields.stream() .map(FieldDto::getFieldName) - .map(n -> String.format("this.%s", n)) + .map(n -> String.format("this.%s.value()", n)) .reduce((a, b) -> a + ", " + b) .orElse(""); @@ -236,11 +254,9 @@ private MethodSpec createMethodBuild( mb.addStatement("$1T result = new $2T<>($3L)", returnType, dtoBaseClass, ctorArgs); } - // Apply setter-based fields + // Apply setter-based fields only when changed for (FieldDto f : setterFields) { - mb.beginControlFlow("if (this.$N != null)", f.getFieldName()) - .addStatement("result.$N(this.$N)", f.getSetterName(), f.getFieldName()) - .endControlFlow(); + mb.addStatement("this.$N.ifChanged(result::$N)", f.getFieldName(), f.getSetterName()); } mb.addStatement("return result"); return mb.build(); diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 3c8e0a53..ec1cbb85 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -110,8 +110,8 @@ void shouldHandleFieldsOfConstructorAndSetters() { "public CtorAndSetterBuilder()", // empty constructor for builder "public CtorAndSetterBuilder name(String name)", // helpermethod for setter "public CtorAndSetterBuilder a(int a)", // helpermethod for constructor param - "CtorAndSetter result = new CtorAndSetter(this.a);", - "result.setName(this.name);"); + "CtorAndSetter result = new CtorAndSetter(this.a.value());", + "this.name.ifChanged(result::setName);"); } @Test @@ -150,14 +150,14 @@ void shouldPreferConstructorOverSetterWhenBothExist() { "public CtorAndSetterSameFieldBuilder()", // Expectations on constructor of builder with instance "public CtorAndSetterSameFieldBuilder(CtorAndSetterSameField instance)", - "this.name = instance.getName();", - "this.a = instance.getA();", + "this.name = initialValue(instance.getName());", + "this.a = initialValue(instance.getA());", // Expectations on helper functions "public CtorAndSetterSameFieldBuilder a(int a)", "public CtorAndSetterSameFieldBuilder name(String name)", // Expectations on build function - "CtorAndSetterSameField result = new CtorAndSetterSameField(this.a);", - "result.setName(this.name);"); + "CtorAndSetterSameField result = new CtorAndSetterSameField(this.a.value());", + "this.name.ifChanged(result::setName);"); // expect no setter application for 'a' on build-function ProcessorAsserts.assertNotContaining(generatedCode, "result.setA(this.a);"); } @@ -236,17 +236,17 @@ void shouldGenerateCollectionSettersAndProviders() { "public WithCollectionsBuilder names(List names)", "public WithCollectionsBuilder names(Supplier> namesSupplier)", "public WithCollectionsBuilder names(String... names)", - "this.names = List.of(names);", - "private List names;", + "this.names = changedValue(List.of(names));", + "private TrackedValue> names = unsetValue();", "public WithCollectionsBuilder tags(Set tags)", "public WithCollectionsBuilder tags(Supplier> tagsSupplier)", "public WithCollectionsBuilder tags(String... tags)", - "this.tags = Set.of(tags);", - "private Set tags;", + "this.tags = changedValue(Set.of(tags));", + "private TrackedValue> tags = unsetValue();", "public WithCollectionsBuilder map(Map map)", "public WithCollectionsBuilder map(Supplier> mapSupplier)", - "this.map = map;", - "private Map map;"); + "this.map = changedValue(map);", + "private TrackedValue> map = unsetValue();"); } @Test @@ -271,8 +271,9 @@ void shouldDeclarePerFieldBackingField() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); - // Expect a private boxed backing field for primitive int setter - ProcessorAsserts.assertContaining(generatedCode, "private Integer x;"); + // Expect a private tracked backing field for primitive int setter + ProcessorAsserts.assertContaining( + generatedCode, "private TrackedValue x = unsetValue();"); ProcessorAsserts.assertNotContaining(generatedCode, " FieldDoc instance;"); } @@ -338,9 +339,7 @@ void shouldGenerateBuildOverrideAndReturnAnInstanceWithCalledSetters() { @Override public BuildDoc build() { BuildDoc result = new BuildDoc(); - if (this.x != null) { - result.setX(this.x); - } + this.x.ifChanged(result::setX); return result; } """); @@ -371,7 +370,7 @@ void shouldReturnThisFromFluentMethods() { ProcessorAsserts.assertContaining( generatedCode, "public FluentReturnBuilder name(String name)", - "this.name = name;", + "this.name = changedValue(name);", "return this;"); } @@ -519,7 +518,7 @@ void shouldGenerateBothConstructorsWithJavadocs() { "@param instance object instance for initialisiation", "public CtorDocBuilder()", "public CtorDocBuilder(CtorDoc instance)", - "private String x;"); + "private TrackedValue x = unsetValue();"); } @Test @@ -574,7 +573,9 @@ public HelperPlain() {} // Backing fields present // TODO: Add Javadoc to fields ProcessorAsserts.assertContaining( - generatedCode, "private String name;", "private HelperPlain helperPlain;"); + generatedCode, + "private TrackedValue name = unsetValue();", + "private TrackedValue helperPlain = unsetValue();"); } @Test @@ -609,7 +610,7 @@ void shouldPropagateFieldJavadocToBuilderParameter() { generatedCode, "@param name the person name", "public FieldWithDocBuilder name(String name)", - "private String name;"); + "private TrackedValue name = unsetValue();"); } @Test @@ -651,11 +652,11 @@ public HelperAnno() {} // Expect consumer-by-builder method using HelperAnnoBuilder and builder.build() ProcessorAsserts.assertContaining( generatedCode, - "private HelperAnno helper;", + "private TrackedValue helper = unsetValue();", "helperBuilderConsumer", "HelperAnnoBuilder builder = new HelperAnnoBuilder();", "helperBuilderConsumer.accept(builder);", - "this.helper = builder.build();"); + "this.helper = changedValue(builder.build());"); } @Test @@ -697,7 +698,7 @@ public HelperPlain() {} generatedCode, "HelperPlain consumer = new HelperPlain();", "helperPlainConsumer.accept(consumer);", - "private HelperPlain helperPlain;"); + "private TrackedValue helperPlain = unsetValue();"); } @Test @@ -727,7 +728,10 @@ void shouldGenerateSupplierMethodForField() { // Expect supplier-based setter usage ProcessorAsserts.assertContaining( - generatedCode, "nameSupplier", "private String name;", "this.name = nameSupplier.get();"); + generatedCode, + "nameSupplier", + "private TrackedValue name = unsetValue();", + "this.name = changedValue(nameSupplier.get());"); } protected Compilation compile(JavaFileObject... sourceFiles) { @@ -804,11 +808,13 @@ public void setAge(int age) { generatedCode, "public PersonBuilder name(String name)", "public PersonBuilder age(int age)", - "private String name;", - "private Integer age;"); + "private TrackedValue name = unsetValue();", + "private TrackedValue age = unsetValue();"); } @Test + // TODO: Replace Optional with other generic wrapper type, because for optional there is a + // feature-request void shouldHandleGenericWrapperTypes() { JavaFileObject optionalWrapperClass = @@ -863,9 +869,9 @@ public class OptionalWrapper { // Backing fields present ProcessorAsserts.assertContaining( optionalBuilder, - "private Optional name;", - "private Optional count;", - "private Optional date;"); + "private TrackedValue> name = unsetValue();", + "private TrackedValue> count = unsetValue();", + "private TrackedValue> date = unsetValue();"); } @Test @@ -908,7 +914,7 @@ public Helper() {} ProcessorAsserts.assertContaining( generatedCode, "public UsesOtherPackageHelperBuilder helper(Helper helper)", - "private Helper helper;"); + "private TrackedValue helper = unsetValue();"); } @Test @@ -939,13 +945,13 @@ void shouldHandleSetOfStrings() { ProcessorAsserts.assertContaining( generatedCode, "public HasSetStringBuilder tags(Set tags)", - "this.tags = tags;", + "this.tags = changedValue(tags);", "public HasSetStringBuilder tags(Consumer> tagsBuilderConsumer)", - "this.tags = builder.build();", + "this.tags = changedValue(builder.build());", "public HasSetStringBuilder tags(String... tags)", - "this.tags = Set.of(tags);", + "this.tags = changedValue(Set.of(tags));", "public HasSetStringBuilder tags(Supplier> tagsSupplier)", - "this.tags = tagsSupplier.get();"); + "this.tags = changedValue(tagsSupplier.get());"); } @Test @@ -984,13 +990,13 @@ public Helper() {} ProcessorAsserts.assertContaining( generatedCode, "public HasSetCustomBuilder helpers(Set helpers)", - "this.helpers = helpers;", + "this.helpers = changedValue(helpers);", "public HasSetCustomBuilder helpers(Consumer> helpersBuilderConsumer)", - "this.helpers = builder.build();", + "this.helpers = changedValue(builder.build());", "public HasSetCustomBuilder helpers(Helper... helpers)", - "this.helpers = Set.of(helpers);", + "this.helpers = changedValue(Set.of(helpers));", "public HasSetCustomBuilder helpers(Supplier> helpersSupplier)", - "this.helpers = helpersSupplier.get();"); + "this.helpers = changedValue(helpersSupplier.get());"); } @Test @@ -1065,7 +1071,7 @@ public void setRisky(int risk) throws Exception {} ProcessorAsserts.assertingResult( generatedCode, contains("public HasVariousMethodsBuilder ok(int ok)"), - contains("this.ok = ok;"), + contains("this.ok = changedValue(ok);"), notContains("nonSetterMethods"), notContains("settingChanged"), notContains("hidden"), @@ -1245,8 +1251,8 @@ void shouldGenerateVarargsConvenienceForCollections() { assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( generatedCode, - "public HasCollectionsConvenienceBuilder names(String... names) { this.names = List.of(names);", - "public HasCollectionsConvenienceBuilder tags(String... tags) { this.tags = Set.of(tags);"); + "public HasCollectionsConvenienceBuilder names(String... names) { this.names = changedValue(List.of(names));", + "public HasCollectionsConvenienceBuilder tags(String... tags) { this.tags = changedValue(Set.of(tags));"); } @Test @@ -1274,7 +1280,8 @@ void shouldHandleJavaTimeField() { String generatedCode = loadGeneratedSource(compilation, builderClassName); assertGenerationSucceeded(compilation, builderClassName, generatedCode); ProcessorAsserts.assertContaining( - generatedCode, "public HasLocalDateBuilder date(LocalDate date) { this.date = date;"); + generatedCode, + "public HasLocalDateBuilder date(LocalDate date) { this.date = changedValue(date);"); } @Test From 4103167737b87d7e02883a74504efd8eca26ecf3 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Tue, 7 Oct 2025 23:12:36 +0200 Subject: [PATCH 27/35] Adding a test for a class with non-empty constructor and final fields --- .../processor/BuilderProcessorTest.java | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index ec1cbb85..12081dbe 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -76,6 +76,52 @@ public abstract class AbstractAnnotated { } "Builder should not be generated for abstract class target"); } + @Test + void shouldGenerateBuilderForNonEmptyConstructorWithFinalFields() { + // Given + String packageName = "test"; + String className = "NonEmptyCtorFinals"; + String builderClassName = className + "Builder"; + + JavaFileObject sourceFile = + ProcessorTestUtils.simpleBuilderClass( + packageName, + className, + """ + private final String name; + private final int age; + + public NonEmptyCtorFinals(String name, int age) { + this.name = name; + this.age = age; + } + + public String getName() { return name; } + public int getAge() { return age; } + """); + + // When + Compilation compilation = compile(sourceFile); + + // Then + String generatedCode = loadGeneratedSource(compilation, builderClassName); + assertGenerationSucceeded(compilation, builderClassName, generatedCode); + + // Expect helper methods for constructor params and proper build invocation + ProcessorAsserts.assertContaining( + generatedCode, + "public NonEmptyCtorFinalsBuilder()", + "public NonEmptyCtorFinalsBuilder(NonEmptyCtorFinals instance)", + "this.name = initialValue(instance.getName());", + "this.age = initialValue(instance.getAge());", + "public NonEmptyCtorFinalsBuilder name(String name)", + "public NonEmptyCtorFinalsBuilder age(int age)", + "NonEmptyCtorFinals result = new NonEmptyCtorFinals(this.name.value(), this.age.value());"); + + // Ensure no setter calls are emitted for final fields + ProcessorAsserts.assertNotContaining(generatedCode, "result.setName", "result.setAge"); + } + @Test void shouldHandleFieldsOfConstructorAndSetters() { // Given From c625013e79493e33fb9d5eee3a00d2588c2a41ad Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Tue, 7 Oct 2025 23:19:20 +0200 Subject: [PATCH 28/35] Adding JavaDoc --- .../builders/core/util/TrackedValue.java | 69 +++++++++++++++++++ .../builders/processor/dtos/TypeName.java | 6 ++ 2 files changed, 75 insertions(+) diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java b/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java index 70e728c9..8d4f6854 100644 --- a/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java +++ b/core/src/main/java/org/javahelpers/simple/builders/core/util/TrackedValue.java @@ -1,23 +1,92 @@ +/* + * MIT License + * + * Copyright (c) 2025 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.core.util; import java.util.function.Consumer; +/** + * Represents a value together with a flag indicating whether it has been explicitly changed. + * + *

This utility is used by generated builders to differentiate between: + * + *

    + *
  • an unset value (no change made), + *
  • an initial value (derived from an existing instance), and + *
  • a changed value (explicitly set via a builder method). + *
+ * + * The {@code isChanged} flag controls whether setter invocations should be performed during {@code + * build()}; only changed values are applied. + * + * @param the value type + * @param value the underlying value (may be {@code null}) + * @param isChanged whether the value was explicitly changed by the builder API + */ public record TrackedValue(T value, boolean isChanged) { + /** + * Executes the provided {@link Consumer} only if this value has been explicitly changed. + * + * @param consumer action to perform with the current {@link #value()} when {@link #isChanged()} + * is {@code true} + */ public void ifChanged(Consumer consumer) { if (isChanged) { consumer.accept(value); } } + /** + * Creates a tracked value representing an unset state (no change made). + * + * @param the value type + * @return an instance with {@code value == null} and {@code isChanged == false} + */ public static TrackedValue unsetValue() { return new TrackedValue<>(null, false); } + /** + * Creates a tracked value representing an initial value copied from an existing instance. The + * value is present but is not considered a change initiated by the builder. + * + * @param the value type + * @param value the initial value (may be {@code null}) + * @return an instance with the given value and {@code isChanged == false} + */ public static TrackedValue initialValue(T value) { return new TrackedValue<>(value, false); } + /** + * Creates a tracked value representing an explicit change performed via the builder API. + * + * @param the value type + * @param value the changed value (may be {@code null}) + * @return an instance with the given value and {@code isChanged == true} + */ public static TrackedValue changedValue(T value) { return new TrackedValue<>(value, true); } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java index 676a5a06..77e83715 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/TypeName.java @@ -80,6 +80,12 @@ public Optional getInnerType() { return Optional.empty(); } + /** + * Creates a new instance of {@code TypeName} from an existing {@code Class}. + * + * @param clazz existing class to retrieve packe and class-name from + * @return + */ public static TypeName of(Class clazz) { return new TypeName(clazz.getPackage().getName(), clazz.getSimpleName()); } From 9c874cff11f2d78dbaf0b4499ec0deea7fdb0e39 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 21:57:44 +0200 Subject: [PATCH 29/35] Adding implementation for usage of SimpleBuilderConstructor annotation --- .../annotations/SimpleBuilderConstructor.java | 2 +- .../util/BuilderDefinitionCreator.java | 17 +++++- .../processor/BuilderProcessorTest.java | 61 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java index cdaa92dc..ad05a2b4 100644 --- a/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java +++ b/core/src/main/java/org/javahelpers/simple/builders/core/annotations/SimpleBuilderConstructor.java @@ -33,6 +33,6 @@ * Annotation to be placed on a constructor, if a specific constructor should be used for building * the resulting DTO. */ -@Target({ElementType.METHOD}) +@Target({ElementType.CONSTRUCTOR}) @Retention(RetentionPolicy.CLASS) public @interface SimpleBuilderConstructor {} diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index c4f062b5..faa988f9 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -46,6 +46,7 @@ import org.apache.commons.lang3.StringUtils; import org.javahelpers.simple.builders.core.annotations.IgnoreInBuilder; import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; +import org.javahelpers.simple.builders.core.annotations.SimpleBuilderConstructor; import org.javahelpers.simple.builders.core.builders.ArrayListBuilder; import org.javahelpers.simple.builders.core.builders.HashMapBuilder; import org.javahelpers.simple.builders.core.builders.HashSetBuilder; @@ -302,14 +303,24 @@ private static Optional createFieldFromSetter( } /** - * Determines which constructor to use for builder initialization. Currently selects the - * constructor with the highest number of parameters. Returns empty if no constructor has - * parameters (i.e., only default constructor or none found). + * Determines which constructor to use for builder initialization. Prioritizes constructors + * annotated with {@link SimpleBuilderConstructor}. If none is annotated, selects the constructor + * with the highest number of parameters. Returns empty if no constructor has parameters (i.e., + * only default constructor or none found). */ private static Optional findConstructorForBuilder( TypeElement annotatedType, Elements elementUtils) { List ctors = ElementFilter.constructorsIn(elementUtils.getAllMembers(annotatedType)); + + // First, check if any constructor is annotated with @SimpleBuilderConstructor + for (ExecutableElement ctor : ctors) { + if (ctor.getAnnotation(SimpleBuilderConstructor.class) != null) { + return Optional.of(ctor); + } + } + + // Fall back to heuristic: select constructor with the most parameters ExecutableElement selected = null; int maxParams = -1; for (ExecutableElement ctor : ctors) { diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 12081dbe..bb54bd81 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -208,6 +208,67 @@ void shouldPreferConstructorOverSetterWhenBothExist() { ProcessorAsserts.assertNotContaining(generatedCode, "result.setA(this.a);"); } + @Test + void shouldUseAnnotatedConstructorWhenMultipleConstructorsExist() { + // Given + String builderClassName = "AnnotatedCtorChoiceBuilder"; + + JavaFileObject sourceFile = + ProcessorTestUtils.forSource( + """ + package test; + + import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; + import org.javahelpers.simple.builders.core.annotations.SimpleBuilderConstructor; + + @SimpleBuilder + public class AnnotatedCtorChoice { + private int a; + private String name; + private boolean flag; + + @SimpleBuilderConstructor + public AnnotatedCtorChoice(int a, String name) { + this.a = a; + this.name = name; + } + + // Alternative constructor that must NOT be used by the builder + public AnnotatedCtorChoice(int a, String name, boolean flag) { + this.a = a; + this.name = name; + this.flag = flag; + } + + public int getA() { return a; } + public String getName() { return name; } + public boolean isFlag() { return flag; } + + public void setName(String name) { this.name = name; } + } + """); + + // When + Compilation compilation = compile(sourceFile); + + // Then + String generatedCode = loadGeneratedSource(compilation, builderClassName); + assertGenerationSucceeded(compilation, builderClassName, generatedCode); + + // Builder must use the annotated (a, name) constructor + ProcessorAsserts.assertContaining( + generatedCode, + "public AnnotatedCtorChoiceBuilder a(int a)", + "public AnnotatedCtorChoiceBuilder name(String name)", + "AnnotatedCtorChoice result = new AnnotatedCtorChoice(this.a.value(), this.name.value());"); + + // Ensure the alternative constructor (with flag) is not used nor exposed via helpers + ProcessorAsserts.assertNotContaining( + generatedCode, + "public AnnotatedCtorChoiceBuilder flag", + "new AnnotatedCtorChoice(this.a.value(), this.flag.value())"); + } + @Test @Disabled("TODO: Records are not supported for builder generation yet") void shouldHandleRecordWithoutGeneratingBuilder() { From db88d6842107cf866d00264ad378ed59d909ca47 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 22:12:29 +0200 Subject: [PATCH 30/35] Refactoring solution to improve code quality --- .../processor/dtos/BuilderDefinitionDto.java | 4 +-- .../util/BuilderDefinitionCreator.java | 32 ----------------- .../processor/util/JavaLangAnalyser.java | 36 +++++++++++++++++++ 3 files changed, 38 insertions(+), 34 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java index d17c5639..f8d54a84 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/dtos/BuilderDefinitionDto.java @@ -49,7 +49,7 @@ public class BuilderDefinitionDto { * List of all fields of target DTO which are supported by builder. A field could be supported by * multiple functions in builder. */ - private final LinkedList fields = new LinkedList<>(); + private final List fields = new LinkedList<>(); /** * Getting type of builder. @@ -108,7 +108,7 @@ public void addFieldInConstructor(FieldDto field) { * * @return list of fields with type {@code org.javahelpers.simple.builders.internal.dtos.FieldDto} */ - public LinkedList getSetterFieldsForBuilder() { + public List getSetterFieldsForBuilder() { return fields; } diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index faa988f9..4560dccb 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -46,7 +46,6 @@ import org.apache.commons.lang3.StringUtils; import org.javahelpers.simple.builders.core.annotations.IgnoreInBuilder; import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; -import org.javahelpers.simple.builders.core.annotations.SimpleBuilderConstructor; import org.javahelpers.simple.builders.core.builders.ArrayListBuilder; import org.javahelpers.simple.builders.core.builders.HashMapBuilder; import org.javahelpers.simple.builders.core.builders.HashSetBuilder; @@ -302,37 +301,6 @@ private static Optional createFieldFromSetter( return Optional.of(result); } - /** - * Determines which constructor to use for builder initialization. Prioritizes constructors - * annotated with {@link SimpleBuilderConstructor}. If none is annotated, selects the constructor - * with the highest number of parameters. Returns empty if no constructor has parameters (i.e., - * only default constructor or none found). - */ - private static Optional findConstructorForBuilder( - TypeElement annotatedType, Elements elementUtils) { - List ctors = - ElementFilter.constructorsIn(elementUtils.getAllMembers(annotatedType)); - - // First, check if any constructor is annotated with @SimpleBuilderConstructor - for (ExecutableElement ctor : ctors) { - if (ctor.getAnnotation(SimpleBuilderConstructor.class) != null) { - return Optional.of(ctor); - } - } - - // Fall back to heuristic: select constructor with the most parameters - ExecutableElement selected = null; - int maxParams = -1; - for (ExecutableElement ctor : ctors) { - int p = ctor.getParameters().size(); - if (p > maxParams) { - maxParams = p; - selected = ctor; - } - } - return (selected != null && maxParams > 0) ? Optional.of(selected) : Optional.empty(); - } - /** * Creates a FieldDto from a constructor parameter, including a simple builder setter to supply * the constructor argument. diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java index 5629e3fe..1395bca4 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaLangAnalyser.java @@ -42,6 +42,7 @@ import javax.lang.model.util.Types; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; +import org.javahelpers.simple.builders.core.annotations.SimpleBuilderConstructor; /** Helperclass for extrating specific information from existing classes. */ public final class JavaLangAnalyser { @@ -285,4 +286,39 @@ public static Optional findGetterForField( } return Optional.empty(); } + + /** + * Determines which constructor to use for builder initialization. Prioritizes constructors + * annotated with {@link SimpleBuilderConstructor}. If none is annotated, selects the constructor + * with the highest number of parameters. Returns empty if no constructor has parameters (i.e., + * only default constructor or none found). + * + * @param annotatedType the type element to search for constructors + * @param elementUtils elements utility + * @return Optional containing the selected constructor, or empty if none suitable + */ + public static Optional findConstructorForBuilder( + TypeElement annotatedType, Elements elementUtils) { + List ctors = + ElementFilter.constructorsIn(elementUtils.getAllMembers(annotatedType)); + + // First, check if any constructor is annotated with @SimpleBuilderConstructor + for (ExecutableElement ctor : ctors) { + if (ctor.getAnnotation(SimpleBuilderConstructor.class) != null) { + return Optional.of(ctor); + } + } + + // Fall back to heuristic: select constructor with the most parameters + ExecutableElement selected = null; + int maxParams = -1; + for (ExecutableElement ctor : ctors) { + int p = ctor.getParameters().size(); + if (p > maxParams) { + maxParams = p; + selected = ctor; + } + } + return (selected != null && maxParams > 0) ? Optional.of(selected) : Optional.empty(); + } } From 24ec529b18497df4e704fee5cddb1beef983fbc8 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 22:22:02 +0200 Subject: [PATCH 31/35] Fixing generated javadoc --- .../processor/util/BuilderDefinitionCreator.java | 2 ++ .../builders/processor/util/JavaCodeGenerator.java | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index 4560dccb..c5208004 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -316,6 +316,8 @@ private static Optional createFieldFromConstructor( FieldDto ctorField = new FieldDto(); ctorField.setFieldName(fieldName); ctorField.setFieldType(fieldType); + // Set javadoc (default to field name if no javadoc found) + ctorField.setJavaDoc(fieldName); // Find matching getter on the DTO type JavaLangAnalyser.findGetterForField(dtoType, fieldName, param.asType(), elementUtils, typeUtils) .ifPresent(getter -> ctorField.setGetterName(getter.getSimpleName().toString())); diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index a182a01d..be6530c6 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -309,19 +309,18 @@ private MethodSpec createMethod( switch (methodDto.getMethodType()) { case PROXY -> methodBuilder.addJavadoc( - "Calling $1N on dto-instance with parameters.\n", - methodDto.getMethodName()); + "Sets the value for $1N.\n", methodDto.getMethodName()); case CONSUMER -> methodBuilder.addJavadoc( - "Calling $1N on dto-instance with value after executing consumer.\n", + "Sets the value for $1N by executing the provided consumer.\n", methodDto.createFieldSetterMethodName()); case CONSUMER_BY_BUILDER -> methodBuilder.addJavadoc( - "Calling $1N on dto-instance with builder result value.\n", + "Sets the value for $1N using a builder consumer that produces the value.\n", methodDto.createFieldSetterMethodName()); case SUPPLIER -> methodBuilder.addJavadoc( - "Calling $1N on instance with value of supplier.\n", + "Sets the value for $1N by invoking the provided supplier.\n", methodDto.createFieldSetterMethodName()); } From 3323a00f29e3cb7673dde52b34a3fc45fd4cd0ca Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 22:43:39 +0200 Subject: [PATCH 32/35] Adding javadoc generation on fields --- .../processor/util/JavaCodeGenerator.java | 4 ++++ .../processor/BuilderProcessorTest.java | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java index be6530c6..59620ea4 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/JavaCodeGenerator.java @@ -224,6 +224,10 @@ private FieldSpec createFieldMember(FieldDto fieldDto) { ParameterizedTypeName wrappedFieldType = ParameterizedTypeName.get(builderFieldWrapper, fieldType); return FieldSpec.builder(wrappedFieldType, fieldDto.getFieldName(), Modifier.PRIVATE) + .addJavadoc( + "Tracked value for $L: $L.\n", + fieldDto.getFieldName(), + fieldDto.getJavaDoc()) .initializer("$T.unsetValue()", builderFieldWrapper) .build(); } diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index bb54bd81..fa28666c 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -643,8 +643,14 @@ void shouldGenerateMethodJavadocsForAllMethodTypes() { private String name; private HelperPlain helperPlain; public String getName() { return name; } + /** + * @param name the name of a specific person + */ public void setName(String name) { this.name = name; } public HelperPlain getHelperPlain() { return helperPlain; } + /** + * @param helperPlain complex helper object + */ public void setHelperPlain(HelperPlain helperPlain) { this.helperPlain = helperPlain; } public void doSomething(int a, String b) { /* no-op */ } """); @@ -668,20 +674,23 @@ public HelperPlain() {} // Note: Proxy methods are no longer supported; do not assert proxy method javadoc here. // Setter method javadoc and code (default values, when nothing is set) - ProcessorAsserts.assertContaining(generatedCode, "@param name name"); + ProcessorAsserts.assertContaining(generatedCode, "@param name the name of a specific person"); // SUPPLIER method javadoc and code - ProcessorAsserts.assertContaining(generatedCode, "@param nameSupplier supplier for name"); + ProcessorAsserts.assertContaining( + generatedCode, "@param nameSupplier supplier for the name of a specific person"); // CONSUMER method javadoc and code ProcessorAsserts.assertContaining( - generatedCode, "@param helperPlainConsumer consumer providing an instance of helperPlain"); + generatedCode, + "@param helperPlainConsumer consumer providing an instance of complex helper object"); - // Backing fields present - // TODO: Add Javadoc to fields + // Backing fields present with javadoc ProcessorAsserts.assertContaining( generatedCode, + "* Tracked value for name: the name of a specific person.", "private TrackedValue name = unsetValue();", + "* Tracked value for helperPlain: complex helper object.", "private TrackedValue helperPlain = unsetValue();"); } From b74d00a461decc2ad7a630d3fb1e2ecf760ec5af Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 23:11:36 +0200 Subject: [PATCH 33/35] Improving tests --- .../processor/BuilderProcessorTest.java | 50 +++++++++++++++++++ .../processor/JavaLangAnalyserTest.java | 15 ++++++ 2 files changed, 65 insertions(+) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index fa28666c..3312f7ac 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -2076,6 +2076,56 @@ public class GenericDto { contains("public static GenericDtoBuilder create()")); } + @Test + void shouldIgnoreSettersWithMethodLevelTypeParameters() { + // Given: a DTO with setters that have their own type parameters + String builderClassName = "DtoWithGenericSettersBuilder"; + JavaFileObject source = + ProcessorTestUtils.forSource( + """ + package test; + import org.javahelpers.simple.builders.core.annotations.SimpleBuilder; + import java.io.Serializable; + @SimpleBuilder + public class DtoWithGenericSetters { + private String name; + private Object fieldWithOwnGeneric; + private T fieldWithClassGeneric; + private T fieldWithExtendedClassGeneric; + public String getName() { return name; } + public void setName(String name) { this.name = name; } + public T getFieldWithClassGeneric() { return fieldWithClassGeneric; } + public void setFieldWithClassGeneric(T fieldWithClassGeneric) { this.fieldWithClassGeneric = fieldWithClassGeneric; } + public T getFieldWithExtendedClassGeneric() { return fieldWithExtendedClassGeneric; } + public void setFieldWithExtendedClassGeneric(F fieldWithExtendedClassGeneric) { + this.fieldWithExtendedClassGeneric = fieldWithExtendedClassGeneric; + } + public Object getFieldWithOwnGeneric() { return fieldWithOwnGeneric; } + // This setter has its own type parameter - should be ignored + public void setFieldWithOwnGeneric(F fieldWithOwnGeneric) { + this.fieldWithOwnGeneric = fieldWithOwnGeneric; + } + } + """); + + // When + Compilation compilation = compile(source); + + // Then + String generatedCode = loadGeneratedSource(compilation, builderClassName); + assertThat(compilation).succeededWithoutWarnings(); + ProcessorAsserts.assertingResult( + generatedCode, + // name field should be present + contains("public DtoWithGenericSettersBuilder name"), + // fieldWithClassGeneric setter should be present + contains("public DtoWithGenericSettersBuilder fieldWithClassGeneric"), + // fieldWithOwnGeneric setter should be ignored because it has method-level type parameter + notContains("public DtoWithGenericSettersBuilder fieldWithOwnGeneric"), + // fieldWithExtendedClassGeneric setter should be ignored because it has method-level type parameter , even if it extends the type parameter of the class + notContains("public DtoWithGenericSettersBuilder fieldWithExtendedClassGeneric")); + } + @Test void shouldNotGenerateBuilderWhenNestedBuilderInterfaceExists() { // Given: an annotated class that already declares a nested interface named like the would-be diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/JavaLangAnalyserTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/JavaLangAnalyserTest.java index 76a073f3..b9893872 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/JavaLangAnalyserTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/JavaLangAnalyserTest.java @@ -203,4 +203,19 @@ void extractParamJavaDoc_shouldAddSpaceBetweenLines() { "first line second line third line", JavaLangAnalyser.extractParamJavaDoc(javaDoc, parameter)); } + + @Test + void findGetterForField_shouldReturnEmpty_whenDtoTypeIsNull() { + assertTrue(JavaLangAnalyser.findGetterForField(null, "fieldName", null, null, null).isEmpty()); + } + + @Test + void findGetterForField_shouldReturnEmpty_whenFieldNameIsNull() { + assertTrue(JavaLangAnalyser.findGetterForField(null, null, null, null, null).isEmpty()); + } + + @Test + void findGetterForField_shouldReturnEmpty_whenFieldTypeMirrorIsNull() { + assertTrue(JavaLangAnalyser.findGetterForField(null, "fieldName", null, null, null).isEmpty()); + } } From 23b5d3041ef18a2b305a1cb76255148729fda49d Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 23:12:20 +0200 Subject: [PATCH 34/35] Fixing implementation, looks like the solution wit comparing generics is not needed anymore --- .../util/BuilderDefinitionCreator.java | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java index c5208004..301ca5ce 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/util/BuilderDefinitionCreator.java @@ -107,8 +107,7 @@ public static BuilderDefinitionDto extractFromElement( // nur public if (isMethodRelevantForBuilder(mth)) { // Avoid adding setter-derived fields that duplicate constructor params - Optional maybeField = - createFieldFromSetter(mth, result.getGenerics(), elementUtils, typeUtils); + Optional maybeField = createFieldFromSetter(mth, elementUtils, typeUtils); if (maybeField.isPresent()) { FieldDto field = maybeField.get(); if (!ctorFieldNames.contains(field.getFieldName())) { @@ -229,10 +228,7 @@ private static void addSupplierMethodsForField( } private static Optional createFieldFromSetter( - ExecutableElement mth, - List dtoGenerics, - Elements elementUtils, - Types typeUtils) { + ExecutableElement mth, Elements elementUtils, Types typeUtils) { String methodName = mth.getSimpleName().toString(); String fieldName = StringUtils.uncapitalize(StringUtils.removeStart(methodName, "set")); @@ -276,17 +272,11 @@ private static Optional createFieldFromSetter( // Finding generics declared on the setter itself (field-specific), e.g., + // If there are field-specific generics, no field in builder could be generated for it, so it + // needs to be ignored if (CollectionUtils.isNotEmpty(mth.getTypeParameters())) { - List dtoGenericsNames = - dtoGenerics.stream().map(GenericParameterDto::getName).toList(); - List mthGenericsNames = - mth.getTypeParameters().stream().map(tp -> tp.getSimpleName().toString()).toList(); - // If there are field-specific generics, no field in builder could be generated for it, so it - // needs to be ignored - if (CollectionUtils.containsAll(dtoGenericsNames, mthGenericsNames)) { - // TODO: Logging - return Optional.empty(); - } + // TODO: Logging + return Optional.empty(); } // simple setter From 98707b99ae476c1dd0b6bc742541420995f6dea4 Mon Sep 17 00:00:00 2001 From: AndreasIgel Date: Wed, 8 Oct 2025 23:14:58 +0200 Subject: [PATCH 35/35] Fixing code format --- .../simple/builders/processor/BuilderProcessorTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java index 3312f7ac..c1fe3630 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/BuilderProcessorTest.java @@ -2120,9 +2120,11 @@ public void setFieldWithOwnGeneric(F fieldWithOwnGeneri contains("public DtoWithGenericSettersBuilder name"), // fieldWithClassGeneric setter should be present contains("public DtoWithGenericSettersBuilder fieldWithClassGeneric"), - // fieldWithOwnGeneric setter should be ignored because it has method-level type parameter + // fieldWithOwnGeneric setter should be ignored because it has method-level type parameter + // notContains("public DtoWithGenericSettersBuilder fieldWithOwnGeneric"), - // fieldWithExtendedClassGeneric setter should be ignored because it has method-level type parameter , even if it extends the type parameter of the class + // fieldWithExtendedClassGeneric setter should be ignored because it has method-level type + // parameter , even if it extends the type parameter of the class notContains("public DtoWithGenericSettersBuilder fieldWithExtendedClassGeneric")); }