Repository navigation
Emit direct setter last within same-name builder method overloads - #325
AndreasIgel merged 5 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ffccf3f to
62f4b9e
Compare
62f4b9e to
2de1c6f
Compare
…ld, name Introduces a member ordering that keeps each field's methods together and emits the direct property setter last within its overload group, so consumers that resolve overloaded builder methods last-wins (e.g. MapStruct's builder detection) bind to the plain setter. The emission order is decided on the generation side: BuilderToGenerationTypeMapper sorts the (field, method) pairs by ordering value, access modifier (public first), non-static before static, originating field (alphabetically, builder-level methods last), method name, direct setter last, then the existing tie-breakers, and emits already-ordered MethodDtos. MethodDto drops its ordering field and comparator — it no longer carries generation-scope metadata. The generator-side signature safety net degrades to keep-first-occurrence with a warning (error in strict mode); real conflict resolution stays priority-based in BuilderDefinitionCreator. Regenerates example builders and golden test fixtures.
2de1c6f to
afb03e0
Compare
| return signatureToMethod.values().stream().sorted(comparator).toList(); | ||
| return List.copyOf(signatureToMethod.values()); |
There was a problem hiding this comment.
Do we need here something, that preserves the ordering?
There was a problem hiding this comment.
The incoming list is already in final emission order (sorted in BuilderToGenerationTypeMapper on the generation side), so preserving insertion order is all that's needed — now explicit via the Set+List dedup in b31b344.
| " Unexpected duplicate method signature: '%s' — keeping first occurrence (safety net)", | ||
| signature); | ||
| for (MethodDto method : methods) { | ||
| if (signatureToMethod.putIfAbsent(method.getSignatureKey(), method) != null) { |
There was a problem hiding this comment.
Putting it to a map and then doing a list out of it does not preserve the ordering!?
There was a problem hiding this comment.
LinkedHashMap does preserve insertion order — but you're right that the map indirection hid the intent. Simplified in b31b344 to a Set<String> of seen signature keys + a List<MethodDto>: seen.add() returns false on duplicates, so the result keeps the mapper's emission order explicitly with take-first semantics.
| if (groupKey1 == null && groupKey2 == null) { | ||
| return 0; | ||
| } | ||
| if (groupKey1 == null) { | ||
| return 1; | ||
| } | ||
| if (groupKey2 == null) { | ||
| return -1; | ||
| } | ||
| return groupKey1.compareTo(groupKey2); |
There was a problem hiding this comment.
Is there nothing from String, Strings or StringUtils?
There was a problem hiding this comment.
Yes — StringUtils.compare(a, b, false) does exactly this (nulls-last). Replaced compareGroupKeys with it in 49efe2c.
| Consumer<DefaultFormatDtoBuilder> yesCondition) { | ||
| return conditional(condition, yesCondition, null); | ||
| } | ||
| /** |
There was a problem hiding this comment.
Here and in whole file the starting spaces are changed now. Looks not correct to me! And is an unnecessary change in git history
There was a problem hiding this comment.
Fixed in 49efe2c — the block keeps its original indentation now, so the diff only shows the ordering changes.
…t compare - FormattingModeTest: keep expectedSection2's original block indentation so the diff shows only ordering changes. - BuilderToGenerationTypeMapper: use StringUtils.compare(a, b, false) for the nulls-last group-key comparison instead of a hand-rolled helper. - RoasterCodeGenerator: note that LinkedHashMap preserves the mapper's emission order through the safety-net dedup.
Keeps a HashSet of seen signature keys and appends each unseen method to the result list, so duplicate signatures keep the first occurrence in emission order and no map indirection is needed.
…eTest The duplicate-signature branch is unreachable through the builder pipeline because BuilderDefinitionCreator resolves conflicts first; feeding identical MethodDto signatures straight into the generator verifies the take-first semantics and the strict-mode warning.
Replaces the three accumulation loops with a Stream.concat pipeline feeding the emission-order sort; semantics unchanged.
|
2 participants
Summary
Preparation for #300 — fixes #328. No behavior change for Java callers.
Generated builders emit several overloads per property name: the direct setter (
name(String),nickNames(List)) plus helper methods (name(Supplier<String>),name(Consumer<...>),nameUpdate(...), varargs/format helpers). Their emission order was incidental — driven by signature sort — which left the direct setter buried before the helpers.This PR makes member order deterministic and keeps each field's methods together — and it moves the decision into the generation scope where field context exists:
BuilderToGenerationTypeMappersorts the (field, method) pairs byordering(create 200 → fluent 1000 → conditional 1100 → build 1200 → toString 2000 → private helpers 2100), access modifier (public first, private last), non-static before static, originating field (alphabetically, builder-level methods last), method name — and within a same-named overload group the direct property setter last — then emits already-orderedMethodDtos.MethodDtodrops itsorderingfield and comparator entirely: it carries no generation-scope metadata. The code generator's remaining signature safety net degrades to keep-first-occurrence with a warning (error in strict mode); the real priority-based conflict resolution stays inBuilderDefinitionCreatoruntouched.Why
Consumers that resolve overloaded builder methods last-wins then bind to the plain setter — the motivating consumer is MapStruct (#300): it takes the last single-argument write accessor per property name on a builder type. Field cohesion is preserved: every
birthdate*/name*cluster stays adjacent instead of setters being pulled to the end. Documented indocs/CUSTOMIZING.mdnext to the generator priority table.Notes
javacresolves overloads by signature, not declaration order.example/generated-example-builder/is regenerated — the large diff is pure method reordering, no content changes.directSetters_emittedAfterHelperOverloadsverifies the setter-last guarantee for every property.