From 9afa6dd722669820df7d5aca7a403279f2c55359 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 14:52:44 -0700 Subject: [PATCH 01/30] Handy tool for making encoding formats that don't have any forbidden characters. --- .../diffplug/spotless/ExceptionPerStep.java | 101 ++++++++ .../FilterByContentPatternFormatterStep.java | 18 +- .../spotless/FilterByFileFormatterStep.java | 14 +- .../java/com/diffplug/spotless/Formatter.java | 30 ++- .../com/diffplug/spotless/FormatterFunc.java | 26 ++ .../com/diffplug/spotless/FormatterStep.java | 17 ++ ...atterStepEqualityOnStateSerialization.java | 9 + .../main/java/com/diffplug/spotless/Lint.java | 242 ++++++++++++++++++ .../spotless/PerCharacterEscaper.java | 152 +++++++++++ .../com/diffplug/spotless/ThrowingEx.java | 13 +- .../java/com/diffplug/spotless/LintTest.java | 47 ++++ .../spotless/PerCharacterEscaperTest.java | 65 +++++ 12 files changed, 720 insertions(+), 14 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java create mode 100644 lib/src/main/java/com/diffplug/spotless/Lint.java create mode 100644 lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java create mode 100644 testlib/src/test/java/com/diffplug/spotless/LintTest.java create mode 100644 testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java diff --git a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java new file mode 100644 index 0000000000..7bf7fe2011 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java @@ -0,0 +1,101 @@ +/* + * 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 ExceptionPerStep extends AbstractList { + private final int size; + private @Nullable Throwable exception; + private int exceptionIdx; + private @Nullable Throwable[] multipleExceptions = null; + + ExceptionPerStep(Formatter formatter) { + this.size = formatter.getSteps().size(); + } + + @Override + public @Nullable Throwable set(int index, Throwable exception) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); + } + if (this.exception == null) { + this.exceptionIdx = index; + this.exception = exception; + return null; + } else if (this.multipleExceptions != null) { + Throwable previousValue = multipleExceptions[index]; + multipleExceptions[index] = exception; + return previousValue; + } else { + if (index == exceptionIdx) { + Throwable previousValue = this.exception; + this.exception = exception; + return previousValue; + } else { + multipleExceptions = new Throwable[size]; + multipleExceptions[exceptionIdx] = this.exception; + multipleExceptions[index] = exception; + return null; + } + } + } + + @Override + public Throwable get(int index) { + if (multipleExceptions != null) { + return multipleExceptions[index]; + } else if (exceptionIdx == index) { + return exception; + } else { + return null; + } + } + + private int indexOfFirstException() { + if (multipleExceptions != null) { + for (int i = 0; i < multipleExceptions.length; i++) { + if (multipleExceptions[i] != null) { + return i; + } + } + return -1; + } else if (exception != null) { + return exceptionIdx; + } else { + return -1; + } + } + + @Override + public int size() { + return size; + } + + /** Rethrows the first exception in the list. */ + public void rethrowFirstIfPresent() { + int firstException = indexOfFirstException(); + if (firstException != -1) { + throw ThrowingEx.asRuntimeRethrowError(get(firstException)); + } + } +} 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/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 1e2e44fe3a..5db20942f5 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -26,6 +26,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; +import java.util.ListIterator; import java.util.Objects; /** Formatter which performs the full formatting. */ @@ -127,12 +128,28 @@ public String computeLineEndings(String unix, File file) { * is guaranteed to also have unix line endings. */ public String compute(String unix, File file) { + ExceptionPerStep exceptionPerStep = new ExceptionPerStep(this); + String result = compute(unix, file, exceptionPerStep); + exceptionPerStep.rethrowFirstIfPresent(); + 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. + *

