Keep //$NON-NLS-n$ markers valid: no inserted space, each marker on the line of its string

[abashev]

Background

Eclipse marks a string literal that is deliberately not externalised with a trailing //$NON-NLS-n$ comment, where n is the position of the literal on that line. The marker has to keep its exact spelling and has to stay on the line of its string. This answer explains the convention. The item comes from the "Future work" list of the old README.

Chains of string concatenation already work: OpsBuilder keeps each marker with its string, and NON-NLS.input / NON-NLS.output cover that. Three other cases break. All of them reproduce with 2.98.0.1, with the jar and the native binary alike, in every style.

1. A space is inserted into the marker

String a = "x"; //$NON-NLS-1$

becomes

String a = "x"; // $NON-NLS-1$

and Eclipse no longer recognises it. This is the most common case, because it hits every statement that fits on one line.

Cause: doc/Comment.computeFlat() prefixes // to any line comment that does not start with it. For a statement that fits on one line commentsHelper.rewrite is never called, so the exemption that JavaCommentsHelper.LINE_COMMENT_MISSING_SPACE_PREFIX already has for //noinspection and //$NON-NLS-n$ is bypassed. computeWidth() carries the matching + 1. A trailing //noinspection gets the same space; on a line of its own it is left alone.

2. Wrapped arguments leave their markers behind

String message = Messages.format("first argument that is long enough to matter", "second argument that is also rather long", "third one"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$

becomes

String message = Messages.format(
        "first argument that is long enough to matter",
        "second argument that is also rather long",
        "third one"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$

The first two literals are now on lines without a marker, and the last line has one literal and three markers.

3. A reflowed literal ends up partly unmarked

String reflowed = "a single string literal that is far too long to fit into one hundred and twenty columns, so the formatter reflows it"; //$NON-NLS-1$

becomes

String reflowed =
        "a single string literal that is far too long to fit into one hundred and twenty columns, so the"
                + " formatter reflows it"; //$NON-NLS-1$

The first piece has no marker. --skip-reflowing-long-strings avoids this case only.

Proposal

• Case 1 is small: apply the existing exemption in Comment.computeFlat() and Comment.computeWidth(), sharing the pattern with JavaCommentsHelper instead of repeating it.
• Cases 2 and 3 need a design. When a statement with markers is wrapped, each marker has to move to the line of its literal and be renumbered for that line. When a literal is split, every piece needs a marker of its own.

Compatibility

All three fixes change the output of files that format today, so they differ from palantir-java-format 2.98.0. The promise for 2.x says output changes wait for 3.0. Case 1 is a bug that makes the formatter corrupt a marker another tool depends on, so it is worth deciding whether it counts as a fix or as a style change.

Prior reports

google-java-format shares this code path and has had it reported: google/google-java-format#221 (closed, 2017) and google/google-java-format#361 (open since 2019). No report was found in palantir-java-format.