diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/ModifierOrderer.java b/open-java-format/src/main/java/com/palantir/javaformat/java/ModifierOrderer.java index 26e83f130..e59bf115c 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/ModifierOrderer.java +++ b/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 { + private final ImmutableList tokens; + + @Nullable + private final Modifier modifier; + + static ModifierTokens create(ImmutableList tokens) { + return new ModifierTokens(tokens, asModifier(tokens)); + } + + static ModifierTokens empty() { + return new ModifierTokens(ImmutableList.of(), null); + } + + private ModifierTokens(ImmutableList 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 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 getToksBefore() { + return first().getToksBefore(); + } + + ImmutableList 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> characterRanges) throws FormatterException { @@ -52,43 +123,37 @@ static JavaInput reorderModifiers(JavaInput javaInput, Collection Iterator it = javaInput.getTokens().iterator(); TreeRangeMap 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 modifierTokens = new ArrayList<>(); - List mods = new ArrayList<>(); + List 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 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 it) { + Token token = it.next(); + ImmutableList.Builder 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 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: diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java b/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java index 5305d7c47..026265920 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java @@ -49,7 +49,7 @@ public final class FileBasedTests { ImmutableMultimap.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, diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/ModifierOrdererTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/ModifierOrdererTest.java index 6a660a7a0..dc3ecc8f0 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/ModifierOrdererTest.java +++ b/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 {}"); + } } diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.input new file mode 100644 index 000000000..f19165843 --- /dev/null +++ b/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 {} +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/Sealed.output new file mode 100644 index 000000000..0d5d0b377 --- /dev/null +++ b/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 {} +}