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

Re-enables errorprone on JDK 10+ #761

Merged
merged 3 commits into from
Aug 20, 2018
Merged

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Aug 20, 2018

This uses the commandline instructions here:
http://errorprone.info/docs/installation

See google/error-prone#860

This uses the commandline instructions here:
  http://errorprone.info/docs/installation

However, there's a glitch as I'm not sure why
`-XepDisableWarningsInGeneratedCode` doesn't work.

See google/error-prone#860
pom.xml Outdated
<annotationProcessorPaths>
<processorPath>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_ant</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need the ant classifier? or will core work?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error_prone_core should work.

pom.xml Outdated
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne</arg>
<!-- TODO: why does this not work? -->
<!--<arg>-XepDisableWarningsInGeneratedCode</arg>-->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this crashes when uncommented:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] javac: invalid flag: -XepDisableWarningsInGeneratedCode
Usage: javac <options> <source files>
use --help for a list of possible options
warning: [options] bootstrap class path not set in conjunction with -source 1.8

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to put all the Error Prone flags in the same <arg> as -Xplugin:ErrorProne like

<arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode</arg>

Here's an example from another project I work on that uses Error Prone as a compiler plugin: https://github.com/TechEmpower/tfb-status/blob/e3e1c5d2684252d2ad585545016c98574bf78e81/pom.xml#L451-L534

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks tons. all good now!

@codefromthecrypt
Copy link
Member Author

Works with the problem of crashing on args (commented about this)

$ ./mvnw clean compile -pl brave
--snip--
[INFO] --- maven-compiler-plugin:3.8.0:compile (default-compile) @ brave ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 50 source files to /Users/acole/oss/brave/brave/target/classes
[WARNING] [options] bootstrap class path not set in conjunction with -source 1.8
[WARNING] WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.errorprone.bugpatterns.FutureReturnValueIgnored (file:/Users/acole/.m2/repository/com/google/errorprone/error_prone_ant/2.3.1/error_prone_ant-2.3.1.jar) to field com.sun.tools.javac.code.Type$StructuralTypeMapping$4.this$0
WARNING: Please consider reporting this to the maintainers of com.google.errorprone.bugpatterns.FutureReturnValueIgnored
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
/Users/acole/oss/brave/brave/src/main/java/brave/propagation/ThreadLocalSpan.java:[99,6] [ThreadLocalUsage] ThreadLocals should be stored in static fields
    (see http://errorprone.info/bugpattern/ThreadLocalUsage)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4.289 s
[INFO] Finished at: 2018-08-20T09:38:30+08:00
[INFO] ------------------------------------------------------------------------

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Aug 20, 2018

oops also need to figure our how to disable this in certain sub-projects, like JMH (instrumentation/benchmarks)

@codefromthecrypt
Copy link
Member Author

all sorted.. thanks @michaelhixson!

Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codefromthecrypt
Copy link
Member Author

circleci is angry for unrelated reasons

@codefromthecrypt codefromthecrypt merged commit f7974f4 into master Aug 20, 2018
@codefromthecrypt codefromthecrypt deleted the errorprone-plugin branch August 20, 2018 08:26
codefromthecrypt pushed a commit to openzipkin/zipkin that referenced this pull request Aug 20, 2018
It is a little tricky to get errorprone and auto-value working at the
same time.

See openzipkin/brave#761
codefromthecrypt pushed a commit to openzipkin/zipkin that referenced this pull request Aug 21, 2018
It is a little tricky to get errorprone and auto-value working at the
same time.

See openzipkin/brave#761
codefromthecrypt pushed a commit to openzipkin-attic/zipkin-classic that referenced this pull request May 17, 2019
It is a little tricky to get errorprone and auto-value working at the
same time.

See openzipkin/brave#761
abesto pushed a commit to abesto/zipkin that referenced this pull request Sep 10, 2019
It is a little tricky to get errorprone and auto-value working at the
same time.

See openzipkin/brave#761
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.

3 participants