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 398ccca1ad..1e0ee03761 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 @@ -18,19 +18,17 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.nio.charset.Charset; import java.nio.file.Files; import java.util.List; import java.util.ListIterator; import java.util.Objects; -import org.eclipse.jgit.diff.DiffFormatter; import org.eclipse.jgit.diff.EditList; -import org.eclipse.jgit.diff.MyersDiff; +import org.eclipse.jgit.diff.HistogramDiff; import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.diff.RawTextComparator; -import com.diffplug.common.base.CharMatcher; import com.diffplug.common.base.Errors; import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Splitter; @@ -168,7 +166,9 @@ private void addIntendedLine(String indent, String line) { * sequence (\n, \r, \r\n). */ private static String diff(Builder builder, File file) throws IOException { - String raw = new String(Files.readAllBytes(file.toPath()), builder.formatter.getEncoding()); + byte[] rawBytes = Files.readAllBytes(file.toPath()); + Charset encoding = builder.formatter.getEncoding(); + String raw = new String(rawBytes, encoding); String rawUnix = LineEnding.toUnix(raw); String formattedUnix; if (builder.isPaddedCell) { @@ -177,61 +177,18 @@ private static String diff(Builder builder, File file) throws IOException { formattedUnix = builder.formatter.compute(rawUnix, file); } - if (rawUnix.equals(formattedUnix)) { - // the formatting is fine, so it's a line-ending issue - String formatted = builder.formatter.computeLineEndings(formattedUnix, file); - return diffWhitespaceLineEndings(raw, formatted, false, true); - } else { - return diffWhitespaceLineEndings(rawUnix, formattedUnix, true, false); - } + String formatted = builder.formatter.computeLineEndings(formattedUnix, file); + // TODO: use per-file encoding (e.g. from .editorconfig) + byte[] formattedBytes = formatted.getBytes(encoding); + return visualizeDiff(rawBytes, formattedBytes, encoding); } - /** - * Returns a git-style diff between the two unix strings. - * - * Output has no trailing newlines. - * - * Boolean args determine whether whitespace or line endings will be visible. - */ - private static String diffWhitespaceLineEndings(String dirty, String clean, boolean whitespace, boolean lineEndings) throws IOException { - dirty = visibleWhitespaceLineEndings(dirty, whitespace, lineEndings); - clean = visibleWhitespaceLineEndings(clean, whitespace, lineEndings); - - RawText a = new RawText(dirty.getBytes(StandardCharsets.UTF_8)); - RawText b = new RawText(clean.getBytes(StandardCharsets.UTF_8)); - EditList edits = new EditList(); - edits.addAll(MyersDiff.INSTANCE.diff(RawTextComparator.DEFAULT, a, b)); - + private static String visualizeDiff(byte[] rawBytes, byte[] formattedBytes, Charset encoding) throws IOException { + RawText a = new RawText(rawBytes); + RawText b = new RawText(formattedBytes); + EditList edits = new HistogramDiff().diff(RawTextComparator.DEFAULT, a, b); ByteArrayOutputStream out = new ByteArrayOutputStream(); - try (DiffFormatter formatter = new DiffFormatter(out)) { - formatter.format(edits, a, b); - } - String formatted = out.toString(StandardCharsets.UTF_8.name()); - - // we don't need the diff to show this, since we display newlines ourselves - formatted = formatted.replace("\\ No newline at end of file\n", ""); - return NEWLINE_MATCHER.trimTrailingFrom(formatted); + new WriteSpaceAwareDiffFormatter(out, encoding).format(edits, a, b); + return new String(out.toByteArray(), encoding); } - - private static final CharMatcher NEWLINE_MATCHER = CharMatcher.is('\n'); - - /** - * Makes the whitespace and/or the lineEndings visible. - * - * MyersDiff wants inputs with only unix line endings. So this ensures that that is the case. - */ - private static String visibleWhitespaceLineEndings(String input, boolean whitespace, boolean lineEndings) { - if (whitespace) { - input = input.replace(' ', MIDDLE_DOT).replace("\t", "\\t"); - } - if (lineEndings) { - input = input.replace("\n", "\\n\n").replace("\r", "\\r"); - } else { - // we want only \n, so if we didn't replace them above, we'll replace them here. - input = input.replace("\r", ""); - } - return input; - } - - private static final char MIDDLE_DOT = '\u00b7'; } diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/WriteSpaceAwareDiffFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/WriteSpaceAwareDiffFormatter.java new file mode 100644 index 0000000000..6581768366 --- /dev/null +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/WriteSpaceAwareDiffFormatter.java @@ -0,0 +1,210 @@ +/* + * 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.extra.integration; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.Charset; + +import org.bouncycastle.util.Arrays; +import org.eclipse.jgit.diff.Edit; +import org.eclipse.jgit.diff.EditList; +import org.eclipse.jgit.diff.RawText; +import org.eclipse.jgit.util.IntList; +import org.eclipse.jgit.util.RawParseUtils; + +/** + * Formats the diff in Git-like style, however it makes whitespace visible for + * edit-like diffs (when one fragment is replaced with another). + */ +class WriteSpaceAwareDiffFormatter { + private static final int CONTEXT_LINES = 3; + private static final String MIDDLE_DOT = "\u00b7"; + private static final String CR = "\u240d"; + private static final String LF = "\u240a"; + private static final String TAB = "\u21e5"; + private static final byte[] ONE_SPACE = new byte[]{' '}; + private static final byte[] CR_SIMPLE = new byte[]{'\\', 'r'}; + private static final byte[] LF_SIMPLE = new byte[]{'\\', 'n'}; + private static final byte[] TAB_SIMPLE = new byte[]{'\\', 't'}; + + private final ByteArrayOutputStream out; + private final byte[] middleDot; + private final byte[] cr; + private final byte[] lf; + private final byte[] tab; + + /** + * Creates the formatter. + * @param out output stream for the resulting diff. The diff would have \n line endings + * @param charset the charset for the text to use + */ + public WriteSpaceAwareDiffFormatter(ByteArrayOutputStream out, Charset charset) { + this.out = out; + this.middleDot = replacementFor(MIDDLE_DOT, charset, ONE_SPACE); + this.cr = replacementFor(CR, charset, CR_SIMPLE); + this.lf = replacementFor(LF, charset, LF_SIMPLE); + this.tab = replacementFor(TAB, charset, TAB_SIMPLE); // \u2409 is missing in lots of the fonts + } + + private static byte[] replacementFor(String value, Charset charset, byte[] defaultValue) { + try { + ByteBuffer buffer = charset.newEncoder() + .replaceWith(ONE_SPACE) + .encode(CharBuffer.wrap(value)); + return Arrays.copyOf(buffer.array(), buffer.remaining()); + } catch (CharacterCodingException e) { + return defaultValue; + } + } + + /** + * Formats the diff. + * @param edits the list of edits to format + * @param a input text a, with \n line endings + * @param b input text b, with \n line endings + * @throws IOException if formatting fails + */ + public void format(EditList edits, RawText a, RawText b) throws IOException { + IntList linesA = RawParseUtils.lineMap(a.getRawContent(), 0, a.getRawContent().length); + IntList linesB = RawParseUtils.lineMap(b.getRawContent(), 0, b.getRawContent().length); + boolean firstLine = true; + for (int i = 0; i < edits.size(); i++) { + Edit edit = edits.get(i); + int lineA = Math.max(0, edit.getBeginA() - CONTEXT_LINES); + int lineB = Math.max(0, edit.getBeginB() - CONTEXT_LINES); + + final int endIdx = findCombinedEnd(edits, i); + final Edit endEdit = edits.get(endIdx); + + int endA = Math.min(a.size(), endEdit.getEndA() + CONTEXT_LINES); + int endB = Math.min(b.size(), endEdit.getEndB() + CONTEXT_LINES); + + if (firstLine) { + firstLine = false; + } else { + out.write('\n'); + } + header(lineA, endA, lineB, endB); + + boolean showWhitespace = edit.getType() == Edit.Type.REPLACE; + + while (lineA < endA || lineB < endB) { + if (lineA < edit.getBeginA()) { + // Common part before the diff + line(' ', a, lineA, linesA, false); + lineA++; + lineB++; + } else if (lineA < edit.getEndA()) { + line('-', a, lineA, linesA, showWhitespace); + lineA++; + } else if (lineB < edit.getEndB()) { + line('+', b, lineB, linesB, showWhitespace); + lineB++; + } else { + // Common part after the diff + line(' ', a, lineA, linesA, false); + lineA++; + lineB++; + } + + if (lineA == edit.getEndA() && lineB == edit.getEndB() && i < endIdx) { + i++; + edit = edits.get(i); + showWhitespace = edit.getType() == Edit.Type.REPLACE; + } + } + } + } + + /** + * There might be multiple adjacent diffs, so we need to figure out the latest one in the group. + * @param edits list of edits + * @param i starting edit + * @return the index of the latest edit in the group + */ + private int findCombinedEnd(EditList edits, int i) { + for (; i < edits.size() - 1; i++) { + Edit current = edits.get(i); + Edit next = edits.get(i + 1); + if (current.getEndA() - next.getBeginA() > 2 * CONTEXT_LINES && + current.getEndB() - next.getBeginB() > 2 * CONTEXT_LINES) { + break; + } + } + return i; + } + + private void header(int lineA, int endA, int lineB, int endB) { + out.write('@'); + out.write('@'); + range('-', lineA + 1, endA - lineA); + range('+', lineB + 1, endB - lineB); + out.write(' '); + out.write('@'); + out.write('@'); + } + + private void range(char prefix, int begin, int length) { + out.write(' '); + out.write(prefix); + if (length == 0) { + writeInt(begin - 1); + out.write(','); + out.write('0'); + } else { + writeInt(begin); + if (length > 1) { + out.write(','); + writeInt(length); + } + } + } + + private void writeInt(int num) { + String str = Integer.toString(num); + for (int i = 0, len = str.length(); i < len; i++) { + out.write(str.charAt(i)); + } + } + + private void line(char prefix, RawText a, int lineA, IntList lines, boolean showWhitespace) throws IOException { + out.write('\n'); + out.write(prefix); + if (!showWhitespace) { + a.writeLine(out, lineA); + return; + } + byte[] bytes = a.getRawContent(); + for (int i = lines.get(lineA + 1), end = lines.get(lineA + 2); i < end; i++) { + byte b = bytes[i]; + if (b == ' ') { + out.write(middleDot); + } else if (b == '\t') { + out.write(tab); + } else if (b == '\r') { + out.write(cr); + } else if (b == '\n') { + out.write(lf); + } else { + out.write(b); + } + } + } +} diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java index a46f9f9824..8f55235fea 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java @@ -77,12 +77,12 @@ public void lineEndingProblem() throws Exception { assertTaskFailure(task, " testFile", " @@ -1,3 +1,3 @@", - " -A\\r\\n", - " -B\\r\\n", - " -C\\r\\n", - " +A\\n", - " +B\\n", - " +C\\n"); + " -A␍␊", + " -B␍␊", + " -C␍␊", + " +A␊", + " +B␊", + " +C␊"); } @Test @@ -95,12 +95,85 @@ public void whitespaceProblem() throws Exception { assertTaskFailure(task, " testFile", " @@ -1,3 +1,3 @@", - " -A·", - " -B\\t", - " -C··", - " +A", - " +B", - " +C"); + " -A·␊", + " -B⇥␊", + " -C··␊", + " +A␊", + " +B␊", + " +C␊"); + } + + @Test + public void whitespaceProblem2() throws Exception { + SpotlessTask task = create(setFile("testFile").toContent( + "u0\nu 1\nu 2\nu 3\n" + + "\t leading space\n" + + "trailing space\t \n" + + "u 4\n" + + " leading and trailing space \n" + + "u 5\nu 6")); + task.addStep(FormatterStep.createNeverUpToDate("trimTrailing", input -> { + Pattern pattern = Pattern.compile("(^[ \t]+)|([ \t]+$)", Pattern.UNIX_LINES | Pattern.MULTILINE); + return pattern.matcher(input).replaceAll(""); + })); + assertTaskFailure(task, + " testFile", + " @@ -2,9 +2,9 @@", + " u 1", + " u 2", + " u 3", + " -⇥·leading·space␊", + " -trailing·space⇥·␊", + " +leading·space␊", + " +trailing·space␊", + " u 4", + " -··leading·and·trailing·space··␊", + " +leading·and·trailing·space␊", + " u 5", + " u 6"); + } + + @Test + public void singleLineCr() throws Exception { + SpotlessTask task = create(setFile("testFile").toContent( + "line without line ending")); + task.addStep(FormatterStep.createNeverUpToDate("trimTrailing", + input -> input.endsWith("\n") ? input : input + "\n")); + assertTaskFailure(task, + " testFile", + " @@ -1 +1 @@", + " -line·without·line·ending", + " +line·without·line·ending␊"); + } + + @Test + public void singleLineUnnecessaryCr() throws Exception { + SpotlessTask task = create(setFile("testFile").toContent( + "line without line ending\r\n")); + task.addStep(FormatterStep.createNeverUpToDate("trimTrailing", + input -> input.replaceAll("[\r\n]+$", ""))); + assertTaskFailure(task, + " testFile", + " @@ -1 +1 @@", + " -line·without·line·ending␍␊", + " +line·without·line·ending"); + } + + @Test + public void trailingWhitespaceAndCRLF() throws Exception { + SpotlessTask task = create(setFile("testFile").toContent( + "line 1\ntrailing whitespace \nline with CRLF\r\nline 2")); + task.addStep(FormatterStep.createNeverUpToDate("trimTrailing", + input -> input.replaceAll("[\r ]+\n", "\n"))); + assertTaskFailure(task, + " testFile", + " @@ -1,4 +1,4 @@", + " line 1", + " -trailing·whitespace··␊", + " -line·with·CRLF␍␊", + " +trailing·whitespace␊", + " +line·with·CRLF␊", + " line 2"); } @Test @@ -111,15 +184,15 @@ public void multipleFiles() throws Exception { assertTaskFailure(task, " A", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " B", " @@ -1,2 +1,2 @@", - " 3\\n", - " -4\\r\\n", - " +4\\n"); + " 3", + " -4␍␊", + " +4␊"); } @Test @@ -132,56 +205,56 @@ public void manyFiles() throws Exception { assertTaskFailure(task, " 0.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 1.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 2.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 3.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 4.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 5.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 6.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 7.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 8.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", + " -1␍␊", + " -2␍␊", " ... (2 more lines that didn't fit)", "Violations also present in:", " 9.txt", @@ -205,56 +278,56 @@ public void manyManyFiles() throws Exception { assertTaskFailure(task, " 0.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 1.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 2.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 3.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 4.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 5.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 6.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 7.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", - " +1\\n", - " +2\\n", + " -1␍␊", + " -2␍␊", + " +1␊", + " +2␊", " 8.txt", " @@ -1,2 +1,2 @@", - " -1\\r\\n", - " -2\\r\\n", + " -1␍␊", + " -2␍␊", " ... (2 more lines that didn't fit)", "Violations also present in " + DiffMessageFormatter.MAX_FILES_TO_LIST + " other files."); } @@ -270,54 +343,54 @@ public void longFile() throws Exception { assertTaskFailure(task, " testFile", " @@ -1,1000 +1,1000 @@", - " -0\\r\\n", - " -1\\r\\n", - " -2\\r\\n", - " -3\\r\\n", - " -4\\r\\n", - " -5\\r\\n", - " -6\\r\\n", - " -7\\r\\n", - " -8\\r\\n", - " -9\\r\\n", - " -10\\r\\n", - " -11\\r\\n", - " -12\\r\\n", - " -13\\r\\n", - " -14\\r\\n", - " -15\\r\\n", - " -16\\r\\n", - " -17\\r\\n", - " -18\\r\\n", - " -19\\r\\n", - " -20\\r\\n", - " -21\\r\\n", - " -22\\r\\n", - " -23\\r\\n", - " -24\\r\\n", - " -25\\r\\n", - " -26\\r\\n", - " -27\\r\\n", - " -28\\r\\n", - " -29\\r\\n", - " -30\\r\\n", - " -31\\r\\n", - " -32\\r\\n", - " -33\\r\\n", - " -34\\r\\n", - " -35\\r\\n", - " -36\\r\\n", - " -37\\r\\n", - " -38\\r\\n", - " -39\\r\\n", - " -40\\r\\n", - " -41\\r\\n", - " -42\\r\\n", - " -43\\r\\n", - " -44\\r\\n", - " -45\\r\\n", - " -46\\r\\n", - " -47\\r\\n", + " -0␍␊", + " -1␍␊", + " -2␍␊", + " -3␍␊", + " -4␍␊", + " -5␍␊", + " -6␍␊", + " -7␍␊", + " -8␍␊", + " -9␍␊", + " -10␍␊", + " -11␍␊", + " -12␍␊", + " -13␍␊", + " -14␍␊", + " -15␍␊", + " -16␍␊", + " -17␍␊", + " -18␍␊", + " -19␍␊", + " -20␍␊", + " -21␍␊", + " -22␍␊", + " -23␍␊", + " -24␍␊", + " -25␍␊", + " -26␍␊", + " -27␍␊", + " -28␍␊", + " -29␍␊", + " -30␍␊", + " -31␍␊", + " -32␍␊", + " -33␍␊", + " -34␍␊", + " -35␍␊", + " -36␍␊", + " -37␍␊", + " -38␍␊", + " -39␍␊", + " -40␍␊", + " -41␍␊", + " -42␍␊", + " -43␍␊", + " -44␍␊", + " -45␍␊", + " -46␍␊", + " -47␍␊", " ... (1952 more lines that didn't fit)"); } }