Reorder non-sealed as one modifier

"non-sealed private interface B" came out as text that no longer
parsed, so the whole file failed with "<identifier> expected". javac
lexes non-sealed as three tokens, "non", "-" and "sealed", and the
modifier sorting looked at one token at a time: it never saw
"non-sealed", took the trailing "sealed" as the modifier to move in
front of "private", and left "non-" behind.

This ports google/google-java-format#1107 by Liam Miller-Cushon: the
sorter now reads the three tokens as one modifier and writes it back
whole. The golden Sealed and the sealedClass test come from upstream,
with the expected output in this project's style; a second unit test
pins the shape that failed.

The 15,747 files of the JDK 21 sources format exactly as before; none
of them writes non-sealed out of order.

Author: cushon

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

@@ -16,6 +16,9 @@
 
 package com.palantir.javaformat.java;
 
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Iterables.getLast;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Ordering;
 import com.google.common.collect.Range;
@@ -26,10 +29,10 @@
 import com.sun.tools.javac.parser.Tokens.TokenKind;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nullable;
 import javax.lang.model.element.Modifier;
 
 /** Fixes sequences of modifiers to be in JLS order. */
@@ -40,6 +43,74 @@ static JavaInput reorderModifiers(String text) throws FormatterException {
         return reorderModifiers(new JavaInput(text), ImmutableList.of(Range.closedOpen(0, text.length())));
     }
 
