diff --git a/CHANGES.md b/CHANGES.md index bf65c5a019..fc2b6e827b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added -* APIs to support linting. (implemented in [#2148](https://github.com/diffplug/spotless/pull/2148) and [#2149](https://github.com/diffplug/spotless/pull/2149)) +* APIs to support linting. (implemented in [#2148](https://github.com/diffplug/spotless/pull/2148), [#2149](https://github.com/diffplug/spotless/pull/2149), [#2307](https://github.com/diffplug/spotless/pull/2307)) * Spotless is still primarily a formatter, not a linter. But when formatting fails, it's more flexible to model those failures as lints so that the formatting can continue and ideally we can also capture the line numbers causing the failure. * `Lint` models a single change. A `FormatterStep` can create a lint by: * throwing an exception during formatting, ideally `throw Lint.atLine(127, "code", "Well what happened was...")` diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 964cd102e1..a5d269aa70 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -80,7 +80,7 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw) { var valuePerStep = new ValuePerStep(formatter); DirtyState state = of(formatter, file, rawBytes, raw, valuePerStep); - LintPolicy.legacyBehavior(formatter, file, valuePerStep); + Formatter.legacyErrorBehavior(formatter, file, valuePerStep); return state; } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java deleted file mode 100644 index 9bafc946ef..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2016-2024 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.spotless; - -import java.util.Objects; -import java.util.Set; -import java.util.TreeSet; - -/** - * A policy for handling exceptions in the format. Any exceptions will - * halt the build except for a specifically excluded path or step. - */ -public class FormatExceptionPolicyStrict extends NoLambda.EqualityBasedOnSerialization { - private static final long serialVersionUID = 1L; - - private final Set excludeSteps = new TreeSet<>(); - private final Set excludePaths = new TreeSet<>(); - - /** Adds a step name to exclude. */ - public void excludeStep(String stepName) { - excludeSteps.add(Objects.requireNonNull(stepName)); - } - - /** Adds a relative path to exclude. */ - public void excludePath(String relativePath) { - excludePaths.add(Objects.requireNonNull(relativePath)); - } - - public void handleError(Throwable e, FormatterStep step, String relativePath) { - Objects.requireNonNull(e, "e"); - Objects.requireNonNull(step, "step"); - Objects.requireNonNull(relativePath, "relativePath"); - if (excludeSteps.contains(step.getName())) { - LintPolicy.warning(e, step, relativePath); - } else { - if (excludePaths.contains(relativePath)) { - LintPolicy.warning(e, step, relativePath); - } else { - LintPolicy.error(e, step, relativePath); - throw ThrowingEx.asRuntimeRethrowError(e); - } - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 970d5eeeb4..0635f8dfe6 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -28,6 +28,9 @@ import java.util.List; import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** Formatter which performs the full formatting. */ public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; @@ -130,10 +133,22 @@ public String computeLineEndings(String unix, File file) { public String compute(String unix, File file) { ValuePerStep exceptionPerStep = new ValuePerStep<>(this); String result = computeWithLint(unix, file, exceptionPerStep); - LintPolicy.legacyBehavior(this, file, exceptionPerStep); + legacyErrorBehavior(this, file, exceptionPerStep); return result; } + static void legacyErrorBehavior(Formatter formatter, File file, ValuePerStep exceptionPerStep) { + for (int i = 0; i < formatter.getSteps().size(); ++i) { + Throwable exception = exceptionPerStep.get(i); + if (exception != null && exception != LintState.formatStepCausedNoChange()) { + logger.error("Step '{}' found problem in '{}':\n{}", formatter.getSteps().get(i).getName(), file.getName(), exception.getMessage(), exception); + throw ThrowingEx.asRuntimeRethrowError(exception); + } + } + } + + private static final Logger logger = LoggerFactory.getLogger(Formatter.class); + /** * Returns the result of calling all of the FormatterSteps, while also * tracking any exceptions which are thrown. diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 4384ed38ef..b48a7bcbd2 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -37,28 +37,28 @@ public static Lint atLine(int line, String ruleId, String detail) { return new Lint(line, ruleId, detail); } - public static Lint atLineRange(int lineStart, int lineEnd, String ruleId, String detail) { - return new Lint(lineStart, lineEnd, ruleId, detail); + public static Lint atLineRange(int lineStart, int lineEnd, String shortCode, String detail) { + return new Lint(lineStart, lineEnd, shortCode, detail); } private static final long serialVersionUID = 1L; private int lineStart, lineEnd; // 1-indexed, inclusive - private String ruleId; // e.g. CN_IDIOM https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#cn-class-implements-cloneable-but-does-not-define-or-use-clone-method-cn-idiom + private String shortCode; // e.g. CN_IDIOM https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#cn-class-implements-cloneable-but-does-not-define-or-use-clone-method-cn-idiom private String detail; - private Lint(int lineStart, int lineEnd, String ruleId, String detail) { + private Lint(int lineStart, int lineEnd, String shortCode, String detail) { if (lineEnd < lineStart) { throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd); } this.lineStart = lineStart; this.lineEnd = lineEnd; - this.ruleId = LineEnding.toUnix(ruleId); + this.shortCode = LineEnding.toUnix(shortCode); this.detail = LineEnding.toUnix(detail); } - private Lint(int line, String ruleId, String detail) { - this(line, line, ruleId, detail); + private Lint(int line, String shortCode, String detail) { + this(line, line, shortCode, detail); } public int getLineStart() { @@ -69,8 +69,8 @@ public int getLineEnd() { return lineEnd; } - public String getRuleId() { - return ruleId; + public String getShortCode() { + return shortCode; } public String getDetail() { @@ -115,12 +115,12 @@ public RuntimeException shortcut() { public String toString() { if (lineStart == lineEnd) { if (lineStart == LINE_UNDEFINED) { - return "LINE_UNDEFINED: (" + ruleId + ") " + detail; + return "LINE_UNDEFINED: (" + shortCode + ") " + detail; } else { - return "L" + lineStart + ": (" + ruleId + ") " + detail; + return "L" + lineStart + ": (" + shortCode + ") " + detail; } } else { - return "L" + lineStart + "-" + lineEnd + ": (" + ruleId + ") " + detail; + return "L" + lineStart + "-" + lineEnd + ": (" + shortCode + ") " + detail; } } @@ -131,12 +131,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; Lint lint = (Lint) o; - return lineStart == lint.lineStart && lineEnd == lint.lineEnd && Objects.equals(ruleId, lint.ruleId) && Objects.equals(detail, lint.detail); + return lineStart == lint.lineStart && lineEnd == lint.lineEnd && Objects.equals(shortCode, lint.shortCode) && Objects.equals(detail, lint.detail); } @Override public int hashCode() { - return Objects.hash(lineStart, lineEnd, ruleId, detail); + return Objects.hash(lineStart, lineEnd, shortCode, detail); } /** Attempts to parse a line number from the given exception. */ @@ -189,4 +189,28 @@ private static String msgFrom(String message) { } public static final int LINE_UNDEFINED = -1; + + public void addWarningMessageTo(StringBuilder buffer, String stepName, boolean oneLine) { + if (lineStart == Lint.LINE_UNDEFINED) { + buffer.append("LINE_UNDEFINED"); + } else { + buffer.append("L"); + buffer.append(lineStart); + if (lineEnd != lineStart) { + buffer.append("-").append(lineEnd); + } + } + buffer.append(" "); + buffer.append(stepName).append("(").append(shortCode).append(") "); + + int firstNewline = detail.indexOf('\n'); + if (firstNewline == -1) { + buffer.append(detail); + } else if (oneLine) { + buffer.append(detail, 0, firstNewline); + buffer.append(" (...)"); + } else { + buffer.append(detail); + } + } } diff --git a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java deleted file mode 100644 index 0b28c36884..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2024 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.spotless; - -import java.io.File; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -class LintPolicy { - private static final Logger logger = LoggerFactory.getLogger(Formatter.class); - - static void error(Throwable e, FormatterStep step, String relativePath) { - logger.error("Step '{}' found problem in '{}':\n{}", step.getName(), relativePath, e.getMessage(), e); - } - - static void warning(Throwable e, FormatterStep step, String relativePath) { - logger.warn("Unable to apply step '{}' to '{}'", step.getName(), relativePath, e); - } - - static void legacyBehavior(Formatter formatter, File file, ValuePerStep exceptionPerStep) { - for (int i = 0; i < formatter.getSteps().size(); ++i) { - Throwable exception = exceptionPerStep.get(i); - if (exception != null && exception != LintState.formatStepCausedNoChange()) { - LintPolicy.error(exception, formatter.getSteps().get(i), file.getName()); - throw ThrowingEx.asRuntimeRethrowError(exception); - } - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index b2b4030e93..3068872256 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -18,9 +18,10 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import javax.annotation.Nullable; @@ -28,7 +29,7 @@ public class LintState { private final DirtyState dirtyState; private final @Nullable List> lintsPerStep; - private LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { + LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { this.dirtyState = dirtyState; this.lintsPerStep = lintsPerStep; } @@ -45,23 +46,61 @@ public boolean isClean() { return dirtyState.isClean() && !isHasLints(); } - public Map> getLints(Formatter formatter) { + public LinkedHashMap> getLintsByStep(Formatter formatter) { if (lintsPerStep == null) { throw new IllegalStateException("Check `isHasLints` first!"); } if (lintsPerStep.size() != formatter.getSteps().size()) { throw new IllegalStateException("LintState was created with a different formatter!"); } - Map> result = new LinkedHashMap<>(); + LinkedHashMap> result = new LinkedHashMap<>(); for (int i = 0; i < lintsPerStep.size(); i++) { List lints = lintsPerStep.get(i); if (lints != null) { - result.put(formatter.getSteps().get(i), lints); + FormatterStep step = formatter.getSteps().get(i); + result.put(step.getName(), lints); } } return result; } + public LintState withRemovedSuppressions(Formatter formatter, String relativePath, List suppressions) { + if (lintsPerStep == null) { + return this; + } + if (formatter.getSteps().size() != lintsPerStep.size()) { + throw new IllegalStateException("LintState was created with a different formatter!"); + } + boolean changed = false; + ValuePerStep> perStepFiltered = new ValuePerStep<>(formatter); + for (int i = 0; i < lintsPerStep.size(); i++) { + FormatterStep step = formatter.getSteps().get(i); + List lintsOriginal = lintsPerStep.get(i); + if (lintsOriginal != null) { + List lints = new ArrayList<>(lintsOriginal); + Iterator iter = lints.iterator(); + while (iter.hasNext()) { + Lint lint = iter.next(); + for (LintSuppression suppression : suppressions) { + if (suppression.suppresses(relativePath, step, lint)) { + changed = true; + iter.remove(); + break; + } + } + } + if (!lints.isEmpty()) { + perStepFiltered.set(i, lints); + } + } + } + if (changed) { + return new LintState(dirtyState, perStepFiltered.indexOfFirstValue() == -1 ? null : perStepFiltered); + } else { + return this; + } + } + public String asStringDetailed(File file, Formatter formatter) { return asString(file, formatter, false); } @@ -81,27 +120,7 @@ private String asString(File file, Formatter formatter, boolean oneLine) { FormatterStep step = formatter.getSteps().get(i); for (Lint lint : lints) { result.append(file.getName()).append(":"); - if (lint.getLineStart() == Lint.LINE_UNDEFINED) { - result.append("LINE_UNDEFINED"); - } else { - result.append("L"); - result.append(lint.getLineStart()); - if (lint.getLineEnd() != lint.getLineStart()) { - result.append("-").append(lint.getLineEnd()); - } - } - result.append(" "); - result.append(step.getName()).append("(").append(lint.getRuleId()).append(") "); - - int firstNewline = lint.getDetail().indexOf('\n'); - if (firstNewline == -1) { - result.append(lint.getDetail()); - } else if (oneLine) { - result.append(lint.getDetail(), 0, firstNewline); - result.append(" (...)"); - } else { - result.append(lint.getDetail()); - } + lint.addWarningMessageTo(result, step.getName(), oneLine); result.append("\n"); } } diff --git a/lib/src/main/java/com/diffplug/spotless/LintSuppression.java b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java new file mode 100644 index 0000000000..d84107bd3a --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java @@ -0,0 +1,93 @@ +/* + * Copyright 2024 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.spotless; + +import java.util.Objects; + +public class LintSuppression implements java.io.Serializable { + private static final long serialVersionUID = 1L; + + private static final String ALL = "*"; + private String path = ALL; + private String step = ALL; + private String shortCode = ALL; + + public String getPath() { + return path; + } + + public void setPath(String path) { + this.path = Objects.requireNonNull(path); + } + + public String getStep() { + return step; + } + + public void setStep(String step) { + this.step = Objects.requireNonNull(step); + } + + public String getShortCode() { + return shortCode; + } + + public void setShortCode(String shortCode) { + this.shortCode = Objects.requireNonNull(shortCode); + } + + public boolean suppresses(String relativePath, FormatterStep formatterStep, Lint lint) { + if (path.equals(ALL) || path.equals(relativePath)) { + if (step.equals(ALL) || formatterStep.getName().equals(this.step)) { + if (shortCode.equals(ALL) || lint.getShortCode().equals(this.shortCode)) { + return true; + } + } + } + return false; + } + + public void ensureDoesNotSuppressAll() { + boolean suppressAll = path.equals(ALL) && step.equals(ALL) && shortCode.equals(ALL); + if (suppressAll) { + throw new IllegalArgumentException("You must specify a specific `file`, `step`, or `shortCode`."); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + LintSuppression that = (LintSuppression) o; + return Objects.equals(path, that.path) && Objects.equals(step, that.step) && Objects.equals(shortCode, that.shortCode); + } + + @Override + public int hashCode() { + return Objects.hash(path, step, shortCode); + } + + @Override + public String toString() { + return "LintSuppression{" + + "file='" + path + '\'' + + ", step='" + step + '\'' + + ", code='" + shortCode + '\'' + + '}'; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierMissingParserException.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierMissingParserException.java index cc04c7b182..9728b61eec 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierMissingParserException.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierMissingParserException.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,12 +19,15 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import javax.annotation.Nonnull; -class PrettierMissingParserException extends RuntimeException { +import com.diffplug.spotless.Lint; + +class PrettierMissingParserException extends RuntimeException implements Lint.Has { private static final long serialVersionUID = 1L; private static final Map EXTENSIONS_TO_PLUGINS; @@ -87,6 +90,11 @@ public PrettierMissingParserException(@Nonnull File file, Exception cause) { this.file = Objects.requireNonNull(file); } + @Override + public List getLints() { + return List.of(Lint.atUndefinedLine("no-parser", "Could not infer a parser. Maybe you need to include a prettier plugin in devDependencies? e.g. " + recommendPlugin(file))); + } + private static String recommendPlugin(File file) { String pluginName = guessPlugin(file); return "A good candidate for file '" + file + "' is '" + pluginName + "\n" diff --git a/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java new file mode 100644 index 0000000000..1e1037a6ef --- /dev/null +++ b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java @@ -0,0 +1,82 @@ +/* + * Copyright 2024 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.spotless; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; + +import org.assertj.core.api.AbstractBooleanAssert; +import org.junit.jupiter.api.Test; + +import com.diffplug.spotless.generic.EndWithNewlineStep; + +public class LintSuppressionTest { + private LintState dummyLintState() { + var lints = new ArrayList(); + lints.add(Lint.atLine(2, "66", "Order 66")); + var perStep = new ArrayList>(); + perStep.add(lints); + return new LintState(DirtyState.clean(), perStep); + } + + private Formatter formatter() { + return Formatter.builder() + .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) + .encoding(StandardCharsets.UTF_8) + .steps(List.of(EndWithNewlineStep.create())) + .build(); + } + + @Test + public void testMatchSingle() { + var noSuppressions = dummyLintState().withRemovedSuppressions(formatter(), "file", List.of()); + assertThat(noSuppressions.isHasLints()).isTrue(); + removesLint(s -> s.setStep("blah")).isFalse(); + removesLint(s -> s.setStep("endWithNewline")).isTrue(); + removesLint(s -> s.setPath("blah")).isFalse(); + removesLint(s -> s.setPath("testFile")).isTrue(); + removesLint(s -> s.setShortCode("blah")).isFalse(); + removesLint(s -> s.setShortCode("66")).isTrue(); + } + + @Test + public void testMatchDouble() { + removesLint(s -> { + s.setStep("endWithNewline"); + s.setShortCode("blah"); + }).isFalse(); + removesLint(s -> { + s.setStep("blah"); + s.setShortCode("66"); + }).isFalse(); + removesLint(s -> { + s.setStep("endWithNewline"); + s.setShortCode("66"); + }).isTrue(); + } + + private AbstractBooleanAssert removesLint(Consumer suppression) { + var s = new LintSuppression(); + suppression.accept(s); + + var ls = dummyLintState().withRemovedSuppressions(formatter(), "testFile", List.of(s)); + return assertThat(!ls.isHasLints()); + } +} diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index c9bdcee643..e5f7aa36ed 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -5,6 +5,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * Support for line ending policy `PRESERVE` which just takes the first line ending of every given file as setting (no matter if `\n`, `\r\n` or `\r`) ([#2304](https://github.com/diffplug/spotless/pull/2304)) +* New `suppressLintsFor` DSL ([docs](https://github.com/diffplug/spotless/tree/main/plugin-gradle#linting)) ([#2307](https://github.com/diffplug/spotless/pull/2307)) + * `ignoreErrorForStep` and `ignoreErrorForPath` are now deprecated aliases of `suppressLintsFor` + * Spotless is still a formatter not a linter, it just models formatting failures as lints rather than stopping execution (resolves [#287](https://github.com/diffplug/spotless/issues/287)) ### Fixed * `ktlint` steps now read from the `string` instead of the `file` so they don't clobber earlier steps. (fixes [#1599](https://github.com/diffplug/spotless/issues/1599)) diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 80fcafb7bb..41c1b6e1a3 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -53,6 +53,7 @@ Spotless supports all of Gradle's built-in performance features (incremental bui - [**Quickstart**](#quickstart) - [Requirements](#requirements) + - [Linting](#linting) - **Languages** - [Java](#java) ([google-java-format](#google-java-format), [eclipse jdt](#eclipse-jdt), [clang-format](#clang-format), [prettier](#prettier), [palantir-java-format](#palantir-java-format), [formatAnnotations](#formatAnnotations), [cleanthat](#cleanthat)) - [Groovy](#groovy) ([eclipse groovy](#eclipse-groovy)) @@ -74,11 +75,8 @@ Spotless supports all of Gradle's built-in performance features (incremental bui - [Gherkin](#gherkin) - Multiple languages - [Prettier](#prettier) ([plugins](#prettier-plugins), [npm detection](#npm-detection), [`.npmrc` detection](#npmrc-detection), [caching `npm install` results](#caching-results-of-npm-install)) - - javascript, jsx, angular, vue, flow, typescript, css, less, scss, html, json, graphql, markdown, ymaml - [clang-format](#clang-format) - - c, c++, c#, objective-c, protobuf, javascript, java - [eclipse web tools platform](#eclipse-web-tools-platform) - - css, html, js, json, xml - [Biome](#biome) ([binary detection](#biome-binary), [config file](#biome-configuration-file), [input language](#biome-input-language)) - **Language independent** - [Generic steps](#generic-steps) @@ -142,6 +140,35 @@ Spotless requires JRE 11+ and Gradle 6.1.1 or newer. - If you're stuck on JRE 8, use [`id 'com.diffplug.spotless' version '6.13.0'` or older](https://github.com/diffplug/spotless/blob/main/plugin-gradle/CHANGES.md#6130---2023-01-14). - If you're stuck on an older version of Gradle, [`id 'com.diffplug.gradle.spotless' version '4.5.1'` supports all the way back to Gradle 2.x](https://github.com/diffplug/spotless/blob/main/plugin-gradle/CHANGES.md#451---2020-07-04). +### Linting + +Starting in version `7.0.0`, Spotless now supports linting in addition to formatting. To Spotless, all lints are errors which must be either fixed or suppressed. Lints show up like this: + +```console +user@machine repo % ./gradlew build +:spotlessKotlinCheck FAILED + There were 2 lint error(s), they must be fixed or suppressed. + src/main/kotlin/com/diffplug/Foo.kt:L7 ktlint(standard:no-wildcard-imports) Wildcard import + src/main/kotlin/com/diffplug/Bar.kt:L9 ktlint(standard:no-wildcard-imports) Wildcard import + Resolve these lints or suppress with `suppressLintsFor` +``` + +To suppress lints, you can do this: + +```gradle +spotless { + kotlin { + ktlint() + suppressLintsFor { + step = 'ktlint' + shortCode = 'standard:no-wildcard-imports' + } + } +} +``` + +Spotless is primarily a formatter, _not_ a linter. In our opinion, a linter is just a broken formatter. But formatters do break sometimes, and representing these failures as lints that can be suppressed is more useful than just giving up. + ## Java 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 efb3b45bdf..56cccc52c9 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 @@ -49,11 +49,11 @@ import org.gradle.util.GradleVersion; import com.diffplug.common.base.Preconditions; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LazyForwardingEquality; import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.LintSuppression; import com.diffplug.spotless.OnMatch; import com.diffplug.spotless.Provisioner; import com.diffplug.spotless.SerializedFunction; @@ -168,16 +168,36 @@ public void setEncoding(Charset charset) { encoding = requireNonNull(charset); } - final FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); + final List lintSuppressions = new ArrayList<>(); - /** Ignores errors in the given step. */ + /** Suppresses any lints which meet the supplied criteria. */ + public void suppressLintsFor(Action lintSuppression) { + LintSuppression suppression = new LintSuppression(); + lintSuppression.execute(suppression); + suppression.ensureDoesNotSuppressAll(); + lintSuppressions.add(suppression); + } + + /** + * Ignores errors in the given step. + * + * @deprecated Use {@link #suppressLintsFor(Action)} instead. + */ + @Deprecated public void ignoreErrorForStep(String stepName) { - exceptionPolicy.excludeStep(requireNonNull(stepName)); + System.err.println("`ignoreErrorForStep('" + stepName + "') is deprecated, use `suppressLintsFor { step = '" + stepName + "' }` instead."); + suppressLintsFor(it -> it.setStep(stepName)); } - /** Ignores errors for the given relative path. */ + /** + * Ignores errors for the given relative path. + * + * @deprecated Use {@link #suppressLintsFor(Action)} instead. + */ + @Deprecated public void ignoreErrorForPath(String relativePath) { - exceptionPolicy.excludePath(requireNonNull(relativePath)); + System.err.println("`ignoreErrorForPath('" + relativePath + "') is deprecated, use `suppressLintsFor { path = '" + relativePath + "' }` instead."); + suppressLintsFor(it -> it.setPath(relativePath)); } /** @@ -996,7 +1016,7 @@ public void toggleOffOnDisable() { /** Sets up a format task according to the values in this extension. */ protected void setupTask(SpotlessTask task) { task.setEncoding(getEncoding().name()); - task.setExceptionPolicy(exceptionPolicy); + task.setLintSuppressions(lintSuppressions); FileCollection totalTarget = targetExclude == null ? target : target.minus(targetExclude); task.setTarget(totalTarget); List steps; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java index a85195fe9f..f38745a6b0 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import java.nio.file.Files; import java.nio.file.StandardCopyOption; +import org.gradle.api.GradleException; import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; @@ -29,11 +30,12 @@ public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { @TaskAction public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); - ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); - if (files.isEmpty()) { + ConfigurableFileTree cleanFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessCleanDirectory().get()); + ConfigurableFileTree lintsFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessLintsDirectory().get()); + if (cleanFiles.isEmpty() && lintsFiles.isEmpty()) { getState().setDidWork(sourceDidWork()); } else { - files.visit(new FileVisitor() { + cleanFiles.visit(new FileVisitor() { @Override public void visitDir(FileVisitDetails fileVisitDetails) { @@ -51,6 +53,10 @@ public void visitFile(FileVisitDetails fileVisitDetails) { } } }); + if (!lintsFiles.isEmpty()) { + boolean detailed = false; + throw new GradleException(super.allLintsErrorMsgDetailed(lintsFiles, detailed)); + } } } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index 95f051371b..dba219ff06 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; +import org.jetbrains.annotations.NotNull; import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.ThrowingEx; @@ -54,65 +55,78 @@ public void performAction() throws IOException { } private void performAction(boolean isTest) throws IOException { - ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); - if (files.isEmpty()) { + ConfigurableFileTree cleanFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessCleanDirectory().get()); + ConfigurableFileTree lintsFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessLintsDirectory().get()); + if (cleanFiles.isEmpty() && lintsFiles.isEmpty()) { getState().setDidWork(sourceDidWork()); } else if (!isTest && applyHasRun()) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); } else { - List problemFiles = new ArrayList<>(); - files.visit(new FileVisitor() { - @Override - public void visitDir(FileVisitDetails fileVisitDetails) { - - } - - @Override - public void visitFile(FileVisitDetails fileVisitDetails) { - String path = fileVisitDetails.getPath(); - File originalSource = new File(getProjectDir().get().getAsFile(), path); - try { - // read the file on disk - byte[] userFile = Files.readAllBytes(originalSource.toPath()); - // and the formatted version from spotlessOutDirectory - byte[] formatted; - { - ByteArrayOutputStream clean = new ByteArrayOutputStream(); - fileVisitDetails.copyTo(clean); - formatted = clean.toByteArray(); - } - // If these two are equal, it means that SpotlessTask left a file - // in its output directory which ought to have been removed. As - // best I can tell, this is a filesytem race which is very hard - // to trigger. GitRatchetGradleTest can *sometimes* reproduce it - // but it's very erratic, and that test writes both to Gradle cache - // and git cache very quickly. Either of Gradle or jgit might be - // caching something wrong because of the fast repeated writes. - if (!Arrays.equals(userFile, formatted)) { - // If the on-disk content is equal to the formatted content, - // just don't add it as a problem file. Easy! - problemFiles.add(originalSource); - } - } catch (IOException e) { - throw ThrowingEx.asRuntime(e); - } - } - }); - if (!problemFiles.isEmpty()) { - Collections.sort(problemFiles); + List unformattedFiles = getUncleanFiles(cleanFiles); + if (!unformattedFiles.isEmpty()) { + // if any files are unformatted, we show those throw new GradleException(DiffMessageFormatter.builder() .runToFix(getRunToFixMessage().get()) .formatterFolder( getProjectDir().get().getAsFile().toPath(), - getSpotlessOutDirectory().get().toPath(), + getSpotlessCleanDirectory().get().toPath(), getEncoding().get()) - .problemFiles(problemFiles) + .problemFiles(unformattedFiles) .getMessage()); + } else { + // We only show lints if there are no unformatted files. + // This is because lint line numbers are relative to the + // formatted content, and formatting often fixes lints. + boolean detailed = false; + throw new GradleException(super.allLintsErrorMsgDetailed(lintsFiles, detailed)); } } } + private @NotNull List getUncleanFiles(ConfigurableFileTree cleanFiles) { + List uncleanFiles = new ArrayList<>(); + cleanFiles.visit(new FileVisitor() { + @Override + public void visitDir(FileVisitDetails fileVisitDetails) { + + } + + @Override + public void visitFile(FileVisitDetails fileVisitDetails) { + String path = fileVisitDetails.getPath(); + File originalSource = new File(getProjectDir().get().getAsFile(), path); + try { + // read the file on disk + byte[] userFile = Files.readAllBytes(originalSource.toPath()); + // and the formatted version from spotlessOutDirectory + byte[] formatted; + { + ByteArrayOutputStream clean = new ByteArrayOutputStream(); + fileVisitDetails.copyTo(clean); + formatted = clean.toByteArray(); + } + // If these two are equal, it means that SpotlessTask left a file + // in its output directory which ought to have been removed. As + // best I can tell, this is a filesytem race which is very hard + // to trigger. GitRatchetGradleTest can *sometimes* reproduce it + // but it's very erratic, and that test writes both to Gradle cache + // and git cache very quickly. Either of Gradle or jgit might be + // caching something wrong because of the fast repeated writes. + if (!Arrays.equals(userFile, formatted)) { + // If the on-disk content is equal to the formatted content, + // just don't add it as a problem file. Easy! + uncleanFiles.add(originalSource); + } + } catch (IOException e) { + throw ThrowingEx.asRuntime(e); + } + } + }); + Collections.sort(uncleanFiles); + return uncleanFiles; + } + @Internal abstract Property getProjectPath(); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index e1e729e852..e31be63143 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -17,6 +17,7 @@ import java.io.File; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -36,10 +37,10 @@ import org.gradle.work.Incremental; import com.diffplug.spotless.ConfigurationCacheHackList; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.LintSuppression; import com.diffplug.spotless.extra.GitRatchet; public abstract class SpotlessTask extends DefaultTask { @@ -121,15 +122,15 @@ public ObjectId getRatchetSha() { return subtreeSha; } - protected FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); + protected List lintSuppressions = new ArrayList<>(); - public void setExceptionPolicy(FormatExceptionPolicyStrict exceptionPolicy) { - this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy); + public void setLintSuppressions(List lintSuppressions) { + this.lintSuppressions = Objects.requireNonNull(lintSuppressions); } @Input - public FormatExceptionPolicyStrict getExceptionPolicy() { - return exceptionPolicy; + public List getLintSuppressions() { + return lintSuppressions; } protected FileCollection target; @@ -149,12 +150,20 @@ public void setTarget(Iterable target) { } } - protected File outputDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(), - "spotless/" + getName()); + protected File cleanDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(), + "spotless-clean/" + getName()); @OutputDirectory - public File getOutputDirectory() { - return outputDirectory; + public File getCleanDirectory() { + return cleanDirectory; + } + + protected File lintsDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(), + "spotless-lints/" + getName()); + + @OutputDirectory + public File getLintsDirectory() { + return lintsDirectory; } private final ConfigurationCacheHackList stepsInternalRoundtrip = ConfigurationCacheHackList.forRoundtrip(); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 17cd6c08a1..9c8d6c49be 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -20,6 +20,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.LinkedHashMap; +import java.util.List; import javax.annotation.Nullable; import javax.inject.Inject; @@ -38,6 +40,7 @@ import com.diffplug.common.annotations.VisibleForTesting; import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.Lint; import com.diffplug.spotless.LintState; import com.diffplug.spotless.extra.GitRatchet; @@ -74,19 +77,31 @@ public void performAction(InputChanges inputs) throws Exception { if (!inputs.isIncremental()) { getLogger().info("Not incremental: removing prior outputs"); - getFs().delete(d -> d.delete(outputDirectory)); - Files.createDirectories(outputDirectory.toPath()); + getFs().delete(d -> d.delete(cleanDirectory)); + getFs().delete(d -> d.delete(lintsDirectory)); + Files.createDirectories(cleanDirectory.toPath()); + Files.createDirectories(lintsDirectory.toPath()); } try (Formatter formatter = buildFormatter()) { GitRatchetGradle ratchet = getRatchet(); for (FileChange fileChange : inputs.getFileChanges(target)) { File input = fileChange.getFile(); + File projectDir = getProjectDir().get().getAsFile(); + String relativePath = FormatExtension.relativize(projectDir, input); + if (relativePath == null) { + throw new IllegalArgumentException(StringPrinter.buildString(printer -> { + printer.println("Spotless error! All target files must be within the project dir."); + printer.println(" project dir: " + projectDir.getAbsolutePath()); + printer.println(" target: " + input.getAbsolutePath()); + })); + } if (fileChange.getChangeType() == ChangeType.REMOVED) { - deletePreviousResults(input); + deletePreviousResults(cleanDirectory, relativePath); + deletePreviousResults(lintsDirectory, relativePath); } else { if (input.isFile()) { - processInputFile(ratchet, formatter, input); + processInputFile(ratchet, formatter, input, relativePath); } } } @@ -94,62 +109,51 @@ public void performAction(InputChanges inputs) throws Exception { } @VisibleForTesting - void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException { - File output = getOutputFileWithBaseDir(input, outputDirectory); - getLogger().debug("Applying format to {} and writing to {}", input, output); + void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input, String relativePath) throws IOException { + File cleanFile = new File(cleanDirectory, relativePath); + File lintFile = new File(lintsDirectory, relativePath); + getLogger().debug("Applying format to {} and writing to {}", input, cleanFile); LintState lintState; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { lintState = LintState.clean(); } else { try { - lintState = LintState.of(formatter, input); + lintState = LintState.of(formatter, input).withRemovedSuppressions(formatter, relativePath, getLintSuppressions()); } catch (Throwable e) { throw new IllegalArgumentException("Issue processing file: " + input, e); } } if (lintState.getDirtyState().isClean()) { // Remove previous output if it exists - Files.deleteIfExists(output.toPath()); + Files.deleteIfExists(cleanFile.toPath()); } else if (lintState.getDirtyState().didNotConverge()) { - getLogger().warn("Skipping '{}' because it does not converge. Run {@code spotlessDiagnose} to understand why", input); + getLogger().warn("Skipping '{}' because it does not converge. Run {@code spotlessDiagnose} to understand why", relativePath); } else { - Path parentDir = output.toPath().getParent(); + Path parentDir = cleanFile.toPath().getParent(); if (parentDir == null) { - throw new IllegalStateException("Every file has a parent folder. But not: " + output); + throw new IllegalStateException("Every file has a parent folder. But not: " + cleanFile); } Files.createDirectories(parentDir); // Need to copy the original file to the tmp location just to remember the file attributes - Files.copy(input.toPath(), output.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); + Files.copy(input.toPath(), cleanFile.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); - getLogger().info(String.format("Writing clean file: %s", output)); - lintState.getDirtyState().writeCanonicalTo(output); + getLogger().info(String.format("Writing clean file: %s", cleanFile)); + lintState.getDirtyState().writeCanonicalTo(cleanFile); } - if (lintState.isHasLints()) { - var lints = lintState.getLints(formatter); - var first = lints.entrySet().iterator().next(); - getExceptionPolicy().handleError(new Throwable(first.getValue().get(0).toString()), first.getKey(), FormatExtension.relativize(getProjectDir().get().getAsFile(), input)); + if (!lintState.isHasLints()) { + Files.deleteIfExists(lintFile.toPath()); + } else { + LinkedHashMap> lints = lintState.getLintsByStep(formatter); + SerializableMisc.toFile(lints, lintFile); } } - private void deletePreviousResults(File input) throws IOException { - File output = getOutputFileWithBaseDir(input, outputDirectory); + private void deletePreviousResults(File baseDir, String subpath) throws IOException { + File output = new File(baseDir, subpath); if (output.isDirectory()) { getFs().delete(d -> d.delete(output)); } else { Files.deleteIfExists(output.toPath()); } } - - private File getOutputFileWithBaseDir(File input, File baseDir) { - File projectDir = getProjectDir().get().getAsFile(); - String outputFileName = FormatExtension.relativize(projectDir, input); - if (outputFileName == null) { - throw new IllegalArgumentException(StringPrinter.buildString(printer -> { - printer.println("Spotless error! All target files must be within the project dir."); - printer.println(" project dir: " + projectDir.getAbsolutePath()); - printer.println(" target: " + input.getAbsolutePath()); - })); - } - return new File(baseDir, outputFileName); - } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index b5a1300ead..b58ff9005a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2023 DiffPlug + * Copyright 2021-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,13 +18,20 @@ import java.io.File; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; import javax.inject.Inject; import org.gradle.api.DefaultTask; +import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.DirectoryProperty; +import org.gradle.api.file.FileVisitDetails; +import org.gradle.api.file.FileVisitor; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; import org.gradle.api.provider.Provider; @@ -36,6 +43,7 @@ import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Unhandled; +import com.diffplug.spotless.Lint; import com.diffplug.spotless.Provisioner; /** @@ -104,7 +112,10 @@ static void usesServiceTolerateTestFailure(DefaultTask task, Provider getSpotlessOutDirectory(); + abstract Property getSpotlessCleanDirectory(); + + @Internal + abstract Property getSpotlessLintsDirectory(); @Internal abstract Property getTaskService(); @@ -117,7 +128,8 @@ static abstract class ClientTask extends DefaultTask { void init(SpotlessTaskImpl impl) { usesServiceTolerateTestFailure(this, impl.getTaskServiceProvider()); - getSpotlessOutDirectory().set(impl.getOutputDirectory()); + getSpotlessCleanDirectory().set(impl.getCleanDirectory()); + getSpotlessLintsDirectory().set(impl.getLintsDirectory()); getTaskService().set(impl.getTaskService()); getProjectDir().set(impl.getProjectDir()); } @@ -154,5 +166,41 @@ protected boolean sourceDidWork() { protected boolean applyHasRun() { return service().apply.containsKey(sourceTaskPath()); } + + protected String allLintsErrorMsgDetailed(ConfigurableFileTree lintsFiles, boolean detailed) { + AtomicInteger total = new AtomicInteger(0); + TreeMap>> allLints = new TreeMap<>(); + lintsFiles.visit(new FileVisitor() { + @Override + public void visitDir(FileVisitDetails fileVisitDetails) { + + } + + @Override + public void visitFile(FileVisitDetails fileVisitDetails) { + String path = fileVisitDetails.getPath(); + getLogger().debug("Reading lints for " + path); + LinkedHashMap> lints = SerializableMisc.fromFile(LinkedHashMap.class, fileVisitDetails.getFile()); + allLints.put(path, lints); + lints.values().forEach(list -> total.addAndGet(list.size())); + } + }); + StringBuilder builder = new StringBuilder(); + builder.append("There were " + total.get() + " lint error(s), they must be fixed or suppressed.\n"); + for (Map.Entry>> lintsPerFile : allLints.entrySet()) { + for (Map.Entry> stepLints : lintsPerFile.getValue().entrySet()) { + String stepName = stepLints.getKey(); + for (Lint lint : stepLints.getValue()) { + builder.append(lintsPerFile.getKey()); + builder.append(":"); + boolean oneLine = !detailed; + lint.addWarningMessageTo(builder, stepName, oneLine); + builder.append("\n"); + } + } + } + builder.append("Resolve these lints or suppress with `suppressLintsFor`"); + return builder.toString(); + } } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index 8391a8757e..0a8dfe7022 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java @@ -387,11 +387,11 @@ void failureWhenNotParseable() throws Exception { "}"); setFile("biome_test.js").toResource("biome/js/fileBefore.js"); - var spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - assertThat(spotlessApply.getOutput()).contains("spotlessMybiome FAILED"); + var spotlessApply = gradleRunner().withArguments("spotlessApply").buildAndFail(); + assertThat(spotlessApply.getOutput()).contains("spotlessMybiomeApply FAILED"); assertFile("biome_test.js").sameAsResource("biome/js/fileBefore.js"); assertThat(spotlessApply.getOutput()).contains("Format with errors is disabled."); - assertThat(spotlessApply.getOutput()).contains("Step 'biome' found problem in 'biome_test.js'"); + assertThat(spotlessApply.getOutput()).contains("biome_test.js:LINE_UNDEFINED biome(java.lang.RuntimeException)"); } /** diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java index 0c21a3644d..e083161a09 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java @@ -20,14 +20,11 @@ import java.util.Arrays; import java.util.List; -import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; -import org.gradle.testkit.runner.TaskOutcome; import org.junit.jupiter.api.Test; -import com.diffplug.common.base.CharMatcher; -import com.diffplug.common.base.Splitter; -import com.diffplug.spotless.LineEnding; +import com.diffplug.selfie.Selfie; +import com.diffplug.selfie.StringSelfie; /** Tests the desired behavior from https://github.com/diffplug/spotless/issues/46. */ class ErrorShouldRethrowTest extends GradleIntegrationHarness { @@ -41,7 +38,7 @@ private void writeBuild(String... toInsert) throws IOException { lines.add(" format 'misc', {"); lines.add(" lineEndings 'UNIX'"); lines.add(" target file('README.md')"); - lines.add(" custom 'no swearing', {"); + lines.add(" custom 'noSwearingStep', {"); lines.add(" if (it.toLowerCase(Locale.ROOT).contains('fubar')) {"); lines.add(" throw com.diffplug.spotless.Lint.atUndefinedLine('swearing', 'No swearing!').shortcut();"); lines.add(" }"); @@ -56,7 +53,7 @@ void passesIfNoException() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fun."); - runWithSuccess("> Task :spotlessMisc"); + expectSuccess(); } @Test @@ -65,11 +62,16 @@ void anyExceptionShouldFail() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithFailure( - "> Task :spotlessMisc FAILED\n" + - "Step 'no swearing' found problem in 'README.md':\n" + - "LINE_UNDEFINED: (swearing) No swearing!\n" + - "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); + expectFailureAndConsoleToBe().toBe("> Task :spotlessMisc", + "> Task :spotlessMiscCheck FAILED", + "", + "FAILURE: Build failed with an exception.", + "", + "* What went wrong:", + "Execution failed for task ':spotlessMiscCheck'.", + "> There were 1 lint error(s), they must be fixed or suppressed.", + " README.md:LINE_UNDEFINED noSwearingStep(swearing) No swearing!", + " Resolve these lints or suppress with `suppressLintsFor`"); } @Test @@ -79,17 +81,17 @@ void unlessEnforceCheckIsFalse() throws Exception { " enforceCheck false", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithSuccess("> Task :processResources NO-SOURCE"); + expectSuccess(); } + @Test void unlessExemptedByStep() throws Exception { writeBuild( - " ignoreErrorForStep 'no swearing'", + " ignoreErrorForStep 'noSwearingStep'", " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithSuccess("> Task :spotlessMisc\n" + - "Unable to apply step 'no swearing' to 'README.md'"); + expectSuccess(); } @Test @@ -99,8 +101,7 @@ void unlessExemptedByPath() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithSuccess("> Task :spotlessMisc\n" + - "Unable to apply step 'no swearing' to 'README.md'"); + expectSuccess(); } @Test @@ -111,33 +112,29 @@ void failsIfNeitherStepNorFileExempted() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithFailure("> Task :spotlessMisc FAILED\n" + - "Step 'no swearing' found problem in 'README.md':\n" + - "LINE_UNDEFINED: (swearing) No swearing!\n" + - "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); + expectFailureAndConsoleToBe().toBe("> Task :spotlessMisc", + "> Task :spotlessMiscCheck FAILED", + "", + "FAILURE: Build failed with an exception.", + "", + "* What went wrong:", + "Execution failed for task ':spotlessMiscCheck'.", + "> There were 1 lint error(s), they must be fixed or suppressed.", + " README.md:LINE_UNDEFINED noSwearingStep(swearing) No swearing!", + " Resolve these lints or suppress with `suppressLintsFor`"); } - private void runWithSuccess(String expectedToStartWith) throws Exception { - BuildResult result = gradleRunner().withArguments("check").build(); - assertResultAndMessages(result, TaskOutcome.SUCCESS, expectedToStartWith); + private void expectSuccess() throws Exception { + gradleRunner().withArguments("check", "--stacktrace").build(); } - private void runWithFailure(String expectedToStartWith) throws Exception { + private StringSelfie expectFailureAndConsoleToBe() throws Exception { BuildResult result = gradleRunner().withArguments("check").buildAndFail(); - assertResultAndMessages(result, TaskOutcome.FAILED, expectedToStartWith); - } - - private void assertResultAndMessages(BuildResult result, TaskOutcome outcome, String expectedToStartWith) { String output = result.getOutput(); int register = output.indexOf(":spotlessInternalRegisterDependencies"); int firstNewlineAfterThat = output.indexOf('\n', register + 1); - String useThisToMatch = output.substring(firstNewlineAfterThat); - - int numNewlines = CharMatcher.is('\n').countIn(expectedToStartWith); - List actualLines = Splitter.on('\n').splitToList(LineEnding.toUnix(useThisToMatch.trim())); - String actualStart = String.join("\n", actualLines.subList(0, numNewlines + 1)); - Assertions.assertThat(actualStart).isEqualTo(expectedToStartWith); - Assertions.assertThat(outcomes(result, outcome).size() + outcomes(result, TaskOutcome.UP_TO_DATE).size() + outcomes(result, TaskOutcome.NO_SOURCE).size()) - .isEqualTo(outcomes(result).size()); + int firstTry = output.indexOf("\n* Try:"); + String useThisToMatch = output.substring(firstNewlineAfterThat, firstTry).trim(); + return Selfie.expectSelfie(useThisToMatch); } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java index 99509d51a4..0d10cf54e3 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java @@ -48,7 +48,7 @@ public BuildServiceParameters.None getParameters() { @Test void testLineEndings() throws Exception { File testFile = setFile("testFile").toContent("\r\n"); - File outputFile = new File(spotlessTask.getOutputDirectory(), "testFile"); + File outputFile = new File(spotlessTask.getCleanDirectory(), "testFile"); spotlessTask.setTarget(Collections.singleton(testFile)); Tasks.execute(spotlessTask); @@ -59,7 +59,7 @@ void testLineEndings() throws Exception { @Test void testStep() throws Exception { File testFile = setFile("testFile").toContent("apple"); - File outputFile = new File(spotlessTask.getOutputDirectory(), "testFile"); + File outputFile = new File(spotlessTask.getCleanDirectory(), "testFile"); spotlessTask.setTarget(Collections.singleton(testFile)); spotlessTask.setSteps(List.of(ReplaceStep.create("double-p", "pp", "p"))); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index 5d68db2492..888f11f294 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -61,7 +61,7 @@ public BuildServiceParameters.None getParameters() { source = createFormatTask(name, step); check = createCheckTask(name, source); apply = createApplyTask(name, source); - outputFile = new File(source.getOutputDirectory() + "/src", file.getName()); + outputFile = new File(source.getCleanDirectory() + "/src", file.getName()); } private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java index 9799fbcf15..b6547204b8 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java @@ -226,7 +226,7 @@ void suggestsMissingJavaCommunityPlugin(String prettierVersion) throws IOExcepti "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - Assertions.assertThat(spotlessApply.getOutput()).contains("could not infer a parser"); + Assertions.assertThat(spotlessApply.getOutput()).contains("Could not infer a parser"); Assertions.assertThat(spotlessApply.getOutput()).contains("prettier-plugin-java"); } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessTaskImplTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessTaskImplTest.java index fc7fab9980..a1f49c7373 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessTaskImplTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessTaskImplTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,6 +41,6 @@ public void testThrowsMessageContainsFilename() throws Exception { File input = Paths.get("unitTests", "projectDir", "someInput").toFile(); Formatter formatter = Mockito.mock(Formatter.class); - Assertions.assertThatThrownBy(() -> task.processInputFile(null, formatter, input)).hasMessageContaining(input.toString()); + Assertions.assertThatThrownBy(() -> task.processInputFile(null, formatter, input, "someInput")).hasMessageContaining(input.toString()); } }