Skip to content

Commit

Permalink
Migrate the maven examples to use the -Xplugin integration
Browse files Browse the repository at this point in the history 
MOE_MIGRATED_REVID=209507984
  • Loading branch information
@cushon
cushon committed Aug 21, 2018
1 parent 34ac40e commit 82b319c
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 211 deletions.
1 change: 0 additions & 1 deletion examples/maven/annotation_processing_bug_repro/README.txt
View file

This file was deleted.

40 changes: 0 additions & 40 deletions examples/maven/annotation_processing_bug_repro/pom.xml
View file

This file was deleted.

23 changes: 0 additions & 23 deletions examples/maven/annotation_processing_bug_repro/src/main/java/NeedsAnnotationProcessing.java
View file

This file was deleted.

2 changes: 0 additions & 2 deletions examples/maven/error_prone_should_flag/README.txt
View file

This file was deleted.

30 changes: 0 additions & 30 deletions examples/maven/error_prone_should_flag/pom.xml
View file

This file was deleted.

96 changes: 57 additions & 39 deletions examples/maven/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,57 +20,75 @@
<modelVersion>4.0.0</modelVersion>

<groupId>com.example</groupId>
<artifactId>maven-plugin-example</artifactId>
<artifactId>errorprone-example</artifactId>
<version>1.0-SNAPSHOT</version>
<packaging>pom</packaging>

<build>
<plugins>
<!-- Turn on Error Prone -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.3</version>
<configuration>
<compilerId>javac-with-errorprone</compilerId>
<forceJavacCompilerUse>true</forceJavacCompilerUse>
<!-- maven-compiler-plugin defaults to targeting Java 5, but our
javac only supports >=6 -->
<source>8</source>
<target>8</target>
<showWarnings>true</showWarnings>
</configuration>
<dependencies>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-compiler-javac-errorprone</artifactId>
<version>2.8</version>
</dependency>
<!-- override plexus-compiler-javac-errorprone's dependency on
Error Prone with the latest version -->
<dependency>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_core</artifactId>
<version>2.3.2-SNAPSHOT</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<javac.version>9+181-r4173-1</javac.version>
</properties>