+    /**
+     * The tokens that make up one modifier. Usually a single token (e.g. for {@code public}), but a modifier that
+     * contains a {@code -} (e.g. {@code non-sealed}) is lexed as three tokens.
+     */
+    static final class ModifierTokens implements Comparable<ModifierTokens> {
+        private final ImmutableList<Token> tokens;
+
+        @Nullable
+        private final Modifier modifier;
+
+        static ModifierTokens create(ImmutableList<Token> tokens) {
+            return new ModifierTokens(tokens, asModifier(tokens));
+        }
+
+        static ModifierTokens empty() {
+            return new ModifierTokens(ImmutableList.of(), null);
+        }
+
+        private ModifierTokens(ImmutableList<Token> tokens, @Nullable Modifier modifier) {
+            this.tokens = tokens;
+            this.modifier = modifier;
+        }
+
+        boolean isEmpty() {
+            return tokens.isEmpty() || modifier == null;
+        }
+
+        @SuppressWarnings("for-rollout:NullAway")
+        Modifier modifier() {
+            return modifier;
+        }
+
+        ImmutableList<Token> tokens() {
+            return tokens;
+        }
+
+        private Token first() {
+            return tokens.get(0);
+        }
+
+        private Token last() {
+            return getLast(tokens);
+        }
+
+        int startPosition() {
+            return first().getTok().getPosition();
+        }
+
+        int endPosition() {
+            return last().getTok().getPosition() + last().getTok().length();
+        }
+
+        ImmutableList<? extends Tok> getToksBefore() {
+            return first().getToksBefore();
+        }
+
+        ImmutableList<? extends Tok> getToksAfter() {
+            return last().getToksAfter();
+        }
+
+        @Override
+        @SuppressWarnings("for-rollout:NullAway")
+        public int compareTo(ModifierTokens o) {
+            checkState(!isEmpty()); // empty ModifierTokens are filtered out prior to sorting
+            return modifier.compareTo(o.modifier);
+        }
+    }
+
     /** Reorders all modifiers in the given text and within the given character ranges to be in JLS order. */
     static JavaInput reorderModifiers(JavaInput javaInput, Collection<Range<Integer>> characterRanges)
             throws FormatterException {
@@ -52,43 +123,37 @@ static JavaInput reorderModifiers(JavaInput javaInput, Collection<Range<Integer>
         Iterator<? extends Token> it = javaInput.getTokens().iterator();
         TreeRangeMap<Integer, String> replacements = TreeRangeMap.create();
         while (it.hasNext()) {
-            Token token = it.next();
-            if (!tokenRanges.contains(token.getTok().getIndex())) {
-                continue;
-            }
-            Modifier mod = asModifier(token);
-            if (mod == null) {
+            ModifierTokens tokens = getModifierTokens(it);
+            if (tokens.isEmpty()
+                    || !tokens.tokens().stream()
+                            .allMatch(token -> tokenRanges.contains(token.getTok().getIndex()))) {
                 continue;
             }
 
-            List<Token> modifierTokens = new ArrayList<>();
-            List<Modifier> mods = new ArrayList<>();
+            List<ModifierTokens> modifierTokens = new ArrayList<>();
 
-            int begin = token.getTok().getPosition();
-            mods.add(mod);
-            modifierTokens.add(token);
+            int begin = tokens.startPosition();
+            modifierTokens.add(tokens);
 
             int end = -1;
             while (it.hasNext()) {
-                token = it.next();
-                mod = asModifier(token);
-                if (mod == null) {
+                tokens = getModifierTokens(it);
+                if (tokens.isEmpty()) {
                     break;
                 }
-                mods.add(mod);
-                modifierTokens.add(token);
-                end = token.getTok().getPosition() + token.getTok().length();
+                modifierTokens.add(tokens);
+                end = tokens.endPosition();
             }
 
-            if (!Ordering.natural().isOrdered(mods)) {
-                Collections.sort(mods);
+            if (!Ordering.natural().isOrdered(modifierTokens)) {
+                List<ModifierTokens> sorted = Ordering.natural().sortedCopy(modifierTokens);
                 StringBuilder replacement = new StringBuilder();
-                for (int i = 0; i < mods.size(); i++) {
+                for (int i = 0; i < sorted.size(); i++) {
                     if (i > 0) {
                         addTrivia(replacement, modifierTokens.get(i).getToksBefore());
                     }
-                    replacement.append(mods.get(i).toString());
-                    if (i < (modifierTokens.size() - 1)) {
+                    replacement.append(sorted.get(i).modifier());
+                    if (i < (sorted.size() - 1)) {
                         addTrivia(replacement, modifierTokens.get(i).getToksAfter());
                     }
                 }
@@ -104,9 +169,45 @@ private static void addTrivia(StringBuilder replacement, ImmutableList<? extends
         }
     }
 
+    /**
+     * Consumes the tokens of one modifier from the iterator: the next token, plus the two after it when it is the
+     * {@code non} of a hyphenated modifier such as {@code non-sealed}. The result is empty if they are not a modifier.
+     */
+    private static ModifierTokens getModifierTokens(Iterator<? extends Token> it) {
+        Token token = it.next();
+        ImmutableList.Builder<Token> result = ImmutableList.builder();
+        result.add(token);
+        if (!token.getTok().getText().equals("non")) {
+            return ModifierTokens.create(result.build());
+        }
+        if (!it.hasNext()) {
+            return ModifierTokens.empty();
+        }
+        Token dash = it.next();
+        result.add(dash);
+        if (!dash.getTok().getText().equals("-") || !it.hasNext()) {
+            return ModifierTokens.empty();
+        }
+        result.add(it.next());
+        return ModifierTokens.create(result.build());
+    }
+
+    @Nullable
+    private static Modifier asModifier(ImmutableList<Token> tokens) {
+        if (tokens.size() == 1) {
+            return asModifier(tokens.get(0));
+        }
+        Modifier modifier = asModifier(getLast(tokens));
+        if (modifier == null) {
+            return null;
+        }
+        return Modifier.valueOf("NON_" + modifier.name());
+    }
+
     /**
      * Returns the given token as a {@link javax.lang.model.element.Modifier}, or {@code null} if it is not a modifier.
      */
+    @Nullable
     @SuppressWarnings("for-rollout:NullAway")
     private static Modifier asModifier(Token token) {
         TokenKind kind = ((JavaInput.Tok) token.getTok()).kind();
@@ -140,8 +241,6 @@ private static Modifier asModifier(Token token) {
             }
         }
         switch (token.getTok().getText()) {
-            case "non-sealed":
-                return Modifier.valueOf("NON_SEALED");
             case "sealed":
                 return Modifier.valueOf("SEALED");
             default:

open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java

@@ -49,7 +49,7 @@ public final class FileBasedTests {
             ImmutableMultimap.<Integer, String>builder()
                     .putAll(14, "Records", "RSL", "Var", "ExpressionSwitch", "I574", "I594")
                     .putAll(15, "I603")
-                    .putAll(16, "I588")
+                    .putAll(16, "I588", "Sealed")
                     .putAll(17, "I683", "I684", "I696")
                     .putAll(
                             21,

open-java-format/src/test/java/com/palantir/javaformat/java/ModifierOrdererTest.java

@@ -99,4 +99,17 @@ public void whitespace() throws FormatterException {
                 .getText();
         assertThat(output).contains("public\n  static int a;");
     }
+
+    @Test
+    public void sealedClass() throws FormatterException {
+        assertThat(ModifierOrderer.reorderModifiers("non-sealed sealed public").getText())
+                .isEqualTo("public sealed non-sealed");
+    }
+
+    @Test
+    public void nonSealedBeforeAccessModifier() throws FormatterException {
+        assertThat(ModifierOrderer.reorderModifiers("non-sealed private interface B extends I {}")
+                        .getText())
+                .isEqualTo("private non-sealed interface B extends I {}");
+    }
 }

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.input

@@ -0,0 +1,6 @@
+class T {
+  sealed interface I extends A permits C, B {}
+  final class C implements I {}
+  sealed private interface A permits I {}
+  non-sealed private interface B extends I {}
+}

open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.output

@@ -0,0 +1,9 @@
+class T {
+    sealed interface I extends A permits C, B {}
+
+    final class C implements I {}
+
+    private sealed interface A permits I {}
+
+    private non-sealed interface B extends I {}
+}