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

CLAUDE.md

@@ -0,0 +1,35 @@
+# Working in this repository
+
+Building and testing: the README's "Building and testing locally" section. Gradle runs on the Temurin 21 that
+`mise install` provides, `./gradlew test` is what CI runs, and `./gradlew formatDiff` formats the files you changed.
+
+## Conventions for new code
+
+- **Collections.** Plain `java.util` types. `List.of`, `List.copyOf`, `Map.of` and `Stream.toList()` give
+  unmodifiable collections, so new code does not need Guava's `ImmutableList` or `ImmutableMap`; use them only
+  where an existing API already hands one over.
+- **Nullability.** One annotation, `javax.annotation.Nullable` (jsr305), the one the code uses; do not add another
+  `Nullable`.
+- **Local variables.** `var` wherever the initializer gives the type unchanged, in `for` loops too. Not where it
+  would box a primitive (`for (var n : List<Integer>)`), and not without an initializer that has a type of its own
+  (a lambda, `null`).
+- **Immutability annotations.** Do not add Error Prone's `@Immutable` or `@SuppressWarnings("Immutable")`.
+- **License header.** A new file starts with the Apache 2.0 header alone, without a copyright line:
+
+  ```java
+  /*
+   * 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.
+   */
+  ```
+
+  Files inherited from google-java-format and palantir-java-format keep their original headers.

open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java

@@ -30,14 +30,18 @@
 import com.palantir.javaformat.doc.DocBuilder;
 import com.palantir.javaformat.doc.FillMode;
 import com.palantir.javaformat.doc.NonBreakingSpace;
+import com.palantir.javaformat.doc.ReservedWidth;
 import com.palantir.javaformat.doc.State;
 import com.palantir.javaformat.doc.Token;
 import com.palantir.javaformat.doc.Token.RealOrImaginary;
 import com.palantir.javaformat.java.FormatterDiagnostic;
 import com.palantir.javaformat.java.InputMetadata;
 import com.palantir.javaformat.java.InputMetadataBuilder;
+import com.palantir.javaformat.java.NonNlsMarkers;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 
 /** An {@code OpsBuilder} creates a list of {@link Op}s, which is turned into a {@link Doc} by {@link DocBuilder}. */
