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

java: ErrorProne generates warnings for codegen #2718

Closed
ejona86 opened this issue Feb 15, 2017 · 6 comments
Closed

java: ErrorProne generates warnings for codegen #2718

ejona86 opened this issue Feb 15, 2017 · 6 comments

Comments

@ejona86
Copy link
Contributor

ejona86 commented Feb 15, 2017

protoc 3.2.0 generates code that fails ErrorProne's MissingOverride, ReferenceEquality, and FunctionalInterfaceClash checks. It's not obvious that the protobuf project has any legitimate problems to fix based on those warnings, but their presence makes it hard to use ErrorProne to check non-generated code.

Maybe the best option is just to add @SuppressWarnings for each of those warnings at the top of the generated class.

@liujisi
Copy link
Contributor

liujisi commented Mar 10, 2017

Protobuf code base should now pass the default ErrorProne settings. Do you have a list of patterns that you'd like to also cover? I guess maven only reports Error patterns, and didn't output anything for Warning patterns. I'd have to manually enable them.

@ejona86
Copy link
Contributor Author

ejona86 commented Mar 10, 2017

Oh, sweet! We use -Werror in our CI to prevent drowning in warnings, so resolving warnings would be good. We do use the default set of options though. The only ones I noticed failing were those I listed initially: MissingOverride, ReferenceEquality, and FunctionalInterfaceClash.

@liujisi
Copy link
Contributor

liujisi commented Mar 11, 2017

It seems like SuppressWarnings doesn't work for those ErrorProne specific warnings -- I tested @SuppressWarnings("FunctionalInterfaceClash"). Could you blacklist those patterns in grpc instead?

@ejona86
Copy link
Contributor Author

ejona86 commented Mar 11, 2017

Huh. That's strange. Did you apply it to the class or the method? I think that one is necessary to suppress at the class level. I just tested and SuppressWarnings worked for me and it is documented.

We are using -Xep:FunctionalInterfaceClash:OFF now in order to build. But that also means the check is ignored for our own code, not just the generated code. I do agree that it doesn't seem quite protobuf's fault to fix. It's a combination of how generated code, errorprone, gradle, and the gradle-protobuf plugin interact. At this point I think I the individual decisions make sense, but overall we're left with an issue and nowhere obvious to fix it.

@jared2501
Copy link

Could you annotate the generated protobuf with the javax.annotation.Generated annotation? Then downstream projects that use protobuf just need to add -XepDisableWarningsInGeneratedCode, and can still add -Werror too!

@haberman
Copy link
Member

The idea of adding generated annotations is covered in this issue: #42

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

6 participants