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

Consider providing a com.sun.source.util.Plugin? #535

Closed
jkeljo opened this issue Feb 17, 2017 · 12 comments
Closed

Consider providing a com.sun.source.util.Plugin? #535

jkeljo opened this issue Feb 17, 2017 · 12 comments

Comments

@jkeljo
Copy link

jkeljo commented Feb 17, 2017

I recently became aware of ErrorProne as @davido is working to integrate it into Buck. Glancing at the code, it looks like the key reason for exposing it as a compiler is to get a TaskListener into place, which is exactly what the Plugin facility lets you do in javac 8 and higher. Plugins can be auto-discovered on the processor path (just like annotation processors), which would make it more straightforward to run ErrorProne even with command-line javac, and of course any build system that supports Plugins (I'm working to add such support to Buck) would no longer require a special integration.

@jkeljo jkeljo changed the title Consider using com.sun.source.util.Plugin? Consider providing a com.sun.source.util.Plugin? Feb 17, 2017
cushon added a commit that referenced this issue Mar 1, 2017
This allows Error Prone to be used as a javac plugin (instead of a
wrapper) via the plugin API that was added in 8:
https://docs.oracle.com/javase/8/docs/jdk/api/javac/tree/com/sun/source/util/Plugin.html

Demo:

```
javac -J-Xbootclasspath/p:javac-9-dev-r3297-3.jar \
  -processorpath error_prone_ant-2.0.18-SNAPSHOT.jar \
  -Xplugin:ErrorProne Test.java
...
ShortSet.java:8: error: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method; its type int is not compatible with its collection's type argument Short
      s.remove(i - 1);
              ^
    (see http://errorprone.info/bugpattern/CollectionIncompatibleType)
```

See #535

MOE_MIGRATED_REVID=148835033
@cushon
Copy link
Collaborator

cushon commented Mar 1, 2017

I pushed an initial implementation in feb4b34, and verified that it works for simple examples. I'd be interested in feedback if you get a chance to try using this with Buck.

javac -J-Xbootclasspath/p:javac-9-dev-r3297-4.jar \
  -processorpath error_prone_ant-2.0.18-SNAPSHOT.jar \
  -Xplugin:ErrorProne Test.java

@cushon cushon closed this as completed Mar 1, 2017
@davido
Copy link
Contributor

davido commented Mar 1, 2017

Would that mean that i could simplify EP in Buck integration, added here facebook/buck#1047?

@ttsugriy
Copy link

@cushon, I'm getting a following stack trace when using modern ant compiler with -processorpath pointing at error_prone_ant-2.2.0.jar and -Xplugin:ErrorProne:

    [javac] A plugin threw an uncaught exception.
    [javac] Consult the following stack trace for details.
    [javac] java.lang.NoSuchMethodError: com.sun.tools.javac.util.JavacMessages.add(Lcom/sun/tools/javac/util/JavacMessages$ResourceBundleHelper;)V
    [javac] 	at com.google.errorprone.BaseErrorProneJavaCompiler.setupMessageBundle(BaseErrorProneJavaCompiler.java:189)
    [javac] 	at com.google.errorprone.ErrorProneJavacPlugin.init(ErrorProneJavacPlugin.java:38)
    [javac] 	at com.sun.tools.javac.main.Main.compile(Main.java:470)
    [javac] 	at com.sun.tools.javac.main.Main.compile(Main.java:381)
    [javac] 	at com.sun.tools.javac.main.Main.compile(Main.java:370)
    [javac] 	at com.sun.tools.javac.main.Main.compile(Main.java:361)
    [javac] 	at com.sun.tools.javac.Main.compile(Main.java:56)
    [javac] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [javac] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    [javac] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [javac] 	at java.lang.reflect.Method.invoke(Method.java:498)
    [javac] 	at org.apache.tools.ant.taskdefs.compilers.Javac13.execute(Javac13.java:57)
    [javac] 	at org.apache.tools.ant.taskdefs.Javac.compile(Javac.java:1407)
    [javac] 	at org.apache.tools.ant.taskdefs.Javac.execute(Javac.java:1133)
    [javac] 	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
    [javac] 	at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [javac] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [javac] 	at java.lang.reflect.Method.invoke(Method.java:498)
    [javac] 	at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
    [javac] 	at org.apache.tools.ant.Task.perform(Task.java:346)
    [javac] 	at java.util.Vector.forEach(Vector.java:1249)
    [javac] 	at org.apache.tools.ant.taskdefs.Sequential.execute(Sequential.java:67)
    [javac] 	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
    [javac] 	at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [javac] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [javac] 	at java.lang.reflect.Method.invoke(Method.java:498)
    [javac] 	at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
    [javac] 	at org.apache.tools.ant.Task.perform(Task.java:346)
    [javac] 	at org.apache.tools.ant.taskdefs.MacroInstance.execute(MacroInstance.java:394)
    [javac] 	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
    [javac] 	at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [javac] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [javac] 	at java.lang.reflect.Method.invoke(Method.java:498)
    [javac] 	at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
    [javac] 	at org.apache.tools.ant.Task.perform(Task.java:346)
    [javac] 	at org.apache.tools.ant.Target.execute(Target.java:448)
    [javac] 	at org.apache.tools.ant.Target.performTasks(Target.java:469)
    [javac] 	at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1399)
    [javac] 	at org.apache.tools.ant.Project.executeTarget(Project.java:1370)
    [javac] 	at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
    [javac] 	at org.apache.tools.ant.Project.executeTargets(Project.java:1260)
    [javac] 	at org.apache.tools.ant.Main.runBuild(Main.java:849)
    [javac] 	at org.apache.tools.ant.Main.startAnt(Main.java:228)
    [javac] 	at org.apache.tools.ant.launch.Launcher.run(Launcher.java:283)
    [javac] 	at org.apache.tools.ant.launch.Launcher.main(Launcher.java:101)