@@ -505,6 +509,10 @@ public OpsOutput build() {
         markForPartialFormat();
         // Rewrite the ops to insert comments.
         Multimap<Integer, Op> tokOps = ArrayListMultimap.create();
+        // The room after a string literal for the NON-NLS markers written on its line, see Comment.makeNonNlsMarkers.
+        var reservations = ArrayListMultimap.<Integer, Op>create();
+        var tokenOps = new HashMap<Input.Token, Integer>();
+        Map<Integer, Input.Token> tokenByIndex = null;
         int opsN = ops.size();
         for (int i = 0; i < opsN; i++) {
             Op op = ops.get(i);
@@ -528,6 +536,7 @@ public OpsOutput build() {
                      * Regular input token. Copy out toksBefore before token, and toksAfter after it. Insert
                      * this token's toksBefore at position j.
                      */
+                    tokenOps.put(token, i);
                     int newlines = 0; // Count of newlines in a row.
                     boolean space = false; // Do we need an extra space after a previous "/*" comment?
                     boolean lastWasComment = false; // Was the last thing we output a comment?
@@ -591,7 +600,29 @@ public OpsOutput build() {
                             } else {
                                 tokOps.put(tokAfterPos, SPACE);
                             }
-                            tokOps.putAll(tokAfterPos, makeComment(tokAfter));
+                            if (tokAfter.isSlashSlashComment() && NonNlsMarkers.isMarkersOnly(tokAfter.getText())) {
+                                if (tokenByIndex == null) {
+                                    tokenByIndex = new HashMap<>();
+                                    for (Input.Token t : input.getTokens()) {
+                                        tokenByIndex.put(t.getTok().getIndex(), t);
+                                    }
+                                }
+                                var literals = nonNlsLiterals(token, tokAfter, tokenByIndex, tokenOps);
+                                var numbers = NonNlsMarkers.numbers(tokAfter.getText());
+                                for (var m = 0; m < literals.size(); m++) {
+                                    var literalOp = literals.get(m) < 0
+                                            ? null
+                                            : tokenOps.get(tokenByIndex.get(literals.get(m)));
+                                    if (literalOp != null) {
+                                        var marker = NonNlsMarkers.marker(numbers.get(m));
+                                        reservations.put(literalOp + 1, ReservedWidth.make(marker.length() + 1));
+                                    }
+                                }
+                                tokOps.put(tokAfterPos, Comment.makeNonNlsMarkers(tokAfter, literals));
+                                tokOps.put(tokAfterPos, Break.makeForced());
+                            } else {
+                                tokOps.putAll(tokAfterPos, makeComment(tokAfter));
+                            }
                             if (breakAfter) {
                                 tokOps.put(tokAfterPos, Break.make(FillMode.FORCED, "", ZERO));
                             }
@@ -630,6 +661,7 @@ public OpsOutput build() {
         ImmutableList.Builder<Op> newOps = ImmutableList.builder();
         boolean afterForcedBreak = false; // Was the last Op a forced break? If so, suppress spaces.
         for (int i = 0; i < opsN; i++) {
+            newOps.addAll(reservations.get(i));
             for (Op op : tokOps.get(i)) {
                 if (!(afterForcedBreak && op instanceof NonBreakingSpace)) {
                     newOps.add(op);
@@ -662,6 +694,39 @@ private static boolean isNonNlsComment(Input.Tok tokAfter) {
         return tokAfter.isSlashSlashComment() && tokAfter.getText().contains("$NON-NLS");
     }
 
+    /**
+     * For each marker of a {@code //$NON-NLS-1$ //$NON-NLS-2$} comment that trails {@code token}, the index of the
+     * string literal it marks, the n-th one on the comment's input line, or -1 when there is none. A literal in
+     * another partial-format region counts as none too: a partial format of its region would otherwise lose the
+     * marker, which belongs to the comment's region. So does a literal without an op of its own.
+     */
+    private List<Integer> nonNlsLiterals(
+            Input.Token token,
+            Input.Tok comment,
+            Map<Integer, Input.Token> tokenByIndex,
+            Map<Input.Token, Integer> tokenOps) {
+        var line = input.getLineNumber(token.getTok().getPosition()) - 1;
+        var toks = input.getRanges(line);
+        var literals = new ArrayList<Integer>();
+        for (int k = toks.lowerEndpoint(); k < Math.min(toks.upperEndpoint(), comment.getIndex()); k++) {
+            var candidate = tokenByIndex.get(k);
+            if (candidate != null && candidate.getTok().getText().startsWith("\"")) {
+                literals.add(k);
+            }
+        }
+        var region = inputMetadataBuilder.regionContaining(comment.getIndex());
+        var result = new ArrayList<Integer>();
+        for (int n : NonNlsMarkers.numbers(comment.getText())) {
+            var literal = 1 <= n && n <= literals.size() ? literals.get(n - 1) : -1;
+            var usable = literal >= 0
+                    && region != null
+                    && region.contains(literal)
+                    && tokenOps.containsKey(tokenByIndex.get(literal));
+            result.add(usable ? literal : -1);
+        }
+        return List.copyOf(result);
+    }
+
     private static boolean isForcedBreak(Op op) {
         return op instanceof Break b && b.isForced();
     }

open-java-format/src/main/java/com/palantir/javaformat/Output.java

@@ -17,6 +17,7 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.Range;
 import com.palantir.javaformat.doc.State;
+import java.util.List;
 
 /** An output from the formatter. */
 public abstract class Output extends InputOutput {
@@ -36,6 +37,13 @@ public abstract class Output extends InputOutput {
      */
     public abstract void append(State state, String text, Range<Integer> range);
 
+    /**
+     * Output a comment of {@code //$NON-NLS-n$} markers, see {@link com.palantir.javaformat.java.NonNlsMarkers}.
+     *
+     * @param literals for each marker, the index of the string literal it marks, or -1
+     */
+    public abstract void appendNonNlsMarkers(State state, String comment, Range<Integer> range, List<Integer> literals);
+
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(this).add("super", super.toString()).toString();

open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java

@@ -19,20 +19,25 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Range;
-import com.google.errorprone.annotations.Immutable;
 import com.palantir.javaformat.CommentsHelper;
 import com.palantir.javaformat.Input;
 import com.palantir.javaformat.Newlines;
 import com.palantir.javaformat.Op;
 import com.palantir.javaformat.Output;
+import com.palantir.javaformat.java.JavaCommentsHelper;
+import com.palantir.javaformat.java.NonNlsMarkers;
+import java.util.List;
 
 /** A leaf node in a {@link Doc} for a non-token. */
-@Immutable
 public final class Comment extends Doc implements Op {
     private final Input.Tok tok;
 
-    private Comment(Input.Tok tok) {
+    /** For a comment of NON-NLS markers, the index of the string literal each marker marks, or -1; else empty. */
+    private final List<Integer> nonNlsLiterals;
+
+    private Comment(Input.Tok tok, List<Integer> nonNlsLiterals) {
         this.tok = tok;
+        this.nonNlsLiterals = nonNlsLiterals;
     }
 
     /**
@@ -42,7 +47,18 @@ private Comment(Input.Tok tok) {
      * @return the new {@code Tok}
      */
     public static Comment make(Input.Tok tok) {
-        return new Comment(tok);
+        return new Comment(tok, List.of());
+    }
+
+    /**
+     * A comment of {@code //$NON-NLS-n$} markers, see {@link NonNlsMarkers}. A marker with a literal is written at
+     * the end of the literal's output line and takes its room there, as a {@link ReservedWidth} after the literal;
+     * only the markers without one take room here.
+     *
+     * @param literals for each marker, the index of the string literal it marks, or -1
+     */
+    public static Comment makeNonNlsMarkers(Input.Tok tok, List<Integer> literals) {
+        return new Comment(tok, List.copyOf(literals));
     }
 
     @Override
@@ -52,14 +68,25 @@ public void add(DocBuilder builder) {
 
     @Override
     protected float computeWidth() {
+        if (!nonNlsLiterals.isEmpty()) {
+            var width = 0;
+            var numbers = NonNlsMarkers.numbers(tok.getOriginalText());
+            for (var i = 0; i < numbers.size(); i++) {
+                if (nonNlsLiterals.get(i) < 0) {
+                    width += (width > 0 ? 1 : 0)
+                            + NonNlsMarkers.marker(numbers.get(i)).length();
+                }
+            }
+            return width;
+        }
         int idx = Newlines.firstBreak(tok.getOriginalText());
         // only count the first line of multi-line block comments
         if (tok.isComment()) {
             if (idx > 0) {
                 return idx;
-            } else if (tok.isSlashSlashComment() && !tok.getOriginalText().startsWith("// ")) {
-                // Account for line comments with missing spaces, see computeFlat.
-                return tok.length() + 1;
+            } else if (tok.isSlashSlashComment()) {
+                // The flat text may have gained the space after the slashes, see computeFlat.
+                return getFlat().length();
             } else {
                 return tok.length();
             }
@@ -69,11 +96,11 @@ protected float computeWidth() {
 
     @Override
     protected String computeFlat() {
-        // TODO(cushon): commentsHelper.rewrite doesn't get called for spans that fit in a single
-        // line. That's fine for multi-line comment reflowing, but problematic for adding missing
-        // spaces in line comments.
-        if (tok.isSlashSlashComment() && !tok.getOriginalText().startsWith("// ")) {
-            return "// " + tok.getOriginalText().substring("//".length());
+        // A level that fits on one line is written from its flat text, and commentsHelper.rewrite is not called for
+        // the comments in it. So the space a line comment gets after its slashes is added here as well, by the rule
+        // that rewrite applies: a trailing `//foo` and a trailing `//$NON-NLS-1$` then come out the same either way.
+        if (tok.isSlashSlashComment()) {
+            return JavaCommentsHelper.addMissingSpace(tok.getOriginalText());
         }
         return tok.getOriginalText();
     }
@@ -101,7 +128,11 @@ public State computeBreaks(
 
     @Override
     public void write(State state, Output output) {
-        output.append(state, state.getTokText(this), range());
+        if (nonNlsLiterals.isEmpty()) {
+            output.append(state, state.getTokText(this), range());
+        } else {
+            output.appendNonNlsMarkers(state, state.getTokText(this), range(), nonNlsLiterals);
+        }
     }
 
     @Override

open-java-format/src/main/java/com/palantir/javaformat/doc/ReservedWidth.java

@@ -0,0 +1,72 @@
+/*
+ * 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.palantir.javaformat.doc;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.Range;
+import com.palantir.javaformat.CommentsHelper;
+import com.palantir.javaformat.Op;
+import com.palantir.javaformat.Output;
+
+/**
+ * A leaf node in a {@link Doc} that takes room on its line without writing anything: the room for a
+ * {@code //$NON-NLS-n$} marker after the string literal it marks, which {@link Comment} writes at the end of the
+ * literal's line once the line breaks are known. So the layout already counts the marker on that line.
+ */
+public final class ReservedWidth extends Doc implements Op {
+    private final int width;
+
+    private ReservedWidth(int width) {
+        this.width = width;
+    }
+
+    public static ReservedWidth make(int width) {
+        return new ReservedWidth(width);
+    }
+
+    @Override
+    public void add(DocBuilder builder) {
+        builder.add(this);
+    }
+
+    @Override
+    protected float computeWidth() {
+        return width;
+    }
+
+    @Override
+    protected String computeFlat() {
+        return "";
+    }
+
+    @Override
+    protected Range<Integer> computeRange() {
+        return Doc.EMPTY_RANGE;
+    }
+
+    @Override
+    public State computeBreaks(
+            CommentsHelper commentsHelper, int maxWidth, State state, Obs.ExplorationNode observationNode) {
+        return state.withColumn(state.column() + width);
+    }
+
+    @Override
+    public void write(State state, Output output) {}
+
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this).add("width", width).toString();
+    }
+}

open-java-format/src/main/java/com/palantir/javaformat/java/InputMetadataBuilder.java

@@ -24,6 +24,7 @@
 import com.palantir.javaformat.OpsBuilder.BlankLineWanted;
 import java.util.HashMap;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 public final class InputMetadataBuilder {
     private final Map<Integer, BlankLineWanted> blankLines = new HashMap<>(); // Info on blank lines.
@@ -50,6 +51,12 @@ public void markForPartialFormat(Input.Token start, Input.Token end) {
         partialFormatRanges.add(Range.closed(lo, hi));
     }
 
+    /** The region marked for partial formatting that holds tok {@code k}, or null. */
+    @Nullable
+    public Range<Integer> regionContaining(int k) {
+        return partialFormatRanges.rangeContaining(k);
+    }
+
     public InputMetadata build() {
         return new InputMetadata(ImmutableMap.copyOf(blankLines), ImmutableRangeSet.copyOf(partialFormatRanges));
     }