GeezerVison

Fixes #148.

What

When a concatenation of string literals ends up on one line, the + between them stays. The string wrapping pass only looks at lines past the column limit, so the output depends on where the author broke the string.

// 21.X.X
throw new SafeIllegalArgumentException(
        "Invalid recipient field - contains multiple " + "foreign key properties.",
        UnsafeArg.of("field", maybeField));
foo("one " + "two " + value + "three " + "four");

// this PR
throw new SafeIllegalArgumentException(
        "Invalid recipient field - contains multiple foreign key properties.",
        UnsafeArg.of("field", maybeField));
foo("one two " + value + "three four");

What the pass does

StringWrapper now joins each run of adjacent string literals that share a line into one literal, after the reflow and the text block passes, whether the author wrote them so or the layout brought them together. The code is then laid out again around the joined literal, so a call whose argument now fits goes onto one line. A line with a quote on both sides of a + sets the pass off, in needWrapping and in linesNeedWrapping (the --output-replacements path), and the pass returns at once when it finds nothing to do.

Literals stay apart:

• on different lines, which is wrapping done on purpose, by the author or by the reflow;
• with a comment between them;
• at a text block (literals after one still join among themselves);
• on a line that ends in a line comment, which may speak of them as they are written: a NON-NLS marker counts the literals of its line (Keep //$NON-NLS-n$ markers valid: no inserted space, each marker on the line of its string #23), and a table of literals has a comment for each row;
• where the left one ends in an octal escape of one or two digits and the right one starts with an octal digit: "\1" + "2" is two characters, "\12" one;
• where either holds a Unicode escape.

A run that a reflowed chain or a text block overlaps is left to that pass.

Differences from palantir#1781

The code is Zayan Khan's, from palantir#1781 (closed there unmerged), which addresses palantir#68; the commit keeps the author. Two rules are new here:

• A line comment at the end of the line. Upstream joins those literals. The JDK's java/util/LocaleISOData.java holds tables of rows such as "aa" + "aar" // Afar, 437 of them, which became "aaaar" // Afar. The rule also keeps our NON-NLS markers with their literals: without it the ojf-issue-23 cases fail. It changes no other JDK file.
• Unicode escapes. Upstream's octal check reads the raw text, so "\u005c1" + "2", a backslash written as a Unicode escape, became "\u005c12", one character instead of two. The AST check of the pass rejects that, and the whole file fails to format.

The goldens and unit tests became case files, and the changelog entry is left out.

Tests

• stringwrapper/adjacentLiteralsAreJoined.case: an argument the formatter brings onto one line, runs around other operands of the +, parentheses, a two-line chain the formatter pulls onto one line, escapes copied as written (\t, \", \\ before a digit, a three-digit octal escape, \1 before 8), literals after a text block, an annotation and a case label.
• stringwrapper/adjacentLiteralsThatStayApart.case: a comment between, a line comment at the end of the line (a table and a call), short octal escapes, Unicode escapes.
• testdata/NON-NLS-* and testdata/palantir-10-*, in all three line endings: the expected outputs join their literals, and nothing else in them changes.
• Upstream's unit tests of joiningWouldChangeEscapes and mayHaveJoinableLiterals were not kept, because the cases already fail when either breaks. I checked that by taking out each rule in turn: the line comment rule, the text block rule, the Unicode rule, the octal rule and the trigger each fail at least one case. Taking out the early return changes no output.
• A plain formatSource leaves all 889 after blocks as they are.
• Tests on JDK 21: FormatterCasesTest 887 → 889; 1410 tests in the module, 1471 in all modules.

JDK 21 corpus

Of the 15,747 files, 787 format differently, in 2,303 places, and each is a join: with the + between two literals taken out and blanks ignored, both versions read the same.

• 737 files join on lines that stay as they are, for example SSLLogger.fine("Discard invalid record: " + "too small record to hold a handshake fragment").

• In 167 the code is then laid out again, mostly a call whose argument now fits on its line, as in java/lang/StackWalker.java:

  -            throw new UnsupportedOperationException(
  -                    "This stack walker " + "does not have RETAIN_CLASS_REFERENCE access");
  +            throw new UnsupportedOperationException("This stack walker does not have RETAIN_CLASS_REFERENCE access");

• 221 places join literals with escapes, such as "\"" + ",offset=" into "\",offset=". The pass compares the folded values of the code before and after, and no file failed.

• The files that change on a second run go down from 67 to 48, and none of them is new. In 19 files, a second run used to move pieces of a chain around, and those pieces are now joined in the first run.

• No slowdown is visible: 42–48 s with the change and 64 s without, though on a machine under load (load average 15–100), so this is not a clean benchmark.

Partial formatting

Joins reach as far as the reflow does today, which is the whole file:

• --lines and --character-ranges: the string pass runs over the whole file, so formatting a few lines can join literals elsewhere in it.
• --output-replacements (IntelliJ): the pass runs when a line in the ranges needs it, and then answers with one replacement for the whole file. A line in the ranges with joinable literals now counts as such a line.
• Eclipse plugin: it goes through getFormatReplacements, which has no string pass (One input, two outputs: getFormatReplacements does not reflow long strings #33), so it joins nothing, just as it wraps nothing today.

Version

This changes the output, so under the version scheme it moves the second number.