Skip to content

Commit

Permalink
Strip the quotation marks from the source code when reconstructing th…
Browse files Browse the repository at this point in the history
…e 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
  • Loading branch information
graememorgan authored and Error Prone Team committed Oct 23, 2024
1 parent 99a0d9d commit 2ce9632
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit 2ce9632

Please sign in to comment.