From 46ac925561c386e2fe8305cb960b65b08033374c Mon Sep 17 00:00:00 2001 From: ghm Date: Mon, 14 Oct 2024 08:45:24 -0700 Subject: [PATCH] MisleadingEscapedSpace: tokenize so that we don't complain about comments. PiperOrigin-RevId: 685720735 --- .../bugpatterns/MisleadingEscapedSpace.java | 20 ++++++++++++++++--- .../MisleadingEscapedSpaceTest.java | 14 +++++++++++++ 2 files changed, 31 insertions(+), 3 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 ff26b540ad2..800d30f98ab 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpace.java @@ -19,12 +19,15 @@ 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; import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher; import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ErrorProneTokens; import com.sun.source.tree.LiteralTree; +import com.sun.tools.javac.parser.Tokens.TokenKind; /** See the summary. */ @BugPattern( @@ -50,15 +53,26 @@ public Description matchLiteral(LiteralTree tree, VisitorState state) { return NO_MATCH; } String source = state.getSourceForNode(tree); + // 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 < source.length(); ++i) { - switch (source.charAt(i)) { + for (int i = 0; i < literal.length(); ++i) { + switch (literal.charAt(i)) { case '\n': seenEscape = false; break; case '\\': i++; - if (source.charAt(i) == 's') { + if (literal.charAt(i) == 's') { seenEscape = true; break; } 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 808edea9e61..348a4058640 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/MisleadingEscapedSpaceTest.java @@ -126,4 +126,18 @@ class Test { }""") .doTest(); } + + @Test + public void withinCommentInBrokenUpString_noFinding() { + assume().that(Runtime.version().feature()).isAtLeast(14); + + testHelper + .addSourceLines( + "Test.class", + """ + class Test { + private static final String FOO = "foo" + /* \\s */ " bar"; + }""") + .doTest(); + } }