Skip to content

Add @lombok.CheckReturnValue to @With and @Builder build() methods for static analysis - #4013

Open
timtebeek wants to merge 7 commits into
projectlombok:masterfrom
timtebeek:timtebeek/check-return-value
Open

Add @lombok.CheckReturnValue to @With and @Builder build() methods for static analysis#4013
timtebeek wants to merge 7 commits into
projectlombok:masterfrom
timtebeek:timtebeek/check-return-value

Conversation

@timtebeek

Copy link
Copy Markdown

…ted methods where the return value should not be ignored.

Adds the annotation to @with, @Withby, @Builder.build(), and @SuperBuilder.build() methods. Controlled via lombok.addCheckReturnValueAnnotation config key (default: true). Static analysis tools (Error Prone, IntelliJ, SpotBugs) recognize @CheckReturnValue by simple name and will warn when the return value is discarded.
…han opt-out to avoid breaking existing tests.
@rzwitserloot

Copy link
Copy Markdown
Collaborator

The concept is approved, but we want to release a JDK26 compatible version first.

A few changes to the design:

  1. We want to kick this off with, as default, not to generate it. For now, you have to opt in to get em. This way we can paper over a few versions: lombok.CheckReturnValue will exist the moment this PR goes live into stable, and we'll then wait a year or two and change the default - that gives a 'window' of two years of lombok releases where the annotation exists but wasn't by default on yet.
  2. Whilst clearly it is very trendy to make your own CheckReturnValue annotation, and we will, it'd be nice if we can accomodate those who have their own fashion sense: A lombok.config setting to tell lombok to use some different common @CheckReturnValue.

We can solve both of those in one swoop with a lombok.config setting (enum) with the following values:

  1. unset - take the lombok default, which for now is none but will later be lombok.
  2. none - do not generate any annotation.
  3. lombok - generate the @lombok.CheckReturnValue annotation.

Later on we can add more values, such as jetbrains or errorprone or what not, or even custom: com.foo.FullyQualifiedNameHere, or as a separate lombok config key. It leaves doors open.

@timtebeek

Copy link
Copy Markdown
Author

Great to hear and thanks for the new release; let me know if you'd want me to make any changes, or if you'd prefer to do so yourself.

…d `lombok.checkReturnValueAnnotation` config key.

Per PR projectlombok#4013 review, swap the boolean `lombok.addCheckReturnValueAnnotation`
for an enum-valued `lombok.checkReturnValueAnnotation` (values: `none`,
`lombok`; default: `none`). This leaves room for additional flavors
(`jetbrains`, `errorprone`, `custom: FQN`) in future releases without
another key rename, and for flipping the default to `lombok` after a
release cycle or two.
@timtebeek

Copy link
Copy Markdown
Author

I've made the above changes on my branch, as well as merged master to update the doc/changelog.markdown; let me know if there's anything else I can do to help this along!

@rzwitserloot

Copy link
Copy Markdown
Collaborator

@timtebeek LGTM, but you haven't added your name to the AUTHORS file. Can you throw one more commit in here with that update?

@timtebeek

Copy link
Copy Markdown
Author

Done, thanks! Looking forward to using this in our analyses.

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 a @CheckReturnValue annotation on generated methods where applicable

2 participants