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

Support google-java-format 1.8+ #563

Merged
merged 13 commits into from
May 3, 2020

Conversation

benkard
Copy link
Contributor

@benkard benkard commented May 2, 2020

In google-java-format 1.8, the signature of
RemoveUnusedImports#removeUnusedImports changed and the
RemoveUnusedImports$JavadocOnlyImports class was removed entirely.
With this patch, GoogleJavaFormatStep detects which version of the
method is available and calls the right one.

Fixes #562.

In google-java-format 1.8, the signature of
RemoveUnusedImports#removeUnusedImports changed and the
RemoveUnusedImports$JavadocOnlyImports class was removed entirely.
With this patch, GoogleJavaFormatStep detects which version of the
method is available and calls the right one.

Fixes diffplug#562.
@nedtwigg
Copy link
Member

nedtwigg commented May 3, 2020

@diffplug/spotless Just FYI, this is the first PR we have received where the formatter requires Java 11+, and there are more coming (#547). As a result, we now need our tests to run on Java 11.

This PR represents the minimal effort needed to merge support for google-java-format 1.8 with tests. We now run the whole build on both 8 and 11. The only things I disabled on 11 are things which are broken there, namely

  • plugin-maven
  • javadoc generation
  • spotbugs (I think already fixed in newer spotbugs)

The trickier part is that now there are some tests which you can't run on 8, because they'll break. Also, Gradle 5.0 is the oldest that can run on Java 11, so you can't run old-gradle-tests on 11.

I'm very open to PRs for a more comprehensive solution, but I think it's fine to play whack-a-mole until a pattern merges. Here's the gist of the 8 / 11 test infrastructure I put in:

public enum JreVersion {
_8, _11;
public static JreVersion thisVm() {

@Test
public void behavior18() throws Exception {
if (JreVersion.thisVm() == JreVersion._8) {
// google-java-format requires JRE 11+
return;
}

/**
* For Java 11+, Gradle 5 is the minimum.
* So if you ask for less than Gradle 5, you get it on Java 8, but on Java 11 you get promoted to Gradle 5.
* If you ask for more than Gradle 5, you'll definitely get it.
*/
protected static String requestGradleForJre8and11(String ver) {
JreVersion jre = JreVersion.thisVm();
// @formatter:off
switch (jre) {
case _8: return ver;
case _11: return Double.parseDouble(ver) < 5.0 ? "5.0" : ver;
default: throw new IllegalStateException("Spotless build is only supported on Java 8 and Java 11");
}
// @formatter:on
}
protected final GradleRunner gradleRunner() throws IOException {
return GradleRunner.create()
.withGradleVersion(requestGradleForJre8and11("2.14"))

@nedtwigg
Copy link
Member

nedtwigg commented May 5, 2020

Released in plugin-gradle 3.29.0, and plugin-maven 1.31.0

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.

ClassNotFoundException with google-java-format 1.8
2 participants