From 2ce963284ef612ee0a8ad734e94f022ef9738a72 Mon Sep 17 00:00:00 2001 From: ghm Date: Wed, 23 Oct 2024 07:04:25 -0700 Subject: [PATCH] Strip the quotation marks from the source code when reconstructing the literal. My comment was a lie. This does obviously matter, because it's important to work out if the \s is at the end of the entire string. PiperOrigin-RevId: 688949631 --- .../bugpatterns/MisleadingEscapedSpace.java | 55 ++++++++++--------- .../MisleadingEscapedSpaceTest.java | 33 +++++++++++ 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java b/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java index 800d30f98ab..3290b2d40b2 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java @@ -19,7 +19,6 @@ import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.SourceVersion.supportsTextBlocks; -import static java.util.stream.Collectors.joining; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; @@ -56,32 +55,38 @@ public Description matchLiteral(LiteralTree tree, VisitorState state) { // Tokenize the source to make sure we omit comments. Desugaring of "a" + "b" into a single // string literal happens really early in compilation, and we want to ensure we don't match // on any "\s" in comments. - // This is quite ugly in that we end up with a string that looks like "foo""bar", i.e. - // including the quotes, but that doesn't matter for this check. var tokens = ErrorProneTokens.getTokens(source, state.context); - var literal = - tokens.stream() - .filter(t -> t.kind().equals(TokenKind.STRINGLITERAL)) - .map(t -> source.substring(t.pos(), t.endPos())) - .collect(joining()); - boolean seenEscape = false; - for (int i = 0; i < literal.length(); ++i) { - switch (literal.charAt(i)) { - case '\n': - seenEscape = false; - break; - case '\\': - i++; - if (literal.charAt(i) == 's') { - seenEscape = true; + for (var token : tokens) { + if (!token.kind().equals(TokenKind.STRINGLITERAL)) { + continue; + } + var sourceWithQuotes = source.substring(token.pos(), token.endPos()); + boolean isBlockLiteral = sourceWithQuotes.startsWith("\"\"\""); + int quoteSize = isBlockLiteral ? 3 : 1; + var literal = sourceWithQuotes.substring(quoteSize, sourceWithQuotes.length() - quoteSize); + boolean seenEscape = false; + for (int i = 0; i < literal.length(); ++i) { + switch (literal.charAt(i)) { + case '\n': + seenEscape = false; + break; + case '\\': + i++; + if (literal.charAt(i) == 's') { + seenEscape = true; + break; + } + // fall through + default: + if (seenEscape) { + return describeMatch(tree); + } break; - } - // fall through - default: - if (seenEscape) { - return describeMatch(tree); - } - break; + } + } + // Catch _trailing_ \s at the end of non-block literals. + if (seenEscape && !isBlockLiteral) { + return describeMatch(tree); } } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java index 348a4058640..8bfa78708b2 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java @@ -140,4 +140,37 @@ class Test { }""") .doTest(); } + + @Test + public void atEndOfString_noFinding() { + assume().that(Runtime.version().feature()).isAtLeast(14); + + testHelper + .addSourceLines( + "Test.class", + """ + class Test { + private static final String FOO = + \""" + foo + bar\\s\"""; + } + """) + .doTest(); + } + + @Test + public void escapedSpaceAtEndOfString() { + assume().that(Runtime.version().feature()).isAtLeast(14); + + testHelper + .addSourceLines( + "Test.class", + """ + class Test { + // BUG: Diagnostic contains: + private static final String FOO = "foo\\s"; + }""") + .doTest(); + } }