Skip to content

A long string containing \\t fails to format: spurious "unclosed string literal" #32

Closed

Description

@abashev

What

Reported upstream as palantir/palantir-java-format#1680 (open since 2026-05-29, no comments). It reproduces here on 2.98.0.2. Minimal case:

class C {
    void m(java.sql.Connection con) throws Exception {
        con.createStatement()
                .execute("ALTER DATABASE tempdb MODIFY FILE (NAME = 'tempdev', FILENAME = 'D:\\tempDb\\DATA\\tempdb.mdf')");
    }
}

open-java-format C.java exits 2 with C.java:5:26: error: unclosed string literal, while javac compiles the same file. Replace the escaped backslashes with forward slashes and the file formats.

In the upstream report the errors point at methods further down the file, which makes the cause hard to see: the line and column come from the rewritten source the formatter parses to check itself, not from the file on disk.

Why it happens

StringWrapper.stringComponents splits the literal's source text at whitespace and at escaped whitespace, and hasEscapedWhitespaceAt/hasEscapedNewlineAt look for \t, \n and \r without asking whether that backslash is itself escaped. In DATA\\tempDb the second backslash and the t look like an escaped tab, so the split lands inside the escaped backslash. StringWrapper.getReflowReplacements on the file above returns:

"ALTER DATABASE tempdb MODIFY FILE (NAME = 'tempdev', FILENAME = 'D:\\tempDb\\DATA\"
        + "\tempdb.mdf')"

The first piece ends in a lone backslash, which escapes the closing quote — that is the error. The second piece starts with \t, a tab, so even if it parsed the string would no longer be the one the author wrote.

\n and \r are mis-read the same way, but that branch advances past the escape, so the split is merely in an odd place and the result still parses and still means the same: the same file with D:\\nempDb formats fine. Only \\t produces code that does not parse.

Fix

Walk the literal left to right consuming escape sequences, so that \\ takes two characters and the t after it is an ordinary letter.

Mind the output promise: files whose long strings contain \\n or \\r format today, and the fix moves where they are split, so their output changes. Measure that over a corpus and decide which release it belongs to, as in #22 and #31.

Done when

  • the file above formats, and formatting it twice is a no-op;
  • a golden covers a wrapped literal with \\t, \\n, \\r, \\\\ and \u escapes, and fails without the fix;
  • a test asserts the string's value is unchanged by wrapping, not only that the result parses;
  • a corpus run says how many files the \\n/\\r split moves.

Activity

  1. abashev commented on Sep 23, 2026

    @abashev
    CollaboratorAuthor

    Fixed in 53ac7d8 on main.

    Cause. As described above: hasEscapedWhitespaceAt and hasEscapedNewlineAt took any \t, \n or \r in the literal's source for an escape, without asking whether its backslash was escaped itself. Besides the crash, reflow forced a line break after every piece ending in \\n or \\r.

    Fix. A backslash with an odd number of backslashes right before it counts as escaped. The check is in both helpers and in reflow's forced break. The loop that cuts the literal is unchanged, so literals without an escaped backslash are cut exactly as before.

    Done when, item by item

    • The file formats, and formatting it twice is a no-op. This is StringWrapperTest.wrapsAStringWithEscapedBackslashes, which failed with 5:26: error: unclosed string literal before. Through the CLI the example now formats to ...FILENAME =" + " 'D:\\tempDb\\DATA\\tempdb.mdf')", and javac compiles the result.
    • A golden with \\t, \\n, \\r, \\\\ and \u escapes that fails without the fix. This is StringWrapperIntegrationTest/16, which also has a real \t and \n as places to cut. Without the fix the golden cut C:\\temp\\n + ew\\r + eports.
    • The string's value is unchanged, not only parsing. StringWrapperTest.wrappingKeepsTheValueOfAStringWithEscapes compares the value javac reads before and after wrapping "copy the file to C:\\temp and back" at 40 columns. At that width the line break falls inside \\t, and the test failed with the parse error before.
    • Corpus. The same 8 files format differently in both corpora: 8 of the 15,747 JDK 21 files and 8 of the 15,368 JDK 25 files. They are resource bundles whose messages contain \\n or \\r, and the line no longer breaks after them: the launcher's es, ko and pt_BR, XSLTErrorResources_ja, and jshell's l10n in four languages. That is an output change for such files. stderr and timings are unchanged.

    Not covered. A backslash can be written as a Unicode escape, \u005c. If one is followed by \t in the source, the file still breaks when the line break lands between the two: javac reads \u005c\t as backslash + t, but the check counts only the backslashes in the source text. It failed the same way before, and the result is an error, not a changed string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions