fix: builder accessor method naming conflicts#4342
Conversation
nated0g
changed the title
landonxjames
left a comment
landonxjames
left a comment
There was a problem hiding this comment.
Assuming that commit I did for the ktlint issues works this all looks good to me. Will require a review from the server team as well.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
I think this will probably cause breaking changes because setters that almost certainly exist in the wild for fields like I think we need to make this context aware so that we only rename setters where the setter itself would actually conflict. |
The setterName() extension function now requires a symbolProvider parameter. Updated call sites to pass the correct symbolProvider.
Verifies that structures with a 'meta' field get properly renamed to 'meta_value' with corresponding setter/getter methods to avoid conflicts with error metadata fields.
Use setterName() extension function instead of manually constructing setter names to handle field renaming (e.g. default -> default_value).
The previous approach renamed every member that the symbol provider touched, which changed `set_default`/`set_build`/`set_builder` (and the matching getters) for any struct with those fields — a breaking change flagged by @rcoh on review. The duplicate-definition bug only occurs for error-struct builders, where `ErrorGenerator` injects `set_meta` for `ErrorMetadata`. Only rename members whose raw name is `meta` and leave every other setter and getter at its pre-existing `set_<raw>` / `get_<raw>` name. Rewrite the regression test to actually reproduce the original bug: it now builds a `TestError` (an `@error` shape with a `meta` member) and asserts `set_default` stays intact, so the earlier failure mode and the guard against unrelated renames are both covered. Also drop a now-unused `toSnakeCase` import in `ServerBuilderGenerator` that surfaced after rebasing onto main.
nated0g
force-pushed
the
fix-builder-accessors
branch
from
b7f1096 to
da74b5d
Compare
alright finally coming back to this because I'm tired of manually tweaking codegen output for a client I've had to regenerate a few times now 😅 @rcoh I think my latest should address your concern |
4 participants
Motivation and Context
#4338
Description
fixes a couple stray compilation errors in @haydenbaker's PR (#4341), and adds a test, adds changelog entry
Testing
Added a test in
FluentClientGeneratorTest.ktto verify the bug no longer existsChecklist
.changelogdirectory, specifying "client," "server," or both in theapplies_tokey.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.