Stop splitting long strings inside an escaped backslash · openjavaformat/open-java-format@53ac7d8 · GitHub
Skip to content

Commit 53ac7d8

Browse files
abashevabashev
committed
Stop splitting long strings inside an escaped backslash
StringWrapper cuts a long string literal before whitespace and before \t, and ends a piece after \n and \r, but never asked whether the backslash of such an escape is escaped itself. In 'D:\\tempDb' the second backslash and the t looked like a tab, so a line break there left a lone backslash that escaped the closing quote, and the file failed with "unclosed string literal" (#32, from palantir#1680). \\n and \\r were misread the same way: they cut words such as C:\\new in two and forced a line break, which moved the break but still parsed. A backslash with an odd number of backslashes right before it now counts as escaped, both where pieces are cut and where a piece forces the line to end. Literals without an escaped backslash are cut as before. Of the JDK 21 and 25 sources, the same 8 files format differently: resource bundles whose messages contain \\n or \\r, which no longer break the line after them.
1 parent 39f69dc commit 53ac7d8

4 files changed

Lines changed: 95 additions & 3 deletions

File tree

‎open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java‎

Lines changed: 15 additions & 3 deletions

‎open-java-format/src/test/java/com/palantir/javaformat/java/StringWrapperTest.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,23 @@
1515
package com.palantir.javaformat.java;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static com.palantir.javaformat.java.JavaFormatterOptions.Style;
1819

1920
import com.google.common.base.Joiner;
21+
import com.google.common.collect.Iterables;
22+
import com.sun.source.tree.BinaryTree;
23+
import com.sun.source.tree.ClassTree;
24+
import com.sun.source.tree.CompilationUnitTree;
25+
import com.sun.source.tree.ExpressionTree;
26+
import com.sun.source.tree.LiteralTree;
27+
import com.sun.source.tree.VariableTree;
28+
import com.sun.source.util.JavacTask;
29+
import java.io.IOException;
30+
import java.net.URI;
31+
import java.util.List;
32+
import javax.tools.JavaFileObject;
33+
import javax.tools.SimpleJavaFileObject;
34+
import javax.tools.ToolProvider;
2035
import org.junit.jupiter.api.Test;
2136
import org.junit.jupiter.api.parallel.Execution;
2237
import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -48,6 +63,60 @@ public void testAwkwardLineEndWrapping() throws Exception {
4863
assertThat(StringWrapper.wrap(100, input, Formatter.create())).isEqualTo(output);
4964
}
5065

66+
@Test
67+
public void wrapsAStringWithEscapedBackslashes() throws Exception {
68+
// In D:\\tempDb the second backslash and the t are not an escaped tab. Splitting there left a lone backslash
69+
// at the end of a piece, where it escaped the closing quote.
70+
String input = lines(
71+
"class C {",
72+
" void m(java.sql.Connection con) throws Exception {",
73+
" con.createStatement()",
74+
" .execute(\"ALTER DATABASE tempdb MODIFY FILE (NAME = 'tempdev',"
75+
+ " FILENAME = 'D:\\\\tempDb\\\\DATA\\\\tempdb.mdf')\");",
76+
" }",
77+
"}");
78+
Formatter formatter = Formatter.createFormatter(
79+
JavaFormatterOptions.builder().style(Style.OJF).build());
80+
81+
String output = formatter.formatSourceAndFixImports(input);
82+
83+
assertThat(formatter.formatSourceAndFixImports(output)).isEqualTo(output);
84+
}
85+
86+
@Test
87+
public void wrappingKeepsTheValueOfAStringWithEscapes() throws Exception {
88+
// At 40 columns the first line fills up right inside C:\\temp, where \\t is not an escaped tab.
89+
String input = lines("class T {", " String s = \"copy the file to C:\\\\temp and back\";", "}");
90+
91+
String output = StringWrapper.wrap(40, input, Formatter.create());
92+
93+
assertThat(output).isNotEqualTo(input);
94+
assertThat(stringValue(output)).isEqualTo(stringValue(input));
95+
}
96+
97+
/** The value javac gives the initializer of the only field: a string literal, or literals joined with +. */
98+
private static String stringValue(String source) throws IOException {
99+
JavaFileObject file = new SimpleJavaFileObject(URI.create("string:///T.java"), JavaFileObject.Kind.SOURCE) {
100+
@Override
101+
public CharSequence getCharContent(boolean ignoreEncodingErrors) {
102+
return source;
103+
}
104+
};
105+
JavacTask task =
106+
(JavacTask) ToolProvider.getSystemJavaCompiler().getTask(null, null, null, null, null, List.of(file));
107+
CompilationUnitTree unit = Iterables.getOnlyElement(task.parse());
108+
ClassTree type = (ClassTree) Iterables.getOnlyElement(unit.getTypeDecls());
109+
VariableTree field = (VariableTree) Iterables.getOnlyElement(type.getMembers());
110+
return concatenation(field.getInitializer());
111+
}
112+
113+
private static String concatenation(ExpressionTree expression) {
114+
if (expression instanceof BinaryTree plus) {
115+
return concatenation(plus.getLeftOperand()) + concatenation(plus.getRightOperand());
116+
}
117+
return (String) ((LiteralTree) expression).getValue();
118+
}
119+
51120
private static String lines(String... line) {
52121
return Joiner.on('\n').join(line) + '\n';
53122
}

‎open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/16.input‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class T {
2+
String s = "fields split on \\t, lines on \\r\\n or \\n in C:\\temp\\new\\reports and \\\\server\\share for caf\u00e9\tthen\ndone";
3+
}

‎open-java-format/src/test/resources/com/palantir/javaformat/java/StringWrapperIntegrationTest/16.output‎

Lines changed: 8 additions & 0 deletions

0 commit comments

Comments
 (0)