From f4df9563cd4d84e4d7ab286625729c5a942ebd23 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 22 Oct 2024 08:51:56 -0700 Subject: [PATCH 01/16] Simplify the TOC. --- plugin-gradle/README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 80fcafb7bb..3ac2cc7120 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -74,11 +74,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) From 8f43b2f45882c487aebe2339ff81a6bf90ac5ca3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 22 Oct 2024 23:58:34 -0700 Subject: [PATCH 02/16] First cut at gradle docs. --- plugin-gradle/README.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 3ac2cc7120..c5adda5d2b 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)) @@ -139,6 +140,39 @@ 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 +:spotlessJavaCheck FAILED + The following files had format violations: + src\main\java\com\diffplug\gradle\spotless\FormatExtension.java + -\t\t····if·(targets.length·==·0)·{ + +\t\tif·(targets.length·==·0)·{ + Resolve these lints or suppress with `suppressLintsFor` +``` + +To suppress lints, you can do this: + +```gradle +spotless { + suppressLintsFor { // applies to all formats + file = 'src/blah/blah' + } + kotlin { + ktfmt() + suppressLintsFor { // applies to only the kotlin formats + step = 'ktlint' + shortCode = 'rename' + } + } +} +``` + +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 From faa56fca8821531824fee361e3570be32c010b3e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 00:12:07 -0700 Subject: [PATCH 03/16] Rename `spotlessOut` to `spotlessClean`, to differentiate it from the coming `spotlessLint`. --- .../gradle/spotless/SpotlessApply.java | 8 +-- .../gradle/spotless/SpotlessCheck.java | 10 ++-- .../gradle/spotless/SpotlessTask.java | 8 +-- .../gradle/spotless/SpotlessTaskImpl.java | 51 +++++++++---------- .../gradle/spotless/SpotlessTaskService.java | 6 +-- .../gradle/spotless/FormatTaskTest.java | 4 +- .../gradle/spotless/PaddedCellTaskTest.java | 12 ++--- .../gradle/spotless/SpotlessTaskImplTest.java | 4 +- 8 files changed, 49 insertions(+), 54 deletions(-) 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..061a2496eb 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. @@ -29,11 +29,11 @@ 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()); + if (cleanFiles.isEmpty()) { getState().setDidWork(sourceDidWork()); } else { - files.visit(new FileVisitor() { + cleanFiles.visit(new FileVisitor() { @Override public void visitDir(FileVisitDetails fileVisitDetails) { 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..bbbbf3faf5 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. @@ -54,15 +54,15 @@ 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()); + if (cleanFiles.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() { + cleanFiles.visit(new FileVisitor() { @Override public void visitDir(FileVisitDetails fileVisitDetails) { @@ -105,7 +105,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { .runToFix(getRunToFixMessage().get()) .formatterFolder( getProjectDir().get().getAsFile().toPath(), - getSpotlessOutDirectory().get().toPath(), + getSpotlessCleanDirectory().get().toPath(), getEncoding().get()) .problemFiles(problemFiles) .getMessage()); 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..04b0e30b33 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 @@ -149,12 +149,12 @@ 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; } 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..22b0c3486a 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 @@ -74,19 +74,27 @@ 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)); + Files.createDirectories(cleanDirectory.toPath()); } try (Formatter formatter = buildFormatter()) { GitRatchetGradle ratchet = getRatchet(); for (FileChange fileChange : inputs.getFileChanges(target)) { File input = fileChange.getFile(); + String subpath = FormatExtension.relativize(getProjectDir().getAsFile().get(), input); + if (subpath == null) { + throw new IllegalArgumentException(StringPrinter.buildString(printer -> { + printer.println("Spotless error! All target files must be within the project dir."); + printer.println(" project dir: " + getProjectDir().getAsFile().get().getAbsolutePath()); + printer.println(" target: " + input.getAbsolutePath()); + })); + } if (fileChange.getChangeType() == ChangeType.REMOVED) { - deletePreviousResults(input); + deletePreviousResults(cleanDirectory, subpath); } else { if (input.isFile()) { - processInputFile(ratchet, formatter, input); + processInputFile(ratchet, formatter, input, subpath); } } } @@ -94,9 +102,9 @@ 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 subpath) throws IOException { + File cleanFile = new File(cleanDirectory, subpath); + 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(); @@ -109,20 +117,20 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in } 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); } 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); @@ -131,25 +139,12 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in } } - 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..d354bca1e0 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. @@ -104,7 +104,7 @@ static void usesServiceTolerateTestFailure(DefaultTask task, Provider getSpotlessOutDirectory(); + abstract Property getSpotlessCleanDirectory(); @Internal abstract Property getTaskService(); @@ -117,7 +117,7 @@ static abstract class ClientTask extends DefaultTask { void init(SpotlessTaskImpl impl) { usesServiceTolerateTestFailure(this, impl.getTaskServiceProvider()); - getSpotlessOutDirectory().set(impl.getOutputDirectory()); + getSpotlessCleanDirectory().set(impl.getCleanDirectory()); getTaskService().set(impl.getTaskService()); getProjectDir().set(impl.getProjectDir()); } 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..e815791a31 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 @@ -49,7 +49,7 @@ public BuildServiceParameters.None getParameters() { } }); File file; - File outputFile; + File cleanFile; SpotlessTaskImpl source; SpotlessCheck check; SpotlessApply apply; @@ -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()); + cleanFile = new File(source.getCleanDirectory() + "/src", file.getName()); } private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { @@ -143,10 +143,10 @@ void paddedCellFormat() throws Exception { converge.format(); diverge.format(); - assertFile(wellbehaved.outputFile).hasContent("42"); // cycle -> first element in cycle - assertFile(cycle.outputFile).hasContent("A"); // cycle -> first element in cycle - assertFile(converge.outputFile).hasContent(""); // converge -> converges - assertThat(diverge.outputFile).doesNotExist(); // diverge -> no change + assertFile(wellbehaved.cleanFile).hasContent("42"); // cycle -> first element in cycle + assertFile(cycle.cleanFile).hasContent("A"); // cycle -> first element in cycle + assertFile(converge.cleanFile).hasContent(""); // converge -> converges + assertThat(diverge.cleanFile).doesNotExist(); // diverge -> no change } @Test 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()); } } From 955ed7757eec097aaac0095a85e310caa9c1e6ab Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 13:10:42 -0700 Subject: [PATCH 04/16] amend prev --- .../diffplug/gradle/spotless/PaddedCellTaskTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 e815791a31..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 @@ -49,7 +49,7 @@ public BuildServiceParameters.None getParameters() { } }); File file; - File cleanFile; + File outputFile; SpotlessTaskImpl source; SpotlessCheck check; SpotlessApply apply; @@ -61,7 +61,7 @@ public BuildServiceParameters.None getParameters() { source = createFormatTask(name, step); check = createCheckTask(name, source); apply = createApplyTask(name, source); - cleanFile = new File(source.getCleanDirectory() + "/src", file.getName()); + outputFile = new File(source.getCleanDirectory() + "/src", file.getName()); } private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { @@ -143,10 +143,10 @@ void paddedCellFormat() throws Exception { converge.format(); diverge.format(); - assertFile(wellbehaved.cleanFile).hasContent("42"); // cycle -> first element in cycle - assertFile(cycle.cleanFile).hasContent("A"); // cycle -> first element in cycle - assertFile(converge.cleanFile).hasContent(""); // converge -> converges - assertThat(diverge.cleanFile).doesNotExist(); // diverge -> no change + assertFile(wellbehaved.outputFile).hasContent("42"); // cycle -> first element in cycle + assertFile(cycle.outputFile).hasContent("A"); // cycle -> first element in cycle + assertFile(converge.outputFile).hasContent(""); // converge -> converges + assertThat(diverge.outputFile).doesNotExist(); // diverge -> no change } @Test From 03493f48dc30bfb69ad07c62e3d63187bb40e397 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 13:12:42 -0700 Subject: [PATCH 05/16] Replace `FormatExceptionPolicyStrict` with `List` --- .../spotless/FormatExceptionPolicyStrict.java | 57 ----------- .../main/java/com/diffplug/spotless/Lint.java | 24 +++++ .../com/diffplug/spotless/LintPolicy.java | 14 +-- .../java/com/diffplug/spotless/LintState.java | 56 ++++++----- .../diffplug/spotless/LintSuppression.java | 99 +++++++++++++++++++ .../gradle/spotless/FormatExtension.java | 19 +++- .../gradle/spotless/SpotlessApply.java | 8 +- .../gradle/spotless/SpotlessCheck.java | 98 ++++++++++-------- .../gradle/spotless/SpotlessTask.java | 21 ++-- .../gradle/spotless/SpotlessTaskImpl.java | 34 ++++--- .../gradle/spotless/SpotlessTaskService.java | 48 +++++++++ 11 files changed, 319 insertions(+), 159 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java create mode 100644 lib/src/main/java/com/diffplug/spotless/LintSuppression.java 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/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 4384ed38ef..e3e3fa9f4b 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -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(ruleId).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 index 0b28c36884..d5fb54e628 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java +++ b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java @@ -21,23 +21,15 @@ 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()); + logger.error("Step '{}' found problem in '{}':\n{}", formatter.getSteps().get(i), file.getName(), exception.getMessage(), exception); throw ThrowingEx.asRuntimeRethrowError(exception); } } } + + private static final Logger logger = LoggerFactory.getLogger(Formatter.class); } diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index b2b4030e93..465cb9b580 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -18,9 +18,9 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import javax.annotation.Nullable; @@ -45,23 +45,49 @@ 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++) { + FormatterStep step = formatter.getSteps().get(i); List lints = lintsPerStep.get(i); if (lints != null) { - result.put(formatter.getSteps().get(i), lints); + result.put(step.getName(), lints); } } return result; } + public void removeSuppressedLints(Formatter formatter, String relativePath, List suppressions) { + if (lintsPerStep == null) { + return; + } + for (int i = 0; i < lintsPerStep.size(); i++) { + FormatterStep step = formatter.getSteps().get(i); + List lints = lintsPerStep.get(i); + if (lints != null) { + Iterator iter = lints.iterator(); + while (iter.hasNext()) { + Lint lint = iter.next(); + for (LintSuppression suppression : suppressions) { + if (suppression.suppresses(relativePath, step, lint)) { + iter.remove(); + break; + } + } + } + if (lints.isEmpty()) { + lintsPerStep.set(i, null); + } + } + } + } + public String asStringDetailed(File file, Formatter formatter) { return asString(file, formatter, false); } @@ -81,27 +107,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..c26eef6b1c --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java @@ -0,0 +1,99 @@ +/* + * 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 file = ALL; + private String step = ALL; + private String shortCode = ALL; + + public String getFile() { + return file; + } + + public void setFile(String file) { + this.file = Objects.requireNonNull(file); + } + + 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 step, Lint lint) { + return !allows(relativePath, step, lint); + } + + private boolean allows(String relativePath, FormatterStep step, Lint lint) { + if (!file.equals(ALL) && !file.equals(relativePath)) { + return false; + } + if (!this.step.equals(ALL) && !this.step.equals(step.getName())) { + return false; + } + if (!this.shortCode.equals(ALL) && !this.shortCode.equals(lint.getRuleId())) { + return false; + } + return true; + } + + public void ensureDoesNotSuppressAll() { + boolean suppressAll = file.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(file, that.file) && Objects.equals(step, that.step) && Objects.equals(shortCode, that.shortCode); + } + + @Override + public int hashCode() { + return Objects.hash(file, step, shortCode); + } + + @Override + public String toString() { + return "LintSuppression{" + + "file='" + file + '\'' + + ", step='" + step + '\'' + + ", code='" + shortCode + '\'' + + '}'; + } +} 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..68f00f5208 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,25 @@ public void setEncoding(Charset charset) { encoding = requireNonNull(charset); } - final FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); + final List lintSuppressions = new ArrayList<>(); + + public void suppressLintsFor(Action lintSuppression) { + LintSuppression suppression = new LintSuppression(); + lintSuppression.execute(suppression); + suppression.ensureDoesNotSuppressAll(); + lintSuppressions.add(suppression); + } /** Ignores errors in the given step. */ + @Deprecated public void ignoreErrorForStep(String stepName) { - exceptionPolicy.excludeStep(requireNonNull(stepName)); + suppressLintsFor(it -> it.setStep(stepName)); } /** Ignores errors for the given relative path. */ + @Deprecated public void ignoreErrorForPath(String relativePath) { - exceptionPolicy.excludePath(requireNonNull(relativePath)); + suppressLintsFor(it -> it.setFile(relativePath)); } /** @@ -996,7 +1005,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 061a2496eb..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 @@ -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; @@ -30,7 +31,8 @@ public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); ConfigurableFileTree cleanFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessCleanDirectory().get()); - if (cleanFiles.isEmpty()) { + ConfigurableFileTree lintsFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessLintsDirectory().get()); + if (cleanFiles.isEmpty() && lintsFiles.isEmpty()) { getState().setDidWork(sourceDidWork()); } else { cleanFiles.visit(new FileVisitor() { @@ -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 bbbbf3faf5..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 @@ -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; @@ -55,64 +56,77 @@ public void performAction() throws IOException { private void performAction(boolean isTest) throws IOException { ConfigurableFileTree cleanFiles = getConfigCacheWorkaround().fileTree().from(getSpotlessCleanDirectory().get()); - if (cleanFiles.isEmpty()) { + 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<>(); - 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! - 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(), 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 04b0e30b33..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; @@ -157,6 +158,14 @@ 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(); private final ConfigurationCacheHackList stepsInternalEquality = ConfigurationCacheHackList.forEquality(); 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 22b0c3486a..27879925b6 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; @@ -75,26 +78,30 @@ public void performAction(InputChanges inputs) throws Exception { if (!inputs.isIncremental()) { getLogger().info("Not incremental: removing prior outputs"); 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(); - String subpath = FormatExtension.relativize(getProjectDir().getAsFile().get(), input); - if (subpath == null) { + 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: " + getProjectDir().getAsFile().get().getAbsolutePath()); + printer.println(" project dir: " + projectDir.getAbsolutePath()); printer.println(" target: " + input.getAbsolutePath()); })); } if (fileChange.getChangeType() == ChangeType.REMOVED) { - deletePreviousResults(cleanDirectory, subpath); + deletePreviousResults(cleanDirectory, relativePath); + deletePreviousResults(lintsDirectory, relativePath); } else { if (input.isFile()) { - processInputFile(ratchet, formatter, input, subpath); + processInputFile(ratchet, formatter, input, relativePath); } } } @@ -102,8 +109,9 @@ public void performAction(InputChanges inputs) throws Exception { } @VisibleForTesting - void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input, String subpath) throws IOException { - File cleanFile = new File(cleanDirectory, subpath); + 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)) { @@ -111,6 +119,7 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in } else { try { lintState = LintState.of(formatter, input); + lintState.removeSuppressedLints(formatter, relativePath, getLintSuppressions()); } catch (Throwable e) { throw new IllegalArgumentException("Issue processing file: " + input, e); } @@ -119,7 +128,7 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in // Remove previous output if it exists 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 = cleanFile.toPath().getParent(); if (parentDir == null) { @@ -132,10 +141,11 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in 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); } } 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 d354bca1e0..ce371ecd61 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 @@ -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; /** @@ -106,6 +114,9 @@ static abstract class ClientTask extends DefaultTask { @Internal abstract Property getSpotlessCleanDirectory(); + @Internal + abstract Property getSpotlessLintsDirectory(); + @Internal abstract Property getTaskService(); @@ -118,6 +129,7 @@ static abstract class ClientTask extends DefaultTask { void init(SpotlessTaskImpl impl) { usesServiceTolerateTestFailure(this, impl.getTaskServiceProvider()); 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.setLength(builder.length() - 1); // remove trailing newline + return builder.toString(); + } } } From 11448d9b4f250904157990f82257ebe032a4237a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 19:32:02 -0700 Subject: [PATCH 06/16] Add a `LintSuppressionTest` --- .../diffplug/spotless/LintSuppression.java | 22 ++--- .../spotless/LintSuppressionTest.java | 84 +++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java diff --git a/lib/src/main/java/com/diffplug/spotless/LintSuppression.java b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java index c26eef6b1c..41a81ae3df 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintSuppression.java +++ b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java @@ -49,21 +49,15 @@ public void setShortCode(String shortCode) { this.shortCode = Objects.requireNonNull(shortCode); } - public boolean suppresses(String relativePath, FormatterStep step, Lint lint) { - return !allows(relativePath, step, lint); - } - - private boolean allows(String relativePath, FormatterStep step, Lint lint) { - if (!file.equals(ALL) && !file.equals(relativePath)) { - return false; - } - if (!this.step.equals(ALL) && !this.step.equals(step.getName())) { - return false; - } - if (!this.shortCode.equals(ALL) && !this.shortCode.equals(lint.getRuleId())) { - return false; + public boolean suppresses(String relativePath, FormatterStep formatterStep, Lint lint) { + if (file.equals(ALL) || file.equals(relativePath)) { + if (step.equals(ALL) || formatterStep.getName().equals(this.step)) { + if (shortCode.equals(ALL) || lint.getRuleId().equals(this.shortCode)) { + return true; + } + } } - return true; + return false; } public void ensureDoesNotSuppressAll() { 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..efe96bc3cb --- /dev/null +++ b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java @@ -0,0 +1,84 @@ +/* + * 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(); + noSuppressions.removeSuppressedLints(formatter(), "file", List.of()); + assertThat(noSuppressions.isHasLints()).isTrue(); + removesLint(s -> s.setStep("blah")).isFalse(); + removesLint(s -> s.setStep("endWithNewline")).isTrue(); + removesLint(s -> s.setFile("blah")).isFalse(); + removesLint(s -> s.setFile("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(); + ls.removeSuppressedLints(formatter(), "testFile", List.of(s)); + return assertThat(!ls.isHasLints()); + } +} From b23118999c0149e11aa9055c7c26e3e0a6a540fe Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 17:23:06 -0700 Subject: [PATCH 07/16] ErrorShouldRethrowTest now tests the new lint messages. --- .../java/com/diffplug/spotless/LintState.java | 12 +++- .../spotless/ErrorShouldRethrowTest.java | 69 +++++++++---------- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index 465cb9b580..fca15ce3ac 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -21,14 +21,15 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; +import java.util.Objects; import javax.annotation.Nullable; public class LintState { private final DirtyState dirtyState; - private final @Nullable List> lintsPerStep; + private @Nullable List> lintsPerStep; - private LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { + LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { this.dirtyState = dirtyState; this.lintsPerStep = lintsPerStep; } @@ -67,6 +68,9 @@ public void removeSuppressedLints(Formatter formatter, String relativePath, List if (lintsPerStep == null) { return; } + if (formatter.getSteps().size() != lintsPerStep.size()) { + throw new IllegalStateException("LintState was created with a different formatter!"); + } for (int i = 0; i < lintsPerStep.size(); i++) { FormatterStep step = formatter.getSteps().get(i); List lints = lintsPerStep.get(i); @@ -83,6 +87,10 @@ public void removeSuppressedLints(Formatter formatter, String relativePath, List } if (lints.isEmpty()) { lintsPerStep.set(i, null); + if (lintsPerStep.stream().allMatch(Objects::isNull)) { + lintsPerStep = null; + return; + } } } } 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..1068f42908 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,15 @@ 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!"); } @Test @@ -79,17 +80,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'", " } // 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 +100,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 +111,28 @@ 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!"); } - 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); } } From cbb03fa4abefed0715179bf4f0e383135bca1127 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 20:04:05 -0700 Subject: [PATCH 08/16] Replace mutable `removeSuppressedLints` with immutable `withRemovedSuppressions`. --- .../java/com/diffplug/spotless/LintState.java | 29 +++++++++++-------- .../spotless/LintSuppressionTest.java | 6 ++-- .../gradle/spotless/SpotlessTaskImpl.java | 3 +- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index fca15ce3ac..42de3e8d58 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -18,16 +18,16 @@ 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.Objects; import javax.annotation.Nullable; public class LintState { private final DirtyState dirtyState; - private @Nullable List> lintsPerStep; + private final @Nullable List> lintsPerStep; LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { this.dirtyState = dirtyState; @@ -64,36 +64,41 @@ public LinkedHashMap> getLintsByStep(Formatter formatter) { return result; } - public void removeSuppressedLints(Formatter formatter, String relativePath, List suppressions) { + public LintState withRemovedSuppressions(Formatter formatter, String relativePath, List suppressions) { if (lintsPerStep == null) { - return; + 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 lints = lintsPerStep.get(i); - if (lints != null) { + 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()) { - lintsPerStep.set(i, null); - if (lintsPerStep.stream().allMatch(Objects::isNull)) { - lintsPerStep = null; - return; - } + 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) { diff --git a/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java index efe96bc3cb..0ad9b8e29e 100644 --- a/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java +++ b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java @@ -46,8 +46,7 @@ private Formatter formatter() { @Test public void testMatchSingle() { - var noSuppressions = dummyLintState(); - noSuppressions.removeSuppressedLints(formatter(), "file", List.of()); + var noSuppressions = dummyLintState().withRemovedSuppressions(formatter(), "file", List.of()); assertThat(noSuppressions.isHasLints()).isTrue(); removesLint(s -> s.setStep("blah")).isFalse(); removesLint(s -> s.setStep("endWithNewline")).isTrue(); @@ -77,8 +76,7 @@ private AbstractBooleanAssert removesLint(Consumer suppressi var s = new LintSuppression(); suppression.accept(s); - var ls = dummyLintState(); - ls.removeSuppressedLints(formatter(), "testFile", List.of(s)); + var ls = dummyLintState().withRemovedSuppressions(formatter(), "testFile", List.of(s)); return assertThat(!ls.isHasLints()); } } 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 27879925b6..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 @@ -118,8 +118,7 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in lintState = LintState.clean(); } else { try { - lintState = LintState.of(formatter, input); - lintState.removeSuppressedLints(formatter, relativePath, getLintSuppressions()); + lintState = LintState.of(formatter, input).withRemovedSuppressions(formatter, relativePath, getLintSuppressions()); } catch (Throwable e) { throw new IllegalArgumentException("Issue processing file: " + input, e); } From a36b44b604a8b54038d044c334fc7a53fc7beb79 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 20:04:14 -0700 Subject: [PATCH 09/16] Update tests for the new format. --- .../com/diffplug/gradle/spotless/BiomeIntegrationTest.java | 6 +++--- .../diffplug/gradle/spotless/ErrorShouldRethrowTest.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 1068f42908..eca02db2ab 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 @@ -86,7 +86,7 @@ void unlessEnforceCheckIsFalse() throws Exception { @Test void unlessExemptedByStep() throws Exception { writeBuild( - " ignoreErrorForStep 'no swearing'", + " ignoreErrorForStep 'noSwearingStep'", " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); From ffc7919d19122b50d69e2282b42a12ec17ef9ad9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 20:10:55 -0700 Subject: [PATCH 10/16] Delete the unneeded LintPolicy. --- .../com/diffplug/spotless/DirtyState.java | 2 +- .../java/com/diffplug/spotless/Formatter.java | 17 ++++++++- .../com/diffplug/spotless/LintPolicy.java | 35 ------------------- 3 files changed, 17 insertions(+), 37 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/LintPolicy.java 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/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 970d5eeeb4..9ab7008515 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), 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/LintPolicy.java b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java deleted file mode 100644 index d5fb54e628..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java +++ /dev/null @@ -1,35 +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 { - 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()) { - logger.error("Step '{}' found problem in '{}':\n{}", formatter.getSteps().get(i), file.getName(), exception.getMessage(), exception); - throw ThrowingEx.asRuntimeRethrowError(exception); - } - } - } - - private static final Logger logger = LoggerFactory.getLogger(Formatter.class); -} From a8dcdb651617b45ecc47530bd3e29679988126a4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 21:18:25 -0700 Subject: [PATCH 11/16] Minor fixup. --- lib/src/main/java/com/diffplug/spotless/Formatter.java | 2 +- lib/src/main/java/com/diffplug/spotless/LintState.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 9ab7008515..0635f8dfe6 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -141,7 +141,7 @@ static void legacyErrorBehavior(Formatter formatter, File file, ValuePerStep> getLintsByStep(Formatter formatter) { } LinkedHashMap> result = new LinkedHashMap<>(); for (int i = 0; i < lintsPerStep.size(); i++) { - FormatterStep step = formatter.getSteps().get(i); List lints = lintsPerStep.get(i); if (lints != null) { + FormatterStep step = formatter.getSteps().get(i); result.put(step.getName(), lints); } } From 2ce9803a9110c067e985e132a87c0673a638cc93 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 21:50:09 -0700 Subject: [PATCH 12/16] Improve prettier's exception to be lint-friendly. --- .../spotless/npm/PrettierMissingParserException.java | 12 ++++++++++-- .../gradle/spotless/PrettierIntegrationTest.java | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) 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/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"); } From b2a52efd5543b36e549a7df1f58de56e1dea17e2 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 22:07:46 -0700 Subject: [PATCH 13/16] Fix some lint names (file -> path, ruleId -> shortCode) --- .../main/java/com/diffplug/spotless/Lint.java | 30 +++++++++---------- .../diffplug/spotless/LintSuppression.java | 22 +++++++------- .../spotless/LintSuppressionTest.java | 4 +-- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index e3e3fa9f4b..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. */ @@ -201,7 +201,7 @@ public void addWarningMessageTo(StringBuilder buffer, String stepName, boolean o } } buffer.append(" "); - buffer.append(stepName).append("(").append(ruleId).append(") "); + buffer.append(stepName).append("(").append(shortCode).append(") "); int firstNewline = detail.indexOf('\n'); if (firstNewline == -1) { diff --git a/lib/src/main/java/com/diffplug/spotless/LintSuppression.java b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java index 41a81ae3df..d84107bd3a 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintSuppression.java +++ b/lib/src/main/java/com/diffplug/spotless/LintSuppression.java @@ -21,16 +21,16 @@ public class LintSuppression implements java.io.Serializable { private static final long serialVersionUID = 1L; private static final String ALL = "*"; - private String file = ALL; + private String path = ALL; private String step = ALL; private String shortCode = ALL; - public String getFile() { - return file; + public String getPath() { + return path; } - public void setFile(String file) { - this.file = Objects.requireNonNull(file); + public void setPath(String path) { + this.path = Objects.requireNonNull(path); } public String getStep() { @@ -50,9 +50,9 @@ public void setShortCode(String shortCode) { } public boolean suppresses(String relativePath, FormatterStep formatterStep, Lint lint) { - if (file.equals(ALL) || file.equals(relativePath)) { + if (path.equals(ALL) || path.equals(relativePath)) { if (step.equals(ALL) || formatterStep.getName().equals(this.step)) { - if (shortCode.equals(ALL) || lint.getRuleId().equals(this.shortCode)) { + if (shortCode.equals(ALL) || lint.getShortCode().equals(this.shortCode)) { return true; } } @@ -61,7 +61,7 @@ public boolean suppresses(String relativePath, FormatterStep formatterStep, Lint } public void ensureDoesNotSuppressAll() { - boolean suppressAll = file.equals(ALL) && step.equals(ALL) && shortCode.equals(ALL); + 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`."); } @@ -74,18 +74,18 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; LintSuppression that = (LintSuppression) o; - return Objects.equals(file, that.file) && Objects.equals(step, that.step) && Objects.equals(shortCode, that.shortCode); + return Objects.equals(path, that.path) && Objects.equals(step, that.step) && Objects.equals(shortCode, that.shortCode); } @Override public int hashCode() { - return Objects.hash(file, step, shortCode); + return Objects.hash(path, step, shortCode); } @Override public String toString() { return "LintSuppression{" + - "file='" + file + '\'' + + "file='" + path + '\'' + ", step='" + step + '\'' + ", code='" + shortCode + '\'' + '}'; diff --git a/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java index 0ad9b8e29e..1e1037a6ef 100644 --- a/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java +++ b/lib/src/test/java/com/diffplug/spotless/LintSuppressionTest.java @@ -50,8 +50,8 @@ public void testMatchSingle() { assertThat(noSuppressions.isHasLints()).isTrue(); removesLint(s -> s.setStep("blah")).isFalse(); removesLint(s -> s.setStep("endWithNewline")).isTrue(); - removesLint(s -> s.setFile("blah")).isFalse(); - removesLint(s -> s.setFile("testFile")).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(); } From 44b1ee4b5b179b383ae6e50d434253f900648e9f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 22:14:39 -0700 Subject: [PATCH 14/16] Add deprecation warnings and javadoc to `FormatExtension`. --- .../gradle/spotless/FormatExtension.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 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 68f00f5208..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 @@ -170,6 +170,7 @@ public void setEncoding(Charset charset) { final List lintSuppressions = new ArrayList<>(); + /** Suppresses any lints which meet the supplied criteria. */ public void suppressLintsFor(Action lintSuppression) { LintSuppression suppression = new LintSuppression(); lintSuppression.execute(suppression); @@ -177,16 +178,26 @@ public void suppressLintsFor(Action lintSuppression) { lintSuppressions.add(suppression); } - /** Ignores errors in the given step. */ + /** + * Ignores errors in the given step. + * + * @deprecated Use {@link #suppressLintsFor(Action)} instead. + */ @Deprecated public void ignoreErrorForStep(String 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) { - suppressLintsFor(it -> it.setFile(relativePath)); + System.err.println("`ignoreErrorForPath('" + relativePath + "') is deprecated, use `suppressLintsFor { path = '" + relativePath + "' }` instead."); + suppressLintsFor(it -> it.setPath(relativePath)); } /** From 59cdaa1fc93bcab628fa11908473a0a562379a13 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 22:19:25 -0700 Subject: [PATCH 15/16] Better error message, and loop that into the README. --- plugin-gradle/README.md | 18 +++++++----------- .../gradle/spotless/SpotlessTaskService.java | 2 +- .../spotless/ErrorShouldRethrowTest.java | 6 ++++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index c5adda5d2b..41c1b6e1a3 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -146,11 +146,10 @@ Starting in version `7.0.0`, Spotless now supports linting in addition to format ```console user@machine repo % ./gradlew build -:spotlessJavaCheck FAILED - The following files had format violations: - src\main\java\com\diffplug\gradle\spotless\FormatExtension.java - -\t\t····if·(targets.length·==·0)·{ - +\t\tif·(targets.length·==·0)·{ +: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` ``` @@ -158,14 +157,11 @@ To suppress lints, you can do this: ```gradle spotless { - suppressLintsFor { // applies to all formats - file = 'src/blah/blah' - } kotlin { - ktfmt() - suppressLintsFor { // applies to only the kotlin formats + ktlint() + suppressLintsFor { step = 'ktlint' - shortCode = 'rename' + shortCode = 'standard:no-wildcard-imports' } } } 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 ce371ecd61..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 @@ -199,7 +199,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { } } } - builder.setLength(builder.length() - 1); // remove trailing newline + builder.append("Resolve these lints or suppress with `suppressLintsFor`"); return builder.toString(); } } 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 eca02db2ab..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 @@ -70,7 +70,8 @@ void anyExceptionShouldFail() throws 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!"); + " README.md:LINE_UNDEFINED noSwearingStep(swearing) No swearing!", + " Resolve these lints or suppress with `suppressLintsFor`"); } @Test @@ -119,7 +120,8 @@ void failsIfNeitherStepNorFileExempted() throws 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!"); + " README.md:LINE_UNDEFINED noSwearingStep(swearing) No swearing!", + " Resolve these lints or suppress with `suppressLintsFor`"); } private void expectSuccess() throws Exception { From 611a45d0826acf304440705acfa1f0e70d82c77b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Oct 2024 22:27:27 -0700 Subject: [PATCH 16/16] Update changelogs. --- CHANGES.md | 2 +- plugin-gradle/CHANGES.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) 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/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))