Skip to content
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

Lombok and Findbug #669

Closed
lombokissues opened this issue Jul 14, 2015 · 9 comments
Closed

Lombok and Findbug #669

lombokissues opened this issue Jul 14, 2015 · 9 comments

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 634)

@lombokissues
Copy link
Author

👤 charles.fendt@ariadnext.com   🕗 Jan 31, 2014 at 14:05 UTC

What steps will reproduce the problem?
1.Create a bean
2.add @ Data annotation
3.launch Findbug analyse with USBR_UNNECESSARY_STORE_BEFORE_RETURN activated
(see http://fb-contrib.sourceforge.net/bugdescriptions.html)

Findbug detects a local variable for the return
"This method stores the return result in a local variable, and then immediately returns the local variable. It would be simpler just to return the value that is assigned to the local variable, directly. "

it is like writing :

Object obj = xxx;
return obj;

It's more logic (and need less RAM) to write :

return xxx;

This is with Lombok version 1.12.x, but i suppose it is the same with older version

@lombokissues
Copy link
Author

👤 r.spilker   🕗 Jan 31, 2014 at 14:09 UTC

I'm pretty sure that it is NOT true that more RAM is being used. Also, what is the code it is complaining about?

@lombokissues
Copy link
Author

👤 charles.fendt@ariadnext.com   🕗 Feb 01, 2014 at 07:19 UTC

The code to reproduce is quiet simple:
@ Data
Public class Foo {
private String bar;
}

And if i'm sure that the jit do all, a new local is a new pointer in the stack... Si 4 bytes everytime i call the method...

@lombokissues
Copy link
Author

👤 Maaartinus   🕗 Feb 02, 2014 at 00:26 UTC

Yes, when the interpreter calls the method, the stack needs 4 more bytes. When the method returns, you get them back. So your program needs up to 4 more bytes (it can't be more as the method calls nothing, no recursion).

That said, the size of generated bytecode might matter a bit and so might the interpreted speed.

@lombokissues
Copy link
Author

👤 r.spilker   🕗 Feb 04, 2014 at 22:02 UTC

I don't know what to do with this issue. There are several parts:

  1. We might be able to generate different code to please Findbugs in this scenario. We need to know what Findbugs is complaining about. The test code is not specific about that. Please tell us it it is the getter, setter, equals, hashcode, constructor or toString and don't make us do all that work as well. We don't have findbugs installed, so make please make it easy for us to find the root cause.

  2. We probably won't be able to please all compilers, lint tools, Eclipse preferences etc. regarding code style and potential problems. If there is a real problem in our code, we'll fix it. But we do generate code that has to always work. In general it is better to somehow convince the tooling not to look at generated code, or make them lombok aware. For PMD, some initiatives have been taken, but nothing publicly available yet.

  3. We might be able to generate @ Generated annotations in the class files to help tooling to skip generated code. Since that also makes bigger class files (I personally couldn't care less), I can imagine a command line parameter for the compiler and have a separate compilation for lint tool processing. We already have delombok formatter switches to generate different source files for different "target audiences". Lint tools might be one of them.

Any suggestions?

@lombokissues
Copy link
Author

👤 r.spilker   🕗 Feb 04, 2014 at 22:06 UTC

  1. Some code is not strictly generated. It would be hard to specify how tools should handle that code. @ Getter generates an entire method, but @ Cleanup moves the rest of the statements in the same block into the try-block of a generated try-statement and generates a finally block. It is not possibly to place an annotation on a try-statement. And the try-block is not generated.

@lombokissues
Copy link
Author

👤 reinierz   🕗 Feb 06, 2014 at 19:52 UTC

We have 2 bits of lombok policy that are pertinent to this issue:

  1. We CANNOT address style / code checker complaints on generated code. There are a bazillion ways to configure these things, and there are mutually exclusive ways to configure these. Therefore, it is literally impossible for us to make code that everyone is happy with unless we start adding hundreds of nitty gritty configuration directives. The only right answer is to make such style checkers ignore generated code, and we'll do whatever we can to enable that. For now, that means @ SuppressWarnings. This may also mean adding @ Generated. See Issue Put a @javax.annotation.Generated annotation on all methods and constructors and classes we generate (?) #658 on further discussion on @ Generated.

  2. We do not accept performance-related issues with generated code unless you can prove an actual, real-life performance issue based on for example a profiler report. 4 extra bytes in non-hotspotted code is completely irrelevant, and hotspot will twist your code into something completely different.

@lombokissues
Copy link
Author

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

You can now tell findbugs to stop generating warnings on lombok generated code:

issue #737

@lombokissues
Copy link
Author

End of migration

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

No branches or pull requests

1 participant