@Wither generates code, which violates best practives

[lombokissues]

Migrated from Google Code (issue 702)

[lombokissues]

👤 mcbeelen   🕗 Jul 04, 2014 at 07:07 UTC

What steps will reproduce the problem?

1. Create a class Book with two fields (Long id, String name)
2. @ Wither on the id-field
3. 'De-lombok' the code

Generated code looks like:
public Book withId(Long id) {
return this.id == id ? this : new Book(id, this.name);
}

What is the expected output? What do you see instead?
public Book withId(Long id) {
return this.id.equals(id) ? this : new Book(id, this.name);
}

What version of the product are you using? On what operating system?
Lombok 1.14.4 on MacOSX (In IntelliJ)

Please provide any additional information below.
The generated code raises a violation in our QA reporting (Sonar):

Bad practice - Suspicious reference comparison

This method compares two reference values using the == or != operator, where the correct way to compare instances of this type is generally with the equals() method. Examples of classes which should generally not be compared by reference are java.lang.Integer, java.lang.Float, etc.
findbugs:RC_REF_COMPARISON Sep12 Reliability > Instruction

[lombokissues]

👤 r.spilker   🕗 Jul 04, 2014 at 07:58 UTC

This is done on purpose. Executing the equals might be way more expensive than just creating a copy of the instance.

[lombokissues]

👤 Maaartinus   🕗 Aug 30, 2014 at 02:15 UTC

While I completely agree with using '=' being the best choice, I wonder if there's any sane way to make the checker (findbugs in my case) shut up. Currently, I see two possibilities:

• adding @ SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") to the whole class.
• Using @ Wither(onMethod=@ __(@ SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ"))).

While the former method is too unspecific, the latter is way too long (especially when @ Wither gets applied to a couple of fields rather than the whole class).

Maybe there could be a configuration key like HUSH_FINDBUGS doing this automatically?

[lombokissues]

👤 reinierz   🕗 Nov 20, 2014 at 13:14 UTC

Re-opening this; now that we have a config system, we can have a config key to add @ SuppressFBWarnings(justification = "Generated code") to everything if the user likes.

[lombokissues]

👤 reinierz   🕗 Feb 01, 2015 at 23:18 UTC

For now the justification is not configurable, it's just 'generated code' (all lowercase, no final dot, I believe that's the proper style), and obviously not enabled by default, but there's a config key to turn it on.

I assume that is enough to make FB shut up? I don't need to provide any particular value to it?

This feature is available in the edge build now, and should be in the next version.

https://projectlombok.org/download-edge.html

[lombokissues]

👤 reinierz   🕗 Feb 01, 2015 at 23:29 UTC

Issue #770 has been merged into this issue.

[lombokissues]

End of migration