diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
index 57a9b94b5e8..4e06db632d0 100644
--- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
+++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
@@ -1279,6 +1279,9 @@ public static Nullness getNullnessValue(
if (type.hasTag(TypeTag.BOOLEAN) && value instanceof Integer) {
return ((Integer) value) == 1;
}
+ if (type.hasTag(TypeTag.CHAR) && value instanceof Integer) {
+ return (char) (int) value;
+ }
return value;
}
diff --git a/check_api/src/main/java/com/google/errorprone/util/SourceCodeEscapers.java b/check_api/src/main/java/com/google/errorprone/util/SourceCodeEscapers.java
index 9e2e683610d..7115a832dc3 100644
--- a/check_api/src/main/java/com/google/errorprone/util/SourceCodeEscapers.java
+++ b/check_api/src/main/java/com/google/errorprone/util/SourceCodeEscapers.java
@@ -65,6 +65,27 @@ public static CharEscaper javaCharEscaper() {
'\\', "\\\\",
'\'', "\\'"));
+ /**
+ * Returns an {@link Escaper} instance that escapes special characters in a string so it can
+ * safely be included in either a Java text block.
+ *
+ *
See: The Java Language Specification for more details.
+ */
+ public static CharEscaper getJavaTextBlockEscaper() {
+ return JAVA_TEXT_BLOCK_ESCAPER;
+ }
+
+ private static final CharEscaper JAVA_TEXT_BLOCK_ESCAPER =
+ new JavaCharEscaper(
+ ImmutableMap.of(
+ '\b', "\\b",
+ '\f', "\\f",
+ '\n', "\\n",
+ '\r', "\\r",
+ '\t', "\\t",
+ '\\', "\\\\"));
+
// This escaper does not produce octal escape sequences. See:
// http://java.sun.com/docs/books/jls/third_edition/html/lexical.html#101089
// "Octal escapes are provided for compatibility with C, but can express
@@ -88,11 +109,11 @@ private static char[] asUnicodeHexEscape(char c) {
r[0] = '\\';
r[1] = 'u';
r[5] = HEX_DIGITS[c & 0xF];
- c >>>= 4;
+ c = (char) (c >>> 4);
r[4] = HEX_DIGITS[c & 0xF];
- c >>>= 4;
+ c = (char) (c >>> 4);
r[3] = HEX_DIGITS[c & 0xF];
- c >>>= 4;
+ c = (char) (c >>> 4);
r[2] = HEX_DIGITS[c & 0xF];
return r;
}
diff --git a/core/pom.xml b/core/pom.xml
index b425e327b32..fe7614e283e 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -72,7 +72,7 @@
com.google.googlejavaformat
google-java-format
- 1.19.1
+ 1.24.0
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StringConcatToTextBlock.java b/core/src/main/java/com/google/errorprone/bugpatterns/StringConcatToTextBlock.java
new file mode 100644
index 00000000000..0bfbef320db
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/StringConcatToTextBlock.java
@@ -0,0 +1,284 @@
+/*
+ * Copyright 2024 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Iterables.getLast;
+import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.method.MethodMatchers.constructor;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static com.google.errorprone.util.ASTHelpers.getReceiver;
+import static com.google.errorprone.util.ASTHelpers.getStartPosition;
+import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.isSameType;
+import static java.util.stream.Collectors.joining;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.ErrorProneToken;
+import com.google.errorprone.util.ErrorProneTokens;
+import com.google.errorprone.util.SourceCodeEscapers;
+import com.sun.source.tree.BinaryTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.LineMap;
+import com.sun.source.tree.LiteralTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.NewClassTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.parser.Tokens;
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(
+ summary = "This string literal can be written more clearly as a text block",
+ severity = WARNING)
+public class StringConcatToTextBlock extends BugChecker
+ implements LiteralTreeMatcher, MethodInvocationTreeMatcher {
+
+ public static final String DELIMITER = "\"\"\"";
+
+ @Override
+ public Description matchLiteral(LiteralTree tree, VisitorState state) {
+ // javac constant folds string concat during parsing, so we don't need to handle binary
+ // expressions
+ // see -XDallowStringFolding=false
+ if (!tree.getKind().equals(Tree.Kind.STRING_LITERAL)) {
+ return NO_MATCH;
+ }
+ if (state.getPath().getParentPath().getLeaf() instanceof BinaryTree parent) {
+ if (parent.getKind().equals(Tree.Kind.PLUS)
+ && isSameType(getType(parent), state.getSymtab().stringType, state)) {
+ // the readability benefit of text blocks is less if they're inside another string
+ // concat expression
+ return NO_MATCH;
+ }
+ }
+ List tokens = state.getTokensForNode(tree);
+ ImmutableList strings =
+ tokens.stream()
+ .filter(t -> t.kind().equals(Tokens.TokenKind.STRINGLITERAL))
+ .map(t -> t.stringVal())
+ .collect(toImmutableList());
+ boolean trailingNewline = getLast(strings).endsWith("\n");
+ // Only migrate if there are enough lines to make it worthwhile. Escaping the trailing newline
+ // slightly reduces the readability benefit of migrating, so require an extra line to make it
+ // worth it.
+ int thresholdToMigrate = trailingNewline ? 2 : 3;
+ if (strings.size() < thresholdToMigrate) {
+ return NO_MATCH;
+ }
+ // only migrate if each piece of the string concat ends in an escaped newline. It would be
+ // possible to migrate more eagerly, and perhaps use escaped newlines to reflow the string, or
+ // recognize APIs that don't care about extra newlines.
+ if (!strings.stream().limit(strings.size() - 1).allMatch(s -> s.endsWith("\n"))) {
+ return NO_MATCH;
+ }
+ // shared logic in 'match' assumes the strings are split at newline boundaries, so trim the
+ // escaped newlines here
+ strings =
+ Streams.concat(
+ strings.stream().limit(strings.size() - 1).map(s -> s.substring(0, s.length() - 1)),
+ Stream.of(getLast(strings)))
+ .collect(toImmutableList());
+ return match(tree, strings, state);
+ }
+
+ @Override
+ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+ if (JOINER_JOIN.matches(tree, state)) {
+ return joiner(tree, state);
+ }
+ if (STRING_JOINER_TO_STRING.matches(tree, state)) {
+ return stringJoiner(tree, state);
+ }
+ if (STRING_JOIN.matches(tree, state)) {
+ return stringJoin(tree, state);
+ }
+ if (FOR_SOURCE_LINES.matches(tree, state)) {
+ return forSourceLines(tree, state);
+ }
+ return NO_MATCH;
+ }
+
+ private Description match(Tree replace, ImmutableList lines, VisitorState state) {
+ return match(replace, getStartPosition(replace), state.getEndPosition(replace), lines, state);
+ }
+
+ private Description match(
+ Tree tree,
+ int replaceFrom,
+ int replaceTo,
+ ImmutableList strings,
+ VisitorState state) {
+ if (strings.isEmpty()) {
+ return NO_MATCH;
+ }
+ ImmutableList tokens =
+ ErrorProneTokens.getTokens(
+ state.getSourceCode().subSequence(replaceFrom, replaceTo).toString(), state.context);
+ if (!tokens.stream()
+ .flatMap(t -> t.comments().stream())
+ .map(c -> c.getText())
+ .allMatch(x -> x.isEmpty())) {
+ // For long string literals with interspersed comments, moving all the comments to the
+ // beginning could regress readability.
+ // TODO: cushon - handle comments?
+ return NO_MATCH;
+ }
+ // bail out if the contents contain a delimiter that would require escaping
+ if (strings.stream().anyMatch(s -> s.contains(DELIMITER))) {
+ return NO_MATCH;
+ }
+ boolean trailingNewline = getLast(strings).endsWith("\n");
+ strings =
+ String.join("\n", strings)
+ .stripIndent()
+ .lines()
+ .map(s -> s.stripTrailing())
+ .collect(toImmutableList());
+ LineMap lineMap = state.getPath().getCompilationUnit().getLineMap();
+ String indent = " ".repeat((int) lineMap.getColumnNumber(replaceFrom) - 1);
+ String suffix = trailingNewline ? "" : "\\";
+ String replacement =
+ strings.stream()
+ .map(line -> line.isEmpty() ? line : (indent + line))
+ .map(SourceCodeEscapers.getJavaTextBlockEscaper()::escape)
+ .collect(joining("\n", DELIMITER + "\n", suffix + "\n" + indent + DELIMITER));
+ SuggestedFix fix = SuggestedFix.replace(replaceFrom, replaceTo, replacement);
+ return describeMatch(tree, fix);
+ }
+
+ private static final Matcher JOINER_JOIN =
+ instanceMethod().onExactClass("com.google.common.base.Joiner").named("join");
+ private static final Matcher JOINER_ON =
+ staticMethod().onClass("com.google.common.base.Joiner").named("on");
+
+ private static final Matcher STRING_JOINER_TO_STRING =
+ instanceMethod().onExactClass("java.util.StringJoiner").named("toString");
+ private static final Matcher STRING_JOINER_ADD =
+ instanceMethod().onExactClass("java.util.StringJoiner").named("add");
+ private static final Matcher STRING_JOINER_CONSTRUCTOR =
+ constructor().forClass("java.util.StringJoiner").withParameters("java.lang.CharSequence");
+
+ private static final Matcher STRING_JOIN =
+ staticMethod().onClass("java.lang.String").named("join");
+
+ private static final Matcher FOR_SOURCE_LINES =
+ staticMethod().onClass("com.google.testing.compile.JavaFileObjects").named("forSourceLines");
+
+ private Description joiner(MethodInvocationTree tree, VisitorState state) {
+ ImmutableList strings = stringLiteralArguments(tree.getArguments());
+ if (strings.isEmpty()) {
+ return NO_MATCH;
+ }
+ if (!(getReceiver(tree) instanceof MethodInvocationTree receiver)) {
+ return NO_MATCH;
+ }
+ if (!JOINER_ON.matches(receiver, state)) {
+ return NO_MATCH;
+ }
+ if (!newlineLiteral(getOnlyElement(receiver.getArguments()))) {
+ return NO_MATCH;
+ }
+ return match(tree, strings, state);
+ }
+
+ private Description stringJoiner(MethodInvocationTree tree, VisitorState state) {
+ Deque strings = new ArrayDeque<>();
+ ExpressionTree current = tree;
+ while (getReceiver(current) instanceof MethodInvocationTree receiver) {
+ if (!STRING_JOINER_ADD.matches(receiver, state)) {
+ break;
+ }
+ Optional string = stringLiteral(getOnlyElement(receiver.getArguments()));
+ if (string.isEmpty()) {
+ return NO_MATCH;
+ }
+ strings.addFirst(string.get());
+ current = receiver;
+ }
+ if (!(getReceiver(current) instanceof NewClassTree constructor
+ && STRING_JOINER_CONSTRUCTOR.matches(constructor, state))) {
+ return NO_MATCH;
+ }
+ if (!newlineLiteral(getOnlyElement(constructor.getArguments()))) {
+ return NO_MATCH;
+ }
+ return match(tree, ImmutableList.copyOf(strings), state);
+ }
+
+ private Description stringJoin(MethodInvocationTree tree, VisitorState state) {
+ ImmutableList strings =
+ stringLiteralArguments(tree.getArguments().subList(1, tree.getArguments().size()));
+ if (!newlineLiteral(tree.getArguments().get(0))) {
+ return NO_MATCH;
+ }
+ return match(tree, strings, state);
+ }
+
+ private Description forSourceLines(MethodInvocationTree tree, VisitorState state) {
+ ImmutableList strings =
+ stringLiteralArguments(tree.getArguments().subList(1, tree.getArguments().size()));
+ return match(
+ tree.getArguments().get(1),
+ getStartPosition(tree.getArguments().get(1)),
+ state.getEndPosition(getLast(tree.getArguments())),
+ strings,
+ state);
+ }
+
+ private static boolean newlineLiteral(ExpressionTree expressionTree) {
+ Object value = ASTHelpers.constValue(expressionTree);
+ if (value == null) {
+ return false;
+ }
+ return value.equals("\n") || value.equals('\n');
+ }
+
+ static ImmutableList stringLiteralArguments(List extends ExpressionTree> arguments) {
+ ImmutableList strings =
+ arguments.stream()
+ .filter(a -> a.getKind().equals(Tree.Kind.STRING_LITERAL))
+ .map(x -> (String) ((LiteralTree) x).getValue())
+ .collect(toImmutableList());
+ if (strings.size() != arguments.size()) {
+ return ImmutableList.of();
+ }
+ return strings;
+ }
+
+ static Optional stringLiteral(ExpressionTree tree) {
+ return tree.getKind().equals(Tree.Kind.STRING_LITERAL)
+ ? Optional.of((String) ((LiteralTree) tree).getValue())
+ : Optional.empty();
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
index 7ddf1061ce9..c4eab2295ac 100644
--- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
+++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -360,6 +360,7 @@
import com.google.errorprone.bugpatterns.StringBuilderInitWithChar;
import com.google.errorprone.bugpatterns.StringCaseLocaleUsage;
import com.google.errorprone.bugpatterns.StringCharset;
+import com.google.errorprone.bugpatterns.StringConcatToTextBlock;
import com.google.errorprone.bugpatterns.StringFormatWithLiteral;
import com.google.errorprone.bugpatterns.StringSplitter;
import com.google.errorprone.bugpatterns.StronglyTypeByteString;
@@ -1082,6 +1083,7 @@ public static ScannerSupplier warningChecks() {
StreamToIterable.class,
StringCaseLocaleUsage.class,
StringCharset.class,
+ StringConcatToTextBlock.class,
StringSplitter.class,
Suggester.class,
SuperCallToObjectMethod.class,
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StringConcatToTextBlockTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StringConcatToTextBlockTest.java
new file mode 100644
index 00000000000..ab05655ac96
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/StringConcatToTextBlockTest.java
@@ -0,0 +1,212 @@
+/*
+ * Copyright 2024 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class StringConcatToTextBlockTest {
+ private final CompilationTestHelper testHelper =
+ CompilationTestHelper.newInstance(StringConcatToTextBlock.class, getClass());
+ private final BugCheckerRefactoringTestHelper refactoringHelper =
+ BugCheckerRefactoringTestHelper.newInstance(StringConcatToTextBlock.class, getClass());
+
+ @Test
+ public void negative() {
+ testHelper
+ .addSourceLines(
+ "Test.java",
+ """
+ class Test {
+ String s = new Object() + "world\\n";
+ String oneToken = "hello\\nworld";
+ String comment =
+ "hello\\\\n"
+ // comment
+ + "world\\\\n";
+ String noNewline = "hello" + "world\\n";
+ String extra = "prefix" + s + "hello\\n" + "world\\n";
+ String noTrailing = "hello\\n" + "world";
+ }
+ """)
+ .doTest();
+ }
+
+ @Test
+ public void positive() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ class Test {
+ String s = "hello\\n" + "world\\n";
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ class Test {
+ String s =
+ \"""
+ hello
+ world
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void noTrailing() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ class Test {
+ String s = "hello\\n" + "foo\\n" + "world";
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ class Test {
+ String s =
+ \"""
+ hello
+ foo
+ world\\
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void escape() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ class Test {
+ String s = "\\n" + "\\nhello\\n" + "foo\\n\\n" + "world";
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ class Test {
+ String s =
+ \"""
+
+
+ hello
+ foo
+
+ world\\
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void guavaJoiner() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ import com.google.common.base.Joiner;
+
+ class Test {
+ String s = Joiner.on('\\n').join("string", "literals");
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ import com.google.common.base.Joiner;
+ class Test {
+ String s =
+ \"""
+ string
+ literals\\
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void stringJoiner() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ import java.util.StringJoiner;
+
+ class Test {
+ String s = new StringJoiner("\\n").add("string").add("literals").toString();
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ import java.util.StringJoiner;
+ class Test {
+ String s =
+ \"""
+ string
+ literals\\
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void stringJoin() {
+ refactoringHelper
+ .addInputLines(
+ "Test.java",
+ """
+ import java.util.StringJoiner;
+
+ class Test {
+ String s = String.join("\\n", "string", "literals");
+ }
+ """)
+ .addOutputLines(
+ "Test.java",
+ """
+ import java.util.StringJoiner;
+ class Test {
+ String s =
+ \"""
+ string
+ literals\\
+ \""";
+ }
+ """)
+ .doTest(TEXT_MATCH);
+ }
+}
diff --git a/test_helpers/pom.xml b/test_helpers/pom.xml
index 66b12d308c3..d3e33bc9d5d 100644
--- a/test_helpers/pom.xml
+++ b/test_helpers/pom.xml
@@ -86,7 +86,7 @@
com.google.googlejavaformat
google-java-format
- 1.19.1
+ 1.24.0
diff --git a/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java b/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
index 60067294e2d..f984f06660a 100644
--- a/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
+++ b/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
@@ -49,6 +49,7 @@
import com.google.errorprone.scanner.ScannerSupplier;
import com.google.googlejavaformat.java.Formatter;
import com.google.googlejavaformat.java.FormatterException;
+import com.google.googlejavaformat.java.StringWrapper;
import com.google.testing.compile.JavaFileObjects;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.util.TreePath;
@@ -94,7 +95,8 @@ void verifyMatch(JavaFileObject refactoredSource, JavaFileObject expectedSource)
private String maybeFormat(String input) {
try {
- return new Formatter().formatSource(input);
+ Formatter formatter = new Formatter();
+ return StringWrapper.wrap(formatter.formatSource(input), formatter);
} catch (FormatterException e) {
return input;
}