From 4aab05ca6e8224e0f3e656535bb62d94f0850dc7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 7 Sep 2020 21:08:33 -0700 Subject: [PATCH 1/3] Spotless now checks to make sure that Gradle is modern enough. --- .../gradle/spotless/SpotlessPlugin.java | 4 ++ .../spotless/SpotlessPluginRedirect.java | 37 ++++++++++++++++++- .../spotless/SpotlessPluginRedirectTest.java | 32 +++++++++++++++- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java index 2583425448..c62dd03418 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java @@ -15,6 +15,7 @@ */ package com.diffplug.gradle.spotless; +import org.gradle.api.GradleException; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.plugins.BasePlugin; @@ -27,6 +28,9 @@ public class SpotlessPlugin implements Plugin { @Override public void apply(Project project) { + if (SpotlessPluginRedirect.gradleIsTooOld(project)) { + throw new GradleException("Spotless requires Gradle " + MINIMUM_GRADLE + " or newer, this was " + project.getGradle().getGradleVersion()); + } // if -PspotlessModern=true, then use the modern stuff instead of the legacy stuff if (project.hasProperty(SPOTLESS_MODERN)) { project.getLogger().warn("'spotlessModern' has no effect as of Spotless 5.0, recommend removing it."); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java index 741b842892..da4d3dcd8b 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java @@ -15,6 +15,9 @@ */ package com.diffplug.gradle.spotless; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.gradle.api.GradleException; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -22,9 +25,35 @@ import com.diffplug.common.base.StringPrinter; public class SpotlessPluginRedirect implements Plugin { + private static final Pattern BAD_SEMVER = Pattern.compile("(\\d+)\\.(\\d+)"); + + private static int badSemver(String input) { + Matcher matcher = BAD_SEMVER.matcher(input); + if (!matcher.find() || matcher.start() != 0) { + throw new IllegalArgumentException("Version must start with " + BAD_SEMVER.pattern()); + } + String major = matcher.group(1); + String minor = matcher.group(2); + return badSemver(Integer.parseInt(major), Integer.parseInt(minor)); + } + + /** Ambiguous after 2147.483647.blah-blah */ + private static int badSemver(int major, int minor) { + return major * 1_000_000 + minor; + } + + static Boolean gradleIsTooOld; + + static boolean gradleIsTooOld(Project project) { + if (gradleIsTooOld == null) { + gradleIsTooOld = badSemver(project.getGradle().getGradleVersion()) < badSemver(SpotlessPlugin.MINIMUM_GRADLE); + } + return gradleIsTooOld.booleanValue(); + } + @Override public void apply(Project project) { - throw new GradleException(StringPrinter.buildStringFromLines( + String errorMsg = StringPrinter.buildStringFromLines( "We have moved from 'com.diffplug.gradle.spotless'", " to 'com.diffplug.spotless'", "To migrate:", @@ -42,6 +71,10 @@ public void apply(Project project) { "to be useful, hope you do too.", "", "If you like the idea behind 'ratchetFrom', you should checkout spotless-changelog", - "https://github.com/diffplug/spotless-changelog")); + "https://github.com/diffplug/spotless-changelog"); + if (gradleIsTooOld(project)) { + errorMsg = errorMsg.replace("To migrate:\n", "To migrate:\n- Upgrade gradle to " + SpotlessPlugin.MINIMUM_GRADLE + " or newer (you're on " + project.getGradle().getGradleVersion() + ")\n"); + } + throw new GradleException(errorMsg); } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java index a5140b4a19..517bcc619e 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java @@ -24,7 +24,7 @@ public class SpotlessPluginRedirectTest extends GradleIntegrationHarness { @Test - public void test() throws IOException { + public void redirectPluginModernGradle() throws IOException { setFile("build.gradle").toLines( "plugins {", " id 'com.diffplug.gradle.spotless'", @@ -34,6 +34,34 @@ public void test() throws IOException { "> Failed to apply plugin [id 'com.diffplug.gradle.spotless']", " > We have moved from 'com.diffplug.gradle.spotless'", " to 'com.diffplug.spotless'", - " To migrate:")); + " To migrate:", + " - Test your build with: id 'com.diffplug.gradle.spotless' version '4.5.1'")); + } + + @Test + public void redirectPluginOldGradle() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.gradle.spotless'", + "}"); + Assertions.assertThat(gradleRunner().withGradleVersion("2.14").buildAndFail().getOutput().replace("\r", "")) + .contains(StringPrinter.buildStringFromLines( + "> Failed to apply plugin [id 'com.diffplug.gradle.spotless']", + " > We have moved from 'com.diffplug.gradle.spotless'", + " to 'com.diffplug.spotless'", + " To migrate:", + " - Upgrade gradle to 5.4 or newer (you're on 2.14)", + " - Test your build with: id 'com.diffplug.gradle.spotless' version '4.5.1'")); + } + + @Test + public void realPluginOldGradle() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}"); + Assertions.assertThat(gradleRunner().withGradleVersion("2.14").buildAndFail().getOutput().replace("\r", "")) + .contains(StringPrinter.buildStringFromLines( + "Spotless requires Gradle 5.4 or newer, this was 2.14")); } } From ebd86438694cae5a1757232f0667c63ec800c21f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 7 Sep 2020 21:13:37 -0700 Subject: [PATCH 2/3] Add changelog entry. --- plugin-gradle/CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index af7fcd7d0e..1ac023365b 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Fixed +* We did not proactively check to ensure that the Gradle version was modern enough, now we do (fixes [#684](https://github.com/diffplug/spotless/pull/684)). ## [5.3.0] - 2020-08-29 ### Added From f59aff6d708941cca7b821b97c0bb23afaec90f3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 7 Sep 2020 21:30:54 -0700 Subject: [PATCH 3/3] Running tests against 2.14 is challenging (kills Java 11). Easier to just test against the Java 11 minimum, which is Gradle 5.0. Nice to know that it works for the older versions too! --- .../gradle/spotless/GradleIntegrationHarness.java | 2 +- .../gradle/spotless/SpotlessPluginRedirectTest.java | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java index 56dff3a52b..63639448e4 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java @@ -38,7 +38,7 @@ public class GradleIntegrationHarness extends ResourceHarness { public enum GradleVersionSupport { - MINIMUM(SpotlessPlugin.MINIMUM_GRADLE), SETTINGS_PLUGINS("6.0"); + JRE_11("5.0"), MINIMUM(SpotlessPlugin.MINIMUM_GRADLE), SETTINGS_PLUGINS("6.0"); final String version; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java index 517bcc619e..caec393b1b 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessPluginRedirectTest.java @@ -44,13 +44,14 @@ public void redirectPluginOldGradle() throws IOException { "plugins {", " id 'com.diffplug.gradle.spotless'", "}"); - Assertions.assertThat(gradleRunner().withGradleVersion("2.14").buildAndFail().getOutput().replace("\r", "")) + Assertions.assertThat(gradleRunner().withGradleVersion(GradleVersionSupport.JRE_11.version) + .buildAndFail().getOutput().replace("\r", "")) .contains(StringPrinter.buildStringFromLines( "> Failed to apply plugin [id 'com.diffplug.gradle.spotless']", " > We have moved from 'com.diffplug.gradle.spotless'", " to 'com.diffplug.spotless'", " To migrate:", - " - Upgrade gradle to 5.4 or newer (you're on 2.14)", + " - Upgrade gradle to 5.4 or newer (you're on 5.0)", " - Test your build with: id 'com.diffplug.gradle.spotless' version '4.5.1'")); } @@ -60,8 +61,9 @@ public void realPluginOldGradle() throws IOException { "plugins {", " id 'com.diffplug.spotless'", "}"); - Assertions.assertThat(gradleRunner().withGradleVersion("2.14").buildAndFail().getOutput().replace("\r", "")) + Assertions.assertThat(gradleRunner().withGradleVersion(GradleVersionSupport.JRE_11.version) + .buildAndFail().getOutput().replace("\r", "")) .contains(StringPrinter.buildStringFromLines( - "Spotless requires Gradle 5.4 or newer, this was 2.14")); + "Spotless requires Gradle 5.4 or newer, this was 5.0")); } }