I'm I doing something obviously wrong? Is -J-Xbootclasspath/p:javac-9-dev-r3297-4.jar required for using ErrorProne plugin? I'm using java version "1.8.0_144"

@6harat
Copy link

6harat commented Mar 23, 2018

I am also getting a similar error

@cushon
Copy link
Collaborator

cushon commented Mar 23, 2018

Please start a new bug, and include the steps to reproduce the problem you're seeing.

@AustinShalit
Copy link

Hi folks, sorry to bring back an old thread. But did anyone solve the error above #535 (comment)?

https://xkcd.com/979/

@cushon
Copy link
Collaborator

cushon commented Aug 10, 2018

I can reproduce:

$ javac -version
javac 1.8.0_162
$ javac -XDcompilePolicy=simple -processorpath error_prone_ant-2.3.1.jar \
  '-Xplugin:ErrorProne -XepDisableAllChecks -Xep:CollectionIncompatibleType:ERROR' \
  ShortSet.java
A plugin threw an uncaught exception.
Consult the following stack trace for details.
java.lang.NoSuchMethodError: com.sun.tools.javac.util.JavacMessages.add(Lcom/sun/tools/javac/util/JavacMessages$ResourceBundleHelper;)V
	at com.google.errorprone.BaseErrorProneJavaCompiler.setupMessageBundle(BaseErrorProneJavaCompiler.java:203)
	at com.google.errorprone.ErrorProneJavacPlugin.init(ErrorProneJavacPlugin.java:40)
	at com.sun.tools.javac.main.Main.compile(Main.java:470)
	at com.sun.tools.javac.main.Main.compile(Main.java:381)
	at com.sun.tools.javac.main.Main.compile(Main.java:370)
	at com.sun.tools.javac.main.Main.compile(Main.java:361)
	at com.sun.tools.javac.Main.compile(Main.java:56)
	at com.sun.tools.javac.Main.main(Main.java:42)

The -Xplugin integration doesn't work with JDK 8.

It does work with JDK 9:

