MisleadingEscapedSpace: tokenize so that we don't complain about comm…

…ents. PiperOrigin-RevId: 685720735
google · Oct 14, 2024 · 46ac925 · 46ac925
1 parent 2bebaf2
commit 46ac925
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 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
@@ -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
@@ -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();
+  }
 }