From 4943dda381dc0631c733716db17dc23fd82c19b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 24 Apr 2022 19:18:23 +0200 Subject: [PATCH] Cleanup trimming logic --- .../pmd/lang/apex/ast/ApexTreeBuilder.java | 2 +- .../sourceforge/pmd/cpd/SimpleRenderer.java | 6 +- .../sourceforge/pmd/lang/document/Chars.java | 16 +++-- .../sourceforge/pmd/util/CollectionUtil.java | 11 +++ .../net/sourceforge/pmd/util/StringUtil.java | 71 +++++++++---------- .../sourceforge/pmd/util/StringUtilTest.java | 25 +++++++ .../pmd/lang/java/ast/ASTStringLiteral.java | 7 +- 7 files changed, 92 insertions(+), 46 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java index c0a8b441c8..ca5e912e3c 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java @@ -436,7 +436,7 @@ final class ApexTreeBuilder extends AstVisitor { if (checkForCommentSuppression && commentText.startsWith("//")) { Chars trimmed = commentText.subSequence("//".length(), commentText.length()).trimStart(); if (trimmed.startsWith(suppressMarker)) { - Chars userMessage = trimmed.subSequence(suppressMarker.length(), trimmed.length()).trim(); + Chars userMessage = trimmed.subSequence(suppressMarker.length()).trim(); suppressMap.put(source.lineColumnAtOffset(startIdx).getLine(), userMessage.toString()); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/SimpleRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/SimpleRenderer.java index 79e8cea468..5ce8ca7d48 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/SimpleRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/SimpleRenderer.java @@ -11,6 +11,7 @@ import java.util.Iterator; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.cpd.renderer.CPDRenderer; +import net.sourceforge.pmd.lang.document.Chars; import net.sourceforge.pmd.util.StringUtil; public class SimpleRenderer implements Renderer, CPDRenderer { @@ -49,8 +50,9 @@ public class SimpleRenderer implements Renderer, CPDRenderer { String source = match.getSourceCodeSlice(); if (trimLeadingWhitespace) { - for (String line : StringUtil.linesWithTrimIndent(source)) { - writer.append(line).append(PMD.EOL); + for (Chars line : StringUtil.linesWithTrimIndent(source)) { + line.writeFully(writer); + writer.append(PMD.EOL); } return; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java index 0b002b22d7..c408bf90b5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/document/Chars.java @@ -363,6 +363,14 @@ public final class Chars implements CharSequence { return slice(start, end - start); } + /** + * Returns the subsequence that starts at the given offset and ends + * at the end of this string. Similar to {@link String#substring(int)}. + */ + public Chars subSequence(int start) { + return slice(start, len - start); + } + /** * Slice a region of text. * @@ -415,11 +423,11 @@ public final class Chars implements CharSequence { } private static void validateRangeWithAssert(int off, int len, int bound) { - assert len >= 0 && off >= 0 && (off + len) <= bound : invalidRange(off, len, bound); + assert len >= 0 && off >= 0 && off + len <= bound : invalidRange(off, len, bound); } private static void validateRange(int off, int len, int bound) { - if (len < 0 || off < 0 || (off + len) > bound) { + if (len < 0 || off < 0 || off + len > bound) { throw new IndexOutOfBoundsException(invalidRange(off, len, bound)); } } @@ -429,7 +437,7 @@ public final class Chars implements CharSequence { } @Override - public String toString() { + public @NonNull String toString() { // this already avoids the copy if start == 0 && len == str.length() return str.substring(start, start + len); } @@ -548,7 +556,7 @@ public final class Chars implements CharSequence { private final int max = start + len; @Override - public int read(char[] cbuf, int off, int len) { + public int read(char @NonNull [] cbuf, int off, int len) { if (len < 0 || off < 0 || off + len > cbuf.length) { throw new IndexOutOfBoundsException(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java index e15cda62cd..82e191cc1a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java @@ -40,6 +40,7 @@ import org.pcollections.PSet; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.internal.util.AssertionUtil; import net.sourceforge.pmd.internal.util.IteratorUtil; +import net.sourceforge.pmd.lang.document.Chars; /** * Generic collection and array-related utility functions for java.util types. @@ -635,6 +636,16 @@ public final class CollectionUtil { return sb; } + public static @NonNull StringBuilder joinCharsIntoStringBuilder(List lines, String delimiter) { + return joinOn( + new StringBuilder(), + lines, + (buf, line) -> line.appendChars(buf), + delimiter + ); + } + + /** * Merge the second map into the first. If some keys are in common, * merge them using the merge function, like {@link Map#merge(Object, Object, BiFunction)}. diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java index 8cc32d3a66..8456b2ccfe 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java @@ -279,8 +279,7 @@ public final class StringUtil { * the left without misaligning them. * *

Note: the spec is described in - * String#stripIndent + * String#stripIndent * * * The minimum indentation (min) is determined as follows: @@ -309,33 +308,36 @@ public final class StringUtil { return maxCommonWs; } + /** + * Returns a list of + */ + public static List linesWithTrimIndent(String source) { + List lines = Arrays.asList(source.split("\n")); + List result = lines.stream().map(Chars::wrap).collect(CollectionUtil.toMutableList()); + trimIndentInPlace(result); + return result; + } /** - * Trims off the leading characters off the strings up to the trimDepth - * specified. Returns the same strings if trimDepth = 0 - * - * @return String[] + * @param lines mutable list */ - private static String[] trimStartOn(String[] strings, int trimDepth) { - - if (trimDepth == 0) { - return strings; + public static void trimIndentInPlace(List lines) { + int trimDepth = maxCommonLeadingWhitespaceForAll(lines); + if (trimDepth > 0) { + lines.replaceAll(chars -> chars.length() >= trimDepth + ? chars.subSequence(trimDepth).trimEnd() + : chars.trimEnd()); } - - String[] results = new String[strings.length]; - for (int i = 0; i < strings.length; i++) { - results[i] = strings[i].substring(trimDepth); - } - return results; } /** * Trim common indentation in the lines of the string, like * {@link #appendWithoutCommonPrefix(List, int, StringBuilder)}. - * */ public static StringBuilder trimIndent(Chars string) { - List lines = string.lineStream().collect(Collectors.toList()); + List lines = string.lineStream().collect(CollectionUtil.toMutableList()); + trimIndentInPlace(lines); + StringBuilder sb = new StringBuilder(string.length()); trimIndentIntoStringBuilder(lines, sb); return sb; @@ -365,7 +367,7 @@ public final class StringUtil { Chars line = lines.get(i); // remove common whitespace prefix if (line.length() >= prefixLength && !StringUtils.isBlank(line)) { - line = line.subSequence(prefixLength, line.length()); + line = line.subSequence(prefixLength); } // trim trailing whitespace line = line.trimEnd(); @@ -398,17 +400,21 @@ public final class StringUtil { } } - int lastNonBlankLine = string.indexOf('\n', offsetOfLastNonBlankChar); - int firstNonBlankLine = string.lastIndexOf('\n', offsetOfFirstNonBlankChar); + // look backwards before the first non-blank char + int cutFromInclusive = string.lastIndexOf('\n', offsetOfFirstNonBlankChar); + // If firstNonBlankLineStart == -1, ie we're on the first line, + // we want to start at zero: then we add 1 to get 0 + // If firstNonBlankLineStart >= 0, then it's the index of the + // \n, we want to cut right after that, so we add 1. + cutFromInclusive += 1; - return string.subSequence( - minus1Default(firstNonBlankLine, 0), - minus1Default(lastNonBlankLine, string.length()) - ); - } + // look forwards after the last non-blank char + int cutUntilExclusive = string.indexOf('\n', offsetOfLastNonBlankChar); + if (cutUntilExclusive == StringUtils.INDEX_NOT_FOUND) { + cutUntilExclusive = string.length(); + } - private static int minus1Default(int i, int defaultValue) { - return i == -1 ? defaultValue : i; + return string.subSequence(cutFromInclusive, cutUntilExclusive); } @@ -420,15 +426,6 @@ public final class StringUtil { return count; } - public static String[] linesWithTrimIndent(String source) { - String[] lines = source.split("\n"); - int trimDepth = maxCommonLeadingWhitespaceForAll(Arrays.asList(lines)); - if (trimDepth > 0) { - lines = trimStartOn(lines, trimDepth); - } - return lines; - } - /** * Are the two String values the same. The Strings can be optionally trimmed diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java index 16fdfa606e..76cb245169 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java @@ -94,6 +94,7 @@ public class StringUtilTest { assertTrimIndent(" \n b \n c\n ", "\nb\nc\n"); + assertTrimIndent("", ""); } private void assertTrimIndent(String input, String output) { @@ -107,4 +108,28 @@ public class StringUtilTest { assertThat(StringUtil.elide("abc", 2, ".."), equalTo("..")); assertThat(StringUtil.elide("abc", 3, ".."), equalTo("abc")); } + + @Test + public void substringAfterLast() { + assertEquals("abc", StringUtil.substringAfterLast("a.abc", '.')); + assertEquals("abc", StringUtil.substringAfterLast("abc", '.')); + } + + @Test + public void trimBlankLines() { + assertTrimBlankLinesEquals(" \n \n abc \n \n de \n \n ", + " abc \n \n de "); + assertTrimBlankLinesEquals("", ""); + } + + private void assertTrimBlankLinesEquals(String input, String output) { + assertEquals( + Chars.wrap(output), + StringUtil.trimBlankLines(Chars.wrap(input)) + ); + } + + @Test + public void linesWithTrimIndent() { + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java index eb6cf5c377..004921b7da 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java @@ -12,6 +12,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import net.sourceforge.pmd.lang.document.Chars; +import net.sourceforge.pmd.util.CollectionUtil; import net.sourceforge.pmd.util.StringUtil; /** @@ -83,8 +84,10 @@ public final class ASTStringLiteral extends AbstractLiteral implements ASTLitera static String determineTextBlockContent(Chars image) { List lines = getContentLines(image); - StringBuilder sb = new StringBuilder(image.length()); - StringUtil.trimIndentIntoStringBuilder(lines, sb); + // remove common prefix + StringUtil.trimIndentInPlace(lines); + // join with normalized end of line + StringBuilder sb = CollectionUtil.joinCharsIntoStringBuilder(lines, "\n"); interpretEscapeSequences(sb); return sb.toString(); }