diff --git a/CHANGES.md b/CHANGES.md index 280eb54065..bf65c5a019 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,15 @@ 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)) + * 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...")` + * or by implementing the `List lint(String content, File file)` method to create multiple of them * 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)) +### Changes +* **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. ([#2148](https://github.com/diffplug/spotless/pull/2148)) + * **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `DirtyState`. ### Fixed * `ktlint` steps now read from the `string` instead of the `file` so they don't clobber earlier steps. (fixes [#1599](https://github.com/diffplug/spotless/issues/1599)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e667955542..0ee619f680 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,7 +93,6 @@ Here's a checklist for creating a new step for Spotless: - [ ] Class name ends in Step, `SomeNewStep`. - [ ] Class has a public static method named `create` that returns a `FormatterStep`. - [ ] Has a test class named `SomeNewStepTest` that uses `StepHarness` or `StepHarnessWithFile` to test the step. - - [ ] Start with `StepHarness.forStep(myStep).supportsRoundTrip(false)`, and then add round trip support as described in the next section. - [ ] Test class has test methods to verify behavior. - [ ] Test class has a test method `equality()` which tests equality using `StepEqualityTester` (see existing methods for examples). @@ -137,6 +136,15 @@ There are many great formatters (prettier, clang-format, black, etc.) which live Because of Spotless' up-to-date checking and [git ratcheting](https://github.com/diffplug/spotless/tree/main/plugin-gradle#ratchet), Spotless actually doesn't have to call formatters very often, so even an expensive shell call for every single invocation isn't that bad. Anything that works is better than nothing, and we can always speed things up later if it feels too slow (but it probably won't). +## Lints + +Spotless is primarily a formatter, not a linter. But, if something goes wrong during formatting, it's better to model that as a lint with line numbers rather than just a naked exception. There are two ways to go about this: + +- at any point during the formatting process, you can throw a `Lint.atLine(int line, ...)` exception. This will be caught and turned into a lint. +- or you can override the `default List lint(String content, File file)` method. This method will only run if the step did not already throw an exception. + +Don't go lint crazy! By default, all lints are build failures. Users have to suppress them explicitly if they want to continue. + ## How to add a new plugin for a build system The gist of it is that you will have to: diff --git a/gradle.properties b/gradle.properties index 660a6a63a3..265b57318f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -32,3 +32,4 @@ VER_JGIT=6.10.0.202406032230-r VER_JUNIT=5.11.2 VER_ASSERTJ=3.26.3 VER_MOCKITO=5.14.2 +VER_SELFIE=2.4.1 \ No newline at end of file diff --git a/gradle/special-tests.gradle b/gradle/special-tests.gradle index 5bf7e7d10a..650c04b84b 100644 --- a/gradle/special-tests.gradle +++ b/gradle/special-tests.gradle @@ -18,7 +18,14 @@ tasks.withType(Test).configureEach { maxFailures = 10 } } - + // selfie https://selfie.dev/jvm/get-started#gradle + environment project.properties.subMap([ + "selfie" + ]) // optional, see "Overwrite everything" below + inputs.files(fileTree("src/test") { + // optional, improves up-to-date checking + include "**/*.ss" + }) // https://docs.gradle.org/8.8/userguide/performance.html#execute_tests_in_parallel maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1 } diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 39d5da47d6..964cd102e1 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -46,7 +46,7 @@ public boolean didNotConverge() { return this == didNotConverge; } - private byte[] canonicalBytes() { + byte[] canonicalBytes() { if (canonicalBytes == null) { throw new IllegalStateException("First make sure that {@code !isClean()} and {@code !didNotConverge()}"); } @@ -74,7 +74,17 @@ public static DirtyState of(Formatter formatter, File file) throws IOException { } public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { - String raw = new String(rawBytes, formatter.getEncoding()); + return of(formatter, file, rawBytes, new String(rawBytes, formatter.getEncoding())); + } + + 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); + return state; + } + + static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw, ValuePerStep exceptionPerStep) { // check that all characters were encodable String encodingError = EncodingErrorMsg.msg(raw, rawBytes, formatter.getEncoding()); if (encodingError != null) { @@ -84,7 +94,7 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { String rawUnix = LineEnding.toUnix(raw); // enforce the format - String formattedUnix = formatter.compute(rawUnix, file); + String formattedUnix = formatter.computeWithLint(rawUnix, file, exceptionPerStep); // convert the line endings if necessary String formatted = formatter.computeLineEndings(formattedUnix, file); @@ -95,13 +105,13 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { } // F(input) != input, so we'll do a padded check - String doubleFormattedUnix = formatter.compute(formattedUnix, file); + String doubleFormattedUnix = formatter.computeWithLint(formattedUnix, file, exceptionPerStep); if (doubleFormattedUnix.equals(formattedUnix)) { // most dirty files are idempotent-dirty, so this is a quick-short circuit for that common case return new DirtyState(formattedBytes); } - PaddedCell cell = PaddedCell.check(formatter, file, rawUnix); + PaddedCell cell = PaddedCell.check(formatter, file, rawUnix, exceptionPerStep); if (!cell.isResolvable()) { return didNotConverge; } diff --git a/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java index 4cc336e101..bcc444ad57 100644 --- a/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.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. @@ -16,8 +16,8 @@ package com.diffplug.spotless; import java.io.File; +import java.util.List; import java.util.Objects; -import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -36,14 +36,24 @@ final class FilterByContentPatternFormatterStep extends DelegateFormatterStep { public @Nullable String format(String raw, File file) throws Exception { Objects.requireNonNull(raw, "raw"); Objects.requireNonNull(file, "file"); - Matcher matcher = contentPattern.matcher(raw); - if (matcher.find() == (onMatch == OnMatch.INCLUDE)) { + if (contentPattern.matcher(raw).find() == (onMatch == OnMatch.INCLUDE)) { return delegateStep.format(raw, file); } else { return raw; } } + @Override + public List lint(String raw, File file) throws Exception { + Objects.requireNonNull(raw, "raw"); + Objects.requireNonNull(file, "file"); + if (contentPattern.matcher(raw).find() == (onMatch == OnMatch.INCLUDE)) { + return delegateStep.lint(raw, file); + } else { + return List.of(); + } + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/lib/src/main/java/com/diffplug/spotless/FilterByFileFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FilterByFileFormatterStep.java index 04a06a4673..bc5ddf6053 100644 --- a/lib/src/main/java/com/diffplug/spotless/FilterByFileFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FilterByFileFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 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. @@ -16,6 +16,7 @@ package com.diffplug.spotless; import java.io.File; +import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -39,6 +40,17 @@ final class FilterByFileFormatterStep extends DelegateFormatterStep { } } + @Override + public List lint(String content, File file) throws Exception { + Objects.requireNonNull(content, "content"); + Objects.requireNonNull(file, "file"); + if (filter.accept(file)) { + return delegateStep.lint(content, file); + } else { + return List.of(); + } + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java deleted file mode 100644 index 50f49e41db..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2016-2023 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * 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.Serializable; - -/** A policy for handling exceptions in the format. */ -public interface FormatExceptionPolicy extends Serializable, NoLambda { - /** Called for every error in the formatter. */ - public void handleError(Throwable e, FormatterStep step, String relativePath); - - /** - * Returns a byte array representation of everything inside this {@code FormatExceptionPolicy}. - *

- * The main purpose of this method is to ensure one can't instantiate this class with lambda - * expressions, which are notoriously difficult to serialize and deserialize properly. - */ - public byte[] toBytes(); - - /** - * A policy which rethrows subclasses of {@code Error} and logs other kinds of Exception. - */ - public static FormatExceptionPolicy failOnlyOnError() { - return new FormatExceptionPolicyLegacy(); - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java index 6fd8371928..9bafc946ef 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 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. @@ -23,7 +23,7 @@ * 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 implements FormatExceptionPolicy { +public class FormatExceptionPolicyStrict extends NoLambda.EqualityBasedOnSerialization { private static final long serialVersionUID = 1L; private final Set excludeSteps = new TreeSet<>(); @@ -39,18 +39,17 @@ public void excludePath(String relativePath) { excludePaths.add(Objects.requireNonNull(relativePath)); } - @Override 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())) { - FormatExceptionPolicyLegacy.warning(e, step, relativePath); + LintPolicy.warning(e, step, relativePath); } else { if (excludePaths.contains(relativePath)) { - FormatExceptionPolicyLegacy.warning(e, step, relativePath); + LintPolicy.warning(e, step, relativePath); } else { - FormatExceptionPolicyLegacy.error(e, step, relativePath); + LintPolicy.error(e, step, relativePath); throw ThrowingEx.asRuntimeRethrowError(e); } } diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 4bc017a834..970d5eeeb4 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -128,28 +128,51 @@ public String computeLineEndings(String unix, File file) { * is guaranteed to also have unix line endings. */ public String compute(String unix, File file) { + ValuePerStep exceptionPerStep = new ValuePerStep<>(this); + String result = computeWithLint(unix, file, exceptionPerStep); + LintPolicy.legacyBehavior(this, file, exceptionPerStep); + return result; + } + + /** + * Returns the result of calling all of the FormatterSteps, while also + * tracking any exceptions which are thrown. + *

+ * The input must have unix line endings, and the output + * is guaranteed to also have unix line endings. + *

+ * It doesn't matter what is inside `ValuePerStep`, the value at every index will be overwritten + * when the method returns. + */ + String computeWithLint(String unix, File file, ValuePerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); - for (FormatterStep step : steps) { + for (int i = 0; i < steps.size(); ++i) { + FormatterStep step = steps.get(i); + Throwable storeForStep; try { String formatted = step.format(unix, file); if (formatted == null) { // This probably means it was a step that only checks // for errors and doesn't actually have any fixes. // No exception was thrown so we can just continue. + storeForStep = LintState.formatStepCausedNoChange(); } else { // Should already be unix-only, but some steps might misbehave. - unix = LineEnding.toUnix(formatted); + String clean = LineEnding.toUnix(formatted); + if (clean.equals(unix)) { + storeForStep = LintState.formatStepCausedNoChange(); + } else { + storeForStep = null; + unix = LineEnding.toUnix(formatted); + } } } catch (Throwable e) { - // TODO: this is bad, but it won't matter when add support for linting - if (e instanceof RuntimeException) { - throw (RuntimeException) e; - } else { - throw new RuntimeException(e); - } + // store the exception which was thrown and keep going + storeForStep = e; } + exceptionPerStep.set(i, storeForStep); } return unix; } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index 800a553225..5e6c44b335 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -16,6 +16,7 @@ package com.diffplug.spotless; import java.io.File; +import java.util.List; import java.util.Objects; /** @@ -32,6 +33,14 @@ default String apply(String unix, File file) throws Exception { return apply(unix); } + /** + * Calculates a list of lints against the given content. + * By default, that's just an throwables thrown by the lint. + */ + default List lint(String content, File file) throws Exception { + return List.of(); + } + /** * {@code Function} and {@code BiFunction} whose implementation * requires a resource which should be released when the function is no longer needed. @@ -74,6 +83,14 @@ public String apply(String unix) throws Exception { @FunctionalInterface interface ResourceFunc { String apply(T resource, String unix) throws Exception; + + /** + * Calculates a list of lints against the given content. + * By default, that's just an throwables thrown by the lint. + */ + default List lint(T resource, String unix) throws Exception { + return List.of(); + } } /** Creates a {@link FormatterFunc.Closeable} which uses the given resource to execute the format function. */ @@ -101,6 +118,10 @@ public String apply(String unix) throws Exception { @FunctionalInterface interface ResourceFuncNeedsFile { String apply(T resource, String unix, File file) throws Exception; + + default List lint(T resource, String content, File file) throws Exception { + return List.of(); + } } /** Creates a {@link FormatterFunc.Closeable} which uses the given resource to execute the file-dependent format function. */ @@ -123,6 +144,11 @@ public String apply(String unix, File file) throws Exception { public String apply(String unix) throws Exception { return apply(unix, Formatter.NO_FILE_SENTINEL); } + + @Override + public List lint(String content, File file) throws Exception { + return function.lint(resource, content, file); + } }; } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 2a5a7d2b2f..870ef0ee18 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -17,6 +17,7 @@ import java.io.File; import java.io.Serializable; +import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -46,6 +47,22 @@ public interface FormatterStep extends Serializable, AutoCloseable { @Nullable String format(String rawUnix, File file) throws Exception; + /** + * Returns a list of lints against the given file content + * + * @param content + * the content to check + * @param file + * the file which {@code content} was obtained from; never null. Pass an empty file using + * {@code new File("")} if and only if no file is actually associated with {@code content} + * @return a list of lints + * @throws Exception if the formatter step experiences a problem + */ + @Nullable + default List lint(String content, File file) throws Exception { + return List.of(); + } + /** * Returns a new {@code FormatterStep} which, observing the value of {@code formatIfMatches}, * will only apply, or not, its changes to files which pass the given filter. diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java index 52bf9fc760..e42e0cb4f9 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.Serializable; import java.util.Arrays; +import java.util.List; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -48,6 +49,14 @@ public String format(String rawUnix, File file) throws Exception { return formatter.apply(rawUnix, file); } + @Override + public List lint(String content, File file) throws Exception { + if (formatter == null) { + formatter = stateToFormatter(state()); + } + return formatter.lint(content, file); + } + @Override public boolean equals(Object o) { if (o == null) { diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java new file mode 100644 index 0000000000..4384ed38ef --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -0,0 +1,192 @@ +/* + * Copyright 2022-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.Serializable; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Models a linted line or line range. Note that there is no concept of severity level - responsibility + * for severity and confidence are pushed down to the configuration of the lint tool. If a lint makes it + * to Spotless, then it is by definition. + */ +public final class Lint implements Serializable { + public static Lint atUndefinedLine(String ruleId, String detail) { + return new Lint(LINE_UNDEFINED, ruleId, detail); + } + + 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); + } + + 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 detail; + + private Lint(int lineStart, int lineEnd, String ruleId, 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.detail = LineEnding.toUnix(detail); + } + + private Lint(int line, String ruleId, String detail) { + this(line, line, ruleId, detail); + } + + public int getLineStart() { + return lineStart; + } + + public int getLineEnd() { + return lineEnd; + } + + public String getRuleId() { + return ruleId; + } + + public String getDetail() { + return detail; + } + + /** Any exception which implements this interface will have its lints extracted and reported cleanly to the user. */ + public interface Has { + List getLints(); + } + + /** An exception for shortcutting execution to report a lint to the user. */ + static class ShortcutException extends RuntimeException implements Has { + public ShortcutException(Lint... lints) { + this(Arrays.asList(lints)); + } + + private final List lints; + + ShortcutException(Collection lints) { + super(lints.iterator().next().detail); + this.lints = List.copyOf(lints); + } + + @Override + public List getLints() { + return lints; + } + } + + /** Returns an exception which will wrap all of the given lints using {@link Has} */ + public static RuntimeException shortcut(Collection lints) { + return new ShortcutException(lints); + } + + /** Returns an exception which will wrap this lint using {@link Has} */ + public RuntimeException shortcut() { + return new ShortcutException(this); + } + + @Override + public String toString() { + if (lineStart == lineEnd) { + if (lineStart == LINE_UNDEFINED) { + return "LINE_UNDEFINED: (" + ruleId + ") " + detail; + } else { + return "L" + lineStart + ": (" + ruleId + ") " + detail; + } + } else { + return "L" + lineStart + "-" + lineEnd + ": (" + ruleId + ") " + detail; + } + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + 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); + } + + @Override + public int hashCode() { + return Objects.hash(lineStart, lineEnd, ruleId, detail); + } + + /** Attempts to parse a line number from the given exception. */ + static Lint createFromThrowable(FormatterStep step, Throwable e) { + Throwable current = e; + while (current != null) { + String message = current.getMessage(); + int lineNumber = lineNumberFor(message); + if (lineNumber != -1) { + return new Lint(lineNumber, step.getName(), msgFrom(message)); + } + current = current.getCause(); + } + String exceptionName = e.getClass().getName(); + String detail = ThrowingEx.stacktrace(e); + if (detail.startsWith(exceptionName + ": ")) { + detail = detail.substring(exceptionName.length() + 2); + } + Matcher matcher = Pattern.compile("line (\\d+)").matcher(detail); + int line = LINE_UNDEFINED; + if (matcher.find()) { + line = Integer.parseInt(matcher.group(1)); + } + return Lint.atLine(line, exceptionName, detail); + } + + private static int lineNumberFor(String message) { + if (message == null) { + return -1; + } + int firstColon = message.indexOf(':'); + if (firstColon == -1) { + return -1; + } + String candidateNum = message.substring(0, firstColon); + try { + return Integer.parseInt(candidateNum); + } catch (NumberFormatException e) { + return -1; + } + } + + private static String msgFrom(String message) { + for (int i = 0; i < message.length(); ++i) { + if (Character.isLetter(message.charAt(i))) { + return message.substring(i); + } + } + return ""; + } + + public static final int LINE_UNDEFINED = -1; +} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java similarity index 68% rename from lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java rename to lib/src/main/java/com/diffplug/spotless/LintPolicy.java index 93ca5d05b6..0b28c36884 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java +++ b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 DiffPlug + * 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. @@ -15,24 +15,14 @@ */ package com.diffplug.spotless; +import java.io.File; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class FormatExceptionPolicyLegacy extends NoLambda.EqualityBasedOnSerialization implements FormatExceptionPolicy { - private static final long serialVersionUID = 1L; - +class LintPolicy { private static final Logger logger = LoggerFactory.getLogger(Formatter.class); - @Override - public void handleError(Throwable e, FormatterStep step, String relativePath) { - if (e instanceof Error) { - error(e, step, relativePath); - throw ((Error) e); - } else { - warning(e, step, relativePath); - } - } - static void error(Throwable e, FormatterStep step, String relativePath) { logger.error("Step '{}' found problem in '{}':\n{}", step.getName(), relativePath, e.getMessage(), e); } @@ -40,4 +30,14 @@ static void error(Throwable e, FormatterStep step, String relativePath) { static void warning(Throwable e, FormatterStep step, String relativePath) { logger.warn("Unable to apply step '{}' to '{}'", step.getName(), relativePath, e); } + + static void legacyBehavior(Formatter formatter, File file, ValuePerStep exceptionPerStep) { + for (int i = 0; i < formatter.getSteps().size(); ++i) { + Throwable exception = exceptionPerStep.get(i); + if (exception != null && exception != LintState.formatStepCausedNoChange()) { + LintPolicy.error(exception, formatter.getSteps().get(i), file.getName()); + throw ThrowingEx.asRuntimeRethrowError(exception); + } + } + } } diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java new file mode 100644 index 0000000000..2fcf18ef6c --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -0,0 +1,178 @@ +/* + * 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 java.io.IOException; +import java.nio.file.Files; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import javax.annotation.Nullable; + +public class LintState { + private final DirtyState dirtyState; + private final @Nullable List> lintsPerStep; + + private LintState(DirtyState dirtyState, @Nullable List> lintsPerStep) { + this.dirtyState = dirtyState; + this.lintsPerStep = lintsPerStep; + } + + public DirtyState getDirtyState() { + return dirtyState; + } + + public boolean isHasLints() { + return lintsPerStep != null; + } + + public boolean isClean() { + return dirtyState.isClean() && !isHasLints(); + } + + public Map> getLints(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<>(); + for (int i = 0; i < lintsPerStep.size(); i++) { + List lints = lintsPerStep.get(i); + if (lints != null) { + result.put(formatter.getSteps().get(i), lints); + } + } + return result; + } + + public String asString(File file, Formatter formatter) { + if (!isHasLints()) { + return "(none)"; + } else { + StringBuilder result = new StringBuilder(); + for (int i = 0; i < lintsPerStep.size(); i++) { + List lints = lintsPerStep.get(i); + if (lints != null) { + 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(") "); + result.append(lint.getDetail()); + result.append("\n"); + } + } + } + result.setLength(result.length() - 1); + return result.toString(); + } + } + + public static LintState of(Formatter formatter, File file) throws IOException { + return of(formatter, file, Files.readAllBytes(file.toPath())); + } + + public static LintState of(Formatter formatter, File file, byte[] rawBytes) { + var exceptions = new ValuePerStep(formatter); + var raw = new String(rawBytes, formatter.getEncoding()); + var dirty = DirtyState.of(formatter, file, rawBytes, raw, exceptions); + + String toLint = LineEnding.toUnix(dirty.isClean() || dirty.didNotConverge() ? raw : new String(dirty.canonicalBytes(), formatter.getEncoding())); + + var lints = new ValuePerStep>(formatter); + // if a step did not throw an exception, then it gets to check for lints if it wants + for (int i = 0; i < formatter.getSteps().size(); i++) { + FormatterStep step = formatter.getSteps().get(i); + Throwable exception = exceptions.get(i); + if (exception == null || exception == formatStepCausedNoChange()) { + try { + var lintsForStep = step.lint(toLint, file); + if (lintsForStep != null && !lintsForStep.isEmpty()) { + lints.set(i, lintsForStep); + } + } catch (Exception e) { + lints.set(i, List.of(Lint.createFromThrowable(step, e))); + } + } + } + // for steps that did throw an exception, we will turn those into lints + // we try to reuse the exception if possible, but that is only possible if other steps + // didn't change the formatted value. so we start at the end, and note when the string + // gets changed by a step. if it does, we rerun the steps to get an exception with accurate line numbers. + boolean nothingHasChangedSinceLast = true; + for (int i = formatter.getSteps().size() - 1; i >= 0; i--) { + FormatterStep step = formatter.getSteps().get(i); + Throwable exception = exceptions.get(i); + if (exception != null && exception != formatStepCausedNoChange()) { + nothingHasChangedSinceLast = false; + } + Throwable exceptionForLint; + if (nothingHasChangedSinceLast) { + exceptionForLint = exceptions.get(i); + } else { + // steps changed the content, so we need to rerun to get an exception with accurate line numbers + try { + step.format(toLint, file); + exceptionForLint = null; // the exception "went away" because it got fixed by a later step + } catch (Throwable e) { + exceptionForLint = e; + } + } + List lintsForStep; + if (exceptionForLint instanceof Lint.Has) { + lintsForStep = ((Lint.Has) exceptionForLint).getLints(); + } else if (exceptionForLint != null && exceptionForLint != formatStepCausedNoChange()) { + lintsForStep = List.of(Lint.createFromThrowable(step, exceptionForLint)); + } else { + lintsForStep = List.of(); + } + if (!lintsForStep.isEmpty()) { + lints.set(i, lintsForStep); + } + } + return new LintState(dirty, lints.indexOfFirstValue() == -1 ? null : lints); + } + + /** Returns the DirtyState which corresponds to {@code isClean()}. */ + public static LintState clean() { + return isClean; + } + + private static final LintState isClean = new LintState(DirtyState.clean(), null); + + static Throwable formatStepCausedNoChange() { + return FormatterCausedNoChange.INSTANCE; + } + + private static class FormatterCausedNoChange extends Exception { + private static final long serialVersionUID = 1L; + + static final FormatterCausedNoChange INSTANCE = new FormatterCausedNoChange(); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java b/lib/src/main/java/com/diffplug/spotless/PaddedCell.java index b0cb64ae90..5ec7ef1cad 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCell.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCell.java @@ -86,29 +86,29 @@ public static PaddedCell check(Formatter formatter, File file) { byte[] rawBytes = ThrowingEx.get(() -> Files.readAllBytes(file.toPath())); String raw = new String(rawBytes, formatter.getEncoding()); String original = LineEnding.toUnix(raw); - return check(formatter, file, original, MAX_CYCLE); + return check(formatter, file, original, MAX_CYCLE, new ValuePerStep<>(formatter)); } public static PaddedCell check(Formatter formatter, File file, String originalUnix) { - return check( - Objects.requireNonNull(formatter, "formatter"), - Objects.requireNonNull(file, "file"), - Objects.requireNonNull(originalUnix, "originalUnix"), - MAX_CYCLE); + return check(formatter, file, originalUnix, new ValuePerStep<>(formatter)); + } + + public static PaddedCell check(Formatter formatter, File file, String originalUnix, ValuePerStep exceptionPerStep) { + return check(formatter, file, originalUnix, MAX_CYCLE, exceptionPerStep); } private static final int MAX_CYCLE = 10; - private static PaddedCell check(Formatter formatter, File file, String original, int maxLength) { + private static PaddedCell check(Formatter formatter, File file, String original, int maxLength, ValuePerStep exceptionPerStep) { if (maxLength < 2) { throw new IllegalArgumentException("maxLength must be at least 2"); } - String appliedOnce = formatter.compute(original, file); + String appliedOnce = formatter.computeWithLint(original, file, exceptionPerStep); if (appliedOnce.equals(original)) { return Type.CONVERGE.create(file, Collections.singletonList(appliedOnce)); } - String appliedTwice = formatter.compute(appliedOnce, file); + String appliedTwice = formatter.computeWithLint(appliedOnce, file, exceptionPerStep); if (appliedOnce.equals(appliedTwice)) { return Type.CONVERGE.create(file, Collections.singletonList(appliedOnce)); } @@ -118,7 +118,7 @@ private static PaddedCell check(Formatter formatter, File file, String original, appliedN.add(appliedTwice); String input = appliedTwice; while (appliedN.size() < maxLength) { - String output = formatter.compute(input, file); + String output = formatter.computeWithLint(input, file, exceptionPerStep); if (output.equals(input)) { return Type.CONVERGE.create(file, appliedN); } else { diff --git a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java index 6eb573b5d8..7e017e0989 100644 --- a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java +++ b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.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. @@ -15,6 +15,9 @@ */ package com.diffplug.spotless; +import java.io.PrintWriter; +import java.io.StringWriter; + /** * Basic functional interfaces which throw exception, along with * static helper methods for calling them. @@ -142,4 +145,12 @@ public WrappedAsRuntimeException(Throwable e) { super(e); } } + + public static String stacktrace(Throwable e) { + StringWriter out = new StringWriter(); + PrintWriter writer = new PrintWriter(out); + e.printStackTrace(writer); + writer.flush(); + return out.toString(); + } } diff --git a/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java new file mode 100644 index 0000000000..314bdc7e60 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java @@ -0,0 +1,93 @@ +/* + * Copyright 2024 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.spotless; + +import java.util.AbstractList; + +import javax.annotation.Nullable; + +/** + * Fixed-size list which maintains a list of exceptions, one per step of the formatter. + * Usually this list will be empty or have only a single value, so it is optimized for stack allocation in those cases. + */ +class ValuePerStep extends AbstractList { + private final int size; + private @Nullable T value; + private int valueIdx; + private @Nullable Object[] multipleValues = null; + + ValuePerStep(Formatter formatter) { + this.size = formatter.getSteps().size(); + } + + @Override + public @Nullable T set(int index, T newValue) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); + } + if (this.value == null) { + this.valueIdx = index; + this.value = newValue; + return null; + } else if (this.multipleValues != null) { + T previousValue = (T) multipleValues[index]; + multipleValues[index] = newValue; + return previousValue; + } else { + if (index == valueIdx) { + T previousValue = this.value; + this.value = newValue; + return previousValue; + } else { + multipleValues = new Object[size]; + multipleValues[valueIdx] = this.value; + multipleValues[index] = newValue; + return null; + } + } + } + + @Override + public T get(int index) { + if (multipleValues != null) { + return (T) multipleValues[index]; + } else if (valueIdx == index) { + return value; + } else { + return null; + } + } + + public int indexOfFirstValue() { + if (multipleValues != null) { + for (int i = 0; i < multipleValues.length; i++) { + if (multipleValues[i] != null) { + return i; + } + } + return -1; + } else if (value != null) { + return valueIdx; + } else { + return -1; + } + } + + @Override + public int size() { + return size; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java index 9ce34675f8..3cb0e3f047 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -27,9 +27,9 @@ import javax.annotation.Nullable; import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.Lint; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -101,7 +101,7 @@ static class ApplyWithin extends BaseStep { } @Override - public String apply(Formatter formatter, String unix, File file) throws Exception { + protected String applySubclass(Formatter formatter, String unix, File file) { List groups = groupsZeroed(); Matcher matcher = regex.matcher(unix); while (matcher.find()) { @@ -130,7 +130,7 @@ private void storeGroups(String unix) { } @Override - public String apply(Formatter formatter, String unix, File file) throws Exception { + protected String applySubclass(Formatter formatter, String unix, File file) { storeGroups(unix); String formatted = formatter.compute(unix, file); return assembleGroups(formatted); @@ -138,7 +138,7 @@ public String apply(Formatter formatter, String unix, File file) throws Exceptio } @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - public static abstract class BaseStep implements Serializable, FormatterStep, FormatterFunc.Closeable.ResourceFuncNeedsFile { + private static abstract class BaseStep implements Serializable, FormatterStep { final String name; private static final long serialVersionUID = -2301848328356559915L; final Pattern regex; @@ -198,8 +198,8 @@ protected String assembleGroups(String unix) { return builder.toString(); } else { // these will be needed to generate Lints later on - // int startLine = 1 + (int) builder.toString().codePoints().filter(c -> c == '\n').count(); - // int endLine = 1 + (int) unix.codePoints().filter(c -> c == '\n').count(); + int startLine = 1 + (int) builder.toString().codePoints().filter(c -> c == '\n').count(); + int endLine = 1 + (int) unix.codePoints().filter(c -> c == '\n').count(); // throw an error with either the full regex, or the nicer open/close pair Matcher openClose = Pattern.compile("\\\\Q([\\s\\S]*?)\\\\E" + "\\Q([\\s\\S]*?)\\E" + "\\\\Q([\\s\\S]*?)\\\\E") @@ -210,7 +210,9 @@ protected String assembleGroups(String unix) { } else { pattern = regex.pattern(); } - throw new Error("An intermediate step removed a match of " + pattern); + throw new Lint.ShortcutException(Lint.create("fenceRemoved", + "An intermediate step removed a match of " + pattern, + startLine, endLine)); } } @@ -221,15 +223,21 @@ public String getName() { private transient Formatter formatter; - @Nullable - @Override - public String format(String rawUnix, File file) throws Exception { + private String apply(String rawUnix, File file) throws Exception { if (formatter == null) { formatter = buildFormatter(); } - return this.apply(formatter, rawUnix, file); + return applySubclass(formatter, rawUnix, file); } + @Nullable + @Override + public String format(String rawUnix, File file) throws Exception { + return apply(rawUnix, file); + } + + protected abstract String applySubclass(Formatter formatter, String unix, File file) throws Exception; + @Override public boolean equals(Object o) { if (this == o) 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 ac1b3c42a5..efb3b45bdf 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 @@ -339,7 +339,8 @@ private static void relativizeIfSubdir(List relativePaths, File root, Fi if (!destPath.startsWith(rootPath)) { return null; } else { - return destPath.substring(rootPath.length()); + String relativized = destPath.substring(rootPath.length()); + return relativized.startsWith("/") || relativized.startsWith("\\") ? relativized.substring(1) : relativized; } } 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 77d5683b6e..e1e729e852 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 @@ -36,7 +36,6 @@ import org.gradle.work.Incremental; import com.diffplug.spotless.ConfigurationCacheHackList; -import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; @@ -122,14 +121,14 @@ public ObjectId getRatchetSha() { return subtreeSha; } - protected FormatExceptionPolicy exceptionPolicy = new FormatExceptionPolicyStrict(); + protected FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); - public void setExceptionPolicy(FormatExceptionPolicy exceptionPolicy) { + public void setExceptionPolicy(FormatExceptionPolicyStrict exceptionPolicy) { this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy); } @Input - public FormatExceptionPolicy getExceptionPolicy() { + public FormatExceptionPolicyStrict getExceptionPolicy() { return exceptionPolicy; } 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 b7586f742b..17cd6c08a1 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 @@ -37,8 +37,8 @@ import com.diffplug.common.annotations.VisibleForTesting; import com.diffplug.common.base.StringPrinter; -import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.LintState; import com.diffplug.spotless.extra.GitRatchet; @CacheableTask @@ -83,7 +83,7 @@ public void performAction(InputChanges inputs) throws Exception { for (FileChange fileChange : inputs.getFileChanges(target)) { File input = fileChange.getFile(); if (fileChange.getChangeType() == ChangeType.REMOVED) { - deletePreviousResult(input); + deletePreviousResults(input); } else { if (input.isFile()) { processInputFile(ratchet, formatter, input); @@ -95,24 +95,22 @@ public void performAction(InputChanges inputs) throws Exception { @VisibleForTesting void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException { - File output = getOutputFile(input); + File output = getOutputFileWithBaseDir(input, outputDirectory); getLogger().debug("Applying format to {} and writing to {}", input, output); - DirtyState dirtyState; + LintState lintState; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { - dirtyState = DirtyState.clean(); + lintState = LintState.clean(); } else { try { - dirtyState = DirtyState.of(formatter, input); - } catch (IOException e) { - throw new IOException("Issue processing file: " + input, e); - } catch (RuntimeException e) { + lintState = LintState.of(formatter, input); + } catch (Throwable e) { throw new IllegalArgumentException("Issue processing file: " + input, e); } } - if (dirtyState.isClean()) { + if (lintState.getDirtyState().isClean()) { // Remove previous output if it exists Files.deleteIfExists(output.toPath()); - } else if (dirtyState.didNotConverge()) { + } 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(); @@ -124,12 +122,17 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in Files.copy(input.toPath(), output.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); getLogger().info(String.format("Writing clean file: %s", output)); - dirtyState.writeCanonicalTo(output); + lintState.getDirtyState().writeCanonicalTo(output); + } + 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)); } } - private void deletePreviousResult(File input) throws IOException { - File output = getOutputFile(input); + private void deletePreviousResults(File input) throws IOException { + File output = getOutputFileWithBaseDir(input, outputDirectory); if (output.isDirectory()) { getFs().delete(d -> d.delete(output)); } else { @@ -137,7 +140,7 @@ private void deletePreviousResult(File input) throws IOException { } } - private File getOutputFile(File input) { + private File getOutputFileWithBaseDir(File input, File baseDir) { File projectDir = getProjectDir().get().getAsFile(); String outputFileName = FormatExtension.relativize(projectDir, input); if (outputFileName == null) { @@ -147,6 +150,6 @@ private File getOutputFile(File input) { printer.println(" target: " + input.getAbsolutePath()); })); } - return new File(outputDirectory, outputFileName); + return new File(baseDir, outputFileName); } } 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 abf8d34fb0..8391a8757e 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 @@ -19,12 +19,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.owasp.encoder.Encode; -import com.diffplug.spotless.tag.ForLintRefactor; - class BiomeIntegrationTest extends GradleIntegrationHarness { /** * Tests that biome can be used as a JSON formatting step, using biome 1.8.3 @@ -376,8 +373,6 @@ void failureWhenExeNotFound() throws Exception { * @throws Exception When a test failure occurs. */ @Test - @Disabled - @ForLintRefactor void failureWhenNotParseable() throws Exception { setFile("build.gradle").toLines( "plugins {", 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 d8e9cbd2fa..0c21a3644d 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 @@ -23,17 +23,13 @@ import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.TaskOutcome; -import org.junit.jupiter.api.Disabled; 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.spotless.tag.ForLintRefactor; /** Tests the desired behavior from https://github.com/diffplug/spotless/issues/46. */ -@Disabled -@ForLintRefactor class ErrorShouldRethrowTest extends GradleIntegrationHarness { private void writeBuild(String... toInsert) throws IOException { List lines = new ArrayList<>(); @@ -47,7 +43,7 @@ private void writeBuild(String... toInsert) throws IOException { lines.add(" target file('README.md')"); lines.add(" custom 'no swearing', {"); lines.add(" if (it.toLowerCase(Locale.ROOT).contains('fubar')) {"); - lines.add(" throw new RuntimeException('No swearing!');"); + lines.add(" throw com.diffplug.spotless.Lint.atUndefinedLine('swearing', 'No swearing!').shortcut();"); lines.add(" }"); lines.add(" }"); lines.addAll(Arrays.asList(toInsert)); @@ -72,8 +68,8 @@ void anyExceptionShouldFail() throws Exception { runWithFailure( "> Task :spotlessMisc FAILED\n" + "Step 'no swearing' found problem in 'README.md':\n" + - "No swearing!\n" + - "java.lang.RuntimeException: No swearing!"); + "LINE_UNDEFINED: (swearing) No swearing!\n" + + "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); } @Test @@ -86,7 +82,6 @@ void unlessEnforceCheckIsFalse() throws Exception { runWithSuccess("> Task :processResources NO-SOURCE"); } - @Test void unlessExemptedByStep() throws Exception { writeBuild( " ignoreErrorForStep 'no swearing'", @@ -118,8 +113,8 @@ void failsIfNeitherStepNorFileExempted() throws Exception { setFile("README.md").toContent("This code is fubar."); runWithFailure("> Task :spotlessMisc FAILED\n" + "Step 'no swearing' found problem in 'README.md':\n" + - "No swearing!\n" + - "java.lang.RuntimeException: No swearing!"); + "LINE_UNDEFINED: (swearing) No swearing!\n" + + "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); } private void runWithSuccess(String expectedToStartWith) throws Exception { diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java index 08a5f7279c..ff39cea174 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinExtensionTest.java @@ -20,11 +20,8 @@ import java.io.File; import java.io.IOException; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import com.diffplug.spotless.tag.ForLintRefactor; - class KotlinExtensionTest extends GradleIntegrationHarness { private static final String HEADER = "// License Header"; private static final String HEADER_WITH_YEAR = "// License Header $YEAR"; @@ -170,8 +167,6 @@ void testSetEditorConfigCanOverrideEditorConfigFile() throws IOException { } @Test - @Disabled - @ForLintRefactor void withCustomRuleSetApply() throws IOException { setFile("build.gradle.kts").toLines( "plugins {", diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java index 1e3d24da47..5bb7424ef9 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java @@ -20,11 +20,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.owasp.encoder.Encode.forXml; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.maven.MavenIntegrationHarness; -import com.diffplug.spotless.tag.ForLintRefactor; /** * Tests for the Biome formatter used via the Maven spotless plugin. @@ -225,8 +223,6 @@ void failureWhenExeNotFound() throws Exception { * @throws Exception When a test failure occurs. */ @Test - @Disabled - @ForLintRefactor void failureWhenNotParseable() throws Exception { writePomWithBiomeSteps("**/*.js", "1.2.0json"); setFile("biome_test.js").toResource("biome/js/fileBefore.js"); diff --git a/testlib/build.gradle b/testlib/build.gradle index 08e735bced..2d10006a60 100644 --- a/testlib/build.gradle +++ b/testlib/build.gradle @@ -13,6 +13,8 @@ dependencies { api "org.junit.jupiter:junit-jupiter:${VER_JUNIT}" api "org.assertj:assertj-core:${VER_ASSERTJ}" api "org.mockito:mockito-core:$VER_MOCKITO" + api "com.diffplug.selfie:selfie-lib:${VER_SELFIE}" + api "com.diffplug.selfie:selfie-runner-junit5:${VER_SELFIE}" runtimeOnly "org.junit.platform:junit-platform-launcher" implementation "com.diffplug.durian:durian-io:${VER_DURIAN}" diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index ae59a8d176..e45fd16a5b 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -21,8 +21,8 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; -import org.assertj.core.api.AbstractStringAssert; -import org.assertj.core.api.Assertions; +import com.diffplug.selfie.Selfie; +import com.diffplug.selfie.StringSelfie; /** An api for testing a {@code FormatterStep} that doesn't depend on the File path. DO NOT ADD FILE SUPPORT TO THIS, use {@link StepHarnessWithFile} if you need that. */ public class StepHarness extends StepHarnessBase { @@ -88,27 +88,26 @@ public StepHarness testResourceUnaffected(String resourceIdempotent) { return testUnaffected(idempotentElement); } - public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { - return testExceptionMsg(ResourceHarness.getTestResource(resourceBefore)); + public StringSelfie expectLintsOfResource(String before) { + return expectLintsOf(ResourceHarness.getTestResource(before)); } - public AbstractStringAssert testExceptionMsg(String before) { - try { - formatter().compute(LineEnding.toUnix(before), Formatter.NO_FILE_SENTINEL); - throw new SecurityException("Expected exception"); - } catch (Throwable e) { - if (e instanceof SecurityException) { - throw new AssertionError(e.getMessage()); - } else { - Throwable rootCause = e; - while (rootCause.getCause() != null) { - if (rootCause instanceof IllegalStateException) { - break; - } - rootCause = rootCause.getCause(); - } - return Assertions.assertThat(rootCause.getMessage()); - } + public StringSelfie expectLintsOf(String before) { + LintState state = LintState.of(formatter(), Formatter.NO_FILE_SENTINEL, before.getBytes(formatter().getEncoding())); + return expectLintsOf(state, formatter()); + } + + static StringSelfie expectLintsOf(LintState state, Formatter formatter) { + String assertAgainst = state.asString(Formatter.NO_FILE_SENTINEL, formatter); + String cleaned = assertAgainst.replace("NO_FILE_SENTINEL:", ""); + + int numLines = 1; + int lineEnding = cleaned.indexOf('\n'); + while (lineEnding != -1 && numLines < 10) { + ++numLines; + lineEnding = cleaned.indexOf('\n', lineEnding + 1); } + String toSnapshot = lineEnding == -1 ? cleaned : (cleaned.substring(0, lineEnding) + "\n(... and more)"); + return Selfie.expectSelfie(toSnapshot); } } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index bf537cf030..f833939bbe 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -18,12 +18,12 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Objects; -import org.assertj.core.api.AbstractStringAssert; -import org.assertj.core.api.Assertions; +import com.diffplug.selfie.StringSelfie; /** An api for testing a {@code FormatterStep} that depends on the File path. */ public class StepHarnessWithFile extends StepHarnessBase { @@ -85,30 +85,17 @@ public StepHarnessWithFile testResourceUnaffected(String resourceIdempotent) { return testUnaffected(file, contentBefore); } - public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { - return testResourceExceptionMsg(resourceBefore, resourceBefore); + public StringSelfie expectLintsOfResource(String resource) { + return expectLintsOfResource(resource, resource); } - public AbstractStringAssert testResourceExceptionMsg(String filename, String resourceBefore) { - String contentBefore = ResourceHarness.getTestResource(resourceBefore); - File file = harness.setFile(filename).toContent(contentBefore); - return testExceptionMsg(file, contentBefore); - } - - public AbstractStringAssert testExceptionMsg(File file, String before) { + public StringSelfie expectLintsOfResource(String filename, String resource) { try { - formatter().compute(LineEnding.toUnix(before), file); - throw new SecurityException("Expected exception"); - } catch (Throwable e) { - if (e instanceof SecurityException) { - throw new AssertionError(e.getMessage()); - } else { - Throwable rootCause = e; - while (rootCause.getCause() != null) { - rootCause = rootCause.getCause(); - } - return Assertions.assertThat(rootCause.getMessage()); - } + File file = harness.setFile(filename).toResource(resource); + LintState state = LintState.of(formatter(), file); + return StepHarness.expectLintsOf(state, formatter()); + } catch (IOException e) { + throw new AssertionError(e); } } } diff --git a/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java b/testlib/src/main/java/selfie/SelfieSettings.java similarity index 56% rename from testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java rename to testlib/src/main/java/selfie/SelfieSettings.java index a6ac9b090b..4a60c15dd0 100644 --- a/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java +++ b/testlib/src/main/java/selfie/SelfieSettings.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2024 DiffPlug + * 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. @@ -13,18 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.diffplug.spotless.tag; +package selfie; -import static java.lang.annotation.ElementType.METHOD; -import static java.lang.annotation.ElementType.TYPE; -import static java.lang.annotation.RetentionPolicy.RUNTIME; +import com.diffplug.selfie.junit5.SelfieSettingsAPI; -import java.lang.annotation.Retention; -import java.lang.annotation.Target; - -import org.junit.jupiter.api.Tag; - -@Target({TYPE, METHOD}) -@Retention(RUNTIME) -@Tag("black") -public @interface ForLintRefactor {} +/** https://selfie.dev/jvm/get-started#quickstart */ +public class SelfieSettings extends SelfieSettingsAPI { + @Override + public boolean getJavaDontUseTripleQuoteLiterals() { + return true; + } +} diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java index 697a199fd2..3d9d2ed0fc 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -83,13 +83,13 @@ void multiple() { @Test void broken() { FormatterStep fence = FenceStep.named("fence").openClose("spotless:off", "spotless:on") - .preserveWithin(Arrays.asList(ToCaseStep.upper())); + .preserveWithin(Arrays.asList(ReplaceStep.create("replace", "spotless:on", "REMOVED"))); // this fails because uppercase turns spotless:off into SPOTLESS:OFF, etc - StepHarness.forStepNoRoundtrip(fence).testExceptionMsg(StringPrinter.buildStringFromLines("A B C", + StepHarness.forStep(fence).expectLintsOf(StringPrinter.buildStringFromLines("A B C", "spotless:off", "D E F", "spotless:on", - "G H I")).isEqualTo("An intermediate step removed a match of spotless:off spotless:on"); + "G H I")).toBe("1-6 fence(fenceRemoved) An intermediate step removed a match of spotless:off spotless:on"); } @Test