Skip to content

Add Configuration Key lombok.builder.toBuilder (Fixes #3758) #3816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Mahmoud-Khawaja
Copy link

Description

This PR introduces the lombok.builder.toBuilder configuration key, allowing toBuilder() to be enabled globally for all @Builder annotations via lombok.config. If not explicitly set in the annotation, the configuration value is applied (default: false).

Changes

  • Added BUILDER_TO_BUILDER key in ConfigurationKeys.java.
  • Integrated config key handling in HandleBuilder.java.
  • Ensured explicit annotation settings override configuration values.

This provides more flexibility in testing and code generation.
Fixes #3758.

@Mahmoud-Khawaja Mahmoud-Khawaja changed the title Add lombok.builder.toBuilder configuration key to enable toBuilder by… Add Configuration Key lombok.builder.toBuilder (Fixes #3758) Feb 5, 2025
@rzwitserloot
Copy link
Collaborator

I'm not entirely sure lombok.config is the right place for this sort of thing. Config is always fighting with itself: On one hand, lombok never was intended to be silent magic - the source file on its own without knowing what lombok.config contains ought to be enough to at least guesstimate what's happening.

Then again, your class still has @Builder on it; it should therefore be somewhat obvious where toBuilder() is coming from.

I'll discuss this one with @rspilker before merging.

One consideration fighting against this, @Mahmoud-Khawaja, is OpenJDK's plans for with. It might be limited to records (all the more reason to use toBuilder() more!), but it would look something like:

LocalDate x = fetchSomeLocalDate();
LocalDate y = x with {
  year = 10;
  month = 12;
};

If that was part of OpenJDK, it reduces the boilerplate considerations (the 'bang' part of the 'bang for the buck' equation that lombok features have to balance out).

@rzwitserloot
Copy link
Collaborator

Talked it over with @rspilker ; we're going to accept this.

@rzwitserloot
Copy link
Collaborator

Reviewed the code:

  • The merge conflict is trivial; master now has a new config key added at the end. The order between yours and the new one is not important.
  • Your one commit contains the update, but no docs (that's allright, we can write them), no tests (acceptable, we'll write them), and a whole boatload of weird docker stuff. That has to go.
  • Please add your name to the AUTHORS file for legal reasons.

If you could add the name to this PR we can accept it. And we'd take it as a kindness if you rebase it, remove the docker stuff.

@Mahmoud-Khawaja
Copy link
Author

done, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add configuration key lombok.builder.toBuilder
2 participants