+ */ + String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); - for (FormatterStep step : steps) { + ListIterator iter = steps.listIterator(); + while (iter.hasNext()) { try { - String formatted = step.format(unix, file); + String formatted = iter.next().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. @@ -142,12 +159,9 @@ public String compute(String unix, File file) { 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 stop execution so we don't alter line numbers + exceptionPerStep.set(iter.previousIndex(), e); + return unix; } } 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..bc6d026330 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -0,0 +1,242 @@ +/* + * 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.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Objects; + +/** + * 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 { + /** 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. */ + public static class ShortcutException extends RuntimeException implements Has { + public ShortcutException(Lint... lints) { + this(Arrays.asList(lints)); + } + + private final List lints; + + public ShortcutException(Collection lints) { + this.lints = List.copyOf(lints); + } + + @Override + public List getLints() { + return lints; + } + } + + private static final long serialVersionUID = 1L; + + private int lineStart, lineEnd; // 1-indexed, inclusive + private String code; // 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 msg; + + private Lint(int lineStart, int lineEnd, String lintCode, String lintMsg) { + this.lineStart = lineStart; + this.lineEnd = lineEnd; + this.code = LineEnding.toUnix(lintCode); + this.msg = LineEnding.toUnix(lintMsg); + } + + public static Lint create(String code, String msg, int lineStart, int lineEnd) { + if (lineEnd < lineStart) { + throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd); + } + return new Lint(lineStart, lineEnd, code, msg); + } + + public static Lint create(String code, String msg, int line) { + return new Lint(line, line, code, msg); + } + + public int getLineStart() { + return lineStart; + } + + public int getLineEnd() { + return lineEnd; + } + + public String getCode() { + return code; + } + + public String getMsg() { + return msg; + } + + @Override + public String toString() { + if (lineStart == lineEnd) { + return lineStart + ": (" + code + ") " + msg; + } else { + return lineStart + "-" + lineEnd + ": (" + code + ") " + msg; + } + } + + @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(code, lint.code) && Objects.equals(msg, lint.msg); + } + + @Override + public int hashCode() { + return Objects.hash(lineStart, lineEnd, code, msg); + } + + /** Guaranteed to have no newlines, but also guarantees to preserve all newlines and parenthesis in code and msg. */ + String asOneLine() { + StringBuilder buffer = new StringBuilder(); + buffer.append(Integer.toString(lineStart)); + if (lineStart != lineEnd) { + buffer.append('-'); + buffer.append(Integer.toString(lineEnd)); + } + buffer.append(OPEN); + buffer.append(safeParensAndNewlines.escape(code)); + buffer.append(CLOSE); + buffer.append(safeParensAndNewlines.escape(msg)); + return buffer.toString(); + } + + private static final String OPEN = ": ("; + private static final String CLOSE = ") "; + + static Lint fromOneLine(String content) { + int codeOpen = content.indexOf(OPEN); + int codeClose = content.indexOf(CLOSE, codeOpen); + + int lineStart, lineEnd; + String lineNumber = content.substring(0, codeOpen); + int idxDash = lineNumber.indexOf('-'); + if (idxDash == -1) { + lineStart = Integer.parseInt(lineNumber); + lineEnd = lineStart; + } else { + lineStart = Integer.parseInt(lineNumber.substring(0, idxDash)); + lineEnd = Integer.parseInt(lineNumber.substring(idxDash + 1)); + } + + String code = safeParensAndNewlines.unescape(content.substring(codeOpen + OPEN.length(), codeClose)); + String msg = safeParensAndNewlines.unescape(content.substring(codeClose + CLOSE.length())); + return Lint.create(code, msg, lineStart, lineEnd); + } + + /** Call .escape to get a string which is guaranteed to have no parenthesis or newlines, and you can call unescape to get the original back. */ + static final PerCharacterEscaper safeParensAndNewlines = PerCharacterEscaper.specifiedEscape("\\\\\nn(₍)₎"); + + /** Converts a list of lints to a String, format is not guaranteed to be consistent from version to version of Spotless. */ + public static String toString(List lints) { + StringBuilder builder = new StringBuilder(); + for (Lint lint : lints) { + builder.append(lint.asOneLine()); + builder.append('\n'); + } + return builder.toString(); + } + + /** Converts a list of lints to a String, format is not guaranteed to be consistent from version to version of Spotless. */ + public static List fromString(String content) { + List lints = new ArrayList<>(); + String[] lines = content.split("\n"); + for (String line : lines) { + line = line.trim(); + if (!line.isEmpty()) { + lints.add(fromOneLine(line)); + } + } + return lints; + } + + public static List fromFile(File file) throws IOException { + byte[] content = Files.readAllBytes(file.toPath()); + return fromString(new String(content, StandardCharsets.UTF_8)); + } + + public static void toFile(List lints, File file) throws IOException { + Path path = file.toPath(); + Path parent = path.getParent(); + if (parent == null) { + throw new IllegalArgumentException("file has no parent dir"); + } + Files.createDirectories(parent); + byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); + Files.write(path, content); + } + + /** Attempts to parse a line number from the given exception. */ + static Lint createFromThrowable(FormatterStep step, String content, Throwable e) { + Throwable current = e; + while (current != null) { + String message = current.getMessage(); + int lineNumber = lineNumberFor(message); + if (lineNumber != -1) { + return Lint.create(step.getName(), msgFrom(message), lineNumber); + } + current = current.getCause(); + } + int numNewlines = (int) content.codePoints().filter(c -> c == '\n').count(); + return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1, 1 + numNewlines); + } + + 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 ""; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java b/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java new file mode 100644 index 0000000000..3138e3e781 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java @@ -0,0 +1,152 @@ +/* + * 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; + +class PerCharacterEscaper { + /** + * If your escape policy is "'a1b2c3d", it means this: + * + * ``` + * abc->abc + * 123->'b'c'd + * I won't->I won'at + * ``` + */ + public static PerCharacterEscaper specifiedEscape(String escapePolicy) { + int[] codePoints = escapePolicy.codePoints().toArray(); + if (codePoints.length % 2 != 0) { + throw new IllegalArgumentException(); + } + int escapeCodePoint = codePoints[0]; + int[] escapedCodePoints = new int[codePoints.length / 2]; + int[] escapedByCodePoints = new int[codePoints.length / 2]; + for (int i = 0; i < escapedCodePoints.length; ++i) { + escapedCodePoints[i] = codePoints[2 * i]; + escapedByCodePoints[i] = codePoints[2 * i + 1]; + } + return new PerCharacterEscaper(escapeCodePoint, escapedCodePoints, escapedByCodePoints); + } + + private final int escapeCodePoint; + private final int[] escapedCodePoints; + private final int[] escapedByCodePoints; + + /** The first character in the string will be uses as the escape character, and all characters will be escaped. */ + private PerCharacterEscaper(int escapeCodePoint, int[] escapedCodePoints, int[] escapedByCodePoints) { + this.escapeCodePoint = escapeCodePoint; + this.escapedCodePoints = escapedCodePoints; + this.escapedByCodePoints = escapedByCodePoints; + } + + public boolean needsEscaping(String input) { + return firstOffsetNeedingEscape(input) != -1; + } + + private int firstOffsetNeedingEscape(String input) { + final int length = input.length(); + int firstOffsetNeedingEscape = -1; + outer: for (int offset = 0; offset < length;) { + int codepoint = input.codePointAt(offset); + for (int escaped : escapedCodePoints) { + if (codepoint == escaped) { + firstOffsetNeedingEscape = offset; + break outer; + } + } + offset += Character.charCount(codepoint); + } + return firstOffsetNeedingEscape; + } + + public String escape(String input) { + final int noEscapes = firstOffsetNeedingEscape(input); + if (noEscapes == -1) { + return input; + } else { + final int length = input.length(); + final int needsEscapes = length - noEscapes; + StringBuilder builder = new StringBuilder(noEscapes + 4 + (needsEscapes * 5 / 4)); + builder.append(input, 0, noEscapes); + for (int offset = noEscapes; offset < length;) { + final int codepoint = input.codePointAt(offset); + offset += Character.charCount(codepoint); + int idx = indexOf(escapedCodePoints, codepoint); + if (idx == -1) { + builder.appendCodePoint(codepoint); + } else { + builder.appendCodePoint(escapeCodePoint); + builder.appendCodePoint(escapedByCodePoints[idx]); + } + } + return builder.toString(); + } + } + + private int firstOffsetNeedingUnescape(String input) { + final int length = input.length(); + int firstOffsetNeedingEscape = -1; + for (int offset = 0; offset < length;) { + int codepoint = input.codePointAt(offset); + if (codepoint == escapeCodePoint) { + firstOffsetNeedingEscape = offset; + break; + } + offset += Character.charCount(codepoint); + } + return firstOffsetNeedingEscape; + } + + public String unescape(String input) { + final int noEscapes = firstOffsetNeedingUnescape(input); + if (noEscapes == -1) { + return input; + } else { + final int length = input.length(); + final int needsEscapes = length - noEscapes; + StringBuilder builder = new StringBuilder(noEscapes + 4 + (needsEscapes * 5 / 4)); + builder.append(input, 0, noEscapes); + for (int offset = noEscapes; offset < length;) { + int codepoint = input.codePointAt(offset); + offset += Character.charCount(codepoint); + // if we need to escape something, escape it + if (codepoint == escapeCodePoint) { + if (offset < length) { + codepoint = input.codePointAt(offset); + int idx = indexOf(escapedByCodePoints, codepoint); + if (idx != -1) { + codepoint = escapedCodePoints[idx]; + } + offset += Character.charCount(codepoint); + } else { + throw new IllegalArgumentException("Escape character '" + new String(new int[]{escapeCodePoint}, 0, 1) + "' can't be the last character in a string."); + } + } + // we didn't escape it, append it raw + builder.appendCodePoint(codepoint); + } + return builder.toString(); + } + } + + private static int indexOf(int[] array, int value) { + for (int i = 0; i < array.length; ++i) { + if (array[i] == value) { + return i; + } + } + return -1; + } +} 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/testlib/src/test/java/com/diffplug/spotless/LintTest.java b/testlib/src/test/java/com/diffplug/spotless/LintTest.java new file mode 100644 index 0000000000..d8fcfba873 --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/LintTest.java @@ -0,0 +1,47 @@ +/* + * 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 org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class LintTest { + @Test + public void examples() { + roundtrip(Lint.create("code", "msg", 5)); + roundtrip(Lint.create("code", "msg", 5, 7)); + roundtrip(Lint.create("(code)", "msg\nwith\nnewlines", 5, 7)); + } + + private void roundtrip(Lint lint) { + Lint roundTripped = Lint.fromOneLine(lint.asOneLine()); + Assertions.assertEquals(lint.asOneLine(), roundTripped.asOneLine()); + } + + @Test + public void perCharacterEscaper() { + roundtrip("abcn123", "abcn123"); + roundtrip("abc\\123", "abc\\\\123"); + roundtrip("abc(123)", "abc\\₍123\\₎"); + roundtrip("abc\n123", "abc\\n123"); + roundtrip("abc\nn123", "abc\\nn123"); + } + + private void roundtrip(String unescaped, String escaped) { + Assertions.assertEquals(escaped, Lint.safeParensAndNewlines.escape(unescaped)); + Assertions.assertEquals(unescaped, Lint.safeParensAndNewlines.unescape(escaped)); + } +} diff --git a/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java b/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java new file mode 100644 index 0000000000..aa450ee43a --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java @@ -0,0 +1,65 @@ +/* + * 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 org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class PerCharacterEscaperTest { + @Test + public void examples() { + roundtrip("abcn123", "abcn123"); + roundtrip("abc/123", "abc//123"); + roundtrip("abc(123)", "abc/₍123/₎"); + roundtrip("abc\n123", "abc/n123"); + roundtrip("abc\nn123", "abc/nn123"); + } + + private void roundtrip(String unescaped, String escaped) { + Assertions.assertEquals(escaped, Lint.safeParensAndNewlines.escape(unescaped)); + Assertions.assertEquals(unescaped, Lint.safeParensAndNewlines.unescape(escaped)); + } + + @Test + public void performanceOptimizationSpecific() { + PerCharacterEscaper escaper = PerCharacterEscaper.specifiedEscape("`a1b2c3d"); + // if nothing gets changed, it should return the exact same value + String abc = "abc"; + Assertions.assertSame(abc, escaper.escape(abc)); + Assertions.assertSame(abc, escaper.unescape(abc)); + + // otherwise it should have the normal behavior + Assertions.assertEquals("`b", escaper.escape("1")); + Assertions.assertEquals("`a", escaper.escape("`")); + Assertions.assertEquals("abc`b`c`d`adef", escaper.escape("abc123`def")); + + // in both directions + Assertions.assertEquals("1", escaper.unescape("`b")); + Assertions.assertEquals("`", escaper.unescape("`a")); + Assertions.assertEquals("abc123`def", escaper.unescape("abc`1`2`3``def")); + } + + @Test + public void cornerCasesSpecific() { + PerCharacterEscaper escaper = PerCharacterEscaper.specifiedEscape("`a1b2c3d"); + // cornercase - escape character without follow-on will throw an error + org.assertj.core.api.Assertions.assertThatThrownBy(() -> escaper.unescape("`")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Escape character '`' can't be the last character in a string."); + // escape character followed by non-escape character is fine + Assertions.assertEquals("e", escaper.unescape("`e")); + } +} From 319fc79e6ef26a1793225b99955eb1e221ff8b6f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 15:28:15 -0700 Subject: [PATCH 02/30] Revert "Handy tool for making encoding formats that don't have any forbidden characters." This reverts commit 9afa6dd722669820df7d5aca7a403279f2c55359. --- .../diffplug/spotless/ExceptionPerStep.java | 101 -------- .../FilterByContentPatternFormatterStep.java | 18 +- .../spotless/FilterByFileFormatterStep.java | 14 +- .../java/com/diffplug/spotless/Formatter.java | 30 +-- .../com/diffplug/spotless/FormatterFunc.java | 26 -- .../com/diffplug/spotless/FormatterStep.java | 17 -- ...atterStepEqualityOnStateSerialization.java | 9 - .../main/java/com/diffplug/spotless/Lint.java | 242 ------------------ .../spotless/PerCharacterEscaper.java | 152 ----------- .../com/diffplug/spotless/ThrowingEx.java | 13 +- .../java/com/diffplug/spotless/LintTest.java | 47 ---- .../spotless/PerCharacterEscaperTest.java | 65 ----- 12 files changed, 14 insertions(+), 720 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java delete mode 100644 lib/src/main/java/com/diffplug/spotless/Lint.java delete mode 100644 lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java delete mode 100644 testlib/src/test/java/com/diffplug/spotless/LintTest.java delete mode 100644 testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java diff --git a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java deleted file mode 100644 index 7bf7fe2011..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java +++ /dev/null @@ -1,101 +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.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 ExceptionPerStep extends AbstractList { - private final int size; - private @Nullable Throwable exception; - private int exceptionIdx; - private @Nullable Throwable[] multipleExceptions = null; - - ExceptionPerStep(Formatter formatter) { - this.size = formatter.getSteps().size(); - } - - @Override - public @Nullable Throwable set(int index, Throwable exception) { - if (index < 0 || index >= size) { - throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); - } - if (this.exception == null) { - this.exceptionIdx = index; - this.exception = exception; - return null; - } else if (this.multipleExceptions != null) { - Throwable previousValue = multipleExceptions[index]; - multipleExceptions[index] = exception; - return previousValue; - } else { - if (index == exceptionIdx) { - Throwable previousValue = this.exception; - this.exception = exception; - return previousValue; - } else { - multipleExceptions = new Throwable[size]; - multipleExceptions[exceptionIdx] = this.exception; - multipleExceptions[index] = exception; - return null; - } - } - } - - @Override - public Throwable get(int index) { - if (multipleExceptions != null) { - return multipleExceptions[index]; - } else if (exceptionIdx == index) { - return exception; - } else { - return null; - } - } - - private int indexOfFirstException() { - if (multipleExceptions != null) { - for (int i = 0; i < multipleExceptions.length; i++) { - if (multipleExceptions[i] != null) { - return i; - } - } - return -1; - } else if (exception != null) { - return exceptionIdx; - } else { - return -1; - } - } - - @Override - public int size() { - return size; - } - - /** Rethrows the first exception in the list. */ - public void rethrowFirstIfPresent() { - int firstException = indexOfFirstException(); - if (firstException != -1) { - throw ThrowingEx.asRuntimeRethrowError(get(firstException)); - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java index bcc444ad57..4cc336e101 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-2024 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -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,24 +36,14 @@ final class FilterByContentPatternFormatterStep extends DelegateFormatterStep { public @Nullable String format(String raw, File file) throws Exception { Objects.requireNonNull(raw, "raw"); Objects.requireNonNull(file, "file"); - if (contentPattern.matcher(raw).find() == (onMatch == OnMatch.INCLUDE)) { + Matcher matcher = contentPattern.matcher(raw); + if (matcher.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 bc5ddf6053..04a06a4673 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-2024 DiffPlug + * Copyright 2016-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,6 @@ package com.diffplug.spotless; import java.io.File; -import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -40,17 +39,6 @@ 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/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 5db20942f5..1e2e44fe3a 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -26,7 +26,6 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; -import java.util.ListIterator; import java.util.Objects; /** Formatter which performs the full formatting. */ @@ -128,28 +127,12 @@ public String computeLineEndings(String unix, File file) { * is guaranteed to also have unix line endings. */ public String compute(String unix, File file) { - ExceptionPerStep exceptionPerStep = new ExceptionPerStep(this); - String result = compute(unix, file, exceptionPerStep); - exceptionPerStep.rethrowFirstIfPresent(); - 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. - *

- */ - String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); - ListIterator iter = steps.listIterator(); - while (iter.hasNext()) { + for (FormatterStep step : steps) { try { - String formatted = iter.next().format(unix, file); + 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. @@ -159,9 +142,12 @@ String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - // store the exception which was thrown, and stop execution so we don't alter line numbers - exceptionPerStep.set(iter.previousIndex(), e); - return unix; + // 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); + } } } 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 5e6c44b335..800a553225 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -16,7 +16,6 @@ package com.diffplug.spotless; import java.io.File; -import java.util.List; import java.util.Objects; /** @@ -33,14 +32,6 @@ 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. @@ -83,14 +74,6 @@ 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. */ @@ -118,10 +101,6 @@ 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. */ @@ -144,11 +123,6 @@ 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 870ef0ee18..2a5a7d2b2f 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -17,7 +17,6 @@ import java.io.File; import java.io.Serializable; -import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -47,22 +46,6 @@ 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 e42e0cb4f9..52bf9fc760 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepEqualityOnStateSerialization.java @@ -18,7 +18,6 @@ import java.io.File; import java.io.Serializable; import java.util.Arrays; -import java.util.List; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -49,14 +48,6 @@ 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 deleted file mode 100644 index bc6d026330..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ /dev/null @@ -1,242 +0,0 @@ -/* - * 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.File; -import java.io.IOException; -import java.io.Serializable; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Objects; - -/** - * 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 { - /** 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. */ - public static class ShortcutException extends RuntimeException implements Has { - public ShortcutException(Lint... lints) { - this(Arrays.asList(lints)); - } - - private final List lints; - - public ShortcutException(Collection lints) { - this.lints = List.copyOf(lints); - } - - @Override - public List getLints() { - return lints; - } - } - - private static final long serialVersionUID = 1L; - - private int lineStart, lineEnd; // 1-indexed, inclusive - private String code; // 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 msg; - - private Lint(int lineStart, int lineEnd, String lintCode, String lintMsg) { - this.lineStart = lineStart; - this.lineEnd = lineEnd; - this.code = LineEnding.toUnix(lintCode); - this.msg = LineEnding.toUnix(lintMsg); - } - - public static Lint create(String code, String msg, int lineStart, int lineEnd) { - if (lineEnd < lineStart) { - throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd); - } - return new Lint(lineStart, lineEnd, code, msg); - } - - public static Lint create(String code, String msg, int line) { - return new Lint(line, line, code, msg); - } - - public int getLineStart() { - return lineStart; - } - - public int getLineEnd() { - return lineEnd; - } - - public String getCode() { - return code; - } - - public String getMsg() { - return msg; - } - - @Override - public String toString() { - if (lineStart == lineEnd) { - return lineStart + ": (" + code + ") " + msg; - } else { - return lineStart + "-" + lineEnd + ": (" + code + ") " + msg; - } - } - - @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(code, lint.code) && Objects.equals(msg, lint.msg); - } - - @Override - public int hashCode() { - return Objects.hash(lineStart, lineEnd, code, msg); - } - - /** Guaranteed to have no newlines, but also guarantees to preserve all newlines and parenthesis in code and msg. */ - String asOneLine() { - StringBuilder buffer = new StringBuilder(); - buffer.append(Integer.toString(lineStart)); - if (lineStart != lineEnd) { - buffer.append('-'); - buffer.append(Integer.toString(lineEnd)); - } - buffer.append(OPEN); - buffer.append(safeParensAndNewlines.escape(code)); - buffer.append(CLOSE); - buffer.append(safeParensAndNewlines.escape(msg)); - return buffer.toString(); - } - - private static final String OPEN = ": ("; - private static final String CLOSE = ") "; - - static Lint fromOneLine(String content) { - int codeOpen = content.indexOf(OPEN); - int codeClose = content.indexOf(CLOSE, codeOpen); - - int lineStart, lineEnd; - String lineNumber = content.substring(0, codeOpen); - int idxDash = lineNumber.indexOf('-'); - if (idxDash == -1) { - lineStart = Integer.parseInt(lineNumber); - lineEnd = lineStart; - } else { - lineStart = Integer.parseInt(lineNumber.substring(0, idxDash)); - lineEnd = Integer.parseInt(lineNumber.substring(idxDash + 1)); - } - - String code = safeParensAndNewlines.unescape(content.substring(codeOpen + OPEN.length(), codeClose)); - String msg = safeParensAndNewlines.unescape(content.substring(codeClose + CLOSE.length())); - return Lint.create(code, msg, lineStart, lineEnd); - } - - /** Call .escape to get a string which is guaranteed to have no parenthesis or newlines, and you can call unescape to get the original back. */ - static final PerCharacterEscaper safeParensAndNewlines = PerCharacterEscaper.specifiedEscape("\\\\\nn(₍)₎"); - - /** Converts a list of lints to a String, format is not guaranteed to be consistent from version to version of Spotless. */ - public static String toString(List lints) { - StringBuilder builder = new StringBuilder(); - for (Lint lint : lints) { - builder.append(lint.asOneLine()); - builder.append('\n'); - } - return builder.toString(); - } - - /** Converts a list of lints to a String, format is not guaranteed to be consistent from version to version of Spotless. */ - public static List fromString(String content) { - List lints = new ArrayList<>(); - String[] lines = content.split("\n"); - for (String line : lines) { - line = line.trim(); - if (!line.isEmpty()) { - lints.add(fromOneLine(line)); - } - } - return lints; - } - - public static List fromFile(File file) throws IOException { - byte[] content = Files.readAllBytes(file.toPath()); - return fromString(new String(content, StandardCharsets.UTF_8)); - } - - public static void toFile(List lints, File file) throws IOException { - Path path = file.toPath(); - Path parent = path.getParent(); - if (parent == null) { - throw new IllegalArgumentException("file has no parent dir"); - } - Files.createDirectories(parent); - byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); - Files.write(path, content); - } - - /** Attempts to parse a line number from the given exception. */ - static Lint createFromThrowable(FormatterStep step, String content, Throwable e) { - Throwable current = e; - while (current != null) { - String message = current.getMessage(); - int lineNumber = lineNumberFor(message); - if (lineNumber != -1) { - return Lint.create(step.getName(), msgFrom(message), lineNumber); - } - current = current.getCause(); - } - int numNewlines = (int) content.codePoints().filter(c -> c == '\n').count(); - return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1, 1 + numNewlines); - } - - 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 ""; - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java b/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java deleted file mode 100644 index 3138e3e781..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java +++ /dev/null @@ -1,152 +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; - -class PerCharacterEscaper { - /** - * If your escape policy is "'a1b2c3d", it means this: - * - * ``` - * abc->abc - * 123->'b'c'd - * I won't->I won'at - * ``` - */ - public static PerCharacterEscaper specifiedEscape(String escapePolicy) { - int[] codePoints = escapePolicy.codePoints().toArray(); - if (codePoints.length % 2 != 0) { - throw new IllegalArgumentException(); - } - int escapeCodePoint = codePoints[0]; - int[] escapedCodePoints = new int[codePoints.length / 2]; - int[] escapedByCodePoints = new int[codePoints.length / 2]; - for (int i = 0; i < escapedCodePoints.length; ++i) { - escapedCodePoints[i] = codePoints[2 * i]; - escapedByCodePoints[i] = codePoints[2 * i + 1]; - } - return new PerCharacterEscaper(escapeCodePoint, escapedCodePoints, escapedByCodePoints); - } - - private final int escapeCodePoint; - private final int[] escapedCodePoints; - private final int[] escapedByCodePoints; - - /** The first character in the string will be uses as the escape character, and all characters will be escaped. */ - private PerCharacterEscaper(int escapeCodePoint, int[] escapedCodePoints, int[] escapedByCodePoints) { - this.escapeCodePoint = escapeCodePoint; - this.escapedCodePoints = escapedCodePoints; - this.escapedByCodePoints = escapedByCodePoints; - } - - public boolean needsEscaping(String input) { - return firstOffsetNeedingEscape(input) != -1; - } - - private int firstOffsetNeedingEscape(String input) { - final int length = input.length(); - int firstOffsetNeedingEscape = -1; - outer: for (int offset = 0; offset < length;) { - int codepoint = input.codePointAt(offset); - for (int escaped : escapedCodePoints) { - if (codepoint == escaped) { - firstOffsetNeedingEscape = offset; - break outer; - } - } - offset += Character.charCount(codepoint); - } - return firstOffsetNeedingEscape; - } - - public String escape(String input) { - final int noEscapes = firstOffsetNeedingEscape(input); - if (noEscapes == -1) { - return input; - } else { - final int length = input.length(); - final int needsEscapes = length - noEscapes; - StringBuilder builder = new StringBuilder(noEscapes + 4 + (needsEscapes * 5 / 4)); - builder.append(input, 0, noEscapes); - for (int offset = noEscapes; offset < length;) { - final int codepoint = input.codePointAt(offset); - offset += Character.charCount(codepoint); - int idx = indexOf(escapedCodePoints, codepoint); - if (idx == -1) { - builder.appendCodePoint(codepoint); - } else { - builder.appendCodePoint(escapeCodePoint); - builder.appendCodePoint(escapedByCodePoints[idx]); - } - } - return builder.toString(); - } - } - - private int firstOffsetNeedingUnescape(String input) { - final int length = input.length(); - int firstOffsetNeedingEscape = -1; - for (int offset = 0; offset < length;) { - int codepoint = input.codePointAt(offset); - if (codepoint == escapeCodePoint) { - firstOffsetNeedingEscape = offset; - break; - } - offset += Character.charCount(codepoint); - } - return firstOffsetNeedingEscape; - } - - public String unescape(String input) { - final int noEscapes = firstOffsetNeedingUnescape(input); - if (noEscapes == -1) { - return input; - } else { - final int length = input.length(); - final int needsEscapes = length - noEscapes; - StringBuilder builder = new StringBuilder(noEscapes + 4 + (needsEscapes * 5 / 4)); - builder.append(input, 0, noEscapes); - for (int offset = noEscapes; offset < length;) { - int codepoint = input.codePointAt(offset); - offset += Character.charCount(codepoint); - // if we need to escape something, escape it - if (codepoint == escapeCodePoint) { - if (offset < length) { - codepoint = input.codePointAt(offset); - int idx = indexOf(escapedByCodePoints, codepoint); - if (idx != -1) { - codepoint = escapedCodePoints[idx]; - } - offset += Character.charCount(codepoint); - } else { - throw new IllegalArgumentException("Escape character '" + new String(new int[]{escapeCodePoint}, 0, 1) + "' can't be the last character in a string."); - } - } - // we didn't escape it, append it raw - builder.appendCodePoint(codepoint); - } - return builder.toString(); - } - } - - private static int indexOf(int[] array, int value) { - for (int i = 0; i < array.length; ++i) { - if (array[i] == value) { - return i; - } - } - return -1; - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java b/lib/src/main/java/com/diffplug/spotless/ThrowingEx.java index 7e017e0989..6eb573b5d8 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-2024 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,9 +15,6 @@ */ 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. @@ -145,12 +142,4 @@ 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/testlib/src/test/java/com/diffplug/spotless/LintTest.java b/testlib/src/test/java/com/diffplug/spotless/LintTest.java deleted file mode 100644 index d8fcfba873..0000000000 --- a/testlib/src/test/java/com/diffplug/spotless/LintTest.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * 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 org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -public class LintTest { - @Test - public void examples() { - roundtrip(Lint.create("code", "msg", 5)); - roundtrip(Lint.create("code", "msg", 5, 7)); - roundtrip(Lint.create("(code)", "msg\nwith\nnewlines", 5, 7)); - } - - private void roundtrip(Lint lint) { - Lint roundTripped = Lint.fromOneLine(lint.asOneLine()); - Assertions.assertEquals(lint.asOneLine(), roundTripped.asOneLine()); - } - - @Test - public void perCharacterEscaper() { - roundtrip("abcn123", "abcn123"); - roundtrip("abc\\123", "abc\\\\123"); - roundtrip("abc(123)", "abc\\₍123\\₎"); - roundtrip("abc\n123", "abc\\n123"); - roundtrip("abc\nn123", "abc\\nn123"); - } - - private void roundtrip(String unescaped, String escaped) { - Assertions.assertEquals(escaped, Lint.safeParensAndNewlines.escape(unescaped)); - Assertions.assertEquals(unescaped, Lint.safeParensAndNewlines.unescape(escaped)); - } -} diff --git a/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java b/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java deleted file mode 100644 index aa450ee43a..0000000000 --- a/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java +++ /dev/null @@ -1,65 +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 org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -public class PerCharacterEscaperTest { - @Test - public void examples() { - roundtrip("abcn123", "abcn123"); - roundtrip("abc/123", "abc//123"); - roundtrip("abc(123)", "abc/₍123/₎"); - roundtrip("abc\n123", "abc/n123"); - roundtrip("abc\nn123", "abc/nn123"); - } - - private void roundtrip(String unescaped, String escaped) { - Assertions.assertEquals(escaped, Lint.safeParensAndNewlines.escape(unescaped)); - Assertions.assertEquals(unescaped, Lint.safeParensAndNewlines.unescape(escaped)); - } - - @Test - public void performanceOptimizationSpecific() { - PerCharacterEscaper escaper = PerCharacterEscaper.specifiedEscape("`a1b2c3d"); - // if nothing gets changed, it should return the exact same value - String abc = "abc"; - Assertions.assertSame(abc, escaper.escape(abc)); - Assertions.assertSame(abc, escaper.unescape(abc)); - - // otherwise it should have the normal behavior - Assertions.assertEquals("`b", escaper.escape("1")); - Assertions.assertEquals("`a", escaper.escape("`")); - Assertions.assertEquals("abc`b`c`d`adef", escaper.escape("abc123`def")); - - // in both directions - Assertions.assertEquals("1", escaper.unescape("`b")); - Assertions.assertEquals("`", escaper.unescape("`a")); - Assertions.assertEquals("abc123`def", escaper.unescape("abc`1`2`3``def")); - } - - @Test - public void cornerCasesSpecific() { - PerCharacterEscaper escaper = PerCharacterEscaper.specifiedEscape("`a1b2c3d"); - // cornercase - escape character without follow-on will throw an error - org.assertj.core.api.Assertions.assertThatThrownBy(() -> escaper.unescape("`")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Escape character '`' can't be the last character in a string."); - // escape character followed by non-escape character is fine - Assertions.assertEquals("e", escaper.unescape("`e")); - } -} From 777fd5cfdab4904919806cc87637f70d1eb70fa3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 15:53:40 -0700 Subject: [PATCH 03/30] Add a `Lint` class, along with `ShortcutException` for sending them. --- .../main/java/com/diffplug/spotless/Lint.java | 156 ++++++++++++++++++ .../com/diffplug/spotless/ThrowingEx.java | 13 +- 2 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/Lint.java 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..81fcefc964 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -0,0 +1,156 @@ +/* + * 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; + +/** + * 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 { + /** 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. */ + public static class ShortcutException extends RuntimeException implements Has { + public ShortcutException(Lint... lints) { + this(Arrays.asList(lints)); + } + + private final List lints; + + public ShortcutException(Collection lints) { + this.lints = List.copyOf(lints); + } + + @Override + public List getLints() { + return lints; + } + } + + private static final long serialVersionUID = 1L; + + private int lineStart, lineEnd; // 1-indexed, inclusive + private String code; // 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 msg; + + private Lint(int lineStart, int lineEnd, String lintCode, String lintMsg) { + this.lineStart = lineStart; + this.lineEnd = lineEnd; + this.code = LineEnding.toUnix(lintCode); + this.msg = LineEnding.toUnix(lintMsg); + } + + public static Lint create(String code, String msg, int lineStart, int lineEnd) { + if (lineEnd < lineStart) { + throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd); + } + return new Lint(lineStart, lineEnd, code, msg); + } + + public static Lint create(String code, String msg, int line) { + return new Lint(line, line, code, msg); + } + + public int getLineStart() { + return lineStart; + } + + public int getLineEnd() { + return lineEnd; + } + + public String getCode() { + return code; + } + + public String getMsg() { + return msg; + } + + @Override + public String toString() { + if (lineStart == lineEnd) { + return lineStart + ": (" + code + ") " + msg; + } else { + return lineStart + "-" + lineEnd + ": (" + code + ") " + msg; + } + } + + @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(code, lint.code) && Objects.equals(msg, lint.msg); + } + + @Override + public int hashCode() { + return Objects.hash(lineStart, lineEnd, code, msg); + } + + /** Attempts to parse a line number from the given exception. */ + static Lint createFromThrowable(FormatterStep step, String content, Throwable e) { + Throwable current = e; + while (current != null) { + String message = current.getMessage(); + int lineNumber = lineNumberFor(message); + if (lineNumber != -1) { + return Lint.create(step.getName(), msgFrom(message), lineNumber); + } + current = current.getCause(); + } + int numNewlines = (int) content.codePoints().filter(c -> c == '\n').count(); + return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1, 1 + numNewlines); + } + + 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 ""; + } +} 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(); + } } From 322d5bf76c24f2807acc82fcb026838fc3d190e0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 15:54:26 -0700 Subject: [PATCH 04/30] Add a `lint` method to the core interfaces: `Formatter[Step|Func]` --- .../com/diffplug/spotless/FormatterFunc.java | 26 +++++++++++++++++++ .../com/diffplug/spotless/FormatterStep.java | 17 ++++++++++++ 2 files changed, 43 insertions(+) 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. From c9f52d5f7e4eed7c38fd01683fd7dbbc6ae79f83 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 15:54:57 -0700 Subject: [PATCH 05/30] Pipe linting through the core FormatterStep implementations. --- .../FilterByContentPatternFormatterStep.java | 18 ++++++++++++++---- .../spotless/FilterByFileFormatterStep.java | 14 +++++++++++++- ...matterStepEqualityOnStateSerialization.java | 9 +++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) 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/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) { From b57bf24a1f0dc4501a9d91885884eda5229a4102 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 15:56:40 -0700 Subject: [PATCH 06/30] Formatter can now capture exceptions per-formatter, rethrows if you don't call it in the "special" way. --- .../diffplug/spotless/ExceptionPerStep.java | 101 ++++++++++++++++++ .../java/com/diffplug/spotless/Formatter.java | 30 ++++-- 2 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java diff --git a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java new file mode 100644 index 0000000000..7bf7fe2011 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java @@ -0,0 +1,101 @@ +/* + * 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 ExceptionPerStep extends AbstractList { + private final int size; + private @Nullable Throwable exception; + private int exceptionIdx; + private @Nullable Throwable[] multipleExceptions = null; + + ExceptionPerStep(Formatter formatter) { + this.size = formatter.getSteps().size(); + } + + @Override + public @Nullable Throwable set(int index, Throwable exception) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); + } + if (this.exception == null) { + this.exceptionIdx = index; + this.exception = exception; + return null; + } else if (this.multipleExceptions != null) { + Throwable previousValue = multipleExceptions[index]; + multipleExceptions[index] = exception; + return previousValue; + } else { + if (index == exceptionIdx) { + Throwable previousValue = this.exception; + this.exception = exception; + return previousValue; + } else { + multipleExceptions = new Throwable[size]; + multipleExceptions[exceptionIdx] = this.exception; + multipleExceptions[index] = exception; + return null; + } + } + } + + @Override + public Throwable get(int index) { + if (multipleExceptions != null) { + return multipleExceptions[index]; + } else if (exceptionIdx == index) { + return exception; + } else { + return null; + } + } + + private int indexOfFirstException() { + if (multipleExceptions != null) { + for (int i = 0; i < multipleExceptions.length; i++) { + if (multipleExceptions[i] != null) { + return i; + } + } + return -1; + } else if (exception != null) { + return exceptionIdx; + } else { + return -1; + } + } + + @Override + public int size() { + return size; + } + + /** Rethrows the first exception in the list. */ + public void rethrowFirstIfPresent() { + int firstException = indexOfFirstException(); + if (firstException != -1) { + throw ThrowingEx.asRuntimeRethrowError(get(firstException)); + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 1e2e44fe3a..5db20942f5 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -26,6 +26,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; +import java.util.ListIterator; import java.util.Objects; /** Formatter which performs the full formatting. */ @@ -127,12 +128,28 @@ public String computeLineEndings(String unix, File file) { * is guaranteed to also have unix line endings. */ public String compute(String unix, File file) { + ExceptionPerStep exceptionPerStep = new ExceptionPerStep(this); + String result = compute(unix, file, exceptionPerStep); + exceptionPerStep.rethrowFirstIfPresent(); + 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. + *

+ */ + String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); - for (FormatterStep step : steps) { + ListIterator iter = steps.listIterator(); + while (iter.hasNext()) { try { - String formatted = step.format(unix, file); + String formatted = iter.next().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. @@ -142,12 +159,9 @@ public String compute(String unix, File file) { 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 stop execution so we don't alter line numbers + exceptionPerStep.set(iter.previousIndex(), e); + return unix; } } return unix; From a94dce9f9b0e62d832f3d033603844a8884fa906 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 16:01:47 -0700 Subject: [PATCH 07/30] Pipe the lints through `FenceStep`, preliminary. --- .../diffplug/spotless/generic/FenceStep.java | 30 ++++++++++++------- .../spotless/generic/FenceStepTest.java | 4 +++ 2 files changed, 23 insertions(+), 11 deletions(-) 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/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java index 697a199fd2..ea9755a804 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -18,12 +18,14 @@ import java.io.File; import java.util.Arrays; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarness; +import com.diffplug.spotless.tag.ForLintRefactor; class FenceStepTest extends ResourceHarness { @Test @@ -80,6 +82,8 @@ void multiple() { "1 2 3")); } + @Disabled + @ForLintRefactor @Test void broken() { FormatterStep fence = FenceStep.named("fence").openClose("spotless:off", "spotless:on") From c2249299c6fad4753a3113ff2c30a144150abfbc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 16:04:10 -0700 Subject: [PATCH 08/30] Remove all of `FormatExceptionPolicy` except `Strict`, which remains for easy API conversion. --- .../spotless/FormatExceptionPolicy.java | 39 ------------------- .../spotless/FormatExceptionPolicyStrict.java | 11 +++--- ...ptionPolicyLegacy.java => LintPolicy.java} | 16 +------- .../gradle/spotless/SpotlessTask.java | 7 ++-- 4 files changed, 10 insertions(+), 63 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java rename lib/src/main/java/com/diffplug/spotless/{FormatExceptionPolicyLegacy.java => LintPolicy.java} (71%) 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/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java similarity index 71% rename from lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java rename to lib/src/main/java/com/diffplug/spotless/LintPolicy.java index 93ca5d05b6..d4477bd822 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. @@ -18,21 +18,9 @@ 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); } 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 19d81eb947..881e266eed 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 @@ -37,7 +37,6 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; -import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; @@ -115,14 +114,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; } From b01e9e902031b86e72272dc2cd320df37b4d0126 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 16:24:03 -0700 Subject: [PATCH 09/30] Rename `ExceptionPerStep` to `ValuePerStep`, and bring `Formatter` closer to the old behavior. --- .../diffplug/spotless/ExceptionPerStep.java | 101 ------------------ .../java/com/diffplug/spotless/Formatter.java | 13 ++- .../com/diffplug/spotless/ValuePerStep.java | 93 ++++++++++++++++ 3 files changed, 101 insertions(+), 106 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java create mode 100644 lib/src/main/java/com/diffplug/spotless/ValuePerStep.java diff --git a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java b/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java deleted file mode 100644 index 7bf7fe2011..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/ExceptionPerStep.java +++ /dev/null @@ -1,101 +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.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 ExceptionPerStep extends AbstractList { - private final int size; - private @Nullable Throwable exception; - private int exceptionIdx; - private @Nullable Throwable[] multipleExceptions = null; - - ExceptionPerStep(Formatter formatter) { - this.size = formatter.getSteps().size(); - } - - @Override - public @Nullable Throwable set(int index, Throwable exception) { - if (index < 0 || index >= size) { - throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); - } - if (this.exception == null) { - this.exceptionIdx = index; - this.exception = exception; - return null; - } else if (this.multipleExceptions != null) { - Throwable previousValue = multipleExceptions[index]; - multipleExceptions[index] = exception; - return previousValue; - } else { - if (index == exceptionIdx) { - Throwable previousValue = this.exception; - this.exception = exception; - return previousValue; - } else { - multipleExceptions = new Throwable[size]; - multipleExceptions[exceptionIdx] = this.exception; - multipleExceptions[index] = exception; - return null; - } - } - } - - @Override - public Throwable get(int index) { - if (multipleExceptions != null) { - return multipleExceptions[index]; - } else if (exceptionIdx == index) { - return exception; - } else { - return null; - } - } - - private int indexOfFirstException() { - if (multipleExceptions != null) { - for (int i = 0; i < multipleExceptions.length; i++) { - if (multipleExceptions[i] != null) { - return i; - } - } - return -1; - } else if (exception != null) { - return exceptionIdx; - } else { - return -1; - } - } - - @Override - public int size() { - return size; - } - - /** Rethrows the first exception in the list. */ - public void rethrowFirstIfPresent() { - int firstException = indexOfFirstException(); - if (firstException != -1) { - throw ThrowingEx.asRuntimeRethrowError(get(firstException)); - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 5db20942f5..0159d147e6 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -128,9 +128,13 @@ public String computeLineEndings(String unix, File file) { * is guaranteed to also have unix line endings. */ public String compute(String unix, File file) { - ExceptionPerStep exceptionPerStep = new ExceptionPerStep(this); + ValuePerStep exceptionPerStep = new ValuePerStep<>(this); String result = compute(unix, file, exceptionPerStep); - exceptionPerStep.rethrowFirstIfPresent(); + int firstExceptionIndex = exceptionPerStep.indexOfFirstValue(); + if (firstExceptionIndex != -1) { + LintPolicy.error(exceptionPerStep.get(firstExceptionIndex), steps.get(firstExceptionIndex), file.getAbsolutePath()); + throw ThrowingEx.asRuntimeRethrowError(exceptionPerStep.get(firstExceptionIndex)); + } return result; } @@ -142,7 +146,7 @@ public String compute(String unix, File file) { * is guaranteed to also have unix line endings. *

*/ - String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { + String compute(String unix, File file, ValuePerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); @@ -159,9 +163,8 @@ String compute(String unix, File file, ExceptionPerStep exceptionPerStep) { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - // store the exception which was thrown, and stop execution so we don't alter line numbers + // store the exception which was thrown and keep going exceptionPerStep.set(iter.previousIndex(), e); - return unix; } } return unix; 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..cfc40337b5 --- /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 exception) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size); + } + if (this.value == null) { + this.valueIdx = index; + this.value = exception; + return null; + } else if (this.multipleValues != null) { + T previousValue = (T) multipleValues[index]; + multipleValues[index] = exception; + return previousValue; + } else { + if (index == valueIdx) { + T previousValue = this.value; + this.value = exception; + return previousValue; + } else { + multipleValues = new Object[size]; + multipleValues[valueIdx] = this.value; + multipleValues[index] = exception; + 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; + } +} From fe92d05135e0796b89c4d8193a7bfc3be272f7ee Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 18:15:47 -0700 Subject: [PATCH 10/30] Update `Formatter` and `ValuePerStep` so that something (perhaps null) gets stored into every slot of `ValuePerStep` per invocation. --- .../java/com/diffplug/spotless/Formatter.java | 36 ++++++++++++------- .../com/diffplug/spotless/ValuePerStep.java | 3 ++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 0159d147e6..bbb5b85f6e 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -26,7 +26,6 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; -import java.util.ListIterator; import java.util.Objects; /** Formatter which performs the full formatting. */ @@ -129,11 +128,13 @@ public String computeLineEndings(String unix, File file) { */ public String compute(String unix, File file) { ValuePerStep exceptionPerStep = new ValuePerStep<>(this); - String result = compute(unix, file, exceptionPerStep); - int firstExceptionIndex = exceptionPerStep.indexOfFirstValue(); - if (firstExceptionIndex != -1) { - LintPolicy.error(exceptionPerStep.get(firstExceptionIndex), steps.get(firstExceptionIndex), file.getAbsolutePath()); - throw ThrowingEx.asRuntimeRethrowError(exceptionPerStep.get(firstExceptionIndex)); + String result = computeWithLint(unix, file, exceptionPerStep); + for (int i = 0; i < steps.size(); ++i) { + Throwable exception = exceptionPerStep.get(i); + if (exception != null && exception != LintState.formatStepCausedNoChange()) { + LintPolicy.error(exception, steps.get(i), file.getAbsolutePath()); + throw ThrowingEx.asRuntimeRethrowError(exception); + } } return result; } @@ -145,27 +146,38 @@ public String compute(String unix, File file) { * 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 compute(String unix, File file, ValuePerStep exceptionPerStep) { + String computeWithLint(String unix, File file, ValuePerStep exceptionPerStep) { Objects.requireNonNull(unix, "unix"); Objects.requireNonNull(file, "file"); - ListIterator iter = steps.listIterator(); - while (iter.hasNext()) { + for (int i = 0; i < steps.size(); ++i) { + FormatterStep step = steps.get(i); + Throwable storeForStep; try { - String formatted = iter.next().format(unix, file); + 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) { // store the exception which was thrown and keep going - exceptionPerStep.set(iter.previousIndex(), e); + storeForStep = e; } + exceptionPerStep.set(i, storeForStep); } return unix; } diff --git a/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java index cfc40337b5..653a8f0400 100644 --- a/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java +++ b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java @@ -51,6 +51,9 @@ class ValuePerStep extends AbstractList { T previousValue = this.value; this.value = exception; return previousValue; + } else if (value == null) { + // everything is assumed null anyway so we don't need to do anything + return null; } else { multipleValues = new Object[size]; multipleValues[valueIdx] = this.value; From f3be100afdf6c342bbbadbff0aa55f9c92df4c27 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 18:34:03 -0700 Subject: [PATCH 11/30] Introduce LintState which efficiently reads lint data from both `format` calls and `lint` calls. --- .../com/diffplug/spotless/DirtyState.java | 19 ++- .../java/com/diffplug/spotless/LintState.java | 135 ++++++++++++++++++ .../com/diffplug/spotless/PaddedCell.java | 20 +-- 3 files changed, 159 insertions(+), 15 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/LintState.java diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 3d133927c6..058147535e 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()}"); } @@ -81,7 +81,7 @@ public static class Calculation { private final Formatter formatter; private final File file; private final byte[] rawBytes; - private final String raw; + final String raw; private Calculation(Formatter formatter, File file, byte[] rawBytes) { this.formatter = formatter; @@ -101,10 +101,19 @@ private Calculation(Formatter formatter, File file, byte[] rawBytes) { * due to diverging idempotence. */ public DirtyState calculateDirtyState() { + return calculateDirtyState(new ValuePerStep<>(formatter)); + } + + /** + * Calculates whether the given file is dirty according to a PaddedCell invocation of the given formatter. + * DirtyState includes the clean state of the file, as well as a warning if we were not able to apply the formatter + * due to diverging idempotence. + */ + DirtyState calculateDirtyState(ValuePerStep exceptionPerStep) { 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); @@ -115,13 +124,13 @@ public DirtyState calculateDirtyState() { } // 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/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java new file mode 100644 index 0000000000..8a6a8b2544 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -0,0 +1,135 @@ +/* + * 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 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 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 dirtyCalculation = DirtyState.of(formatter, file, rawBytes); + var dirty = dirtyCalculation.calculateDirtyState(exceptions); + + String toLint = LineEnding.toUnix(dirty.isClean() || dirty.didNotConverge() ? dirtyCalculation.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, toLint, 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) { + lintsForStep = List.of(Lint.createFromThrowable(step, toLint, exceptionForLint)); + } else { + lintsForStep = List.of(); + } + if (!lintsForStep.isEmpty()) { + lints.set(i, lintsForStep); + } + } + return new LintState(dirty, lints.indexOfFirstValue() == -1 ? null : lints); + } + + 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 { From 5c424579c8f3cf125fd34f3c1895f279b4c84200 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 30 May 2024 18:41:59 -0700 Subject: [PATCH 12/30] Restore the "legacy" error printing for both `Formatter` and `DirtyState`. --- .../main/java/com/diffplug/spotless/DirtyState.java | 5 ++++- .../main/java/com/diffplug/spotless/Formatter.java | 8 +------- .../main/java/com/diffplug/spotless/LintPolicy.java | 12 ++++++++++++ .../gradle/spotless/BiomeIntegrationTest.java | 5 ----- .../gradle/spotless/ErrorShouldRethrowTest.java | 6 ++++-- .../gradle/spotless/KotlinExtensionTest.java | 5 ----- .../spotless/maven/biome/BiomeMavenTest.java | 4 ---- 7 files changed, 21 insertions(+), 24 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 058147535e..fbbbc284b4 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -101,7 +101,10 @@ private Calculation(Formatter formatter, File file, byte[] rawBytes) { * due to diverging idempotence. */ public DirtyState calculateDirtyState() { - return calculateDirtyState(new ValuePerStep<>(formatter)); + ValuePerStep exceptionPerStep = new ValuePerStep<>(formatter); + DirtyState result = calculateDirtyState(exceptionPerStep); + LintPolicy.legacyBehavior(formatter, file, exceptionPerStep); + return result; } /** diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index bbb5b85f6e..5160f674c2 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -129,13 +129,7 @@ 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); - for (int i = 0; i < steps.size(); ++i) { - Throwable exception = exceptionPerStep.get(i); - if (exception != null && exception != LintState.formatStepCausedNoChange()) { - LintPolicy.error(exception, steps.get(i), file.getAbsolutePath()); - throw ThrowingEx.asRuntimeRethrowError(exception); - } - } + LintPolicy.legacyBehavior(this, file, exceptionPerStep); return result; } diff --git a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java index d4477bd822..0b28c36884 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintPolicy.java +++ b/lib/src/main/java/com/diffplug/spotless/LintPolicy.java @@ -15,6 +15,8 @@ */ package com.diffplug.spotless; +import java.io.File; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,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/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index aff28664c5..ba39af1705 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 @@ -21,12 +21,9 @@ import java.io.IOException; -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 generic formatting step. @@ -323,8 +320,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..5dd435fd1a 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 @@ -32,8 +32,6 @@ 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<>(); @@ -86,6 +84,8 @@ void unlessEnforceCheckIsFalse() throws Exception { runWithSuccess("> Task :processResources NO-SOURCE"); } + @Disabled + @ForLintRefactor @Test void unlessExemptedByStep() throws Exception { writeBuild( @@ -97,6 +97,8 @@ void unlessExemptedByStep() throws Exception { "Unable to apply step 'no swearing' to 'README.md'"); } + @Disabled + @ForLintRefactor @Test void unlessExemptedByPath() throws Exception { writeBuild( 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 aa82c371f3..cf8f9cc739 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"; @@ -150,8 +147,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 24ce1d09b7..ff763c3cf8 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; class BiomeMavenTest extends MavenIntegrationHarness { /** @@ -192,8 +190,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"); From d467545ff597af3184cde7061d0b7a3bc5f504bc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 31 May 2024 10:34:47 -0700 Subject: [PATCH 13/30] Fix spotbugs. --- .../java/com/diffplug/spotless/ValuePerStep.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java index 653a8f0400..314bdc7e60 100644 --- a/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java +++ b/lib/src/main/java/com/diffplug/spotless/ValuePerStep.java @@ -34,30 +34,27 @@ class ValuePerStep extends AbstractList { } @Override - public @Nullable T set(int index, T exception) { + 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 = exception; + this.value = newValue; return null; } else if (this.multipleValues != null) { T previousValue = (T) multipleValues[index]; - multipleValues[index] = exception; + multipleValues[index] = newValue; return previousValue; } else { if (index == valueIdx) { T previousValue = this.value; - this.value = exception; + this.value = newValue; return previousValue; - } else if (value == null) { - // everything is assumed null anyway so we don't need to do anything - return null; } else { multipleValues = new Object[size]; multipleValues[valueIdx] = this.value; - multipleValues[index] = exception; + multipleValues[index] = newValue; return null; } } From 700114fb372a1a7acc6d81b91826420ecc845e50 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 3 Jun 2024 23:48:45 -0700 Subject: [PATCH 14/30] Add Selfie, and configure it to not use triple quote literals. --- gradle.properties | 1 + testlib/build.gradle | 2 ++ .../src/main/java/selfie/SelfieSettings.java | 26 +++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 testlib/src/main/java/selfie/SelfieSettings.java diff --git a/gradle.properties b/gradle.properties index 5d0c627d65..e328419de3 100644 --- a/gradle.properties +++ b/gradle.properties @@ -32,3 +32,4 @@ VER_JGIT=6.7.0.202309050840-r VER_JUNIT=5.10.2 VER_ASSERTJ=3.26.0 VER_MOCKITO=5.12.0 +VER_SELFIE=2.2.0 diff --git a/testlib/build.gradle b/testlib/build.gradle index 38e598369e..2364ea770d 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/selfie/SelfieSettings.java b/testlib/src/main/java/selfie/SelfieSettings.java new file mode 100644 index 0000000000..4a60c15dd0 --- /dev/null +++ b/testlib/src/main/java/selfie/SelfieSettings.java @@ -0,0 +1,26 @@ +/* + * 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 selfie; + +import com.diffplug.selfie.junit5.SelfieSettingsAPI; + +/** https://selfie.dev/jvm/get-started#quickstart */ +public class SelfieSettings extends SelfieSettingsAPI { + @Override + public boolean getJavaDontUseTripleQuoteLiterals() { + return true; + } +} From ffc911ebf8f3fd9474b2a894aad7802c30897343 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 3 Jun 2024 23:54:03 -0700 Subject: [PATCH 15/30] Add a way to test for lints, and use that to bring back FenceStepTest.broken --- .../java/com/diffplug/spotless/LintState.java | 26 +++++++++++++++++++ .../com/diffplug/spotless/StepHarness.java | 10 +++++++ .../spotless/generic/FenceStepTest.java | 10 +++---- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index 8a6a8b2544..a0e01088ae 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -58,6 +58,32 @@ public Map> getLints(Formatter formatter) { 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(":").append(lint.getLineStart()); + if (lint.getLineEnd() != lint.getLineStart()) { + result.append("-").append(lint.getLineEnd()); + } + result.append(" "); + result.append(step.getName()).append("(").append(lint.getCode()).append(") "); + result.append(lint.getMsg()); + 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())); } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index ae59a8d176..4f7f6b5c27 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -24,6 +24,9 @@ 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 { private StepHarness(Formatter formatter, RoundTrip roundTrip) { @@ -111,4 +114,11 @@ public AbstractStringAssert testExceptionMsg(String before) { } } } + + public StringSelfie expectLintsOf(String before) { + LintState state = LintState.of(formatter(), Formatter.NO_FILE_SENTINEL, before.getBytes(formatter().getEncoding())); + String assertAgainst = state.asString(Formatter.NO_FILE_SENTINEL, formatter()); + String cleaned = assertAgainst.replace("NO_FILE_SENTINEL:", ""); + return Selfie.expectSelfie(cleaned); + } } 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 ea9755a804..3d9d2ed0fc 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -18,14 +18,12 @@ import java.io.File; import java.util.Arrays; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarness; -import com.diffplug.spotless.tag.ForLintRefactor; class FenceStepTest extends ResourceHarness { @Test @@ -82,18 +80,16 @@ void multiple() { "1 2 3")); } - @Disabled - @ForLintRefactor @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 From 6bbe556f0f52b4b9f6fe9d28176e41cd53b6f49e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Jun 2024 02:00:08 -0700 Subject: [PATCH 16/30] Deal with `DirtyState.Calculation` is gone. --- .../java/com/diffplug/spotless/DirtyState.java | 15 +++++++++++---- .../java/com/diffplug/spotless/LintState.java | 6 +++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 706b8f14d4..a4f737a701 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -74,7 +74,14 @@ 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) { + return of(formatter, file, rawBytes, raw, new ValuePerStep<>(formatter)); + } + + public 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 +91,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 +102,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/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index a0e01088ae..95254a923e 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -90,10 +90,10 @@ public static LintState of(Formatter formatter, File file) throws IOException { public static LintState of(Formatter formatter, File file, byte[] rawBytes) { var exceptions = new ValuePerStep(formatter); - var dirtyCalculation = DirtyState.of(formatter, file, rawBytes); - var dirty = dirtyCalculation.calculateDirtyState(exceptions); + var raw = new String(rawBytes, formatter.getEncoding()); + var dirty = DirtyState.of(formatter, file, rawBytes, raw, exceptions); - String toLint = LineEnding.toUnix(dirty.isClean() || dirty.didNotConverge() ? dirtyCalculation.raw : new String(dirty.canonicalBytes(), formatter.getEncoding())); + 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 From d08f44202221c75b8df5cb1f4374f6b83d77b9b5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 15:17:02 -0700 Subject: [PATCH 17/30] Two maven tests which fail because errors are getting swallowed as lints. --- .../com/diffplug/spotless/maven/biome/BiomeMavenTest.java | 4 ++++ .../java/com/diffplug/spotless/maven/kotlin/KtlintTest.java | 4 ++++ 2 files changed, 8 insertions(+) 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 5bb7424ef9..1e3d24da47 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,9 +20,11 @@ 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. @@ -223,6 +225,8 @@ 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/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java index 5586335449..2ece678533 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java @@ -17,10 +17,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.ProcessRunner; import com.diffplug.spotless.maven.MavenIntegrationHarness; +import com.diffplug.spotless.tag.ForLintRefactor; class KtlintTest extends MavenIntegrationHarness { @Test @@ -78,6 +80,8 @@ void testSetEditorConfigCanOverrideEditorConfigFile() throws Exception { } @Test + @Disabled + @ForLintRefactor void testWithCustomRuleSetApply() throws Exception { writePomWithKotlinSteps("\n" + " \n" + From 53e31f5325c35da619feec0c179334085a9a15fb Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 15:29:43 -0700 Subject: [PATCH 18/30] Some gradle tests which fail because errors are getting swallowed as lints. --- .../com/diffplug/gradle/spotless/BiomeIntegrationTest.java | 5 +++++ .../com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java | 4 ++++ .../com/diffplug/gradle/spotless/KotlinExtensionTest.java | 5 +++++ 3 files changed, 14 insertions(+) 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..abf8d34fb0 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,9 +19,12 @@ 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 @@ -373,6 +376,8 @@ 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 5dd435fd1a..5050ef93e4 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 @@ -62,6 +62,8 @@ void passesIfNoException() throws Exception { } @Test + @Disabled + @ForLintRefactor void anyExceptionShouldFail() throws Exception { writeBuild( " } // format", @@ -111,6 +113,8 @@ void unlessExemptedByPath() throws Exception { } @Test + @Disabled + @ForLintRefactor void failsIfNeitherStepNorFileExempted() throws Exception { writeBuild( " ignoreErrorForStep 'nope'", 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 ff39cea174..08a5f7279c 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,8 +20,11 @@ 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"; @@ -167,6 +170,8 @@ void testSetEditorConfigCanOverrideEditorConfigFile() throws IOException { } @Test + @Disabled + @ForLintRefactor void withCustomRuleSetApply() throws IOException { setFile("build.gradle.kts").toLines( "plugins {", From 9af488072b69e97b129b5f01617f8a562fe785ed Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 15:52:24 -0700 Subject: [PATCH 19/30] DirtyState now uses `LintPolicy.legacyBehavior` unless you passed in the `ValuePerStep` explicitly. That's enough to bring most of our tests back to life. --- lib/src/main/java/com/diffplug/spotless/DirtyState.java | 5 ++++- .../com/diffplug/gradle/spotless/BiomeIntegrationTest.java | 5 ----- .../com/diffplug/gradle/spotless/KotlinExtensionTest.java | 5 ----- .../com/diffplug/spotless/maven/biome/BiomeMavenTest.java | 4 ---- .../java/com/diffplug/spotless/maven/kotlin/KtlintTest.java | 4 ---- 5 files changed, 4 insertions(+), 19 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index a4f737a701..e3f2d8bcfe 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -78,7 +78,10 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) { } public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw) { - return of(formatter, file, rawBytes, raw, new ValuePerStep<>(formatter)); + var valuePerStep = new ValuePerStep(formatter); + DirtyState state = of(formatter, file, rawBytes, raw, valuePerStep); + LintPolicy.legacyBehavior(formatter, file, valuePerStep); + return state; } public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw, ValuePerStep exceptionPerStep) { 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/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/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java index 2ece678533..5586335449 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/kotlin/KtlintTest.java @@ -17,12 +17,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.spotless.ProcessRunner; import com.diffplug.spotless.maven.MavenIntegrationHarness; -import com.diffplug.spotless.tag.ForLintRefactor; class KtlintTest extends MavenIntegrationHarness { @Test @@ -80,8 +78,6 @@ void testSetEditorConfigCanOverrideEditorConfigFile() throws Exception { } @Test - @Disabled - @ForLintRefactor void testWithCustomRuleSetApply() throws Exception { writePomWithKotlinSteps("\n" + " \n" + From dfca397aabf8bc1da4d0c19d02fd0b7f15755377 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 21:46:34 -0700 Subject: [PATCH 20/30] Rework SpotlessTask to use `LintState` so that we can know which step is failing and properly use the `FormatExceptionPolicyStrict`. --- .../com/diffplug/spotless/DirtyState.java | 2 +- .../main/java/com/diffplug/spotless/Lint.java | 10 ++++ .../java/com/diffplug/spotless/LintState.java | 13 ++++- .../gradle/spotless/FormatExtension.java | 3 +- .../gradle/spotless/SpotlessTask.java | 8 +++ .../gradle/spotless/SpotlessTaskImpl.java | 52 ++++++++++++------- .../spotless/ErrorShouldRethrowTest.java | 21 ++------ .../spotless/tag/ForLintRefactor.java | 30 ----------- 8 files changed, 70 insertions(+), 69 deletions(-) delete mode 100644 testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index e3f2d8bcfe..964cd102e1 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -84,7 +84,7 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, Str return state; } - public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw, ValuePerStep exceptionPerStep) { + 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) { diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 81fcefc964..b35c0ea32f 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -27,6 +27,16 @@ * to Spotless, then it is by definition. */ public final class Lint implements Serializable { + /** Returns a runtime exception which, if thrown, will lint the entire file. */ + public static ShortcutException entireFile(String code, String msg) { + return new ShortcutException(Lint.create(code, msg, -1)); + } + + /** Returns a runtime exception which, if thrown, will lint a specific line. */ + public static ShortcutException atLine(int line, String code, String msg) { + return new ShortcutException(Lint.create(code, msg, line)); + } + /** Any exception which implements this interface will have its lints extracted and reported cleanly to the user. */ public interface Has { List getLints(); diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index 95254a923e..90b363c81a 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -41,6 +41,10 @@ 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!"); @@ -137,7 +141,7 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) { List lintsForStep; if (exceptionForLint instanceof Lint.Has) { lintsForStep = ((Lint.Has) exceptionForLint).getLints(); - } else if (exceptionForLint != null) { + } else if (exceptionForLint != null && exceptionForLint != formatStepCausedNoChange()) { lintsForStep = List.of(Lint.createFromThrowable(step, toLint, exceptionForLint)); } else { lintsForStep = List.of(); @@ -149,6 +153,13 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) { 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; } 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..2dfd44506d 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.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 e1e729e852..005920b8a6 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 @@ -157,6 +157,14 @@ public File getOutputDirectory() { return outputDirectory; } + protected File lintDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(), + "spotless-lint/" + getName()); + + @OutputDirectory + public File getLintDirectory() { + return lintDirectory; + } + 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 b7586f742b..1d8dd114bf 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,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.List; import javax.annotation.Nullable; import javax.inject.Inject; @@ -37,8 +38,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 +84,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 +96,23 @@ 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); + File lint = getOutputFileWithBaseDir(input, lintDirectory); 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,20 +124,32 @@ 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)); + // Files.createDirectories(lint.toPath().getParent()); + // Files.write(lint.toPath(), lintState.asString().getBytes()); + } else { + Files.deleteIfExists(lint.toPath()); } } - private void deletePreviousResult(File input) throws IOException { - File output = getOutputFile(input); - if (output.isDirectory()) { - getFs().delete(d -> d.delete(output)); - } else { - Files.deleteIfExists(output.toPath()); + private void deletePreviousResults(File input) throws IOException { + File output = getOutputFileWithBaseDir(input, outputDirectory); + File lint = getOutputFileWithBaseDir(input, lintDirectory); + for (File file : List.of(output, lint)) { + if (file.isDirectory()) { + getFs().delete(d -> d.delete(file)); + } else { + Files.deleteIfExists(file.toPath()); + } } } - 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 +159,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/ErrorShouldRethrowTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java index 5050ef93e4..0d99c6bd5d 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,13 +23,11 @@ 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. */ class ErrorShouldRethrowTest extends GradleIntegrationHarness { @@ -45,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.entireFile('swearing', 'No swearing!');"); lines.add(" }"); lines.add(" }"); lines.addAll(Arrays.asList(toInsert)); @@ -62,8 +60,6 @@ void passesIfNoException() throws Exception { } @Test - @Disabled - @ForLintRefactor void anyExceptionShouldFail() throws Exception { writeBuild( " } // format", @@ -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!"); + "-1: (swearing) No swearing!\n" + + "java.lang.Throwable: -1: (swearing) No swearing!"); } @Test @@ -86,9 +82,6 @@ void unlessEnforceCheckIsFalse() throws Exception { runWithSuccess("> Task :processResources NO-SOURCE"); } - @Disabled - @ForLintRefactor - @Test void unlessExemptedByStep() throws Exception { writeBuild( " ignoreErrorForStep 'no swearing'", @@ -99,8 +92,6 @@ void unlessExemptedByStep() throws Exception { "Unable to apply step 'no swearing' to 'README.md'"); } - @Disabled - @ForLintRefactor @Test void unlessExemptedByPath() throws Exception { writeBuild( @@ -113,8 +104,6 @@ void unlessExemptedByPath() throws Exception { } @Test - @Disabled - @ForLintRefactor void failsIfNeitherStepNorFileExempted() throws Exception { writeBuild( " ignoreErrorForStep 'nope'", @@ -124,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!"); + "-1: (swearing) No swearing!\n" + + "java.lang.Throwable: -1: (swearing) No swearing!"); } private void runWithSuccess(String expectedToStartWith) throws Exception { diff --git a/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java b/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java deleted file mode 100644 index a6ac9b090b..0000000000 --- a/testlib/src/main/java/com/diffplug/spotless/tag/ForLintRefactor.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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. - * 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.tag; - -import static java.lang.annotation.ElementType.METHOD; -import static java.lang.annotation.ElementType.TYPE; -import static java.lang.annotation.RetentionPolicy.RUNTIME; - -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 {} From 29cba64047d47fd193d2cdcc91d7e54dfb157c0f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 21:54:32 -0700 Subject: [PATCH 21/30] We don't need to move lints to their own place *yet*. --- .../diffplug/gradle/spotless/SpotlessTask.java | 8 -------- .../gradle/spotless/SpotlessTaskImpl.java | 17 ++++------------- 2 files changed, 4 insertions(+), 21 deletions(-) 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 005920b8a6..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 @@ -157,14 +157,6 @@ public File getOutputDirectory() { return outputDirectory; } - protected File lintDirectory = new File(getProject().getLayout().getBuildDirectory().getAsFile().get(), - "spotless-lint/" + getName()); - - @OutputDirectory - public File getLintDirectory() { - return lintDirectory; - } - 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 1d8dd114bf..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 @@ -20,7 +20,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.util.List; import javax.annotation.Nullable; import javax.inject.Inject; @@ -97,7 +96,6 @@ public void performAction(InputChanges inputs) throws Exception { @VisibleForTesting void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File input) throws IOException { File output = getOutputFileWithBaseDir(input, outputDirectory); - File lint = getOutputFileWithBaseDir(input, lintDirectory); getLogger().debug("Applying format to {} and writing to {}", input, output); LintState lintState; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { @@ -130,22 +128,15 @@ void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File in 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)); - // Files.createDirectories(lint.toPath().getParent()); - // Files.write(lint.toPath(), lintState.asString().getBytes()); - } else { - Files.deleteIfExists(lint.toPath()); } } private void deletePreviousResults(File input) throws IOException { File output = getOutputFileWithBaseDir(input, outputDirectory); - File lint = getOutputFileWithBaseDir(input, lintDirectory); - for (File file : List.of(output, lint)) { - if (file.isDirectory()) { - getFs().delete(d -> d.delete(file)); - } else { - Files.deleteIfExists(file.toPath()); - } + if (output.isDirectory()) { + getFs().delete(d -> d.delete(output)); + } else { + Files.deleteIfExists(output.toPath()); } } From 43cd94b5b577696311ccb71010fa015e10b59f90 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 22:06:37 -0700 Subject: [PATCH 22/30] Make an explicit constant for a lint at an undefined line. --- .../main/java/com/diffplug/spotless/Lint.java | 19 +++++++++++++++---- .../spotless/ErrorShouldRethrowTest.java | 10 +++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index b35c0ea32f..4708c27a2c 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -27,9 +27,9 @@ * to Spotless, then it is by definition. */ public final class Lint implements Serializable { - /** Returns a runtime exception which, if thrown, will lint the entire file. */ - public static ShortcutException entireFile(String code, String msg) { - return new ShortcutException(Lint.create(code, msg, -1)); + /** Returns a runtime exception which, if thrown, will create a lint at an undefined line. */ + public static ShortcutException atUndefinedLine(String code, String msg) { + return new ShortcutException(Lint.create(code, msg, LINE_UNDEFINED)); } /** Returns a runtime exception which, if thrown, will lint a specific line. */ @@ -37,6 +37,11 @@ public static ShortcutException atLine(int line, String code, String msg) { return new ShortcutException(Lint.create(code, msg, line)); } + /** Returns a runtime exception which, if thrown, will lint a specific line range. */ + public static ShortcutException atLineRange(int lineStart, int lineEnd, String code, String msg) { + return new ShortcutException(Lint.create(code, msg, lineStart, lineEnd)); + } + /** Any exception which implements this interface will have its lints extracted and reported cleanly to the user. */ public interface Has { List getLints(); @@ -103,7 +108,11 @@ public String getMsg() { @Override public String toString() { if (lineStart == lineEnd) { - return lineStart + ": (" + code + ") " + msg; + if (lineStart == LINE_UNDEFINED) { + return "LINE_UNDEFINED: (" + code + ") " + msg; + } else { + return lineStart + ": (" + code + ") " + msg; + } } else { return lineStart + "-" + lineEnd + ": (" + code + ") " + msg; } @@ -163,4 +172,6 @@ private static String msgFrom(String message) { } return ""; } + + public static final int LINE_UNDEFINED = -1; } 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 0d99c6bd5d..f425743bf0 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 @@ -43,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 com.diffplug.spotless.Lint.entireFile('swearing', 'No swearing!');"); + lines.add(" throw com.diffplug.spotless.Lint.atUndefinedLine('swearing', 'No swearing!');"); lines.add(" }"); lines.add(" }"); lines.addAll(Arrays.asList(toInsert)); @@ -68,8 +68,8 @@ void anyExceptionShouldFail() throws Exception { runWithFailure( "> Task :spotlessMisc FAILED\n" + "Step 'no swearing' found problem in 'README.md':\n" + - "-1: (swearing) No swearing!\n" + - "java.lang.Throwable: -1: (swearing) No swearing!"); + "LINE_UNDEFINED: (swearing) No swearing!\n" + + "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); } @Test @@ -113,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" + - "-1: (swearing) No swearing!\n" + - "java.lang.Throwable: -1: (swearing) No swearing!"); + "LINE_UNDEFINED: (swearing) No swearing!\n" + + "java.lang.Throwable: LINE_UNDEFINED: (swearing) No swearing!"); } private void runWithSuccess(String expectedToStartWith) throws Exception { From 6619a769d2cd8b52c3d0f2fe1a8f43621ccc8221 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 22:22:55 -0700 Subject: [PATCH 23/30] Fix windows. --- .../main/java/com/diffplug/gradle/spotless/FormatExtension.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2dfd44506d..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 @@ -340,7 +340,7 @@ private static void relativizeIfSubdir(List relativePaths, File root, Fi return null; } else { String relativized = destPath.substring(rootPath.length()); - return relativized.startsWith("/") ? relativized.substring(1) : relativized; + return relativized.startsWith("/") || relativized.startsWith("\\") ? relativized.substring(1) : relativized; } } From a9bd0a60cac0f127188dbeec8d9ba2bafb996fed Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 22:30:35 -0700 Subject: [PATCH 24/30] Update changelog. --- CHANGES.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 69bbaa1fd2..853601237b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,15 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [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 +### 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`. ## [3.0.0.BETA3] - 2024-10-15 ### Added From 6c480d8030c6f049690d35ff096e3732ef021a89 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Oct 2024 22:38:22 -0700 Subject: [PATCH 25/30] Add info to CONTRIBUTING --- CONTRIBUTING.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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: From ae31ea54a2e1cccc598f0f2da3601cf4e5901a8e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Oct 2024 08:55:56 -0700 Subject: [PATCH 26/30] Setup selfie. --- gradle/special-tests.gradle | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 } From 8300fda64127ab1718b3d8006c7dc8833a41ae44 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Oct 2024 08:58:13 -0700 Subject: [PATCH 27/30] Fix ambiguities in the `Lint` class around treating it as data vs exception. --- .../main/java/com/diffplug/spotless/Lint.java | 127 ++++++++++-------- 1 file changed, 71 insertions(+), 56 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 4708c27a2c..4384ed38ef 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -20,6 +20,8 @@ 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 @@ -27,19 +29,52 @@ * to Spotless, then it is by definition. */ public final class Lint implements Serializable { - /** Returns a runtime exception which, if thrown, will create a lint at an undefined line. */ - public static ShortcutException atUndefinedLine(String code, String msg) { - return new ShortcutException(Lint.create(code, msg, LINE_UNDEFINED)); + public static Lint atUndefinedLine(String ruleId, String detail) { + return new Lint(LINE_UNDEFINED, ruleId, detail); } - /** Returns a runtime exception which, if thrown, will lint a specific line. */ - public static ShortcutException atLine(int line, String code, String msg) { - return new ShortcutException(Lint.create(code, msg, line)); + public static Lint atLine(int line, String ruleId, String detail) { + return new Lint(line, ruleId, detail); } - /** Returns a runtime exception which, if thrown, will lint a specific line range. */ - public static ShortcutException atLineRange(int lineStart, int lineEnd, String code, String msg) { - return new ShortcutException(Lint.create(code, msg, lineStart, lineEnd)); + 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. */ @@ -48,14 +83,15 @@ public interface Has { } /** An exception for shortcutting execution to report a lint to the user. */ - public static class ShortcutException extends RuntimeException implements Has { + static class ShortcutException extends RuntimeException implements Has { public ShortcutException(Lint... lints) { this(Arrays.asList(lints)); } private final List lints; - public ShortcutException(Collection lints) { + ShortcutException(Collection lints) { + super(lints.iterator().next().detail); this.lints = List.copyOf(lints); } @@ -65,56 +101,26 @@ public List getLints() { } } - private static final long serialVersionUID = 1L; - - private int lineStart, lineEnd; // 1-indexed, inclusive - private String code; // 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 msg; - - private Lint(int lineStart, int lineEnd, String lintCode, String lintMsg) { - this.lineStart = lineStart; - this.lineEnd = lineEnd; - this.code = LineEnding.toUnix(lintCode); - this.msg = LineEnding.toUnix(lintMsg); + /** Returns an exception which will wrap all of the given lints using {@link Has} */ + public static RuntimeException shortcut(Collection lints) { + return new ShortcutException(lints); } - public static Lint create(String code, String msg, int lineStart, int lineEnd) { - if (lineEnd < lineStart) { - throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd); - } - return new Lint(lineStart, lineEnd, code, msg); - } - - public static Lint create(String code, String msg, int line) { - return new Lint(line, line, code, msg); - } - - public int getLineStart() { - return lineStart; - } - - public int getLineEnd() { - return lineEnd; - } - - public String getCode() { - return code; - } - - public String getMsg() { - return msg; + /** 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: (" + code + ") " + msg; + return "LINE_UNDEFINED: (" + ruleId + ") " + detail; } else { - return lineStart + ": (" + code + ") " + msg; + return "L" + lineStart + ": (" + ruleId + ") " + detail; } } else { - return lineStart + "-" + lineEnd + ": (" + code + ") " + msg; + return "L" + lineStart + "-" + lineEnd + ": (" + ruleId + ") " + detail; } } @@ -125,27 +131,36 @@ 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(code, lint.code) && Objects.equals(msg, lint.msg); + 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, code, msg); + return Objects.hash(lineStart, lineEnd, ruleId, detail); } /** Attempts to parse a line number from the given exception. */ - static Lint createFromThrowable(FormatterStep step, String content, Throwable e) { + 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 Lint.create(step.getName(), msgFrom(message), lineNumber); + return new Lint(lineNumber, step.getName(), msgFrom(message)); } current = current.getCause(); } - int numNewlines = (int) content.codePoints().filter(c -> c == '\n').count(); - return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1, 1 + numNewlines); + 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) { From 9658d411729c1b1ba46fa548204ec4d804dab36a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Oct 2024 08:58:23 -0700 Subject: [PATCH 28/30] Improve LintState's toString. --- .../java/com/diffplug/spotless/LintState.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/LintState.java b/lib/src/main/java/com/diffplug/spotless/LintState.java index 90b363c81a..2fcf18ef6c 100644 --- a/lib/src/main/java/com/diffplug/spotless/LintState.java +++ b/lib/src/main/java/com/diffplug/spotless/LintState.java @@ -72,13 +72,19 @@ public String asString(File file, Formatter formatter) { if (lints != null) { FormatterStep step = formatter.getSteps().get(i); for (Lint lint : lints) { - result.append(file.getName()).append(":").append(lint.getLineStart()); - if (lint.getLineEnd() != lint.getLineStart()) { - result.append("-").append(lint.getLineEnd()); + 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.getCode()).append(") "); - result.append(lint.getMsg()); + result.append(step.getName()).append("(").append(lint.getRuleId()).append(") "); + result.append(lint.getDetail()); result.append("\n"); } } @@ -111,7 +117,7 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) { lints.set(i, lintsForStep); } } catch (Exception e) { - lints.set(i, List.of(Lint.createFromThrowable(step, toLint, e))); + lints.set(i, List.of(Lint.createFromThrowable(step, e))); } } } @@ -142,7 +148,7 @@ public static LintState of(Formatter formatter, File file, byte[] rawBytes) { if (exceptionForLint instanceof Lint.Has) { lintsForStep = ((Lint.Has) exceptionForLint).getLints(); } else if (exceptionForLint != null && exceptionForLint != formatStepCausedNoChange()) { - lintsForStep = List.of(Lint.createFromThrowable(step, toLint, exceptionForLint)); + lintsForStep = List.of(Lint.createFromThrowable(step, exceptionForLint)); } else { lintsForStep = List.of(); } From d00a4ab8984fb66e95ce7b71dcae9f7c320e5238 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Oct 2024 08:58:57 -0700 Subject: [PATCH 29/30] Adapt gradle test for the API. --- .../com/diffplug/gradle/spotless/ErrorShouldRethrowTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f425743bf0..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 @@ -43,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 com.diffplug.spotless.Lint.atUndefinedLine('swearing', 'No swearing!');"); + lines.add(" throw com.diffplug.spotless.Lint.atUndefinedLine('swearing', 'No swearing!').shortcut();"); lines.add(" }"); lines.add(" }"); lines.addAll(Arrays.asList(toInsert)); From 195bc165ce94a2a9086736b28fb2c3dd06740e03 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Oct 2024 08:59:42 -0700 Subject: [PATCH 30/30] Remove `testResourceExceptionMsg` and replace with `expectLintsOfResource`. --- .../com/diffplug/spotless/StepHarness.java | 43 +++++++------------ .../spotless/StepHarnessWithFile.java | 33 +++++--------- 2 files changed, 26 insertions(+), 50 deletions(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index 4f7f6b5c27..e45fd16a5b 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -21,9 +21,6 @@ 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; @@ -91,34 +88,26 @@ public StepHarness testResourceUnaffected(String resourceIdempotent) { return testUnaffected(idempotentElement); } - public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { - return testExceptionMsg(ResourceHarness.getTestResource(resourceBefore)); - } - - 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 expectLintsOfResource(String before) { + return expectLintsOf(ResourceHarness.getTestResource(before)); } public StringSelfie expectLintsOf(String before) { LintState state = LintState.of(formatter(), Formatter.NO_FILE_SENTINEL, before.getBytes(formatter().getEncoding())); - String assertAgainst = state.asString(Formatter.NO_FILE_SENTINEL, formatter()); + 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:", ""); - return Selfie.expectSelfie(cleaned); + + 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); } } }