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.

Sign up for GitHub

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

Prefer using plugin extensions over deprecated conventions #1618

Merged
merged 8 commits into from
Apr 5, 2023
Merged

Prefer using plugin extensions over deprecated conventions #1618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cb9433f
Prefer using plugin extensions over deprecated conventions
Goooler Mar 11, 2023
4410531
Reuse getSources logic for Jvm languages
Goooler Mar 11, 2023
1c21d12
Centralize gradle versions so we can remove version-specific workarou…
nedtwigg Mar 28, 2023
1470666
More version centralizing.
nedtwigg Apr 5, 2023
9f49068
Improve the GroovyExtension error message when there is no target and…
nedtwigg Apr 5, 2023
45e078d
Update changelog.
nedtwigg Apr 5, 2023
d7ca743
Fix groovy test for new error message.
nedtwigg Apr 5, 2023
91439e5
spotlessApply
nedtwigg Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions plugin-gradle/CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Changes
* **POTENTIALLY BREAKING** Drop support for `googleJavaFormat` versions < `1.8`. ([#1630](https://github.com/diffplug/spotless/pull/1630))
* Bump default `googleJavaFormat` version `1.15.0` -> `1.16.0`. ([#1630](https://github.com/diffplug/spotless/pull/1630))
### Fixed
* Stop using deprecated conventions when used in Gradle >= `7.1`. ([#1618](https://github.com/diffplug/spotless/pull/1618))

## [6.17.0] - 2023-03-13
### Added
Expand Down
43 changes: 24 additions & 19 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GroovyExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,18 @@

import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.file.FileCollection;
import org.gradle.api.internal.plugins.DslObject;
import org.gradle.api.plugins.GroovyBasePlugin;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.tasks.GroovySourceDirectorySet;
import org.gradle.api.tasks.GroovySourceSet;
import org.gradle.api.tasks.SourceSet;
import org.gradle.util.GradleVersion;

import com.diffplug.spotless.extra.EquoBasedStepBuilder;
import com.diffplug.spotless.extra.groovy.GrEclipseFormatterStep;
import com.diffplug.spotless.generic.LicenseHeaderStep;
import com.diffplug.spotless.java.ImportOrderStep;

public class GroovyExtension extends FormatExtension implements HasBuiltinDelimiterForLicense {
public class GroovyExtension extends FormatExtension implements HasBuiltinDelimiterForLicense, JvmLang {
static final String NAME = "groovy";

@Inject
Expand Down Expand Up @@ -112,22 +111,28 @@ public GrEclipseConfig withP2Mirrors(Map<String, String> mirrors) {
@Override
protected void setupTask(SpotlessTask task) {
if (target == null) {
JavaPluginConvention convention = getProject().getConvention().getPlugin(JavaPluginConvention.class);
if (convention == null || !getProject().getPlugins().hasPlugin(GroovyBasePlugin.class)) {
throw new GradleException("You must apply the groovy plugin before the spotless plugin if you are using the groovy extension.");
final String message = "You must either specify 'target' manually or apply the 'groovy' plugin.";
if (!getProject().getPlugins().hasPlugin(GroovyBasePlugin.class)) {
throw new GradleException(message);
}
//Add all Groovy files (may contain Java files as well)

FileCollection union = getProject().files();
for (SourceSet sourceSet : convention.getSourceSets()) {
GroovySourceSet groovySourceSet = new DslObject(sourceSet).getConvention().getPlugin(GroovySourceSet.class);
if (excludeJava) {
union = union.plus(groovySourceSet.getAllGroovy());
} else {
union = union.plus(groovySourceSet.getGroovy());
}
}
target = union;
target = getSources(getProject(),
message,
sourceSet -> {
if (GradleVersion.current().compareTo(GradleVersion.version(SpotlessPlugin.VER_GRADLE_javaPluginExtension)) >= 0) {
return sourceSet.getExtensions().getByType(GroovySourceDirectorySet.class);
} else {
final GroovySourceSet groovySourceSet = new DslObject(sourceSet).getConvention().getPlugin(GroovySourceSet.class);
return groovySourceSet.getGroovy();
}
},
file -> {
final String name = file.getName();
if (excludeJava) {
return name.endsWith(".groovy");
} else {
return name.endsWith(".groovy") || name.endsWith(".java");
}
});
} else if (excludeJava) {
throw new IllegalArgumentException("'excludeJava' is not supported in combination with a custom 'target'.");
}
Expand Down
18 changes: 5 additions & 13 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@

import javax.inject.Inject;

import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.tasks.SourceSet;

import com.diffplug.spotless.FormatterStep;
Expand All @@ -43,7 +40,7 @@
import com.diffplug.spotless.java.PalantirJavaFormatStep;
import com.diffplug.spotless.java.RemoveUnusedImportsStep;

public class JavaExtension extends FormatExtension implements HasBuiltinDelimiterForLicense {
public class JavaExtension extends FormatExtension implements HasBuiltinDelimiterForLicense, JvmLang {
static final String NAME = "java";

@Inject
Expand Down Expand Up @@ -366,15 +363,10 @@ private FormatterStep createStep() {
@Override
protected void setupTask(SpotlessTask task) {
if (target == null) {
JavaPluginConvention javaPlugin = getProject().getConvention().findPlugin(JavaPluginConvention.class);
if (javaPlugin == null) {
throw new GradleException("You must either specify 'target' manually or apply the 'java' plugin.");
}
FileCollection union = getProject().files();
for (SourceSet sourceSet : javaPlugin.getSourceSets()) {
union = union.plus(sourceSet.getAllJava());
}
target = union;
target = getSources(getProject(),
"You must either specify 'target' manually or apply the 'java' plugin.",
SourceSet::getAllJava,
file -> file.getName().endsWith(".java"));
}

steps.replaceAll(step -> {
Expand Down
55 changes: 55 additions & 0 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLang.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.gradle.spotless;

import java.io.File;
import java.util.function.Function;

import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.SourceDirectorySet;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.plugins.JavaPluginExtension;
import org.gradle.api.specs.Spec;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;
import org.gradle.util.GradleVersion;

interface JvmLang {

default FileCollection getSources(Project project, String message, Function<SourceSet, SourceDirectorySet> sourceSetSourceDirectory, Spec<? super File> filterSpec) {
final SourceSetContainer sourceSets;
FileCollection union = project.files();
if (GradleVersion.current().compareTo(GradleVersion.version(SpotlessPlugin.VER_GRADLE_javaPluginExtension)) >= 0) {
final JavaPluginExtension javaPluginExtension = project.getExtensions().findByType(JavaPluginExtension.class);
if (javaPluginExtension == null) {
throw new GradleException(message);
}
sourceSets = javaPluginExtension.getSourceSets();
} else {
final JavaPluginConvention javaPluginConvention = project.getConvention().findPlugin(JavaPluginConvention.class);
if (javaPluginConvention == null) {
throw new GradleException(message);
}
sourceSets = javaPluginConvention.getSourceSets();
}
for (SourceSet sourceSet : sourceSets) {
union = union.plus(sourceSetSourceDirectory.apply(sourceSet).filter(filterSpec));
}
return union;
}
}
24 changes: 8 additions & 16 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/KotlinExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@
import javax.annotation.Nullable;
import javax.inject.Inject;

import org.gradle.api.GradleException;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.tasks.SourceSet;

import com.diffplug.common.collect.ImmutableSortedMap;
Expand All @@ -41,7 +38,7 @@
import com.diffplug.spotless.kotlin.KtfmtStep.KtfmtFormattingOptions;
import com.diffplug.spotless.kotlin.KtfmtStep.Style;

public class KotlinExtension extends FormatExtension implements HasBuiltinDelimiterForLicense {
public class KotlinExtension extends FormatExtension implements HasBuiltinDelimiterForLicense, JvmLang {
static final String NAME = "kotlin";

@Inject
Expand Down Expand Up @@ -230,18 +227,13 @@ private FormatterStep createStep() {
@Override
protected void setupTask(SpotlessTask task) {
if (target == null) {
JavaPluginConvention javaPlugin = getProject().getConvention().findPlugin(JavaPluginConvention.class);
if (javaPlugin == null) {
throw new GradleException("You must either specify 'target' manually or apply a kotlin plugin.");
}
FileCollection union = getProject().files();
for (SourceSet sourceSet : javaPlugin.getSourceSets()) {
union = union.plus(sourceSet.getAllSource().filter(file -> {
String name = file.getName();
return name.endsWith(".kt") || name.endsWith(".kts");
}));
}
target = union;
target = getSources(getProject(),
"You must either specify 'target' manually or apply a kotlin plugin.",
SourceSet::getAllSource,
file -> {
final String name = file.getName();
return name.endsWith(".kt") || name.endsWith(".kts");
});
}
super.setupTask(task);
}
Expand Down
26 changes: 9 additions & 17 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/ScalaExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2022 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,15 +21,12 @@
import javax.annotation.Nullable;
import javax.inject.Inject;

import org.gradle.api.GradleException;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.tasks.SourceSet;

import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.scala.ScalaFmtStep;

public class ScalaExtension extends FormatExtension {
public class ScalaExtension extends FormatExtension implements JvmLang {
static final String NAME = "scala";

@Inject
Expand Down Expand Up @@ -79,18 +76,13 @@ private FormatterStep createStep() {
@Override
protected void setupTask(SpotlessTask task) {
if (target == null) {
JavaPluginConvention javaPlugin = getProject().getConvention().findPlugin(JavaPluginConvention.class);
if (javaPlugin == null) {
throw new GradleException("You must either specify 'target' manually or apply the 'scala' plugin.");
}
FileCollection union = getProject().files();
for (SourceSet sourceSet : javaPlugin.getSourceSets()) {
union = union.plus(sourceSet.getAllSource().filter(file -> {
String name = file.getName();
return name.endsWith(".scala") || name.endsWith(".sc");
}));
}
target = union;
target = getSources(getProject(),
"You must either specify 'target' manually or apply the 'scala' plugin.",
SourceSet::getAllSource,
file -> {
final String name = file.getName();
return name.endsWith(".scala") || name.endsWith(".sc");
});
}
super.setupTask(task);
}
Expand Down
5 changes: 3 additions & 2 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@

public class SpotlessPlugin implements Plugin<Project> {
static final String SPOTLESS_MODERN = "spotlessModern";
static final String MINIMUM_GRADLE = "6.1.1";
static final String VER_GRADLE_min = "6.1.1";
static final String VER_GRADLE_javaPluginExtension = "7.1";
Comment on lines +29 to +30
Copy link
Member Author

@Goooler Goooler Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

May need to bump VER_GRADLE_min to 7.1 in the future, using conventions will start emitting deprecation warnings in Gradle 8.1, and will be removed in the later Gradle 8 versions.

Copy link
Member

Choose a reason for hiding this comment

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

Roger. If we are throwing warnings that we can't easily workaround with a version check, then I'm happy to bump the minimum.

Copy link
Member Author

@Goooler Goooler Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@nedtwigg Are you planning to move on Gradle plugin 7? If so can we bump the min Gradle requirement to 7.1 for something like this?

Copy link
Member

Choose a reason for hiding this comment

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

It thought that this

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JvmLang.java

Lines 37 to 49 in ed88d33

if (GradleVersion.current().compareTo(GradleVersion.version(SpotlessPlugin.VER_GRADLE_javaPluginExtension)) >= 0) {
final JavaPluginExtension javaPluginExtension = project.getExtensions().findByType(JavaPluginExtension.class);
if (javaPluginExtension == null) {
throw new GradleException(message);
}
sourceSets = javaPluginExtension.getSourceSets();
} else {
final JavaPluginConvention javaPluginConvention = project.getConvention().findPlugin(JavaPluginConvention.class);
if (javaPluginConvention == null) {
throw new GradleException(message);
}
sourceSets = javaPluginConvention.getSourceSets();
}

would let us keep 6.1.1 as our min and not emit warnings. But if we are emitting warnings and its very difficult to workaround, then sure I'm okay with bumping the minimum.

Copy link
Member Author

Choose a reason for hiding this comment

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

And 682dcd6.

private static final int MINIMUM_JRE = 11;

@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());
throw new GradleException("Spotless requires Gradle " + VER_GRADLE_min + " or newer, this was " + project.getGradle().getGradleVersion());
}
if (Jvm.version() < MINIMUM_JRE) {
throw new GradleException("Spotless requires JRE " + MINIMUM_JRE + " or newer, this was " + JavaVersion.current() + ".\n"
Expand Down
6 changes: 3 additions & 3 deletions plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPluginRedirect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 DiffPlug
* Copyright 2020-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,7 +46,7 @@ private static int badSemver(int major, int minor) {

static boolean gradleIsTooOld(Project project) {
if (gradleIsTooOld == null) {
gradleIsTooOld = badSemver(project.getGradle().getGradleVersion()) < badSemver(SpotlessPlugin.MINIMUM_GRADLE);
gradleIsTooOld = badSemver(project.getGradle().getGradleVersion()) < badSemver(SpotlessPlugin.VER_GRADLE_min);
}
return gradleIsTooOld.booleanValue();
}
Expand All @@ -73,7 +73,7 @@ public void apply(Project project) {
"If you like the idea behind 'ratchetFrom', you should checkout 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");
errorMsg = errorMsg.replace("To migrate:\n", "To migrate:\n- Upgrade gradle to " + SpotlessPlugin.VER_GRADLE_min + " or newer (you're on " + project.getGradle().getGradleVersion() + ")\n");
}
throw new GradleException(errorMsg);
}
Expand Down
2 changes: 1 addition & 1 deletion plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

public class GradleIntegrationHarness extends ResourceHarness {
public enum GradleVersionSupport {
JRE_11("5.0"), MINIMUM(SpotlessPlugin.MINIMUM_GRADLE),
JRE_11("5.0"), MINIMUM(SpotlessPlugin.VER_GRADLE_min),
// technically, this API exists in 6.5, but the flags for it change in 6.6, so we build to that
CONFIGURATION_CACHE("6.6"),
// https://docs.gradle.org/7.5/userguide/configuration_cache.html#config_cache:stable
Expand Down
4 changes: 2 additions & 2 deletions plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GroovyExtensionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -103,7 +103,7 @@ void groovyPluginMissingCheck() throws IOException {

Throwable error = assertThrows(Throwable.class,
() -> gradleRunner().withArguments("spotlessApply").build());
assertThat(error).hasMessageContaining("must apply the groovy plugin before");
assertThat(error).hasMessageContaining("You must either specify 'target' manually or apply the 'groovy' plugin.");
}

}