<profiles>
<profile>
<id>fixerrors</id>
<id>jdk8</id>
<activation>
<jdk>1.8</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.0</version>
<configuration>
<compilerArgs combine.children="append">
<compilerArg>-XepPatchLocation:${basedir}</compilerArg>
<compilerArg>-XepPatchChecks:DeadException,GetClassOnClass</compilerArg>
<forceJavacCompilerUse>true</forceJavacCompilerUse>
<fork>true</fork>
<source>8</source>
<target>8</target>
<compilerArgs>
<arg>-J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar</arg>
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne</arg>
</compilerArgs>
<annotationProcessorPaths>
<path>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_core</artifactId>
<version>2.3.2-SNAPSHOT</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>jdk9</id>
<activation>
<jdk>[9,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.0</version>
<configuration>
<forceJavacCompilerUse>true</forceJavacCompilerUse>
<fork>true</fork>
<source>8</source>
<target>8</target>
<compilerArgs>

This comment has been minimized.

Sign in to view

Copy link
@tbroyer

tbroyer Aug 21, 2018

Contributor

Instead of duplicating everything, you can put the jdk9 configuration outside profile, and only use a jdk8 profile to add the bootclasspath entry (using the combine.children="append" attribute on the compilerArgs to merge the lists)

This comment has been minimized.

Sign in to view

Copy link
@cushon

cushon Aug 21, 2018

Author Collaborator

Thanks! How does this look?

  <profiles>
    <profile>
      <id>jdk8</id>
      <activation>
        <jdk>1.8</jdk>
      </activation>
      <build>
        <plugins>
          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <version>3.8.0</version>
            <configuration>
              <compilerArgs combine.children="append">
                <arg>-J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar</arg>
              </compilerArgs>
            </configuration>
          </plugin>
        </plugins>
      </build>
    </profile>
  </profiles>

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.8.0</version>
        <configuration>
          <forceJavacCompilerUse>true</forceJavacCompilerUse>
          <fork>true</fork>
          <source>8</source>
          <target>8</target>
          <compilerArgs>
            <arg>-XDcompilePolicy=simple</arg>
            <arg>-Xplugin:ErrorProne</arg>
          </compilerArgs>
          <annotationProcessorPaths>
            <path>
              <groupId>com.google.errorprone</groupId>
              <artifactId>error_prone_core</artifactId>
              <version>2.3.2-SNAPSHOT</version>
            </path>
          </annotationProcessorPaths>
        </configuration>
      </plugin>
    </plugins>
  </build>

This comment has been minimized.

Sign in to view

Copy link
@Stephan202

Stephan202 Aug 21, 2018

Contributor

Yes, that should work. You could go one step further by wrapping both <plugins> sections in a <pluginManagement> section. Then:

  • It's more explicit that your leave whether and when to execute the maven-compiler-plugin to the default Maven lifecycle.
  • You can drop the <version>3.8.0</version> declaration from the Java 1.8 profile, since it'll be inherited.

This comment has been minimized.

Sign in to view

Copy link
@Stephan202

Stephan202 Aug 21, 2018

Contributor

(Maybe the first <version>3.8.0</version> can be dropped in any case, but without <pluginManagement> perhaps then the ordering of the sections may matter (some people prefer <profile>s at the bottom), and we get into Maven config territory I'm not familiar with. Our company parent POM uses <pluginManagement> extensively, and I can confirm that way works nicely.)

This comment has been minimized.

Sign in to view

Copy link
@Stephan202

Stephan202 Aug 21, 2018

Contributor

Thinking about it some more: Since the Java 1.8 profile performs an override, it's probably anyway best to present that section second, as that way it'll be easier for people to reason about the configuration.

This comment has been minimized.

Sign in to view

Copy link
@tbroyer

tbroyer Aug 21, 2018

Contributor

You can drop the version in the profile, whether or not you declare it through a pluginManagement. IMO, pluginManagement would complicate the sample without making it really any better.

Also, move the <fork>true</fork> to the jdk8 profile (unless it's really needed for JDK9+ too, but I don't think so)

nit: move the profile after the build section (common configuration first, jdk8-specific next)

nit 2: forceJavacCompilerUse probably is not really needed as that should be the default behavior anyway (see https://github.com/codehaus-plexus/plexus-compiler/blob/e355785479431e2855db9ebffe4ada1f5bd12e31/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java#L172), it's also useless if you <fork>true</fork>.

That'd leave us with:

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.8.0</version>
        <configuration>
          <source>8</source>
          <target>8</target>
          <compilerArgs>
            <arg>-XDcompilePolicy=simple</arg>
            <arg>-Xplugin:ErrorProne</arg>
          </compilerArgs>
          <annotationProcessorPaths>
            <path>
              <groupId>com.google.errorprone</groupId>
              <artifactId>error_prone_core</artifactId>
              <version>2.3.2-SNAPSHOT</version>
            </path>
          </annotationProcessorPaths>
        </configuration>
      </plugin>
    </plugins>
  </build>

  <profiles>
    <profile>
      <id>jdk8</id>
      <activation>
        <jdk>1.8</jdk>
      </activation>
      <build>
        <plugins>
          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <configuration>
              <fork>true</fork>
              <compilerArgs combine.children="append">
                <arg>-J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar</arg>
              </compilerArgs>
            </configuration>
          </plugin>
        </plugins>
      </build>
    </profile>
  </profiles>

This comment has been minimized.

Sign in to view

Copy link
@cushon

cushon Aug 21, 2018

Author Collaborator

Thanks for all of the suggestions! I tried to incorporate all o the improvements into e8c1889 and 99dda7f.
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne</arg>
</compilerArgs>
<annotationProcessorPaths>
<path>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_core</artifactId>
<version>2.3.2-SNAPSHOT</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
</plugins>
Expand Down
50 changes: 0 additions & 50 deletions examples/maven/refaster-based-cleanup/pom.xml
View file

This file was deleted.

26 changes: 0 additions & 26 deletions examples/maven/refaster-based-cleanup/src/main/java/Main.java
View file

This file was deleted.

0 ...prone_should_flag/src/main/java/Main.java → examples/maven/src/main/java/Main.java
View file
File renamed without changes.
0 ...uld_flag/src/main/java/SomethingElse.java → ...es/maven/src/main/java/SomethingElse.java
View file
File renamed without changes.
Empty file added .../maven/target/maven-status/maven-compiler-plugin/compile/default-compile/createdFiles.lst
View file
Empty file.
2 changes: 2 additions & 0 deletions ...es/maven/target/maven-status/maven-compiler-plugin/compile/default-compile/inputFiles.lst
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/usr/local/google/home/cushon/src/error-prone/examples/maven/src/main/java/SomethingElse.java
/usr/local/google/home/cushon/src/error-prone/examples/maven/src/main/java/Main.java

0 comments on commit 82b319c

Please sign in to comment.