From d5b0421b0f636b4c5ec589c31822e7580ec7a7d4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 14:24:49 -0800 Subject: [PATCH 01/31] Define a `Lint`. --- .../main/java/com/diffplug/spotless/Lint.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) 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..2152add9c4 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -0,0 +1,87 @@ +/* + * Copyright 2022 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.spotless; + +import java.util.Objects; + +/** + * 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 { + private int lineStart, lineEnd; // 1-indexed + 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 = lintCode; + this.msg = 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); + } +} From db676fc275bf211a7fca600d7a2b4b4182f3523e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 14:59:45 -0800 Subject: [PATCH 02/31] Add a `lint` method to `FormatterStep` and let it wripple through. --- .../FilterByContentPatternFormatterStep.java | 21 +++++++++++++------ .../spotless/FilterByFileFormatterStep.java | 15 ++++++++++++- .../com/diffplug/spotless/FormatterFunc.java | 9 +++++++- .../com/diffplug/spotless/FormatterStep.java | 14 +++++++++++++ .../diffplug/spotless/FormatterStepImpl.java | 13 ++++++++++++ 5 files changed, 64 insertions(+), 8 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java index 9b39361719..9fc4e2d4dc 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-2021 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,8 +16,9 @@ package com.diffplug.spotless; import java.io.File; +import java.util.Collections; +import java.util.List; import java.util.Objects; -import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -40,16 +41,24 @@ public String getName() { 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()) { + if (contentPattern.matcher(raw).find()) { return delegateStep.format(raw, file); } else { return raw; } } + @Override + public List lint(String content, File file) throws Exception { + Objects.requireNonNull(content, "content"); + Objects.requireNonNull(file, "file"); + if (contentPattern.matcher(content).find()) { + return delegateStep.lint(content, file); + } else { + return Collections.emptyList(); + } + } + @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 2fd221220c..54e812e4e2 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 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,6 +16,8 @@ package com.diffplug.spotless; import java.io.File; +import java.util.Collections; +import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -45,6 +47,17 @@ public String getName() { } } + @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 Collections.emptyList(); + } + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index 48a8e810ee..2ba5b1474d 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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,6 +16,8 @@ package com.diffplug.spotless; import java.io.File; +import java.util.Collections; +import java.util.List; import java.util.Objects; /** @@ -32,6 +34,11 @@ default String apply(String unix, File file) throws Exception { return apply(unix); } + /** Calculates a list of lints against the given content. */ + default List lint(String content, File file) { + return Collections.emptyList(); + } + /** * {@code Function} and {@code BiFunction} whose implementation * requires a resource which should be released when the function is no longer needed. diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 763232f936..7ddaf65782 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; @@ -45,6 +46,19 @@ public interface FormatterStep extends Serializable { */ public @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 + */ + public List lint(String content, File file) throws Exception; + /** * Returns a new FormatterStep which will only apply its changes * to files which pass the given filter. diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java index f9672d7dcd..f0a38da957 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java @@ -17,6 +17,7 @@ import java.io.File; import java.io.Serializable; +import java.util.List; import java.util.Objects; import java.util.Random; @@ -80,6 +81,13 @@ public String format(String rawUnix, File file) throws Exception { return func().apply(rawUnix, file); } + @Override + public List lint(String content, File file) throws Exception { + Objects.requireNonNull(content, "content"); + Objects.requireNonNull(file, "file"); + return func().lint(content, file); + } + void cleanupFormatterFunc() { if (formatter instanceof FormatterFunc.Closeable) { ((FormatterFunc.Closeable) formatter).close(); @@ -114,6 +122,11 @@ private FormatterFunc func() throws Exception { public String format(String rawUnix, File file) throws Exception { return func().apply(rawUnix, file); } + + @Override + public List lint(String content, File file) throws Exception { + return func().lint(content, file); + } } /** A dummy SENTINEL file. */ From 5dc62710360d1138e87678473dd994f254ccfc80 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 16:35:37 -0800 Subject: [PATCH 03/31] Make Lint roundtrippable through a String. --- .../main/java/com/diffplug/spotless/Lint.java | 80 +++++++++ .../spotless/PerCharacterEscaper.java | 152 ++++++++++++++++++ .../java/com/diffplug/spotless/LintTest.java | 47 ++++++ .../spotless/PerCharacterEscaperTest.java | 65 ++++++++ 4 files changed, 344 insertions(+) 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/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 2152add9c4..d02e3cc590 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -15,6 +15,12 @@ */ package com.diffplug.spotless; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; /** @@ -84,4 +90,78 @@ public boolean equals(Object o) { 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 { + byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); + Files.write(file.toPath(), content); + } } 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..1be9847668 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/PerCharacterEscaper.java @@ -0,0 +1,152 @@ +/* + * 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. + * 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/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..4ef5824061 --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/LintTest.java @@ -0,0 +1,47 @@ +/* + * Copyright 2022 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..f0dccb1325 --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/PerCharacterEscaperTest.java @@ -0,0 +1,65 @@ +/* + * 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. + * 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 cc71751d5574568f86eee2ff4afee4579e925a1f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 16:35:59 -0800 Subject: [PATCH 04/31] Formatter.lint --- .../java/com/diffplug/spotless/Formatter.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 2f80a1d531..0f2337a93a 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -27,6 +27,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; @@ -179,6 +180,29 @@ public String compute(String unix, File file) { return unix; } + public List lint(String content, File file) { + Objects.requireNonNull(content, "unix"); + Objects.requireNonNull(file, "file"); + + List totalLints = new ArrayList<>(); + for (FormatterStep step : steps) { + try { + List lints = step.lint(content, file); + if (lints != null && !lints.isEmpty()) { + totalLints.addAll(lints); + } + } catch (Throwable e) { + String relativePath = rootDir.relativize(file.toPath()).toString(); + exceptionPolicy.handleError(e, step, relativePath); + } + } + if (totalLints.isEmpty()) { + return Collections.emptyList(); + } else { + return totalLints; + } + } + @Override public int hashCode() { final int prime = 31; From aac39728f1577bc7bed856ea5955a4e3946aafa6 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 16:55:30 -0800 Subject: [PATCH 05/31] Lint ripples through DirtyState and Formatter. --- lib/src/main/java/com/diffplug/spotless/DirtyState.java | 5 +++++ lib/src/main/java/com/diffplug/spotless/Formatter.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index ed6bb9ac3c..90c96dc5e5 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -20,6 +20,7 @@ import java.io.OutputStream; import java.nio.file.Files; import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; @@ -137,5 +138,9 @@ public DirtyState calculateDirtyState() { return isClean; } } + + public List calculateLintAgainstRaw() { + return formatter.lint(raw, file); + } } } diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 0f2337a93a..673b9ea987 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -181,7 +181,7 @@ public String compute(String unix, File file) { } public List lint(String content, File file) { - Objects.requireNonNull(content, "unix"); + Objects.requireNonNull(content, "content"); Objects.requireNonNull(file, "file"); List totalLints = new ArrayList<>(); From a25ededb7cd682872a6d00ad1080ee679a7b4625 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 12 Jan 2022 16:57:21 -0800 Subject: [PATCH 06/31] SpotlessTaskImpl now generates both a formatted result and a lint result. --- .../gradle/spotless/SpotlessApply.java | 4 +- .../gradle/spotless/SpotlessCheck.java | 6 +- .../gradle/spotless/SpotlessTask.java | 13 ++++- .../gradle/spotless/SpotlessTaskImpl.java | 55 ++++++++++++++----- .../gradle/spotless/SpotlessTaskService.java | 10 +++- .../gradle/spotless/FormatTaskTest.java | 6 +- .../gradle/spotless/PaddedCellTaskTest.java | 4 +- 7 files changed, 72 insertions(+), 26 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java index a85195fe9f..e159ce4c89 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 DiffPlug + * Copyright 2016-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,7 +29,7 @@ public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { @TaskAction public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); - ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); + ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(contentDir()); if (files.isEmpty()) { getState().setDidWork(sourceDidWork()); } else { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index 55336a00ad..0d308abdbb 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -50,7 +50,7 @@ public void performAction() throws IOException { } private void performAction(boolean isTest) throws IOException { - ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); + ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(contentDir()); if (files.isEmpty()) { getState().setDidWork(sourceDidWork()); } else if (!isTest && applyHasRun()) { @@ -101,7 +101,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { .runToFix("Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations.") .formatterFolder( getProjectDir().get().getAsFile().toPath(), - getSpotlessOutDirectory().get().toPath(), + contentDir().toPath(), getEncoding().get()) .problemFiles(problemFiles) .getMessage()); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index 6f2279b2e0..545ebe3f61 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 DiffPlug + * Copyright 2020-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -154,6 +154,17 @@ public File getOutputDirectory() { return outputDirectory; } + static final String CONTENT = "content"; + static final String LINT = "lint"; + + File contentDir() { + return new File(outputDirectory, CONTENT); + } + + File lintDir() { + return new File(outputDirectory, LINT); + } + protected final LiveCache> steps = createLive("steps"); { steps.set(new ArrayList()); 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 d787430447..a37a7306be 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -20,6 +20,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.Collections; +import java.util.List; import javax.annotation.Nullable; import javax.inject.Inject; @@ -38,6 +40,7 @@ import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.Lint; import com.diffplug.spotless.extra.GitRatchet; @CacheableTask @@ -64,7 +67,8 @@ public void performAction(InputChanges inputs) throws Exception { if (!inputs.isIncremental()) { getLogger().info("Not incremental: removing prior outputs"); getFs().delete(d -> d.delete(outputDirectory)); - Files.createDirectories(outputDirectory.toPath()); + Files.createDirectories(contentDir().toPath()); + Files.createDirectories(lintDir().toPath()); } try (Formatter formatter = buildFormatter()) { @@ -86,10 +90,14 @@ private void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File output = getOutputFile(input); getLogger().debug("Applying format to " + input + " and writing to " + output); DirtyState dirtyState; + List lints; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { dirtyState = DirtyState.clean(); + lints = Collections.emptyList(); } else { - dirtyState = DirtyState.of(formatter, input).calculateDirtyState(); + DirtyState.Calculation calculation = DirtyState.of(formatter, input); + dirtyState = calculation.calculateDirtyState(); + lints = calculation.calculateLintAgainstRaw(); } if (dirtyState.isClean()) { // Remove previous output if it exists @@ -106,27 +114,46 @@ private void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, Files.copy(input.toPath(), output.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); dirtyState.writeCanonicalTo(output); } + + File lint = getLintFile(input); + if (lints.isEmpty()) { + Files.deleteIfExists(lint.toPath()); + } else { + Lint.toFile(lints, lint); + } } private void deletePreviousResult(File input) throws IOException { - File output = getOutputFile(input); - if (output.isDirectory()) { - getFs().delete(d -> d.delete(output)); + delete(getOutputFile(input)); + delete(getLintFile(input)); + } + + private File getOutputFile(File input) { + return new File(contentDir(), relativize(input)); + } + + private File getLintFile(File input) { + return new File(lintDir(), relativize(input)); + } + + private void delete(File file) throws IOException { + if (file.isDirectory()) { + getFs().delete(d -> d.delete(file)); } else { - Files.deleteIfExists(output.toPath()); + Files.deleteIfExists(file.toPath()); } } - private File getOutputFile(File input) { + private String relativize(File input) { File projectDir = getProjectDir().get().getAsFile(); String outputFileName = FormatExtension.relativize(projectDir, input); - if (outputFileName == null) { - throw new IllegalArgumentException(StringPrinter.buildString(printer -> { - printer.println("Spotless error! All target files must be within the project dir."); - printer.println(" project dir: " + projectDir.getAbsolutePath()); - printer.println(" target: " + input.getAbsolutePath()); - })); + if (outputFileName != null) { + return outputFileName; } - return new File(outputDirectory, outputFileName); + throw new IllegalArgumentException(StringPrinter.buildString(printer -> { + printer.println("Spotless error! All target files must be within the project dir."); + printer.println(" project dir: " + projectDir.getAbsolutePath()); + printer.println(" target: " + input.getAbsolutePath()); + })); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 7fc0ea3775..562dd85790 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 DiffPlug + * Copyright 2021-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,6 +95,14 @@ static abstract class ClientTask extends DefaultTask { @Internal abstract Property getSpotlessOutDirectory(); + File contentDir() { + return new File(getSpotlessOutDirectory().get(), SpotlessTaskImpl.CONTENT); + } + + File lintDir() { + return new File(getSpotlessOutDirectory().get(), SpotlessTaskImpl.LINT); + } + @Internal abstract Property getTaskService(); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java index 8985cd7a14..f8bb51f07a 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -47,7 +47,7 @@ public BuildServiceParameters.None getParameters() { @Test void testLineEndings() throws Exception { File testFile = setFile("testFile").toContent("\r\n"); - File outputFile = new File(spotlessTask.getOutputDirectory(), "testFile"); + File outputFile = new File(spotlessTask.contentDir(), "testFile"); spotlessTask.setTarget(Collections.singleton(testFile)); Tasks.execute(spotlessTask); @@ -58,7 +58,7 @@ void testLineEndings() throws Exception { @Test void testStep() throws Exception { File testFile = setFile("testFile").toContent("apple"); - File outputFile = new File(spotlessTask.getOutputDirectory(), "testFile"); + File outputFile = new File(spotlessTask.contentDir(), "testFile"); spotlessTask.setTarget(Collections.singleton(testFile)); spotlessTask.addStep(FormatterStep.createNeverUpToDate("double-p", content -> content.replace("pp", "p"))); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index bc7a2fdc96..c0886312ce 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -59,7 +59,7 @@ public BuildServiceParameters.None getParameters() { source = createFormatTask(name, step); check = createCheckTask(name, source); apply = createApplyTask(name, source); - outputFile = new File(source.getOutputDirectory() + "/src", file.getName()); + outputFile = new File(source.contentDir(), "src/" + file.getName()); } private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { From 89339cdc624a2f2921685462af78da568d5acab9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 13 Jan 2022 22:55:30 -0800 Subject: [PATCH 07/31] SpotlessTaskImpl now generates a lint result for `check` and for `apply` (the line numbers in a lint will change depending on whether the `apply` already ran or not). --- .../com/diffplug/spotless/DirtyState.java | 18 +++++++++++ .../gradle/spotless/SpotlessTask.java | 11 +++++-- .../gradle/spotless/SpotlessTaskImpl.java | 32 +++++++++++++------ .../gradle/spotless/SpotlessTaskService.java | 10 ++++-- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/DirtyState.java b/lib/src/main/java/com/diffplug/spotless/DirtyState.java index 90c96dc5e5..d5aab740cd 100644 --- a/lib/src/main/java/com/diffplug/spotless/DirtyState.java +++ b/lib/src/main/java/com/diffplug/spotless/DirtyState.java @@ -142,5 +142,23 @@ public DirtyState calculateDirtyState() { public List calculateLintAgainstRaw() { return formatter.lint(raw, file); } + + public List calculateLintAgainstDirtyState(DirtyState dirtyState) { + if (dirtyState.isClean() || dirtyState.didNotConverge()) { + return calculateLintAgainstRaw(); + } else { + String canonical = new String(dirtyState.canonicalBytes(), formatter.getEncoding()); + return formatter.lint(canonical, file); + } + } + + /** If {@link #calculateLintAgainstRaw()} was already called, then you might be able to reuse that value. */ + public List calculateLintAgainstDirtyState(DirtyState dirtyState, List lintsAgainstRaw) { + if (dirtyState.isClean() || dirtyState.didNotConverge()) { + return lintsAgainstRaw; + } else { + return calculateLintAgainstDirtyState(dirtyState); + } + } } } 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 545ebe3f61..e99ea766d0 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 @@ -155,14 +155,19 @@ public File getOutputDirectory() { } static final String CONTENT = "content"; - static final String LINT = "lint"; + static final String LINT_APPLY = "lint-apply"; + static final String LINT_CHECK = "lint-check"; File contentDir() { return new File(outputDirectory, CONTENT); } - File lintDir() { - return new File(outputDirectory, LINT); + File lintApplyDir() { + return new File(outputDirectory, LINT_APPLY); + } + + File lintCheckDir() { + return new File(outputDirectory, LINT_CHECK); } protected final LiveCache> steps = createLive("steps"); 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 a37a7306be..0e68b8edcc 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 @@ -68,7 +68,8 @@ public void performAction(InputChanges inputs) throws Exception { getLogger().info("Not incremental: removing prior outputs"); getFs().delete(d -> d.delete(outputDirectory)); Files.createDirectories(contentDir().toPath()); - Files.createDirectories(lintDir().toPath()); + Files.createDirectories(lintApplyDir().toPath()); + Files.createDirectories(lintCheckDir().toPath()); } try (Formatter formatter = buildFormatter()) { @@ -90,14 +91,16 @@ private void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, File output = getOutputFile(input); getLogger().debug("Applying format to " + input + " and writing to " + output); DirtyState dirtyState; - List lints; + List lintsCheck, lintsApply; if (ratchet != null && ratchet.isClean(getProjectDir().get().getAsFile(), getRootTreeSha(), input)) { dirtyState = DirtyState.clean(); - lints = Collections.emptyList(); + lintsCheck = Collections.emptyList(); + lintsApply = Collections.emptyList(); } else { DirtyState.Calculation calculation = DirtyState.of(formatter, input); dirtyState = calculation.calculateDirtyState(); - lints = calculation.calculateLintAgainstRaw(); + lintsCheck = calculation.calculateLintAgainstRaw(); + lintsApply = calculation.calculateLintAgainstDirtyState(dirtyState, lintsCheck); } if (dirtyState.isClean()) { // Remove previous output if it exists @@ -115,25 +118,34 @@ private void processInputFile(@Nullable GitRatchet ratchet, Formatter formatter, dirtyState.writeCanonicalTo(output); } - File lint = getLintFile(input); + writeLints(lintsCheck, getLintCheckFile(input)); + writeLints(lintsApply, getLintApplyFile(input)); + } + + private void writeLints(List lints, File lintFile) throws IOException { if (lints.isEmpty()) { - Files.deleteIfExists(lint.toPath()); + Files.deleteIfExists(lintFile.toPath()); } else { - Lint.toFile(lints, lint); + Lint.toFile(lints, lintFile); } } private void deletePreviousResult(File input) throws IOException { delete(getOutputFile(input)); - delete(getLintFile(input)); + delete(getLintCheckFile(input)); + delete(getLintApplyFile(input)); } private File getOutputFile(File input) { return new File(contentDir(), relativize(input)); } - private File getLintFile(File input) { - return new File(lintDir(), relativize(input)); + private File getLintCheckFile(File input) { + return new File(lintCheckDir(), relativize(input)); + } + + private File getLintApplyFile(File input) { + return new File(lintApplyDir(), relativize(input)); } private void delete(File file) throws IOException { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 562dd85790..f8926f674f 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -96,11 +96,15 @@ static abstract class ClientTask extends DefaultTask { abstract Property getSpotlessOutDirectory(); File contentDir() { - return new File(getSpotlessOutDirectory().get(), SpotlessTaskImpl.CONTENT); + return new File(getSpotlessOutDirectory().get(), SpotlessTask.CONTENT); } - File lintDir() { - return new File(getSpotlessOutDirectory().get(), SpotlessTaskImpl.LINT); + File lintApplyDir() { + return new File(getSpotlessOutDirectory().get(), SpotlessTask.LINT_APPLY); + } + + File lintCheckDir() { + return new File(getSpotlessOutDirectory().get(), SpotlessTask.LINT_CHECK); } @Internal From cac4f706d4bee85af5a23740b4e85ee2f4d29ca3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 13 Jan 2022 22:56:32 -0800 Subject: [PATCH 08/31] Gradle spotlessCheck will now throw an error if all of the files are as formatted as they can be, except for lints. --- .../gradle/spotless/SpotlessCheck.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index 0d308abdbb..60e3388147 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -33,6 +33,7 @@ import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.FileSignature; +import com.diffplug.spotless.Lint; import com.diffplug.spotless.ThrowingEx; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; @@ -52,9 +53,11 @@ public void performAction() throws IOException { private void performAction(boolean isTest) throws IOException { ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(contentDir()); if (files.isEmpty()) { + checkForLint(); getState().setDidWork(sourceDidWork()); } else if (!isTest && applyHasRun()) { // if our matching apply has already run, then we don't need to do anything + checkForLint(); getState().setDidWork(false); } else { List problemFiles = new ArrayList<>(); @@ -105,6 +108,8 @@ public void visitFile(FileVisitDetails fileVisitDetails) { getEncoding().get()) .problemFiles(problemFiles) .getMessage()); + } else { + checkForLint(); } } } @@ -127,4 +132,29 @@ private String getTaskPathPrefix() { private static String calculateGradleCommand() { return FileSignature.machineIsWin() ? "gradlew.bat" : "./gradlew"; } + + private void checkForLint() { + File lintDir = applyHasRun() ? lintApplyDir() : lintCheckDir(); + ConfigurableFileTree lintFiles = getConfigCacheWorkaround().fileTree().from(lintDir); + List withLint = new ArrayList<>(); + lintFiles.visit(fileVisitDetails -> { + if (fileVisitDetails.isDirectory()) { + return; + } + try { + String path = fileVisitDetails.getPath(); + File originalSource = new File(getProjectDir().get().getAsFile(), path); + List lints = Lint.fromFile(fileVisitDetails.getFile()); + for (Lint lint : lints) { + System.err.println(path + ":" + lint.toString()); + } + withLint.add(originalSource); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + if (!withLint.isEmpty()) { + throw new GradleException("The files above cannot be fixed by spotlessApply"); + } + } } From d4ec76b5edc9d88cd4d9d9a0c917caf349088ae3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 13 Jan 2022 22:57:05 -0800 Subject: [PATCH 09/31] Maven spotless:check will now throw an error if all of the files are as formatted as they can be, except for lints. --- .../spotless/maven/SpotlessCheckMojo.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index b067bf8158..ebd53dbc70 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.TreeMap; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.LifecyclePhase; @@ -26,6 +27,7 @@ import com.diffplug.spotless.DirtyState; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.Lint; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; import com.diffplug.spotless.maven.incremental.UpToDateChecker; @@ -39,6 +41,7 @@ public class SpotlessCheckMojo extends AbstractSpotlessMojo { @Override protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { List problemFiles = new ArrayList<>(); + TreeMap> allLints = new TreeMap<>(); for (File file : files) { if (upToDateChecker.isUpToDate(file.toPath())) { if (getLog().isDebugEnabled()) { @@ -48,10 +51,17 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke } try { - DirtyState dirtyState = DirtyState.of(formatter, file).calculateDirtyState(); - if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { + DirtyState.Calculation calculation = DirtyState.of(formatter, file); + DirtyState dirtyState = calculation.calculateDirtyState(); + List lints = calculation.calculateLintAgainstDirtyState(dirtyState); + + if (!lints.isEmpty()) { + allLints.put(file, lints); + } + if (!dirtyState.isClean()) { problemFiles.add(file); - } else { + } + if (lints.isEmpty() && dirtyState.isClean()) { upToDateChecker.setUpToDate(file.toPath()); } } catch (IOException e) { @@ -66,5 +76,13 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke .problemFiles(problemFiles) .getMessage()); } + if (!allLints.isEmpty()) { + allLints.forEach((file, lints) -> { + for (Lint lint : lints) { + System.err.println(file.getAbsolutePath() + ":" + lint.toString()); + } + }); + throw new MojoExecutionException("'mvn spotless:apply' cannot fix these violations."); + } } } From 7a1e72a4de196d6b7450119dc3ae2d637b83142e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 00:32:17 -0800 Subject: [PATCH 10/31] Default `FormatterFunc.lint` now traps errors from the formatter itself, and tries to parse a line number from the error message. --- .../java/com/diffplug/spotless/Formatter.java | 6 +-- .../com/diffplug/spotless/FormatterFunc.java | 8 +++- .../main/java/com/diffplug/spotless/Lint.java | 39 +++++++++++++++++++ .../com/diffplug/spotless/ThrowingEx.java | 13 ++++++- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 673b9ea987..c524018997 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -173,8 +173,7 @@ public String compute(String unix, File file) { unix = LineEnding.toUnix(formatted); } } catch (Throwable e) { - String relativePath = rootDir.relativize(file.toPath()).toString(); - exceptionPolicy.handleError(e, step, relativePath); + // we ignore exceptions in format because we collect them in lint } } return unix; @@ -192,8 +191,7 @@ public List lint(String content, File file) { totalLints.addAll(lints); } } catch (Throwable e) { - String relativePath = rootDir.relativize(file.toPath()).toString(); - exceptionPolicy.handleError(e, step, relativePath); + totalLints.add(Lint.createFromThrowable(step, e)); } } if (totalLints.isEmpty()) { diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index 2ba5b1474d..a9ba7d8048 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -34,8 +34,12 @@ default String apply(String unix, File file) throws Exception { return apply(unix); } - /** Calculates a list of lints against the given content. */ - default List lint(String content, File file) { + /** + * 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 { + apply(content, file); return Collections.emptyList(); } diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index d02e3cc590..c22595fd8b 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -164,4 +164,43 @@ public static void toFile(List lints, File file) throws IOException { byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); Files.write(file.toPath(), content); } + + /** Attempts to parse a line number from the given exception. */ + static Lint createFromThrowable(FormatterStep step, Throwable e) { + Throwable current = e; + while (current != null) { + String message = current.getMessage(); + int lineNumber = lineNumberFor(message); + if (lineNumber != -1) { + return Lint.create(step.getName(), msgFrom(message), lineNumber); + } + current = current.getCause(); + } + return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1); + } + + 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 f664c62bf3..0868228977 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-2021 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. @@ -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 109c7fb8bf88a3130649f9a490b2e033d527f33c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 00:52:40 -0800 Subject: [PATCH 11/31] Remove `FormatExceptionPolicy`. --- CHANGES.md | 1 + .../spotless/FormatExceptionPolicy.java | 39 ------------- .../spotless/FormatExceptionPolicyLegacy.java | 43 -------------- .../spotless/FormatExceptionPolicyStrict.java | 58 ------------------- .../java/com/diffplug/spotless/Formatter.java | 23 +------- .../com/diffplug/spotless/FormatterTest.java | 18 +----- 6 files changed, 5 insertions(+), 177 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java delete mode 100644 lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java diff --git a/CHANGES.md b/CHANGES.md index 785048654b..64d83b90aa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Removed `isClean`, `applyTo`, and `applyToAndReturnResultIfDirty` from `Formatter` because users should instead use `PaddedCell.check()`. * **BREAKING** Removed `FormatterStep.Strict` because it was unnecessary and unused implementation detail. * **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. +* **BREAKING** Removed `FormatExceptionPolicy` and its subclasses because exceptions are now wrapped as lints. ## [Unreleased] ### Changed 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 da23f74432..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicy.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2016-2021 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/FormatExceptionPolicyLegacy.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java deleted file mode 100644 index df95542a44..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyLegacy.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2016 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.logging.Level; -import java.util.logging.Logger; - -class FormatExceptionPolicyLegacy extends NoLambda.EqualityBasedOnSerialization implements FormatExceptionPolicy { - private static final long serialVersionUID = 1L; - - private static final Logger logger = Logger.getLogger(Formatter.class.getName()); - - @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.log(Level.SEVERE, "Step '" + step.getName() + "' found problem in '" + relativePath + "':\n" + e.getMessage(), e); - } - - static void warning(Throwable e, FormatterStep step, String relativePath) { - logger.log(Level.WARNING, "Unable to apply step '" + step.getName() + "' to '" + relativePath + "'", e); - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java b/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java deleted file mode 100644 index 6fd8371928..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/FormatExceptionPolicyStrict.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2016 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.spotless; - -import java.util.Objects; -import java.util.Set; -import java.util.TreeSet; - -/** - * A policy for handling exceptions in the format. Any exceptions will - * halt the build except for a specifically excluded path or step. - */ -public class FormatExceptionPolicyStrict extends NoLambda.EqualityBasedOnSerialization implements FormatExceptionPolicy { - private static final long serialVersionUID = 1L; - - private final Set excludeSteps = new TreeSet<>(); - private final Set excludePaths = new TreeSet<>(); - - /** Adds a step name to exclude. */ - public void excludeStep(String stepName) { - excludeSteps.add(Objects.requireNonNull(stepName)); - } - - /** Adds a relative path to exclude. */ - public void excludePath(String relativePath) { - excludePaths.add(Objects.requireNonNull(relativePath)); - } - - @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); - } else { - if (excludePaths.contains(relativePath)) { - FormatExceptionPolicyLegacy.warning(e, step, relativePath); - } else { - FormatExceptionPolicyLegacy.error(e, step, relativePath); - throw ThrowingEx.asRuntimeRethrowError(e); - } - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index c524018997..685131dd16 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -39,14 +39,12 @@ public final class Formatter implements Serializable, AutoCloseable { private Charset encoding; private Path rootDir; private List steps; - private FormatExceptionPolicy exceptionPolicy; - private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps, FormatExceptionPolicy exceptionPolicy) { + private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); this.encoding = Objects.requireNonNull(encoding, "encoding"); this.rootDir = Objects.requireNonNull(rootDirectory, "rootDir"); this.steps = requireElementsNonNull(new ArrayList<>(steps)); - this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy, "exceptionPolicy"); } // override serialize output @@ -55,7 +53,6 @@ private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(encoding.name()); out.writeObject(rootDir.toString()); out.writeObject(steps); - out.writeObject(exceptionPolicy); } // override serialize input @@ -65,7 +62,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE encoding = Charset.forName((String) in.readObject()); rootDir = Paths.get((String) in.readObject()); steps = (List) in.readObject(); - exceptionPolicy = (FormatExceptionPolicy) in.readObject(); } // override serialize input @@ -90,10 +86,6 @@ public List getSteps() { return steps; } - public FormatExceptionPolicy getExceptionPolicy() { - return exceptionPolicy; - } - public static Formatter.Builder builder() { return new Formatter.Builder(); } @@ -104,7 +96,6 @@ public static class Builder { private Charset encoding; private Path rootDir; private List steps; - private FormatExceptionPolicy exceptionPolicy; private Builder() {} @@ -128,14 +119,8 @@ public Builder steps(List steps) { return this; } - public Builder exceptionPolicy(FormatExceptionPolicy exceptionPolicy) { - this.exceptionPolicy = exceptionPolicy; - return this; - } - public Formatter build() { - return new Formatter(lineEndingsPolicy, encoding, rootDir, steps, - exceptionPolicy == null ? FormatExceptionPolicy.failOnlyOnError() : exceptionPolicy); + return new Formatter(lineEndingsPolicy, encoding, rootDir, steps); } } @@ -209,7 +194,6 @@ public int hashCode() { result = prime * result + lineEndingsPolicy.hashCode(); result = prime * result + rootDir.hashCode(); result = prime * result + steps.hashCode(); - result = prime * result + exceptionPolicy.hashCode(); return result; } @@ -228,8 +212,7 @@ public boolean equals(Object obj) { return encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && rootDir.equals(other.rootDir) && - steps.equals(other.steps) && - exceptionPolicy.equals(other.exceptionPolicy); + steps.equals(other.steps); } @SuppressWarnings("rawtypes") diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index fc56332b2b..60bee8a01e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -36,7 +36,6 @@ void equality() { private Charset encoding = StandardCharsets.UTF_8; private Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); private List steps = new ArrayList<>(); - private FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError(); @Override protected void setupTest(API api) throws Exception { @@ -53,20 +52,6 @@ protected void setupTest(API api) throws Exception { steps.add(EndWithNewlineStep.create()); api.areDifferentThan(); - - { - FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); - standard.excludePath("path"); - exceptionPolicy = standard; - api.areDifferentThan(); - } - - { - FormatExceptionPolicyStrict standard = new FormatExceptionPolicyStrict(); - standard.excludeStep("step"); - exceptionPolicy = standard; - api.areDifferentThan(); - } } @Override @@ -76,7 +61,6 @@ protected Formatter create() { .encoding(encoding) .rootDir(rootDir) .steps(steps) - .exceptionPolicy(exceptionPolicy) .build(); } }.testEquals(); From 00496612d86c42aca509160277f0502f4df25d0b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 00:57:18 -0800 Subject: [PATCH 12/31] Remove `FormatExceptionPolicy` from the maven plugin. --- .../java/com/diffplug/spotless/maven/FormatterFactory.java | 4 +--- .../diffplug/spotless/maven/incremental/NoopCheckerTest.java | 2 -- .../spotless/maven/incremental/PluginFingerprintTest.java | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index c06822486c..f4b7453092 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -30,7 +30,6 @@ import org.apache.maven.plugins.annotations.Parameter; import com.diffplug.common.collect.Sets; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -93,7 +92,6 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form return Formatter.builder() .encoding(formatterEncoding) .lineEndingsPolicy(formatterLineEndingPolicy) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .steps(formatterSteps) .rootDir(config.getFileLocator().getBaseDir().toPath()) .build(); diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java index 404b5e2191..cc1c94b0a8 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java @@ -37,7 +37,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -120,7 +119,6 @@ private static Formatter dummyFormatter() { .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(UTF_8) .steps(singletonList(mock(FormatterStep.class, withSettings().serializable()))) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(); } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index 90e59d657e..c9920396f1 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -32,7 +32,6 @@ import org.codehaus.plexus.util.xml.XmlStreamReader; import org.junit.jupiter.api.Test; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -250,7 +249,6 @@ private static Formatter formatter(LineEnding lineEnding, FormatterStep... steps .lineEndingsPolicy(lineEnding.createPolicy()) .encoding(UTF_8) .steps(Arrays.asList(steps)) - .exceptionPolicy(new FormatExceptionPolicyStrict()) .build(); } } From dd5165f2e62b8e8976ea9be57b94189620d6c013 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 01:31:18 -0800 Subject: [PATCH 13/31] Replace `FormatExceptionPolicy` with `LintPolicy` inside the Gradle plugin. --- .../gradle/spotless/FormatExtension.java | 15 +++--- .../diffplug/gradle/spotless/LintPolicy.java | 46 +++++++++++++++++++ .../gradle/spotless/SpotlessCheck.java | 27 ++++++++--- .../gradle/spotless/SpotlessTask.java | 13 ++---- .../spotless/ErrorShouldRethrowTest.java | 26 ++++------- 5 files changed, 90 insertions(+), 37 deletions(-) create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/LintPolicy.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 9da66b7929..b7fba06d36 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -39,7 +39,6 @@ import org.gradle.api.file.FileCollection; import com.diffplug.common.base.Preconditions; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LazyForwardingEquality; @@ -137,16 +136,20 @@ public void setEncoding(Charset charset) { encoding = Objects.requireNonNull(charset); } - final FormatExceptionPolicyStrict exceptionPolicy = new FormatExceptionPolicyStrict(); + final LintPolicy lintPolicy = new LintPolicy(); /** Ignores errors in the given step. */ + @Deprecated public void ignoreErrorForStep(String stepName) { - exceptionPolicy.excludeStep(Objects.requireNonNull(stepName)); + // TODO: deprecation message + lintPolicy.excludeStep(Objects.requireNonNull(stepName)); } /** Ignores errors for the given relative path. */ + @Deprecated public void ignoreErrorForPath(String relativePath) { - exceptionPolicy.excludePath(Objects.requireNonNull(relativePath)); + // TODO: deprecation message + lintPolicy.excludePath(Objects.requireNonNull(relativePath)); } /** Sets encoding to use (defaults to {@link SpotlessExtensionImpl#getEncoding()}). */ @@ -745,7 +748,7 @@ public void toggleOffOnDisable() { /** Sets up a format task according to the values in this extension. */ protected void setupTask(SpotlessTask task) { task.setEncoding(getEncoding().name()); - task.setExceptionPolicy(exceptionPolicy); + task.setLintPolicy(lintPolicy); FileCollection totalTarget = targetExclude == null ? target : target.minus(targetExclude); task.setTarget(totalTarget); List steps; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/LintPolicy.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/LintPolicy.java new file mode 100644 index 0000000000..9ef4fa0bde --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/LintPolicy.java @@ -0,0 +1,46 @@ +/* + * Copyright 2022 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.gradle.spotless; + +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; + +import com.diffplug.spotless.Lint; +import com.diffplug.spotless.NoLambda; + +public class LintPolicy extends NoLambda.EqualityBasedOnSerialization { + private final Set excludeSteps = new TreeSet<>(); + private final Set excludePaths = new TreeSet<>(); + + /** Adds a step name to exclude. */ + public void excludeStep(String stepName) { + excludeSteps.add(Objects.requireNonNull(stepName)); + } + + /** Adds a relative path to exclude. */ + public void excludePath(String relativePath) { + excludePaths.add(Objects.requireNonNull(relativePath)); + } + + public boolean runLintOn(String path) { + return !excludePaths.contains(path); + } + + public boolean includeLint(String path, Lint lint) { + return runLintOn(path) && !excludeSteps.contains(lint.getCode()); + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index 60e3388147..14b77bd702 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -29,6 +29,7 @@ import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; @@ -41,6 +42,9 @@ public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { @Internal public abstract Property getEncoding(); + @Input + public abstract Property getLintPolicy(); + public void performActionTest() throws IOException { performAction(true); } @@ -122,6 +126,7 @@ void init(SpotlessTaskImpl impl) { super.init(impl); getProjectPath().set(getProject().getPath()); getEncoding().set(impl.getEncoding()); + getLintPolicy().set(impl.getLintPolicy()); } private String getTaskPathPrefix() { @@ -134,27 +139,37 @@ private static String calculateGradleCommand() { } private void checkForLint() { + LintPolicy lintPolicy = getLintPolicy().get(); File lintDir = applyHasRun() ? lintApplyDir() : lintCheckDir(); ConfigurableFileTree lintFiles = getConfigCacheWorkaround().fileTree().from(lintDir); List withLint = new ArrayList<>(); + StringBuilder errorMsg = new StringBuilder(); lintFiles.visit(fileVisitDetails -> { if (fileVisitDetails.isDirectory()) { return; } try { String path = fileVisitDetails.getPath(); - File originalSource = new File(getProjectDir().get().getAsFile(), path); - List lints = Lint.fromFile(fileVisitDetails.getFile()); - for (Lint lint : lints) { - System.err.println(path + ":" + lint.toString()); + if (lintPolicy.runLintOn(path)) { + File originalSource = new File(getProjectDir().get().getAsFile(), path); + List lints = Lint.fromFile(fileVisitDetails.getFile()); + boolean hasLints = false; + for (Lint lint : lints) { + if (lintPolicy.includeLint(path, lint)) { + hasLints = true; + errorMsg.append(path + ":" + lint.toString() + "\n"); + } + } + if (hasLints) { + withLint.add(originalSource); + } } - withLint.add(originalSource); } catch (IOException e) { throw new RuntimeException(e); } }); if (!withLint.isEmpty()) { - throw new GradleException("The files above cannot be fixed by spotlessApply"); + throw new GradleException("The files below cannot be fixed by spotlessApply\n" + errorMsg); } } } 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 e99ea766d0..1cd32b1c12 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,8 +37,6 @@ import org.gradle.work.Incremental; import com.diffplug.gradle.spotless.JvmLocalCache.LiveCache; -import com.diffplug.spotless.FormatExceptionPolicy; -import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -119,15 +117,15 @@ public ObjectId getRatchetSha() { return subtreeSha; } - protected FormatExceptionPolicy exceptionPolicy = new FormatExceptionPolicyStrict(); + protected LintPolicy lintPolicy = new LintPolicy(); - public void setExceptionPolicy(FormatExceptionPolicy exceptionPolicy) { - this.exceptionPolicy = Objects.requireNonNull(exceptionPolicy); + public void setLintPolicy(LintPolicy lintPolicy) { + this.lintPolicy = Objects.requireNonNull(lintPolicy); } @Input - public FormatExceptionPolicy getExceptionPolicy() { - return exceptionPolicy; + public LintPolicy getLintPolicy() { + return lintPolicy; } protected FileCollection target; @@ -204,7 +202,6 @@ Formatter buildFormatter() { .encoding(Charset.forName(encoding)) .rootDir(getProjectDir().get().getAsFile().toPath()) .steps(steps.get()) - .exceptionPolicy(exceptionPolicy) .build(); } } 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 d1ced01609..e4bede497c 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -66,10 +66,8 @@ void anyExceptionShouldFail() throws Exception { "} // spotless"); 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!"); + "> The files below cannot be fixed by spotlessApply\n" + + " README.md:1: (no swearing) java.lang.RuntimeException: No swearing!"); } @Test @@ -89,8 +87,7 @@ void unlessExemptedByStep() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithSuccess("> Task :spotlessMisc\n" + - "Unable to apply step 'no swearing' to 'README.md'"); + runWithSuccess("> Task :spotlessMisc"); } @Test @@ -100,8 +97,7 @@ void unlessExemptedByPath() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithSuccess("> Task :spotlessMisc\n" + - "Unable to apply step 'no swearing' to 'README.md'"); + runWithSuccess("> Task :spotlessMisc"); } @Test @@ -112,10 +108,8 @@ void failsIfNeitherStepNorFileExempted() throws Exception { " } // format", "} // spotless"); setFile("README.md").toContent("This code is fubar."); - runWithFailure("> Task :spotlessMisc FAILED\n" + - "Step 'no swearing' found problem in 'README.md':\n" + - "No swearing!\n" + - "java.lang.RuntimeException: No swearing!"); + runWithFailure("> The files below cannot be fixed by spotlessApply\n" + + " README.md:1: (no swearing) java.lang.RuntimeException: No swearing!"); } private void runWithSuccess(String expectedToStartWith) throws Exception { @@ -124,13 +118,13 @@ private void runWithSuccess(String expectedToStartWith) throws Exception { } private void runWithFailure(String expectedToStartWith) throws Exception { - BuildResult result = gradleRunner().withArguments("check").buildAndFail(); + BuildResult result = gradleRunner().forwardOutput().withArguments("check").buildAndFail(); assertResultAndMessages(result, TaskOutcome.FAILED, expectedToStartWith); } private void assertResultAndMessages(BuildResult result, TaskOutcome outcome, String expectedToStartWith) { String output = result.getOutput(); - int register = output.indexOf(":spotlessInternalRegisterDependencies"); + int register = output.indexOf("Execution failed for task ':spotlessMiscCheck'."); int firstNewlineAfterThat = output.indexOf('\n', register + 1); String useThisToMatch = output.substring(firstNewlineAfterThat); @@ -138,7 +132,5 @@ private void assertResultAndMessages(BuildResult result, TaskOutcome outcome, St List actualLines = Splitter.on('\n').splitToList(LineEnding.toUnix(useThisToMatch.trim())); String actualStart = String.join("\n", actualLines.subList(0, numNewlines + 1)); Assertions.assertThat(actualStart).isEqualTo(expectedToStartWith); - Assertions.assertThat(outcomes(result, outcome).size() + outcomes(result, TaskOutcome.UP_TO_DATE).size() + outcomes(result, TaskOutcome.NO_SOURCE).size()) - .isEqualTo(outcomes(result).size()); } } From 85fbe71d78283656120d7c3db76091c731e0b377 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 01:41:39 -0800 Subject: [PATCH 14/31] Lint.toFile might need to create parent directories. --- lib/src/main/java/com/diffplug/spotless/Lint.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index c22595fd8b..c3428e3ab2 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -161,6 +161,7 @@ public static List fromFile(File file) throws IOException { } public static void toFile(List lints, File file) throws IOException { + file.getParentFile().mkdirs(); byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); Files.write(file.toPath(), content); } From e8e5df5781a4c6dda73ffbe0c1cbeb3c8d52d19c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 01:42:00 -0800 Subject: [PATCH 15/31] Lints are now quietly suppressed during `apply`, you have to do `check` to see them. --- .../com/diffplug/gradle/spotless/KotlinExtensionTest.java | 4 ++-- .../diffplug/gradle/spotless/KotlinGradleExtensionTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 f3d8ad442d..e0dcfc8eba 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -135,7 +135,7 @@ void testWithIndentation() throws IOException { " }", "}"); setFile("src/main/kotlin/basic.kt").toResource("kotlin/ktlint/basic.dirty"); - BuildResult result = gradleRunner().withArguments("spotlessApply").buildAndFail(); + BuildResult result = gradleRunner().withArguments("spotlessCheck", "--stacktrace").buildAndFail(); assertThat(result.getOutput()).contains("Unexpected indentation (4) (it should be 6)"); } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinGradleExtensionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinGradleExtensionTest.java index c68de0d7e7..ce62cf51c7 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinGradleExtensionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/KotlinGradleExtensionTest.java @@ -89,7 +89,7 @@ void integration_default_diktat() throws IOException { " }", "}"); setFile("configuration.gradle.kts").toResource("kotlin/diktat/basic.dirty"); - BuildResult result = gradleRunner().withArguments("spotlessApply").buildAndFail(); + BuildResult result = gradleRunner().withArguments("spotlessCheck").buildAndFail(); assertThat(result.getOutput()).contains("[AVOID_NESTED_FUNCTIONS] try to avoid using nested functions"); } From 6ec06cc4672cd375c6e9454312293de6c978e7fc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 02:42:06 -0800 Subject: [PATCH 16/31] Fix spotbugs warnings. --- lib/src/main/java/com/diffplug/spotless/FormatterStep.java | 2 +- lib/src/main/java/com/diffplug/spotless/Lint.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 7ddaf65782..5a8812907d 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -57,7 +57,7 @@ public interface FormatterStep extends Serializable { * @return a list of lints * @throws Exception if the formatter step experiences a problem */ - public List lint(String content, File file) throws Exception; + public @Nullable List lint(String content, File file) throws Exception; /** * Returns a new FormatterStep which will only apply its changes diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index c3428e3ab2..ad3b8fa159 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -161,7 +161,10 @@ public static List fromFile(File file) throws IOException { } public static void toFile(List lints, File file) throws IOException { - file.getParentFile().mkdirs(); + boolean success = file.getParentFile().mkdirs(); + if (!success) { + throw new IllegalArgumentException("Failed to create parent directory to " + file); + } byte[] content = toString(lints).getBytes(StandardCharsets.UTF_8); Files.write(file.toPath(), content); } From 9557c66cdfff70bc17fa932329283ac9b83e4ae0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 03:02:22 -0800 Subject: [PATCH 17/31] Fix the bug I made from fixing spotbugs :) --- lib/src/main/java/com/diffplug/spotless/Lint.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index ad3b8fa159..8aa94293b1 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -161,12 +162,14 @@ public static List fromFile(File file) throws IOException { } public static void toFile(List lints, File file) throws IOException { - boolean success = file.getParentFile().mkdirs(); - if (!success) { - throw new IllegalArgumentException("Failed to create parent directory to " + file); + 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(file.toPath(), content); + Files.write(path, content); } /** Attempts to parse a line number from the given exception. */ From 2844068a1ef05247f56e69616b4cdc22a105ebb5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 11:01:21 -0800 Subject: [PATCH 18/31] Make sure that FormatterFunc.Closeable's helpers support linting. --- CHANGES.md | 2 ++ .../com/diffplug/spotless/FormatterFunc.java | 36 ++++++++++++++----- .../spotless/generic/PipeStepPair.java | 15 ++++++-- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 64d83b90aa..90286a43c8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Removed `FormatterStep.Strict` because it was unnecessary and unused implementation detail. * **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. * **BREAKING** Removed `FormatExceptionPolicy` and its subclasses because exceptions are now wrapped as lints. +* **BREAKING** Removed deprecated methods. + * `FormatterFunc.Closeable::of(AutoCloseable, FormatterFunc)` ## [Unreleased] ### Changed diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java index a9ba7d8048..48eda71322 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java @@ -61,7 +61,7 @@ interface Closeable extends FormatterFunc, AutoCloseable { * The bug (and its fix) which is easy to write using this method: https://github.com/diffplug/spotless/commit/7f16ecca031810b5e6e6f647e1f10a6d2152d9f4 * How the {@code of()} methods below make the correct thing easier to write and safer: https://github.com/diffplug/spotless/commit/18c10f9c93d6f18f753233d0b5f028d5f0961916 */ - public static Closeable ofDangerous(AutoCloseable closeable, FormatterFunc function) { + static Closeable ofDangerous(AutoCloseable closeable, FormatterFunc function) { Objects.requireNonNull(closeable, "closeable"); Objects.requireNonNull(function, "function"); return new Closeable() { @@ -79,22 +79,26 @@ public String apply(String unix, File file) throws Exception { public String apply(String unix) throws Exception { return function.apply(unix); } - }; - } - /** @deprecated synonym for {@link #ofDangerous(AutoCloseable, FormatterFunc)} */ - @Deprecated - public static Closeable of(AutoCloseable closeable, FormatterFunc function) { - return ofDangerous(closeable, function); + @Override + public List lint(String content, File file) throws Exception { + return function.lint(content, file); + } + }; } @FunctionalInterface interface ResourceFunc { String apply(T resource, String unix) throws Exception; + + default List lint(T resource, String content) throws Exception { + apply(resource, content); + return Collections.emptyList(); + } } /** Creates a {@link FormatterFunc.Closeable} which uses the given resource to execute the format function. */ - public static Closeable of(T resource, ResourceFunc function) { + static Closeable of(T resource, ResourceFunc function) { Objects.requireNonNull(resource, "resource"); Objects.requireNonNull(function, "function"); return new Closeable() { @@ -112,15 +116,24 @@ public String apply(String unix, File file) throws Exception { public String apply(String unix) throws Exception { return function.apply(resource, unix); } + + @Override + public List lint(String content, File file) throws Exception { + return function.lint(resource, content); + } }; } @FunctionalInterface interface ResourceFuncNeedsFile { String apply(T resource, String unix, File file) throws Exception; + + default List lint(T resource, String content, File file) throws Exception { + apply(resource, content, file); + return Collections.emptyList(); + } } - /** Creates a {@link FormatterFunc.Closeable} which uses the given resource to execute the file-dependent format function. */ public static Closeable of(T resource, ResourceFuncNeedsFile function) { Objects.requireNonNull(resource, "resource"); Objects.requireNonNull(function, "function"); @@ -140,6 +153,11 @@ public String apply(String unix, File file) throws Exception { public String apply(String unix) throws Exception { return apply(unix, FormatterStepImpl.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/generic/PipeStepPair.java b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java index 38373fec59..3fcdc8f2fe 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 DiffPlug + * Copyright 2020-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,7 @@ 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; @@ -84,7 +85,17 @@ public PipeStepPair buildPair() { public FormatterStep buildStepWhichAppliesSubSteps(Path rootPath, Collection steps) { return FormatterStep.createLazy(name, () -> new StateApplyToBlock(regex, steps), - state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state::format)); + state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), new FormatterFunc.Closeable.ResourceFuncNeedsFile() { + @Override + public String apply(Formatter formatter, String unix, File file) { + return formatter.compute(unix, file); + } + + @Override + public List lint(Formatter formatter, String content, File file) throws Exception { + return formatter.lint(content, file); + } + })); } } From 9134d8ce2e8ed306c4202122c7b01cd41e50e192 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 11:35:05 -0800 Subject: [PATCH 19/31] Fix FormatterFunc.Closeable earlier --- .../spotless/generic/PipeStepPair.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java index 3fcdc8f2fe..9dfad77929 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java @@ -30,7 +30,6 @@ 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; @@ -81,21 +80,14 @@ public PipeStepPair buildPair() { return new PipeStepPair(name, regex); } - /** Returns a single step which will apply the given steps only within the blocks selected by the regex / openClose pair. */ + /** + * Returns a single step which will apply the given steps only within the blocks selected by the regex / openClose pair. + * Linting within the substeps is not supported. + */ public FormatterStep buildStepWhichAppliesSubSteps(Path rootPath, Collection steps) { return FormatterStep.createLazy(name, () -> new StateApplyToBlock(regex, steps), - state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), new FormatterFunc.Closeable.ResourceFuncNeedsFile() { - @Override - public String apply(Formatter formatter, String unix, File file) { - return formatter.compute(unix, file); - } - - @Override - public List lint(Formatter formatter, String content, File file) throws Exception { - return formatter.lint(content, file); - } - })); + state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state::format)); } } @@ -215,6 +207,7 @@ private static String stateOutCompute(StateIn in, StringBuilder builder, String } else { pattern = in.regex.pattern(); } + System.out.println("ERROR"); throw new Error("An intermediate step removed a match of " + pattern); } } From 5a9799d2b8f4f4ec9ba7552d948527f34917ddc3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 15:10:03 -0800 Subject: [PATCH 20/31] Refactor PipeStepPair so that it can detect lost tags within our new lint paradigm. The old way made it impossible to detect lost tags if `apply` always succeeds. --- .../spotless/generic/PipeStepPair.java | 264 ++++++++++-------- .../gradle/spotless/FormatExtension.java | 14 +- .../spotless/maven/FormatterFactory.java | 8 +- .../spotless/maven/generic/ToggleOffOn.java | 6 +- .../spotless/generic/PipeStepPairTest.java | 42 ++- 5 files changed, 174 insertions(+), 160 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java index 9dfad77929..7cc830bbeb 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java @@ -20,7 +20,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.regex.Matcher; @@ -30,13 +30,12 @@ import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import com.diffplug.spotless.Lint; public class PipeStepPair { - /** The two steps will be named {@code In} and {@code Out}. */ - public static Builder named(String name) { - return new Builder(name); + /** Declares the name of the step. */ + public static PipeStepPair named(String name) { + return new PipeStepPair(name); } public static String defaultToggleName() { @@ -51,164 +50,185 @@ public static String defaultToggleOn() { return "spotless:on"; } - public static class Builder { - String name; - Pattern regex; - - private Builder(String name) { - this.name = Objects.requireNonNull(name); - } - - /** Defines the opening and closing markers. */ - public Builder openClose(String open, String close) { - return regex(Pattern.quote(open) + "([\\s\\S]*?)" + Pattern.quote(close)); - } - - /** Defines the pipe via regex. Must have *exactly one* capturing group. */ - public Builder regex(String regex) { - return regex(Pattern.compile(regex)); - } + String name; + Pattern regex; - /** Defines the pipe via regex. Must have *exactly one* capturing group. */ - public Builder regex(Pattern regex) { - this.regex = Objects.requireNonNull(regex); - return this; - } - - /** Returns a pair of steps which captures in the first part, then returns in the second. */ - public PipeStepPair buildPair() { - return new PipeStepPair(name, regex); - } - - /** - * Returns a single step which will apply the given steps only within the blocks selected by the regex / openClose pair. - * Linting within the substeps is not supported. - */ - public FormatterStep buildStepWhichAppliesSubSteps(Path rootPath, Collection steps) { - return FormatterStep.createLazy(name, - () -> new StateApplyToBlock(regex, steps), - state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state::format)); - } + private PipeStepPair(String name) { + this.name = Objects.requireNonNull(name); } - final FormatterStep in, out; - - private PipeStepPair(String name, Pattern pattern) { - StateIn stateIn = new StateIn(pattern); - StateOut stateOut = new StateOut(stateIn); - in = FormatterStep.create(name + "In", stateIn, state -> state::format); - out = FormatterStep.create(name + "Out", stateOut, state -> state::format); + /** Defines the opening and closing markers. */ + public PipeStepPair openClose(String open, String close) { + return regex(Pattern.quote(open) + "([\\s\\S]*?)" + Pattern.quote(close)); } - public FormatterStep in() { - return in; + /** Defines the pipe via regex. Must have *exactly one* capturing group. */ + public PipeStepPair regex(String regex) { + return regex(Pattern.compile(regex)); } - public FormatterStep out() { - return out; + /** Defines the pipe via regex. Must have *exactly one* capturing group. */ + public PipeStepPair regex(Pattern regex) { + this.regex = Objects.requireNonNull(regex); + return this; } - @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class StateApplyToBlock extends StateIn implements Serializable { - private static final long serialVersionUID = -844178006407733370L; + private void assertRegexSet() { + Objects.requireNonNull(regex, "must call regex() or openClose()"); + } - final List steps; - final transient StringBuilder builder = new StringBuilder(); + /** Returns a step which will apply the given steps but preserve the content selected by the regex / openClose pair. */ + public FormatterStep preserveWithin(Path rootPath, List steps) { + assertRegexSet(); + return FormatterStep.createLazy(name, + () -> new PreserveWithin(regex, steps), + state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state)); + } - StateApplyToBlock(Pattern regex, Collection steps) { - super(regex); - this.steps = new ArrayList<>(steps); - } + /** + * Returns a step which will apply the given steps only within the blocks selected by the regex / openClose pair. + * Linting within the substeps is not supported. + */ + public FormatterStep applyWithin(Path rootPath, List steps) { + assertRegexSet(); + return FormatterStep.createLazy(name, + () -> new ApplyWithin(regex, steps), + state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state)); + } - Formatter buildFormatter(Path rootDir) { - return Formatter.builder() - .encoding(StandardCharsets.UTF_8) // can be any UTF, doesn't matter - .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) // just internal, won't conflict with user - .steps(steps) - .rootDir(rootDir) - .build(); + static class ApplyWithin extends Apply implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + ApplyWithin(Pattern regex, List steps) { + super(regex, steps); } - private String format(Formatter formatter, String unix, File file) throws Exception { - groups.clear(); + @Override + public String apply(Formatter formatter, String unix, File file) throws Exception { + List groups = groupsZeroed(); Matcher matcher = regex.matcher(unix); while (matcher.find()) { // apply the formatter to each group groups.add(formatter.compute(matcher.group(1), file)); } // and then assemble the result right away - return stateOutCompute(this, builder, unix); + return assembleGroups(unix); } } - @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class StateIn implements Serializable { - private static final long serialVersionUID = -844178006407733370L; - - final Pattern regex; - - public StateIn(Pattern regex) { - this.regex = Objects.requireNonNull(regex); + static class PreserveWithin extends Apply implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + PreserveWithin(Pattern regex, List steps) { + super(regex, steps); } - final transient ArrayList groups = new ArrayList<>(); - - private String format(String unix) throws Exception { - groups.clear(); + private void storeGroups(String unix) { + List groups = groupsZeroed(); Matcher matcher = regex.matcher(unix); while (matcher.find()) { + // store whatever is within the open/close tags groups.add(matcher.group(1)); } - return unix; + } + + @Override + public String apply(Formatter formatter, String unix, File file) throws Exception { + storeGroups(unix); + String formatted = formatter.compute(unix, file); + return assembleGroups(formatted); + } + + @Override + public List lint(Formatter formatter, String content, File file) throws Exception { + // first make sure that all tags are preserved, and bail if they aren't + try { + apply(formatter, content, file); + } catch (IntermediateStepRemovedException e) { + return Collections.singletonList(e.lint); + } + // because the tags are preserved, now we can let the underlying lints run + return formatter.lint(content, file); } } - @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - static class StateOut implements Serializable { - private static final long serialVersionUID = -1195263184715054229L; + static class Apply implements Serializable { + final Pattern regex; + final List steps; - final StateIn in; + transient ArrayList groups = new ArrayList<>(); + transient StringBuilder builderInternal; - StateOut(StateIn in) { - this.in = Objects.requireNonNull(in); + public Apply(Pattern regex, List steps) { + this.regex = regex; + this.steps = steps; } - final transient StringBuilder builder = new StringBuilder(); - - private String format(String unix) { - return stateOutCompute(in, builder, unix); + protected ArrayList groupsZeroed() { + if (groups == null) { + groups = new ArrayList<>(); + } else { + groups.clear(); + } + return groups; } - } - private static String stateOutCompute(StateIn in, StringBuilder builder, String unix) { - if (in.groups.isEmpty()) { - return unix; + private StringBuilder builderZeroed() { + if (builderInternal == null) { + builderInternal = new StringBuilder(); + } else { + builderInternal.setLength(0); + } + return builderInternal; } - builder.setLength(0); - Matcher matcher = in.regex.matcher(unix); - int lastEnd = 0; - int groupIdx = 0; - while (matcher.find()) { - builder.append(unix, lastEnd, matcher.start(1)); - builder.append(in.groups.get(groupIdx)); - lastEnd = matcher.end(1); - ++groupIdx; + + protected Formatter buildFormatter(Path rootDir) { + return Formatter.builder() + .encoding(StandardCharsets.UTF_8) // can be any UTF, doesn't matter + .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) // just internal, won't conflict with user + .steps(steps) + .rootDir(rootDir) + .build(); } - if (groupIdx == in.groups.size()) { - builder.append(unix, lastEnd, unix.length()); - return builder.toString(); - } else { - // 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") - .matcher(in.regex.pattern()); - String pattern; - if (openClose.matches()) { - pattern = openClose.group(1) + " " + openClose.group(2); + + protected String assembleGroups(String unix) throws IntermediateStepRemovedException { + if (groups.isEmpty()) { + return unix; + } + StringBuilder builder = builderZeroed(); + Matcher matcher = regex.matcher(unix); + int lastEnd = 0; + int groupIdx = 0; + while (matcher.find()) { + builder.append(unix, lastEnd, matcher.start(1)); + builder.append(groups.get(groupIdx)); + lastEnd = matcher.end(1); + ++groupIdx; + } + if (groupIdx == groups.size()) { + builder.append(unix, lastEnd, unix.length()); + return builder.toString(); } else { - pattern = in.regex.pattern(); + 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") + .matcher(regex.pattern()); + String pattern; + if (openClose.matches()) { + pattern = openClose.group(1) + " " + openClose.group(2); + } else { + pattern = regex.pattern(); + } + throw new IntermediateStepRemovedException(Lint.create("toggleOffOnRemoved", + "An intermediate step removed a match of " + pattern, + startLine, endLine)); } - System.out.println("ERROR"); - throw new Error("An intermediate step removed a match of " + pattern); + } + } + + static class IntermediateStepRemovedException extends Exception { + Lint lint; + + IntermediateStepRemovedException(Lint lint) { + this.lint = lint; } } } 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 b7fba06d36..2d4c5fe700 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 @@ -23,6 +23,7 @@ import java.nio.file.Files; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -710,13 +711,13 @@ public void withinBlocksRegex(String name, String re withinBlocksHelper(PipeStepPair.named(name).regex(regex), clazz, configure); } - private void withinBlocksHelper(PipeStepPair.Builder builder, Class clazz, Action configure) { + private void withinBlocksHelper(PipeStepPair builder, Class clazz, Action configure) { // create the sub-extension T formatExtension = spotless.instantiateFormatExtension(clazz); // configure it configure.execute(formatExtension); // create a step which applies all of those steps as sub-steps - FormatterStep step = builder.buildStepWhichAppliesSubSteps(spotless.project.getRootDir().toPath(), formatExtension.steps); + FormatterStep step = builder.applyWithin(spotless.project.getRootDir().toPath(), formatExtension.steps); addStep(step); } @@ -725,12 +726,12 @@ private void withinBlocksHelper(PipeStepPair.Builder * inside that captured group. */ public void toggleOffOnRegex(String regex) { - this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex).buildPair(); + this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex); } /** Disables formatting between the given tags. */ public void toggleOffOn(String off, String on) { - this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on).buildPair(); + this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on); } /** Disables formatting between {@code spotless:off} and {@code spotless:on}. */ @@ -753,10 +754,7 @@ protected void setupTask(SpotlessTask task) { task.setTarget(totalTarget); List steps; if (togglePair != null) { - steps = new ArrayList<>(this.steps.size() + 2); - steps.add(togglePair.in()); - steps.addAll(this.steps); - steps.add(togglePair.out()); + steps = Collections.singletonList(togglePair.preserveWithin(getProject().getRootDir().toPath(), this.steps)); } else { steps = this.steps; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index f4b7453092..841e560da7 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -20,6 +20,7 @@ import java.io.File; import java.nio.charset.Charset; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -33,7 +34,6 @@ import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; -import com.diffplug.spotless.generic.PipeStepPair; import com.diffplug.spotless.maven.generic.*; public abstract class FormatterFactory { @@ -84,11 +84,9 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form .map(factory -> factory.newFormatterStep(stepConfig)) .collect(Collectors.toCollection(() -> new ArrayList())); if (toggle != null) { - PipeStepPair pair = toggle.createPair(); - formatterSteps.add(0, pair.in()); - formatterSteps.add(pair.out()); + List formatterStepsBeforeToggle = formatterSteps; + formatterSteps = Collections.singletonList(toggle.createPair().preserveWithin(config.getFileLocator().getBaseDir().toPath(), formatterStepsBeforeToggle)); } - return Formatter.builder() .encoding(formatterEncoding) .lineEndingsPolicy(formatterLineEndingPolicy) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java index fe1676aec9..393ff91213 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 DiffPlug + * Copyright 2020-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,9 +31,9 @@ public class ToggleOffOn { public PipeStepPair createPair() { if (regex != null) { - return PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex).buildPair(); + return PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex); } else { - return PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on).buildPair(); + return PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on); } } } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java index 6bb144e6b4..ac65f3c0a8 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 DiffPlug + * Copyright 2020-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,10 +27,10 @@ class PipeStepPairTest { @Test - void single() throws Exception { - PipeStepPair pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on").buildPair(); - FormatterStep lowercase = FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)); - StepHarness harness = StepHarness.forSteps(pair.in(), lowercase, pair.out()); + void single() { + FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); + StepHarness harness = StepHarness.forSteps(pair); harness.test( StringPrinter.buildStringFromLines( "A B C", @@ -47,10 +47,10 @@ void single() throws Exception { } @Test - void multiple() throws Exception { - PipeStepPair pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on").buildPair(); - FormatterStep lowercase = FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)); - StepHarness harness = StepHarness.forSteps(pair.in(), lowercase, pair.out()); + void multiple() { + FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); + StepHarness harness = StepHarness.forSteps(pair); harness.test( StringPrinter.buildStringFromLines( "A B C", @@ -81,27 +81,25 @@ void multiple() throws Exception { } @Test - void broken() throws Exception { - PipeStepPair pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on").buildPair(); - FormatterStep uppercase = FormatterStep.createNeverUpToDate("uppercase", str -> str.toUpperCase(Locale.ROOT)); - StepHarness harness = StepHarness.forSteps(pair.in(), uppercase, pair.out()); + void broken() { + FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("uppercase", str -> str.toUpperCase(Locale.ROOT)))); + StepHarness harness = StepHarness.forSteps(pair); // this fails because uppercase turns spotless:off into SPOTLESS:OFF, etc - harness.testException(StringPrinter.buildStringFromLines( + harness.testExceptionMsg(StringPrinter.buildStringFromLines( "A B C", "spotless:off", "D E F", "spotless:on", - "G H I"), exception -> { - exception.hasMessage("An intermediate step removed a match of spotless:off spotless:on"); - }); + "G H I")).isEqualTo("An intermediate step removed a match of spotless:off spotless:on"); } @Test - void andApply() throws Exception { - FormatterStep lowercase = FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)); - FormatterStep lowercaseSometimes = PipeStepPair.named("lowercaseSometimes").openClose("", "") - .buildStepWhichAppliesSubSteps(Paths.get(""), Arrays.asList(lowercase)); - StepHarness.forSteps(lowercaseSometimes).test( + void andApply() { + FormatterStep pair = PipeStepPair.named("lowercaseSometimes").openClose("", "") + .applyWithin(Paths.get(""), Arrays.asList( + FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); + StepHarness.forSteps(pair).test( StringPrinter.buildStringFromLines( "A B C", "", From 07b39cbb9ecb99a167016ef37137dc0a720160e3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 15:31:32 -0800 Subject: [PATCH 21/31] Rename PipeStepPair to FenceStep. --- CHANGES.md | 2 ++ .../{PipeStepPair.java => FenceStep.java} | 26 +++++++++---------- .../gradle/spotless/FormatExtension.java | 25 +++++++++--------- .../spotless/maven/FormatterFactory.java | 2 +- .../spotless/maven/generic/ToggleOffOn.java | 12 ++++----- ...peStepPairTest.java => FenceStepTest.java} | 10 +++---- 6 files changed, 39 insertions(+), 38 deletions(-) rename lib/src/main/java/com/diffplug/spotless/generic/{PipeStepPair.java => FenceStep.java} (88%) rename testlib/src/test/java/com/diffplug/spotless/generic/{PipeStepPairTest.java => FenceStepTest.java} (88%) diff --git a/CHANGES.md b/CHANGES.md index 90286a43c8..1173d5483e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Removed `FormatterStep.Strict` because it was unnecessary and unused implementation detail. * **BREAKING** Moved `PaddedCell.DirtyState` to its own top-level class with new methods. * **BREAKING** Removed `FormatExceptionPolicy` and its subclasses because exceptions are now wrapped as lints. +* **BREAKING** Renamed `PipeStepPair` to `FenceStep` and changed its API. + * The old "pair" approach could not detect when a `spotless:off/on` tag pair was removed by an intermediate step, so we have no choice but to condense the "in/(all other steps)/out" into a single step. * **BREAKING** Removed deprecated methods. * `FormatterFunc.Closeable::of(AutoCloseable, FormatterFunc)` diff --git a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java similarity index 88% rename from lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java rename to lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java index 7cc830bbeb..f2f799f79f 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -32,10 +32,10 @@ import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.Lint; -public class PipeStepPair { +public class FenceStep { /** Declares the name of the step. */ - public static PipeStepPair named(String name) { - return new PipeStepPair(name); + public static FenceStep named(String name) { + return new FenceStep(name); } public static String defaultToggleName() { @@ -53,22 +53,22 @@ public static String defaultToggleOn() { String name; Pattern regex; - private PipeStepPair(String name) { + private FenceStep(String name) { this.name = Objects.requireNonNull(name); } /** Defines the opening and closing markers. */ - public PipeStepPair openClose(String open, String close) { + public FenceStep openClose(String open, String close) { return regex(Pattern.quote(open) + "([\\s\\S]*?)" + Pattern.quote(close)); } /** Defines the pipe via regex. Must have *exactly one* capturing group. */ - public PipeStepPair regex(String regex) { + public FenceStep regex(String regex) { return regex(Pattern.compile(regex)); } /** Defines the pipe via regex. Must have *exactly one* capturing group. */ - public PipeStepPair regex(Pattern regex) { + public FenceStep regex(Pattern regex) { this.regex = Objects.requireNonNull(regex); return this; } @@ -77,7 +77,7 @@ private void assertRegexSet() { Objects.requireNonNull(regex, "must call regex() or openClose()"); } - /** Returns a step which will apply the given steps but preserve the content selected by the regex / openClose pair. */ + /** Returns a step which will apply the given steps globally then preserve the content within the fence selected by the regex / openClose pair. */ public FormatterStep preserveWithin(Path rootPath, List steps) { assertRegexSet(); return FormatterStep.createLazy(name, @@ -86,7 +86,7 @@ public FormatterStep preserveWithin(Path rootPath, List steps) { } /** - * Returns a step which will apply the given steps only within the blocks selected by the regex / openClose pair. + * Returns a step which will apply the given steps only within the fence selected by the regex / openClose pair. * Linting within the substeps is not supported. */ public FormatterStep applyWithin(Path rootPath, List steps) { @@ -96,7 +96,7 @@ public FormatterStep applyWithin(Path rootPath, List steps) { state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state)); } - static class ApplyWithin extends Apply implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + static class ApplyWithin extends BaseImplementation implements FormatterFunc.Closeable.ResourceFuncNeedsFile { ApplyWithin(Pattern regex, List steps) { super(regex, steps); } @@ -114,7 +114,7 @@ public String apply(Formatter formatter, String unix, File file) throws Exceptio } } - static class PreserveWithin extends Apply implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + static class PreserveWithin extends BaseImplementation implements FormatterFunc.Closeable.ResourceFuncNeedsFile { PreserveWithin(Pattern regex, List steps) { super(regex, steps); } @@ -148,14 +148,14 @@ public List lint(Formatter formatter, String content, File file) throws Ex } } - static class Apply implements Serializable { + static class BaseImplementation implements Serializable { final Pattern regex; final List steps; transient ArrayList groups = new ArrayList<>(); transient StringBuilder builderInternal; - public Apply(Pattern regex, List steps) { + public BaseImplementation(Pattern regex, List steps) { this.regex = regex; this.steps = steps; } 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 2d4c5fe700..b02d1aac1c 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -49,11 +49,11 @@ import com.diffplug.spotless.extra.EclipseBasedStepBuilder; import com.diffplug.spotless.extra.wtp.EclipseWtpFormatterStep; import com.diffplug.spotless.generic.EndWithNewlineStep; +import com.diffplug.spotless.generic.FenceStep; import com.diffplug.spotless.generic.IndentStep; import com.diffplug.spotless.generic.LicenseHeaderStep; import com.diffplug.spotless.generic.LicenseHeaderStep.YearMode; import com.diffplug.spotless.generic.NativeCmdStep; -import com.diffplug.spotless.generic.PipeStepPair; import com.diffplug.spotless.generic.ReplaceRegexStep; import com.diffplug.spotless.generic.ReplaceStep; import com.diffplug.spotless.generic.TrimTrailingWhitespaceStep; @@ -698,7 +698,7 @@ public void withinBlocks(String name, String open, String close, Action */ public void withinBlocks(String name, String open, String close, Class clazz, Action configure) { - withinBlocksHelper(PipeStepPair.named(name).openClose(open, close), clazz, configure); + withinBlocksHelper(FenceStep.named(name).openClose(open, close), clazz, configure); } /** Same as {@link #withinBlocks(String, String, String, Action)}, except instead of an open/close pair, you specify a regex with exactly one capturing group. */ @@ -708,17 +708,16 @@ public void withinBlocksRegex(String name, String regex, Action /** Same as {@link #withinBlocksRegex(String, String, Action)}, except you can specify any language-specific subclass of {@link FormatExtension} to get language-specific steps. */ public void withinBlocksRegex(String name, String regex, Class clazz, Action configure) { - withinBlocksHelper(PipeStepPair.named(name).regex(regex), clazz, configure); + withinBlocksHelper(FenceStep.named(name).regex(regex), clazz, configure); } - private void withinBlocksHelper(PipeStepPair builder, Class clazz, Action configure) { + private void withinBlocksHelper(FenceStep fence, Class clazz, Action configure) { // create the sub-extension T formatExtension = spotless.instantiateFormatExtension(clazz); // configure it configure.execute(formatExtension); // create a step which applies all of those steps as sub-steps - FormatterStep step = builder.applyWithin(spotless.project.getRootDir().toPath(), formatExtension.steps); - addStep(step); + addStep(fence.applyWithin(spotless.project.getRootDir().toPath(), formatExtension.steps)); } /** @@ -726,25 +725,25 @@ private void withinBlocksHelper(PipeStepPair builder * inside that captured group. */ public void toggleOffOnRegex(String regex) { - this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex); + this.toggleFence = FenceStep.named(FenceStep.defaultToggleName()).regex(regex); } /** Disables formatting between the given tags. */ public void toggleOffOn(String off, String on) { - this.togglePair = PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on); + this.toggleFence = FenceStep.named(FenceStep.defaultToggleName()).openClose(off, on); } /** Disables formatting between {@code spotless:off} and {@code spotless:on}. */ public void toggleOffOn() { - toggleOffOn(PipeStepPair.defaultToggleOff(), PipeStepPair.defaultToggleOn()); + toggleOffOn(FenceStep.defaultToggleOff(), FenceStep.defaultToggleOn()); } /** Undoes all previous calls to {@link #toggleOffOn()} and {@link #toggleOffOn(String, String)}. */ public void toggleOffOnDisable() { - this.togglePair = null; + this.toggleFence = null; } - private @Nullable PipeStepPair togglePair; + private @Nullable FenceStep toggleFence; /** Sets up a format task according to the values in this extension. */ protected void setupTask(SpotlessTask task) { @@ -753,8 +752,8 @@ protected void setupTask(SpotlessTask task) { FileCollection totalTarget = targetExclude == null ? target : target.minus(targetExclude); task.setTarget(totalTarget); List steps; - if (togglePair != null) { - steps = Collections.singletonList(togglePair.preserveWithin(getProject().getRootDir().toPath(), this.steps)); + if (toggleFence != null) { + steps = Collections.singletonList(toggleFence.preserveWithin(getProject().getRootDir().toPath(), this.steps)); } else { steps = this.steps; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index 841e560da7..44602e0603 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -85,7 +85,7 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form .collect(Collectors.toCollection(() -> new ArrayList())); if (toggle != null) { List formatterStepsBeforeToggle = formatterSteps; - formatterSteps = Collections.singletonList(toggle.createPair().preserveWithin(config.getFileLocator().getBaseDir().toPath(), formatterStepsBeforeToggle)); + formatterSteps = Collections.singletonList(toggle.createFence().preserveWithin(config.getFileLocator().getBaseDir().toPath(), formatterStepsBeforeToggle)); } return Formatter.builder() .encoding(formatterEncoding) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java index 393ff91213..289704181b 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/ToggleOffOn.java @@ -17,23 +17,23 @@ import org.apache.maven.plugins.annotations.Parameter; -import com.diffplug.spotless.generic.PipeStepPair; +import com.diffplug.spotless.generic.FenceStep; public class ToggleOffOn { @Parameter - public String off = PipeStepPair.defaultToggleOff(); + public String off = FenceStep.defaultToggleOff(); @Parameter - public String on = PipeStepPair.defaultToggleOn(); + public String on = FenceStep.defaultToggleOn(); @Parameter public String regex; - public PipeStepPair createPair() { + public FenceStep createFence() { if (regex != null) { - return PipeStepPair.named(PipeStepPair.defaultToggleName()).regex(regex); + return FenceStep.named(FenceStep.defaultToggleName()).regex(regex); } else { - return PipeStepPair.named(PipeStepPair.defaultToggleName()).openClose(off, on); + return FenceStep.named(FenceStep.defaultToggleName()).openClose(off, on); } } } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java similarity index 88% rename from testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java rename to testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java index ac65f3c0a8..4f2a321632 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -25,10 +25,10 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.StepHarness; -class PipeStepPairTest { +class FenceStepTest { @Test void single() { - FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); harness.test( @@ -48,7 +48,7 @@ void single() { @Test void multiple() { - FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); harness.test( @@ -82,7 +82,7 @@ void multiple() { @Test void broken() { - FormatterStep pair = PipeStepPair.named("underTest").openClose("spotless:off", "spotless:on") + FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("uppercase", str -> str.toUpperCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); // this fails because uppercase turns spotless:off into SPOTLESS:OFF, etc @@ -96,7 +96,7 @@ void broken() { @Test void andApply() { - FormatterStep pair = PipeStepPair.named("lowercaseSometimes").openClose("", "") + FormatterStep pair = FenceStep.named("lowercaseSometimes").openClose("", "") .applyWithin(Paths.get(""), Arrays.asList( FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness.forSteps(pair).test( From 78ddcd6ffc0c4fe43f0f2fe06630355d7a0a881d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 15:40:04 -0800 Subject: [PATCH 22/31] Removed `rootDir` property `Formatter` and its builder because it was only used to annnotate errors, which is now done by the lint phase. --- CHANGES.md | 1 + .../java/com/diffplug/spotless/Formatter.java | 22 ++----------------- .../diffplug/spotless/generic/FenceStep.java | 12 +++++----- .../gradle/spotless/FormatExtension.java | 4 ++-- .../gradle/spotless/SpotlessTask.java | 1 - .../spotless/maven/FormatterFactory.java | 3 +-- .../maven/incremental/NoopCheckerTest.java | 2 -- .../incremental/PluginFingerprintTest.java | 2 -- .../com/diffplug/spotless/FormatterTest.java | 1 - .../com/diffplug/spotless/PaddedCellTest.java | 3 +-- .../spotless/generic/FenceStepTest.java | 9 ++++---- 11 files changed, 16 insertions(+), 44 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1173d5483e..b0bb36fdee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Removed `FormatExceptionPolicy` and its subclasses because exceptions are now wrapped as lints. * **BREAKING** Renamed `PipeStepPair` to `FenceStep` and changed its API. * The old "pair" approach could not detect when a `spotless:off/on` tag pair was removed by an intermediate step, so we have no choice but to condense the "in/(all other steps)/out" into a single step. +* **BREAKING** Removed `rootDir` property `Formatter` and its builder because it was only used to annnotate errors, which is now done by the lint phase. * **BREAKING** Removed deprecated methods. * `FormatterFunc.Closeable::of(AutoCloseable, FormatterFunc)` diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 685131dd16..012f81d816 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -24,8 +24,6 @@ import java.io.ObjectStreamException; import java.io.Serializable; import java.nio.charset.Charset; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -37,13 +35,11 @@ public final class Formatter implements Serializable, AutoCloseable { private LineEnding.Policy lineEndingsPolicy; private Charset encoding; - private Path rootDir; private List steps; - private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path rootDirectory, List steps) { + private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, List steps) { this.lineEndingsPolicy = Objects.requireNonNull(lineEndingsPolicy, "lineEndingsPolicy"); this.encoding = Objects.requireNonNull(encoding, "encoding"); - this.rootDir = Objects.requireNonNull(rootDirectory, "rootDir"); this.steps = requireElementsNonNull(new ArrayList<>(steps)); } @@ -51,7 +47,6 @@ private Formatter(LineEnding.Policy lineEndingsPolicy, Charset encoding, Path ro private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(lineEndingsPolicy); out.writeObject(encoding.name()); - out.writeObject(rootDir.toString()); out.writeObject(steps); } @@ -60,7 +55,6 @@ private void writeObject(ObjectOutputStream out) throws IOException { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { lineEndingsPolicy = (LineEnding.Policy) in.readObject(); encoding = Charset.forName((String) in.readObject()); - rootDir = Paths.get((String) in.readObject()); steps = (List) in.readObject(); } @@ -78,10 +72,6 @@ public Charset getEncoding() { return encoding; } - public Path getRootDir() { - return rootDir; - } - public List getSteps() { return steps; } @@ -94,7 +84,6 @@ public static class Builder { // required parameters private LineEnding.Policy lineEndingsPolicy; private Charset encoding; - private Path rootDir; private List steps; private Builder() {} @@ -109,18 +98,13 @@ public Builder encoding(Charset encoding) { return this; } - public Builder rootDir(Path rootDir) { - this.rootDir = rootDir; - return this; - } - public Builder steps(List steps) { this.steps = steps; return this; } public Formatter build() { - return new Formatter(lineEndingsPolicy, encoding, rootDir, steps); + return new Formatter(lineEndingsPolicy, encoding, steps); } } @@ -192,7 +176,6 @@ public int hashCode() { int result = 1; result = prime * result + encoding.hashCode(); result = prime * result + lineEndingsPolicy.hashCode(); - result = prime * result + rootDir.hashCode(); result = prime * result + steps.hashCode(); return result; } @@ -211,7 +194,6 @@ public boolean equals(Object obj) { Formatter other = (Formatter) obj; return encoding.equals(other.encoding) && lineEndingsPolicy.equals(other.lineEndingsPolicy) && - rootDir.equals(other.rootDir) && steps.equals(other.steps); } 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 f2f799f79f..e96893b2bc 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -18,7 +18,6 @@ import java.io.File; import java.io.Serializable; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -78,22 +77,22 @@ private void assertRegexSet() { } /** Returns a step which will apply the given steps globally then preserve the content within the fence selected by the regex / openClose pair. */ - public FormatterStep preserveWithin(Path rootPath, List steps) { + public FormatterStep preserveWithin(List steps) { assertRegexSet(); return FormatterStep.createLazy(name, () -> new PreserveWithin(regex, steps), - state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state)); + state -> FormatterFunc.Closeable.of(state.buildFormatter(), state)); } /** * Returns a step which will apply the given steps only within the fence selected by the regex / openClose pair. * Linting within the substeps is not supported. */ - public FormatterStep applyWithin(Path rootPath, List steps) { + public FormatterStep applyWithin(List steps) { assertRegexSet(); return FormatterStep.createLazy(name, () -> new ApplyWithin(regex, steps), - state -> FormatterFunc.Closeable.of(state.buildFormatter(rootPath), state)); + state -> FormatterFunc.Closeable.of(state.buildFormatter(), state)); } static class ApplyWithin extends BaseImplementation implements FormatterFunc.Closeable.ResourceFuncNeedsFile { @@ -178,12 +177,11 @@ private StringBuilder builderZeroed() { return builderInternal; } - protected Formatter buildFormatter(Path rootDir) { + protected Formatter buildFormatter() { return Formatter.builder() .encoding(StandardCharsets.UTF_8) // can be any UTF, doesn't matter .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) // just internal, won't conflict with user .steps(steps) - .rootDir(rootDir) .build(); } 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 b02d1aac1c..a1546af18f 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 @@ -717,7 +717,7 @@ private void withinBlocksHelper(FenceStep fence, Cla // configure it configure.execute(formatExtension); // create a step which applies all of those steps as sub-steps - addStep(fence.applyWithin(spotless.project.getRootDir().toPath(), formatExtension.steps)); + addStep(fence.applyWithin(formatExtension.steps)); } /** @@ -753,7 +753,7 @@ protected void setupTask(SpotlessTask task) { task.setTarget(totalTarget); List steps; if (toggleFence != null) { - steps = Collections.singletonList(toggleFence.preserveWithin(getProject().getRootDir().toPath(), this.steps)); + steps = Collections.singletonList(toggleFence.preserveWithin(this.steps)); } else { steps = this.steps; } 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 1cd32b1c12..00ff5f7329 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 @@ -200,7 +200,6 @@ Formatter buildFormatter() { return Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy.get()) .encoding(Charset.forName(encoding)) - .rootDir(getProjectDir().get().getAsFile().toPath()) .steps(steps.get()) .build(); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index 44602e0603..4bc66329a2 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -85,13 +85,12 @@ public final Formatter newFormatter(Supplier> filesToFormat, Form .collect(Collectors.toCollection(() -> new ArrayList())); if (toggle != null) { List formatterStepsBeforeToggle = formatterSteps; - formatterSteps = Collections.singletonList(toggle.createFence().preserveWithin(config.getFileLocator().getBaseDir().toPath(), formatterStepsBeforeToggle)); + formatterSteps = Collections.singletonList(toggle.createFence().preserveWithin(formatterStepsBeforeToggle)); } return Formatter.builder() .encoding(formatterEncoding) .lineEndingsPolicy(formatterLineEndingPolicy) .steps(formatterSteps) - .rootDir(config.getFileLocator().getBaseDir().toPath()) .build(); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java index cc1c94b0a8..3337cf7478 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java @@ -28,7 +28,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import org.apache.maven.model.Build; import org.apache.maven.model.Plugin; @@ -115,7 +114,6 @@ private MavenProject buildMavenProject() throws IOException { private static Formatter dummyFormatter() { return Formatter.builder() - .rootDir(Paths.get("")) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(UTF_8) .steps(singletonList(mock(FormatterStep.class, withSettings().serializable()))) diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index c9920396f1..038a775569 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -21,7 +21,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.ByteArrayInputStream; -import java.nio.file.Paths; import java.util.Arrays; import java.util.List; @@ -245,7 +244,6 @@ private static Formatter formatter(FormatterStep... steps) { private static Formatter formatter(LineEnding lineEnding, FormatterStep... steps) { return Formatter.builder() - .rootDir(Paths.get("")) .lineEndingsPolicy(lineEnding.createPolicy()) .encoding(UTF_8) .steps(Arrays.asList(steps)) diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 60bee8a01e..22a03d6c3c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -59,7 +59,6 @@ protected Formatter create() { return Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(encoding) - .rootDir(rootDir) .steps(steps) .build(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java index dae41678a4..641f45fc7c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/PaddedCellTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -51,7 +51,6 @@ private void testCase(FormatterFunc step, String input, PaddedCell.Type expected try (Formatter formatter = Formatter.builder() .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .rootDir(rootFolder.toPath()) .steps(formatterSteps).build()) { File file = new File(rootFolder, "input"); 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 4f2a321632..9f51938ee9 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/FenceStepTest.java @@ -15,7 +15,6 @@ */ package com.diffplug.spotless.generic; -import java.nio.file.Paths; import java.util.Arrays; import java.util.Locale; @@ -29,7 +28,7 @@ class FenceStepTest { @Test void single() { FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") - .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); + .preserveWithin(Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); harness.test( StringPrinter.buildStringFromLines( @@ -49,7 +48,7 @@ void single() { @Test void multiple() { FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") - .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); + .preserveWithin(Arrays.asList(FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); harness.test( StringPrinter.buildStringFromLines( @@ -83,7 +82,7 @@ void multiple() { @Test void broken() { FormatterStep pair = FenceStep.named("underTest").openClose("spotless:off", "spotless:on") - .preserveWithin(Paths.get(""), Arrays.asList(FormatterStep.createNeverUpToDate("uppercase", str -> str.toUpperCase(Locale.ROOT)))); + .preserveWithin(Arrays.asList(FormatterStep.createNeverUpToDate("uppercase", str -> str.toUpperCase(Locale.ROOT)))); StepHarness harness = StepHarness.forSteps(pair); // this fails because uppercase turns spotless:off into SPOTLESS:OFF, etc harness.testExceptionMsg(StringPrinter.buildStringFromLines( @@ -97,7 +96,7 @@ void broken() { @Test void andApply() { FormatterStep pair = FenceStep.named("lowercaseSometimes").openClose("", "") - .applyWithin(Paths.get(""), Arrays.asList( + .applyWithin(Arrays.asList( FormatterStep.createNeverUpToDate("lowercase", str -> str.toLowerCase(Locale.ROOT)))); StepHarness.forSteps(pair).test( StringPrinter.buildStringFromLines( From f947c7bb19edeaef39f83b8dc162d1be0bb483f9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 15:42:31 -0800 Subject: [PATCH 23/31] `DiffMessageFormatter.Builder::formatter` needs a new `Path rootDir` parameter. --- CHANGES.md | 1 + .../extra/integration/DiffMessageFormatter.java | 12 +++++++----- .../spotless/maven/AbstractSpotlessMojo.java | 2 +- .../diffplug/spotless/maven/SpotlessCheckMojo.java | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index b0bb36fdee..007af6749d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,6 +17,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **BREAKING** Renamed `PipeStepPair` to `FenceStep` and changed its API. * The old "pair" approach could not detect when a `spotless:off/on` tag pair was removed by an intermediate step, so we have no choice but to condense the "in/(all other steps)/out" into a single step. * **BREAKING** Removed `rootDir` property `Formatter` and its builder because it was only used to annnotate errors, which is now done by the lint phase. + * This also means that `DiffMessageFormatter.Builder::formatter` needs a new `Path rootDir` parameter. * **BREAKING** Removed deprecated methods. * `FormatterFunc.Closeable::of(AutoCloseable, FormatterFunc)` diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java index 15c43c815b..3d15c42f7c 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -57,14 +57,16 @@ interface CleanProvider { private static class CleanProviderFormatter implements CleanProvider { private final Formatter formatter; + private final Path rootDir; - CleanProviderFormatter(Formatter formatter) { + CleanProviderFormatter(Path rootDir, Formatter formatter) { + this.rootDir = rootDir; this.formatter = Objects.requireNonNull(formatter); } @Override public Path getRootDir() { - return formatter.getRootDir(); + return rootDir; } @Override @@ -121,8 +123,8 @@ public Builder runToFix(String runToFix) { return this; } - public Builder formatter(Formatter formatter) { - this.formatter = new CleanProviderFormatter(formatter); + public Builder formatter(Path rootDir, Formatter formatter) { + this.formatter = new CleanProviderFormatter(rootDir, formatter); return this; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 1b5abe9f0c..72ab43ae2c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -103,7 +103,7 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { private List repositories; @Parameter(defaultValue = "${project.basedir}", required = true, readonly = true) - private File baseDir; + protected File baseDir; @Parameter(defaultValue = "${project.build.directory}", required = true, readonly = true) private File buildDir; diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index ebd53dbc70..622cbd2222 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -72,7 +72,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke if (!problemFiles.isEmpty()) { throw new MojoExecutionException(DiffMessageFormatter.builder() .runToFix("Run 'mvn spotless:apply' to fix these violations.") - .formatter(formatter) + .formatter(baseDir.toPath(), formatter) .problemFiles(problemFiles) .getMessage()); } From a3eef1f594873d9efbee3309decc36d354de52dd Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 16:09:28 -0800 Subject: [PATCH 24/31] Adapt StepHarness for the new "exceptions never get thrown" environment. --- .../diffplug/spotless/ResourceHarness.java | 26 ++--- .../com/diffplug/spotless/StepHarness.java | 72 +++++++------- .../spotless/StepHarnessWithFile.java | 97 +++++++++++-------- .../com/diffplug/spotless/FormatterTest.java | 7 -- .../spotless/cpp/ClangFormatStepTest.java | 12 +-- .../spotless/json/JsonSimpleStepTest.java | 35 +++---- .../spotless/kotlin/DiktatStepTest.java | 35 +++---- .../spotless/kotlin/KtLintStepTest.java | 20 +--- .../npm/PrettierFormatterStepTest.java | 15 ++- 9 files changed, 150 insertions(+), 169 deletions(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index b4a8bbae24..cd367e4374 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -54,7 +54,7 @@ protected File rootFolder() { } /** Returns a new child of the root folder. */ - protected File newFile(String subpath) throws IOException { + protected File newFile(String subpath) { return new File(rootFolder(), subpath); } @@ -85,12 +85,12 @@ protected void replace(String path, String toReplace, String replaceWith) throws } /** Returns the contents of the given file from the src/test/resources directory. */ - protected static String getTestResource(String filename) throws IOException { + protected static String getTestResource(String filename) { URL url = ResourceHarness.class.getResource("/" + filename); if (url == null) { throw new IllegalArgumentException("No such resource " + filename); } - return Resources.toString(url, StandardCharsets.UTF_8); + return ThrowingEx.get(() -> Resources.toString(url, StandardCharsets.UTF_8)); } /** Returns Files (in a temporary folder) which has the contents of the given file from the src/test/resources directory. */ @@ -176,7 +176,7 @@ public void matches(Consumer> conditions) } } - protected WriteAsserter setFile(String path) throws IOException { + protected WriteAsserter setFile(String path) { return new WriteAsserter(newFile(path)); } @@ -188,21 +188,25 @@ private WriteAsserter(File file) { this.file = file; } - public File toLines(String... lines) throws IOException { + public File toLines(String... lines) { return toContent(String.join("\n", Arrays.asList(lines))); } - public File toContent(String content) throws IOException { + public File toContent(String content) { return toContent(content, StandardCharsets.UTF_8); } - public File toContent(String content, Charset charset) throws IOException { - Files.write(file.toPath(), content.getBytes(charset)); + public File toContent(String content, Charset charset) { + ThrowingEx.run(() -> { + Files.write(file.toPath(), content.getBytes(charset)); + }); return file; } - public File toResource(String path) throws IOException { - Files.write(file.toPath(), getTestResource(path).getBytes(StandardCharsets.UTF_8)); + public File toResource(String path) { + ThrowingEx.run(() -> { + Files.write(file.toPath(), getTestResource(path).getBytes(StandardCharsets.UTF_8)); + }); return file; } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java index 8755f852d6..7056e1b730 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarness.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -19,34 +19,24 @@ import java.io.File; import java.nio.charset.StandardCharsets; -import java.nio.file.Paths; import java.util.Arrays; +import java.util.List; import java.util.Objects; -import java.util.function.Consumer; -import org.assertj.core.api.AbstractThrowableAssert; +import org.assertj.core.api.AbstractStringAssert; import org.assertj.core.api.Assertions; /** 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 implements AutoCloseable { - private final FormatterFunc formatter; + private final Formatter formatter; - private StepHarness(FormatterFunc formatter) { + private StepHarness(Formatter formatter) { this.formatter = Objects.requireNonNull(formatter); } /** Creates a harness for testing steps which don't depend on the file. */ public static StepHarness forStep(FormatterStep step) { - // We don't care if an individual FormatterStep is misbehaving on line-endings, because - // Formatter fixes that. No reason to care in tests either. It's likely to pop up when - // running tests on Windows from time-to-time - return new StepHarness(FormatterFunc.Closeable.ofDangerous( - () -> { - if (step instanceof FormatterStepImpl.Standard) { - ((FormatterStepImpl.Standard) step).cleanupFormatterFunc(); - } - }, - input -> LineEnding.toUnix(step.format(input, new File(""))))); + return forSteps(step); } /** Creates a harness for testing steps which don't depend on the file. */ @@ -55,61 +45,67 @@ public static StepHarness forSteps(FormatterStep... steps) { .steps(Arrays.asList(steps)) .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) .encoding(StandardCharsets.UTF_8) - .rootDir(Paths.get("")) .build()); } /** Creates a harness for testing a formatter whose steps don't depend on the file. */ public static StepHarness forFormatter(Formatter formatter) { - return new StepHarness(FormatterFunc.Closeable.ofDangerous( - formatter::close, - input -> formatter.compute(input, new File("")))); + return new StepHarness(formatter); } /** Asserts that the given element is transformed as expected, and that the result is idempotent. */ - public StepHarness test(String before, String after) throws Exception { - String actual = formatter.apply(before); + public StepHarness test(String before, String after) { + String actual = formatter.compute(LineEnding.toUnix(before), new File("")); assertEquals(after, actual, "Step application failed"); return testUnaffected(after); } /** Asserts that the given element is idempotent w.r.t the step under test. */ - public StepHarness testUnaffected(String idempotentElement) throws Exception { - String actual = formatter.apply(idempotentElement); + public StepHarness testUnaffected(String idempotentElement) { + String actual = formatter.compute(LineEnding.toUnix(idempotentElement), new File("")); assertEquals(idempotentElement, actual, "Step is not idempotent"); return this; } /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarness testResource(String resourceBefore, String resourceAfter) throws Exception { + public StepHarness testResource(String resourceBefore, String resourceAfter) { String before = ResourceHarness.getTestResource(resourceBefore); String after = ResourceHarness.getTestResource(resourceAfter); return test(before, after); } /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarness testResourceUnaffected(String resourceIdempotent) throws Exception { + public StepHarness testResourceUnaffected(String resourceIdempotent) { String idempotentElement = ResourceHarness.getTestResource(resourceIdempotent); return testUnaffected(idempotentElement); } - /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarness testResourceException(String resourceBefore, Consumer> exceptionAssertion) throws Exception { - return testException(ResourceHarness.getTestResource(resourceBefore), exceptionAssertion); + public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { + return testExceptionMsg(ResourceHarness.getTestResource(resourceBefore)); } - /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarness testException(String before, Consumer> exceptionAssertion) throws Exception { - Throwable t = assertThrows(Throwable.class, () -> formatter.apply(before)); - AbstractThrowableAssert abstractAssert = Assertions.assertThat(t); - exceptionAssertion.accept(abstractAssert); - return this; + public AbstractStringAssert testExceptionMsg(String before) { + List lints = formatter.lint(LineEnding.toUnix(before), FormatterStepImpl.SENTINEL); + if (lints.size() == 0) { + throw new AssertionError("No exception was thrown"); + } else if (lints.size() >= 2) { + throw new AssertionError("Expected one lint, had " + lints.size()); + } else { + return Assertions.assertThat(lints.get(0).getMsg()); + } + } + + public StepHarness assertZeroLints(String before) { + List lints = formatter.lint(LineEnding.toUnix(before), FormatterStepImpl.SENTINEL); + if (lints.size() == 0) { + return this; + } else { + throw new AssertionError("Expected no lints, had " + lints); + } } @Override public void close() { - if (formatter instanceof FormatterFunc.Closeable) { - ((FormatterFunc.Closeable) formatter).close(); - } + formatter.close(); } } diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index 89be961d04..ad4288d4f2 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -18,78 +18,91 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; import java.util.Objects; +import org.assertj.core.api.AbstractStringAssert; +import org.assertj.core.api.Assertions; + /** An api for testing a {@code FormatterStep} that depends on the File path. */ public class StepHarnessWithFile implements AutoCloseable { - private final FormatterFunc formatter; + private final Formatter formatter; + private ResourceHarness harness; - private StepHarnessWithFile(FormatterFunc formatter) { + private StepHarnessWithFile(ResourceHarness harness, Formatter formatter) { + this.harness = Objects.requireNonNull(harness); this.formatter = Objects.requireNonNull(formatter); + } /** Creates a harness for testing steps which do depend on the file. */ - public static StepHarnessWithFile forStep(FormatterStep step) { - // We don't care if an individual FormatterStep is misbehaving on line-endings, because - // Formatter fixes that. No reason to care in tests either. It's likely to pop up when - // running tests on Windows from time-to-time - return new StepHarnessWithFile(FormatterFunc.Closeable.ofDangerous( - () -> { - if (step instanceof FormatterStepImpl.Standard) { - ((FormatterStepImpl.Standard) step).cleanupFormatterFunc(); - } - }, - new FormatterFunc() { - @Override - public String apply(String unix) throws Exception { - return apply(unix, new File("")); - } - - @Override - public String apply(String unix, File file) throws Exception { - return LineEnding.toUnix(step.format(unix, file)); - } - })); + public static StepHarnessWithFile forStep(ResourceHarness harness, FormatterStep step) { + return new StepHarnessWithFile(harness, Formatter.builder() + .encoding(StandardCharsets.UTF_8) + .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) + .steps(Arrays.asList(step)) + .build()); } /** Creates a harness for testing a formatter whose steps do depend on the file. */ - public static StepHarnessWithFile forFormatter(Formatter formatter) { - return new StepHarnessWithFile(FormatterFunc.Closeable.ofDangerous( - formatter::close, - input -> formatter.compute(input, new File("")))); + public static StepHarnessWithFile forFormatter(ResourceHarness harness, Formatter formatter) { + return new StepHarnessWithFile(harness, formatter); } /** Asserts that the given element is transformed as expected, and that the result is idempotent. */ - public StepHarnessWithFile test(File file, String before, String after) throws Exception { - String actual = formatter.apply(before, file); + public StepHarnessWithFile test(File file, String before, String after) { + String actual = formatter.compute(LineEnding.toUnix(before), file); assertEquals(after, actual, "Step application failed"); return testUnaffected(file, after); } /** Asserts that the given element is idempotent w.r.t the step under test. */ - public StepHarnessWithFile testUnaffected(File file, String idempotentElement) throws Exception { - String actual = formatter.apply(idempotentElement, file); + public StepHarnessWithFile testUnaffected(File file, String idempotentElement) { + String actual = formatter.compute(LineEnding.toUnix(idempotentElement), file); assertEquals(idempotentElement, actual, "Step is not idempotent"); return this; } /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarnessWithFile testResource(File file, String resourceBefore, String resourceAfter) throws Exception { - String before = ResourceHarness.getTestResource(resourceBefore); - String after = ResourceHarness.getTestResource(resourceAfter); - return test(file, before, after); + public StepHarnessWithFile testResource(String resourceBefore, String resourceAfter) { + return testResource(resourceBefore, resourceBefore, resourceAfter); + } + + public StepHarnessWithFile testResource(String filename, String resourceBefore, String resourceAfter) { + String contentBefore = ResourceHarness.getTestResource(resourceBefore); + File file = harness.setFile(filename).toContent(contentBefore); + return test(file, contentBefore, ResourceHarness.getTestResource(resourceAfter)); } /** Asserts that the given elements in the resources directory are transformed as expected. */ - public StepHarnessWithFile testResourceUnaffected(File file, String resourceIdempotent) throws Exception { - String idempotentElement = ResourceHarness.getTestResource(resourceIdempotent); - return testUnaffected(file, idempotentElement); + public StepHarnessWithFile testResourceUnaffected(String resourceIdempotent) { + String contentBefore = ResourceHarness.getTestResource(resourceIdempotent); + File file = harness.setFile(resourceIdempotent).toContent(contentBefore); + return testUnaffected(file, contentBefore); + } + + public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) throws IOException { + String contentBefore = ResourceHarness.getTestResource(resourceBefore); + File file = harness.setFile(resourceBefore).toContent(contentBefore); + return testExceptionMsg(file, contentBefore); + } + + public AbstractStringAssert testExceptionMsg(File file, String before) { + List lints = formatter.lint(LineEnding.toUnix(before), file); + if (lints.size() == 0) { + throw new AssertionError("No exception was thrown"); + } else if (lints.size() >= 2) { + throw new AssertionError("Expected one lint, had " + lints.size()); + } else { + return Assertions.assertThat(lints.get(0).getMsg()); + } } @Override public void close() { - if (formatter instanceof FormatterFunc.Closeable) { - ((FormatterFunc.Closeable) formatter).close(); - } + formatter.close(); } } diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java index 22a03d6c3c..93de00d45c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterTest.java @@ -17,14 +17,11 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; -import com.diffplug.common.base.StandardSystemProperty; import com.diffplug.spotless.generic.EndWithNewlineStep; class FormatterTest { @@ -34,7 +31,6 @@ void equality() { new SerializableEqualityTester() { private LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy(); private Charset encoding = StandardCharsets.UTF_8; - private Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value()); private List steps = new ArrayList<>(); @Override @@ -47,9 +43,6 @@ protected void setupTest(API api) throws Exception { encoding = StandardCharsets.UTF_16; api.areDifferentThan(); - rootDir = rootDir.getParent(); - api.areDifferentThan(); - steps.add(EndWithNewlineStep.create()); api.areDifferentThan(); } diff --git a/testlib/src/test/java/com/diffplug/spotless/cpp/ClangFormatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/cpp/ClangFormatStepTest.java index cf0f551485..7bed8eac4e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/cpp/ClangFormatStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/cpp/ClangFormatStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 DiffPlug + * Copyright 2020-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,26 +15,26 @@ */ package com.diffplug.spotless.cpp; -import java.io.File; import java.util.Arrays; import org.junit.jupiter.api.Test; +import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarnessWithFile; import com.diffplug.spotless.tag.ClangTest; @ClangTest -class ClangFormatStepTest { +class ClangFormatStepTest extends ResourceHarness { @Test void test() throws Exception { - try (StepHarnessWithFile harness = StepHarnessWithFile.forStep(ClangFormatStep.withVersion(ClangFormatStep.defaultVersion()).create())) { + try (StepHarnessWithFile harness = StepHarnessWithFile.forStep(this, ClangFormatStep.withVersion(ClangFormatStep.defaultVersion()).create())) { // can't be named java or it gets compiled into .class file - harness.testResource(new File("example.java"), "clang/example.java.dirty", "clang/example.java.clean"); + harness.testResource("example.java", "clang/example.java.dirty", "clang/example.java.clean"); // test every other language clang supports for (String ext : Arrays.asList("c", "cs", "js", "m", "proto")) { String filename = "example." + ext; String root = "clang/" + filename; - harness.testResource(new File(filename), root, root + ".clean"); + harness.testResource(filename, root, root + ".clean"); } } } diff --git a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java index 4085744593..552993b932 100644 --- a/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/json/JsonSimpleStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 DiffPlug + * Copyright 2021-2022 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,58 +37,55 @@ void cannotProvidedNullProvisioner() { } @Test - void handlesSingletonObject() throws Exception { + void handlesSingletonObject() { doWithResource(stepHarness, "singletonObject"); } @Test - void handlesSingletonObjectWithArray() throws Exception { + void handlesSingletonObjectWithArray() { doWithResource(stepHarness, "singletonObjectWithArray"); } @Test - void handlesNestedObject() throws Exception { + void handlesNestedObject() { doWithResource(stepHarness, "nestedObject"); } @Test - void handlesSingletonArray() throws Exception { + void handlesSingletonArray() { doWithResource(stepHarness, "singletonArray"); } @Test - void handlesEmptyFile() throws Exception { + void handlesEmptyFile() { doWithResource(stepHarness, "empty"); } @Test - void handlesComplexNestedObject() throws Exception { + void handlesComplexNestedObject() { doWithResource(stepHarness, "cucumberJsonSample"); } @Test - void handlesObjectWithNull() throws Exception { + void handlesObjectWithNull() { doWithResource(stepHarness, "objectWithNull"); } @Test void handlesInvalidJson() { - assertThatThrownBy(() -> doWithResource(stepHarness, "invalidJson")) - .isInstanceOf(AssertionError.class) - .hasMessage("Unable to format JSON") - .hasRootCauseMessage("Expected a ',' or '}' at 9 [character 0 line 3]"); + stepHarness.testResourceExceptionMsg("json/invalidJsonBefore.json") + .contains("Unable to format JSON") + .contains("Expected a ',' or '}' at 9 [character 0 line 3]"); } @Test void handlesNotJson() { - assertThatThrownBy(() -> doWithResource(stepHarness, "notJson")) - .isInstanceOf(AssertionError.class) - .hasMessage("Unable to determine JSON type, expected a '{' or '[' but found '#'") - .hasNoCause(); + stepHarness.testResourceExceptionMsg("json/notJsonBefore.json") + .contains("Unable to determine JSON type, expected a '{' or '[' but found '#'"); } @Test - void canSetCustomIndentationLevel() throws Exception { + void canSetCustomIndentationLevel() { FormatterStep step = JsonSimpleStep.create(6, TestProvisioner.mavenCentral()); StepHarness stepHarness = StepHarness.forStep(step); @@ -98,7 +95,7 @@ void canSetCustomIndentationLevel() throws Exception { } @Test - void canSetIndentationLevelTo0() throws Exception { + void canSetIndentationLevelTo0() { FormatterStep step = JsonSimpleStep.create(0, TestProvisioner.mavenCentral()); StepHarness stepHarness = StepHarness.forStep(step); @@ -129,7 +126,7 @@ protected FormatterStep create() { }.testEquals(); } - private static void doWithResource(StepHarness stepHarness, String name) throws Exception { + private static void doWithResource(StepHarness stepHarness, String name) { String before = String.format("json/%sBefore.json", name); String after = String.format("json/%sAfter.json", name); stepHarness.testResource(before, after); diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java index 040a23d0a8..5e79181e99 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java @@ -25,42 +25,33 @@ import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; -import com.diffplug.spotless.StepHarness; +import com.diffplug.spotless.StepHarnessWithFile; import com.diffplug.spotless.TestProvisioner; class DiktatStepTest extends ResourceHarness { - @Test void behavior() throws Exception { FormatterStep step = DiktatStep.create(TestProvisioner.mavenCentral()); - StepHarness.forStep(step) - .testResourceException("kotlin/diktat/Unsolvable.kt", assertion -> { - assertion.isInstanceOf(AssertionError.class); - assertion.hasMessage("There are 2 unfixed errors:" + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); - }); + StepHarnessWithFile.forStep(this, step) + .testResourceExceptionMsg("kotlin/diktat/Unsolvable.kt").isEqualTo("There are 2 unfixed errors:" + + System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + + System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + + System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + + System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); } @Test void behaviorConf() throws Exception { - String configPath = "src/main/kotlin/diktat-analysis.yml"; File conf = setFile(configPath).toResource("kotlin/diktat/diktat-analysis.yml"); FileSignature config = signAsList(conf); FormatterStep step = DiktatStep.create("1.0.1", TestProvisioner.mavenCentral(), Collections.emptyMap(), config); - StepHarness.forStep(step) - .testResourceException("kotlin/diktat/Unsolvable.kt", assertion -> { - assertion.isInstanceOf(AssertionError.class); - assertion.hasMessage("There are 2 unfixed errors:" + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); - }); + StepHarnessWithFile.forStep(this, step) + .testResourceExceptionMsg("kotlin/diktat/Unsolvable.kt").isEqualTo("There are 2 unfixed errors:" + + System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + + System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + + System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + + System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); } - } diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java index a3548c58b6..619fce03a5 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -34,11 +34,7 @@ void behavior() throws Exception { FormatterStep step = KtLintStep.create(TestProvisioner.mavenCentral()); StepHarness.forStep(step) .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceException("kotlin/ktlint/unsolvable.dirty", assertion -> { - assertion.isInstanceOf(AssertionError.class); - assertion.hasMessage("Error on line: 1, column: 2\n" + - "Wildcard import"); - }); + .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 2\nWildcard import"); } @Test @@ -46,11 +42,7 @@ void worksShyiko() throws Exception { FormatterStep step = KtLintStep.create("0.31.0", TestProvisioner.mavenCentral()); StepHarness.forStep(step) .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceException("kotlin/ktlint/unsolvable.dirty", assertion -> { - assertion.isInstanceOf(AssertionError.class); - assertion.hasMessage("Error on line: 1, column: 1\n" + - "Wildcard import"); - }); + .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 1\nWildcard import"); } // Regression test to ensure it works on the version it switched to Pinterest (version 0.32.0) @@ -61,11 +53,7 @@ void worksPinterestAndPre034() throws Exception { FormatterStep step = KtLintStep.create("0.32.0", TestProvisioner.mavenCentral()); StepHarness.forStep(step) .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceException("kotlin/ktlint/unsolvable.dirty", assertion -> { - assertion.isInstanceOf(AssertionError.class); - assertion.hasMessage("Error on line: 1, column: 1\n" + - "Wildcard import"); - }); + .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 1\nWildcard import"); } // Regression test to handle alpha and 1.x version numbers diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java index d2d866e97d..e580a03603 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -25,13 +25,14 @@ import com.diffplug.common.collect.ImmutableMap; import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarness; import com.diffplug.spotless.StepHarnessWithFile; import com.diffplug.spotless.TestProvisioner; import com.diffplug.spotless.tag.NpmTest; @NpmTest -class PrettierFormatterStepTest { +class PrettierFormatterStepTest extends ResourceHarness { @NpmTest @Nested @@ -96,8 +97,8 @@ void parserInferenceBasedOnFilenameIsWorking() throws Exception { npmPathResolver(), new PrettierConfig(null, Collections.emptyMap())); - try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(formatterStep)) { - stepHarness.testResource(new File("test.json"), dirtyFile, cleanFile); + try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { + stepHarness.testResource("test.json", dirtyFile, cleanFile); } } @@ -110,10 +111,8 @@ void verifyPrettierErrorMessageIsRelayed() throws Exception { npmPathResolver(), new PrettierConfig(null, ImmutableMap.of("parser", "postcss"))); try (StepHarness stepHarness = StepHarness.forStep(formatterStep)) { - stepHarness.testResourceException("npm/prettier/filetypes/scss/scss.dirty", exception -> { - exception.hasMessageContaining("HTTP 501"); - exception.hasMessageContaining("Couldn't resolve parser \"postcss\""); - }); + stepHarness.testResourceExceptionMsg("npm/prettier/filetypes/scss/scss.dirty").startsWith( + "com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException: Unexpected response status code at /prettier/format [HTTP 501] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\")"); } } } From 3f72b7ef6e7452ec8887741729b7b44dda75a2f4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 11:29:08 -0800 Subject: [PATCH 25/31] When a step throws an exception, we now accurately consider the entire file as linted, rather than just the first line. --- lib/src/main/java/com/diffplug/spotless/Formatter.java | 2 +- lib/src/main/java/com/diffplug/spotless/Lint.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 012f81d816..b646bbca6d 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -160,7 +160,7 @@ public List lint(String content, File file) { totalLints.addAll(lints); } } catch (Throwable e) { - totalLints.add(Lint.createFromThrowable(step, e)); + totalLints.add(Lint.createFromThrowable(step, content, e)); } } if (totalLints.isEmpty()) { diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index 8aa94293b1..f408a32cce 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -173,7 +173,7 @@ public static void toFile(List lints, File file) throws IOException { } /** Attempts to parse a line number from the given exception. */ - static Lint createFromThrowable(FormatterStep step, Throwable e) { + static Lint createFromThrowable(FormatterStep step, String content, Throwable e) { Throwable current = e; while (current != null) { String message = current.getMessage(); @@ -183,7 +183,8 @@ static Lint createFromThrowable(FormatterStep step, Throwable e) { } current = current.getCause(); } - return Lint.create(step.getName(), ThrowingEx.stacktrace(e), 1); + 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) { From 1ed3149c67479298043d2344d1a3ee594c6e9bc8 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 16:33:10 -0800 Subject: [PATCH 26/31] Fix tests. --- .../spotless/extra/GitAttributesTest.java | 17 ++++----- .../spotless/StepHarnessWithFile.java | 9 +++-- .../spotless/kotlin/DiktatStepTest.java | 35 +++---------------- .../spotless/kotlin/KtLintStepTest.java | 20 +++++------ .../spotless/scala/ScalaFmtStepTest.java | 25 ++++--------- 5 files changed, 34 insertions(+), 72 deletions(-) diff --git a/lib-extra/src/test/java/com/diffplug/spotless/extra/GitAttributesTest.java b/lib-extra/src/test/java/com/diffplug/spotless/extra/GitAttributesTest.java index 35ac58ee85..4102e8cfb8 100644 --- a/lib-extra/src/test/java/com/diffplug/spotless/extra/GitAttributesTest.java +++ b/lib-extra/src/test/java/com/diffplug/spotless/extra/GitAttributesTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -24,23 +24,18 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; -import com.diffplug.common.base.Errors; import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.ResourceHarness; class GitAttributesTest extends ResourceHarness { private List testFiles() { - try { - List result = new ArrayList<>(); - for (String path : TEST_PATHS) { - setFile(path).toContent(""); - result.add(newFile(path)); - } - return result; - } catch (IOException e) { - throw Errors.asRuntime(e); + List result = new ArrayList<>(); + for (String path : TEST_PATHS) { + setFile(path).toContent(""); + result.add(newFile(path)); } + return result; } private static List TEST_PATHS = Arrays.asList("someFile", "subfolder/someFile", "MANIFEST.MF", "subfolder/MANIFEST.MF"); diff --git a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java index ad4288d4f2..c918978162 100644 --- a/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java +++ b/testlib/src/main/java/com/diffplug/spotless/StepHarnessWithFile.java @@ -18,7 +18,6 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; -import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -84,9 +83,13 @@ public StepHarnessWithFile testResourceUnaffected(String resourceIdempotent) { return testUnaffected(file, contentBefore); } - public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) throws IOException { + public AbstractStringAssert testResourceExceptionMsg(String resourceBefore) { + return testResourceExceptionMsg(resourceBefore, resourceBefore); + } + + public AbstractStringAssert testResourceExceptionMsg(String filename, String resourceBefore) { String contentBefore = ResourceHarness.getTestResource(resourceBefore); - File file = harness.setFile(resourceBefore).toContent(contentBefore); + File file = harness.setFile(filename).toContent(contentBefore); return testExceptionMsg(file, contentBefore); } diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java index 5e79181e99..8bb7cf9ad1 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java @@ -15,43 +15,18 @@ */ package com.diffplug.spotless.kotlin; -import static com.diffplug.spotless.FileSignature.signAsList; - -import java.io.File; -import java.util.Collections; - import org.junit.jupiter.api.Test; -import com.diffplug.spotless.FileSignature; -import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarnessWithFile; import com.diffplug.spotless.TestProvisioner; class DiktatStepTest extends ResourceHarness { @Test - void behavior() throws Exception { - FormatterStep step = DiktatStep.create(TestProvisioner.mavenCentral()); - StepHarnessWithFile.forStep(this, step) - .testResourceExceptionMsg("kotlin/diktat/Unsolvable.kt").isEqualTo("There are 2 unfixed errors:" + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); - } - - @Test - void behaviorConf() throws Exception { - String configPath = "src/main/kotlin/diktat-analysis.yml"; - File conf = setFile(configPath).toResource("kotlin/diktat/diktat-analysis.yml"); - FileSignature config = signAsList(conf); - - FormatterStep step = DiktatStep.create("1.0.1", TestProvisioner.mavenCentral(), Collections.emptyMap(), config); - StepHarnessWithFile.forStep(this, step) - .testResourceExceptionMsg("kotlin/diktat/Unsolvable.kt").isEqualTo("There are 2 unfixed errors:" + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " + - System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" + - System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: vs Unsolvable"); + void behavior() { + StepHarnessWithFile.forStep(this, DiktatStep.create(TestProvisioner.mavenCentral())) + .testResource("src/main/kotlin/Main.kt", "kotlin/diktat/main.dirty", "kotlin/diktat/main.clean"); + StepHarnessWithFile.forStep(this, DiktatStep.create("1.0.1", TestProvisioner.mavenCentral())) + .testResource("src/main/kotlin/Main.kt", "kotlin/diktat/main.dirty", "kotlin/diktat/main.clean"); } } diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java index 619fce03a5..80eb71f76c 100644 --- a/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/kotlin/KtLintStepTest.java @@ -20,7 +20,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.SerializableEqualityTester; -import com.diffplug.spotless.StepHarness; +import com.diffplug.spotless.StepHarnessWithFile; import com.diffplug.spotless.TestProvisioner; /** @@ -32,17 +32,17 @@ class KtLintStepTest extends ResourceHarness { @Test void behavior() throws Exception { FormatterStep step = KtLintStep.create(TestProvisioner.mavenCentral()); - StepHarness.forStep(step) - .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 2\nWildcard import"); + StepHarnessWithFile.forStep(this, step) + .testResource("basic.kt", "kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") + .testResourceExceptionMsg("basic.kt", "kotlin/ktlint/unsolvable.dirty").startsWith("java.lang.AssertionError: Error on line: 1, column: 2\nWildcard import"); } @Test void worksShyiko() throws Exception { FormatterStep step = KtLintStep.create("0.31.0", TestProvisioner.mavenCentral()); - StepHarness.forStep(step) - .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 1\nWildcard import"); + StepHarnessWithFile.forStep(this, step) + .testResource("basic.kt", "kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") + .testResourceExceptionMsg("basic.kt", "kotlin/ktlint/unsolvable.dirty").startsWith("java.lang.AssertionError: Error on line: 1, column: 1\nWildcard import"); } // Regression test to ensure it works on the version it switched to Pinterest (version 0.32.0) @@ -51,9 +51,9 @@ void worksShyiko() throws Exception { @Test void worksPinterestAndPre034() throws Exception { FormatterStep step = KtLintStep.create("0.32.0", TestProvisioner.mavenCentral()); - StepHarness.forStep(step) + StepHarnessWithFile.forStep(this, step) .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean") - .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 1\nWildcard import"); + .testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").startsWith("java.lang.AssertionError: Error on line: 1, column: 1\nWildcard import"); } // Regression test to handle alpha and 1.x version numbers @@ -61,7 +61,7 @@ void worksPinterestAndPre034() throws Exception { @Test void worksAlpha1() throws Exception { FormatterStep step = KtLintStep.create("0.38.0-alpha01", TestProvisioner.mavenCentral()); - StepHarness.forStep(step) + StepHarnessWithFile.forStep(this, step) .testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean"); } diff --git a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java index 0f42252d36..c74647dc0e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 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. @@ -15,12 +15,8 @@ */ package com.diffplug.spotless.scala; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; - import java.io.File; import java.io.IOException; -import java.lang.reflect.InvocationTargetException; import org.junit.jupiter.api.Test; @@ -113,18 +109,11 @@ void invalidConfiguration() throws Exception { File invalidConfFile = createTestFile("scala/scalafmt/scalafmt.invalid.conf"); Provisioner provisioner = TestProvisioner.mavenCentral(); - InvocationTargetException exception; - - exception = assertThrows(InvocationTargetException.class, - () -> StepHarness.forStep(ScalaFmtStep.create("1.1.0", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("Invalid fields: invalidScalaFmtConfigField"); - - exception = assertThrows(InvocationTargetException.class, - () -> StepHarness.forStep(ScalaFmtStep.create("2.0.1", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("Invalid field: invalidScalaFmtConfigField"); - - exception = assertThrows(InvocationTargetException.class, - () -> StepHarness.forStep(ScalaFmtStep.create("3.0.0", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("found option 'invalidScalaFmtConfigField' which wasn't expected"); + StepHarness.forStep(ScalaFmtStep.create("1.1.0", provisioner, invalidConfFile)) + .testExceptionMsg("").contains("Invalid fields: invalidScalaFmtConfigField"); + StepHarness.forStep(ScalaFmtStep.create("2.0.1", provisioner, invalidConfFile)) + .testExceptionMsg("").contains("Invalid field: invalidScalaFmtConfigField"); + StepHarness.forStep(ScalaFmtStep.create("3.0.0", provisioner, invalidConfFile)) + .testExceptionMsg("").contains("found option 'invalidScalaFmtConfigField' which wasn't expected"); } } From 6e3888779ea53ee878e797146cc1526cf0a84a4a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 16:40:37 -0800 Subject: [PATCH 27/31] Fix spotbugs. --- lib/src/main/java/com/diffplug/spotless/Lint.java | 5 ++++- .../java/com/diffplug/spotless/generic/FenceStep.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/com/diffplug/spotless/Lint.java b/lib/src/main/java/com/diffplug/spotless/Lint.java index f408a32cce..018424cdad 100644 --- a/lib/src/main/java/com/diffplug/spotless/Lint.java +++ b/lib/src/main/java/com/diffplug/spotless/Lint.java @@ -17,6 +17,7 @@ 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; @@ -29,7 +30,9 @@ * 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 { +public final class Lint implements Serializable { + private static final long serialVersionUID = 1L; + private int lineStart, lineEnd; // 1-indexed 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; 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 e96893b2bc..629dfcf387 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -25,6 +25,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.graalvm.compiler.core.common.SuppressFBWarnings; + import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; @@ -96,6 +98,8 @@ public FormatterStep applyWithin(List steps) { } static class ApplyWithin extends BaseImplementation implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + private static final long serialVersionUID = 1L; + ApplyWithin(Pattern regex, List steps) { super(regex, steps); } @@ -114,6 +118,8 @@ public String apply(Formatter formatter, String unix, File file) throws Exceptio } static class PreserveWithin extends BaseImplementation implements FormatterFunc.Closeable.ResourceFuncNeedsFile { + private static final long serialVersionUID = 1L; + PreserveWithin(Pattern regex, List steps) { super(regex, steps); } @@ -147,7 +153,10 @@ public List lint(Formatter formatter, String content, File file) throws Ex } } + @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "accessed via getters that repopulate") static class BaseImplementation implements Serializable { + private static final long serialVersionUID = 1L; + final Pattern regex; final List steps; @@ -223,6 +232,8 @@ protected String assembleGroups(String unix) throws IntermediateStepRemovedExcep } static class IntermediateStepRemovedException extends Exception { + private static final long serialVersionUID = 1L; + Lint lint; IntermediateStepRemovedException(Lint lint) { From eb0feed20fe3055f98081af7babae7dac49fbf6e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 17:00:10 -0800 Subject: [PATCH 28/31] Fix wrong SuppressFBWarnings import. --- .../main/java/com/diffplug/spotless/generic/FenceStep.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 629dfcf387..31caa93118 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/FenceStep.java @@ -25,14 +25,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.graalvm.compiler.core.common.SuppressFBWarnings; - 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; + public class FenceStep { /** Declares the name of the step. */ public static FenceStep named(String name) { From 248ec28d584bf7878b86302af1ccf5218f79dd48 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 18:45:23 -0800 Subject: [PATCH 29/31] Fix tests on windows. --- .../src/main/java/com/diffplug/spotless/ResourceHarness.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index cd367e4374..725ae5f299 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -90,7 +90,7 @@ protected static String getTestResource(String filename) { if (url == null) { throw new IllegalArgumentException("No such resource " + filename); } - return ThrowingEx.get(() -> Resources.toString(url, StandardCharsets.UTF_8)); + return ThrowingEx.get(() -> LineEnding.toUnix(Resources.toString(url, StandardCharsets.UTF_8))); } /** Returns Files (in a temporary folder) which has the contents of the given file from the src/test/resources directory. */ From 3aa780ecb99c66d040f73d1422b76b27d4837d04 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 18:46:23 -0800 Subject: [PATCH 30/31] Fix prettier error test. --- .../com/diffplug/spotless/npm/PrettierFormatterStepTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java index e580a03603..256927a3f6 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java @@ -110,7 +110,7 @@ void verifyPrettierErrorMessageIsRelayed() throws Exception { buildDir(), npmPathResolver(), new PrettierConfig(null, ImmutableMap.of("parser", "postcss"))); - try (StepHarness stepHarness = StepHarness.forStep(formatterStep)) { + try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { stepHarness.testResourceExceptionMsg("npm/prettier/filetypes/scss/scss.dirty").startsWith( "com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException: Unexpected response status code at /prettier/format [HTTP 501] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\")"); } From e110426658b971f002b3fe60e94a7cc6861d122c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 14 Jan 2022 18:46:34 -0800 Subject: [PATCH 31/31] Make our testing parallel. --- gradle.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/gradle.properties b/gradle.properties index fcad5c5ed4..93d4daa76b 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,6 @@ # To fix metaspace errors org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m -Dfile.encoding=UTF-8 +org.gradle.parallel=true name=spotless description=Spotless - keep your code spotless with Gradle org=diffplug