$ javac -fullversion
javac full version "9+181"
$ javac -XDcompilePolicy=simple -processorpath error_prone_ant-2.3.1.jar \
  '-Xplugin:ErrorProne -XepDisableAllChecks -Xep:CollectionIncompatibleType:ERROR' \
  ShortSet.java
ShortSet.java:7: error: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method; its type int is not compatible with its collection's type argument Short
      s.remove(i - 1);
              ^
    (see http://errorprone.info/bugpattern/CollectionIncompatibleType)
1 error

And it also works if you override JDK 8's javac with the Error Prone javac:

$ javac -J-Xbootclasspath/p:javac-9+181-r4173-1.jar -XDcompilePolicy=simple \
  -processorpath error_prone_ant-2.3.1.jar \
  '-Xplugin:ErrorProne -XepDisableAllChecks -Xep:CollectionIncompatibleType:ERROR' \
  ShortSet.java
ShortSet.java:7: error: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method; its type int is not compatible with its collection's type argument Short
      s.remove(i - 1);
              ^
    (see http://errorprone.info/bugpattern/CollectionIncompatibleType)
1 error

@tbroyer is there a way to make gradle do the equivalent of overriding the host JDK's javac using -J-Xbootclasspath/p:javac-9+181-r4173-1.jar?

@tbroyer
Copy link
Contributor

tbroyer commented Aug 10, 2018

It might be possible to pass that as part of forkOptions.jvmArgs (and forking the compiler) yes.
For now, I'd recommend using the net.ltgt.errorprone plugin with JDK 8 (and not forking), and net.ltgt.errorpone-javacplugin with JDK 9+.
I'm still trying to bring the same DSL from net.ltgt.errorprone-javacplugin to net.ltgt.errorprone to make it easier to switch between the plugins (depending on JDK version), but maybe I should rather invest in trying to make net.ltgt.errorprone-javacplugin work with JDK 8 by forking and setting up the bootclasspath? (assuming that'd work)
I'll try to find time in the coming weeks to work on those plugins again. In the mean time, it's not that hard to use -Xplugin "by hand" (i.e. without a Gradle plugin), and configure the bootclasspath for JDK 8 (declare the dependency in a configuration, or "filter" the annotationProcessorPath, and conditionally configure forking in a doFirst of the JavaCompile tasks; don't forget to declare the tasks' inputs for the up-to-date checks)

@AustinShalit
Copy link

I ended up doing this: wpilibsuite/allwpilib#1265

@tbroyer
Copy link
Contributor

tbroyer commented Aug 20, 2018

@cushon So, apparently, net.ltgt.errorprone-javacplugin can be made to work with Java 8 by using:

configurations {
  javac
}
dependencies {
  javac 'com.google.errorprone:javac:9+181-r4173-1'
}
tasks.withType(JavaCompile) {
  options.fork = true
  options.forkOptions.jvmArgs.add("-Xbootclasspath/p:${configurations.javac.asPath}" })
}

(this is -Xbootclasspath/p: and not -J-Xbootclasspath/p: because Gradle does not call javac directly, but through javax.tools.JavaCompiler API)

@cushon
Copy link
Collaborator

cushon commented Aug 20, 2018

Nice. Do you think we should be recommending that configuration and trying to converge on the Xplugin integration?

@tbroyer
Copy link
Contributor

tbroyer commented Aug 20, 2018

Do you think we should be recommending that configuration and trying to converge on the Xplugin integration?

If you're ready to recommend that on the command line too, then yes; that would make things simpler for everyone (same configuration whichever the JDK, except for an additional bootclasspath entry when using JDK 8).
@oehme (and others at Gradle, Inc.) would likely agree too as that would mean no longer using internal APIs for the Gradle plugin.
The main downside is that it requires forking when using JDK 8; Gradle can fork a daemon process that's reused between compilation tasks (similar to Bazel workers), but I believe Ant or Maven won't do that.

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

No branches or pull requests

7 participants