From 09e267fb3fae8f841bbb8cfbd36ffd25e48974c6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 10:28:42 -0700 Subject: [PATCH 1/4] Add plumbing to FormatExtension to differentiate between parsing with the intent to include files, versus parsing with the intent to exclude files. --- .../gradle/spotless/FormatExtension.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 99610f614e..bd6cfc43f6 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -136,7 +136,7 @@ public void encoding(String charset) { * Anything else gets passed to getProject().files(). */ public void target(Object... targets) { - this.target = parseTargets(targets); + this.target = parseTargetsIsExclude(targets, false); } /** @@ -150,22 +150,22 @@ public void target(Object... targets) { * Anything else gets passed to getProject().files(). */ public void targetExclude(Object... targets) { - this.targetExclude = parseTargets(targets); + this.targetExclude = parseTargetsIsExclude(targets, true); } - private FileCollection parseTargets(Object[] targets) { + private FileCollection parseTargetsIsExclude(Object[] targets, boolean isExclude) { requireElementsNonNull(targets); if (targets.length == 0) { return getProject().files(); } else if (targets.length == 1) { - return parseTarget(targets[0]); + return parseTargetIsExclude(targets[0], isExclude); } else { if (Stream.of(targets).allMatch(o -> o instanceof String)) { - return parseTarget(Arrays.asList(targets)); + return parseTargetIsExclude(Arrays.asList(targets), isExclude); } else { FileCollection union = getProject().files(); for (Object target : targets) { - union = union.plus(parseTarget(target)); + union = union.plus(parseTargetIsExclude(target, isExclude)); } return union; } @@ -178,8 +178,11 @@ private FileCollection parseTargets(Object[] targets) { * List are treated as the 'includes' arg to fileTree, with project.rootDir as the dir. * Anything else gets passed to getProject().files(). */ - @SuppressWarnings("unchecked") - protected FileCollection parseTarget(Object target) { + protected final FileCollection parseTarget(Object target) { + return parseTargetIsExclude(target, false); + } + + private final FileCollection parseTargetIsExclude(Object target, boolean isExclude) { if (target instanceof FileCollection) { return (FileCollection) target; } else if (target instanceof String || @@ -203,7 +206,9 @@ protected FileCollection parseTarget(Object target) { return (FileCollection) getProject().fileTree(dir).include((String) target).exclude(excludes); } else { // target can only be a List at this point - return (FileCollection) getProject().fileTree(dir).include((List) target).exclude(excludes); + @SuppressWarnings("unchecked") + List targetList = (List) target; + return (FileCollection) getProject().fileTree(dir).include(targetList).exclude(excludes); } } else { return getProject().files(target); From 0e911d49fc36976a27058e3c595919ae05fd6b06 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 10:34:02 -0700 Subject: [PATCH 2/4] Moved the "exclude" logic around to make it easier to turn exclusions off. Pure refactor, no behavior change yet. --- .../gradle/spotless/FormatExtension.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index bd6cfc43f6..6273885603 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -28,6 +28,7 @@ import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.file.FileCollection; +import org.gradle.api.tasks.util.PatternFilterable; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.FormatterFunc; @@ -187,9 +188,18 @@ private final FileCollection parseTargetIsExclude(Object target, boolean isExclu return (FileCollection) target; } else if (target instanceof String || (target instanceof List && ((List) target).stream().allMatch(o -> o instanceof String))) { + File dir = getProject().getProjectDir(); + PatternFilterable userExact; // exactly the collection that the user specified + if (target instanceof String) { + userExact = getProject().fileTree(dir).include((String) target); + } else { + // target can only be a List at this point + @SuppressWarnings("unchecked") + List targetList = (List) target; + userExact = getProject().fileTree(dir).include(targetList); + } // since people are likely to do '**/*.md', we want to make sure to exclude folders // they don't want to format which will slow down the operation greatly - File dir = getProject().getProjectDir(); List excludes = new ArrayList<>(); // no git excludes.add(".git"); @@ -202,14 +212,8 @@ private final FileCollection parseTargetIsExclude(Object target, boolean isExclu for (Project subproject : getProject().getSubprojects()) { relativizeIfSubdir(excludes, dir, subproject.getBuildDir()); } - if (target instanceof String) { - return (FileCollection) getProject().fileTree(dir).include((String) target).exclude(excludes); - } else { - // target can only be a List at this point - @SuppressWarnings("unchecked") - List targetList = (List) target; - return (FileCollection) getProject().fileTree(dir).include(targetList).exclude(excludes); - } + userExact = userExact.exclude(excludes); + return (FileCollection) userExact; } else { return getProject().files(target); } From 8bd94d676bc6c06fe8ce0856df50ca3467faefd4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 18:27:58 -0700 Subject: [PATCH 3/4] FormatExtension is now more selective in what gets excluded, as described in #399. --- .../gradle/spotless/FormatExtension.java | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 6273885603..11fb290ca6 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -135,6 +135,9 @@ public void encoding(String charset) { * Strings are treated as the 'include' arg to fileTree, with project.rootDir as the dir. * List are treated as the 'includes' arg to fileTree, with project.rootDir as the dir. * Anything else gets passed to getProject().files(). + * + * If you pass any strings that start with "**\/*", this method will automatically filter out + * "build", ".gradle", and ".git" folders. */ public void target(Object... targets) { this.target = parseTargetsIsExclude(targets, false); @@ -198,21 +201,32 @@ private final FileCollection parseTargetIsExclude(Object target, boolean isExclu List targetList = (List) target; userExact = getProject().fileTree(dir).include(targetList); } + boolean filterOutGitAndGradle; // since people are likely to do '**/*.md', we want to make sure to exclude folders // they don't want to format which will slow down the operation greatly - List excludes = new ArrayList<>(); - // no git - excludes.add(".git"); - // no .gradle - if (getProject() == getProject().getRootProject()) { - excludes.add(".gradle"); + // but we only want to do that if they are *including* - if they are specifying + // what they want to exclude, we shouldn't filter at all + if (target instanceof String && !isExclude) { + String str = (String) target; + filterOutGitAndGradle = str.startsWith("**/*") || str.startsWith("**\\*"); + } else { + filterOutGitAndGradle = false; } - // no build folders (flatInclude means that subproject might not be subfolders, see https://github.com/diffplug/spotless/issues/121) - relativizeIfSubdir(excludes, dir, getProject().getBuildDir()); - for (Project subproject : getProject().getSubprojects()) { - relativizeIfSubdir(excludes, dir, subproject.getBuildDir()); + if (filterOutGitAndGradle) { + List excludes = new ArrayList<>(); + // no git + excludes.add(".git"); + // no .gradle + if (getProject() == getProject().getRootProject()) { + excludes.add(".gradle"); + } + // no build folders (flatInclude means that subproject might not be subfolders, see https://github.com/diffplug/spotless/issues/121) + relativizeIfSubdir(excludes, dir, getProject().getBuildDir()); + for (Project subproject : getProject().getSubprojects()) { + relativizeIfSubdir(excludes, dir, subproject.getBuildDir()); + } + userExact = userExact.exclude(excludes); } - userExact = userExact.exclude(excludes); return (FileCollection) userExact; } else { return getProject().files(target); From fbdaad5696eca1665ca87b7ad14e9f1c9000a1c0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 19:24:34 -0700 Subject: [PATCH 4/4] Update changelog. --- plugin-gradle/CHANGES.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 888f70e0c5..152e04f531 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -6,13 +6,14 @@ * Fixes [#410](https://github.com/diffplug/spotless/issues/410) AccessDeniedException in MinGW/GitBash. * Also fixes occasional [hang on NFS due to filesystem timers](https://github.com/diffplug/spotless/pull/407#issuecomment-514824364). * Eclipse-based formatters used to leave temporary files around ([#447](https://github.com/diffplug/spotless/issues/447)). This is now fixed, but only for eclipse 4.12+, no back-port to older Eclipse formatter versions is planned. ([#451](https://github.com/diffplug/spotless/issues/451)) -* Fixed a bad but simple bug in `paddedCell()` (https://github.com/diffplug/spotless/pull/455) +* Fixed a bad but simple bug in `paddedCell()` ([#455](https://github.com/diffplug/spotless/pull/455)) - if a formatter was behaving correctly on a given file (was idempotent) - but the file was not properly formatted - `spotlessCheck` would improperly say "all good" even though `spotlessApply` would properly change them - combined with up-to-date checking, could lead to even more confusing results, - (https://github.com/diffplug/spotless/issues/338) + ([#338](https://github.com/diffplug/spotless/issues/338)) - Fixed now! +* When you specify `targetExclude()`, spotless no longer silently removes `build` directories from the exclusion ([#457](https://github.com/diffplug/spotless/pull/457)). ### Version 3.24.2 - August 19th 2019 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-plugin-gradle/3.24.1/), [jcenter](https://bintray.com/diffplug/opensource/spotless-plugin-gradle/3.24.1))