Skip to content

Commit 7fd9300

Browse files
cushongoogle-java-format Team
authored and
google-java-format Team
committed
Fix modifier order handling for non-sealed
`non-sealed` is tokenized as three tokens, the modifier sorting logic was assuming it would show up as a single token. PiperOrigin-RevId: 640534518
1 parent db08589 commit 7fd9300

File tree

5 files changed

+140
-27
lines changed

5 files changed

+140
-27
lines changed

core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java

Lines changed: 116 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package com.google.googlejavaformat.java;
1818

19+
import static com.google.common.base.Preconditions.checkState;
20+
import static com.google.common.collect.Iterables.getLast;
21+
1922
import com.google.common.collect.ImmutableList;
2023
import com.google.common.collect.Ordering;
2124
import com.google.common.collect.Range;
@@ -26,12 +29,12 @@
2629
import com.sun.tools.javac.parser.Tokens.TokenKind;
2730
import java.util.ArrayList;
2831
import java.util.Collection;
29-
import java.util.Collections;
3032
import java.util.Iterator;
3133
import java.util.List;
3234
import java.util.Map;
3335
import java.util.Map.Entry;
3436
import javax.lang.model.element.Modifier;
37+
import org.jspecify.annotations.Nullable;
3538

3639
/** Fixes sequences of modifiers to be in JLS order. */
3740
final class ModifierOrderer {
@@ -42,6 +45,71 @@ static JavaInput reorderModifiers(String text) throws FormatterException {
4245
new JavaInput(text), ImmutableList.of(Range.closedOpen(0, text.length())));
4346
}
4447

48+
/**
49+
* A class that contains the tokens corresponding to a modifier. This is usually a single token
50+
* (e.g. for {@code public}), but may be multiple tokens for modifiers containing {@code -} (e.g.
51+
* {@code non-sealed}).
52+
*/
53+
static class ModifierTokens implements Comparable<ModifierTokens> {
54+
private final ImmutableList<Token> tokens;
55+
private final Modifier modifier;
56+
57+
static ModifierTokens create(ImmutableList<Token> tokens) {
58+
return new ModifierTokens(tokens, asModifier(tokens));
59+
}
60+
61+
static ModifierTokens empty() {
62+
return new ModifierTokens(ImmutableList.of(), null);
63+
}
64+
65+
ModifierTokens(ImmutableList<Token> tokens, Modifier modifier) {
66+
this.tokens = tokens;
67+
this.modifier = modifier;
68+
}
69+
70+
boolean isEmpty() {
71+
return tokens.isEmpty() || modifier == null;
72+
}
73+
74+
Modifier modifier() {
75+
return modifier;
76+
}
77+
78+
ImmutableList<Token> tokens() {
79+
return tokens;
80+
}
81+
82+
private Token first() {
83+
return tokens.get(0);
84+
}
85+
86+
private Token last() {
87+
return getLast(tokens);
88+
}
89+
90+
int startPosition() {
91+
return first().getTok().getPosition();
92+
}
93+
94+
int endPosition() {
95+
return last().getTok().getPosition() + last().getTok().getText().length();
96+
}
97+
98+
ImmutableList<? extends Tok> getToksBefore() {
99+
return first().getToksBefore();
100+
}
101+
102+
ImmutableList<? extends Tok> getToksAfter() {
103+
return last().getToksAfter();
104+
}
105+
106+
@Override
107+
public int compareTo(ModifierTokens o) {
108+
checkState(!isEmpty()); // empty ModifierTokens are filtered out prior to sorting
109+
return modifier.compareTo(o.modifier);
110+
}
111+
}
112+
45113
/**
46114
* Reorders all modifiers in the given text and within the given character ranges to be in JLS
47115
* order.
@@ -57,43 +125,37 @@ static JavaInput reorderModifiers(JavaInput javaInput, Collection<Range<Integer>
57125
Iterator<? extends Token> it = javaInput.getTokens().iterator();
58126
TreeRangeMap<Integer, String> replacements = TreeRangeMap.create();
59127
while (it.hasNext()) {
60-
Token token = it.next();
61-
if (!tokenRanges.contains(token.getTok().getIndex())) {
62-
continue;
63-
}
64-
Modifier mod = asModifier(token);
65-
if (mod == null) {
128+
ModifierTokens tokens = getModifierTokens(it);
129+
if (tokens.isEmpty()
130+
|| !tokens.tokens().stream()
131+
.allMatch(token -> tokenRanges.contains(token.getTok().getIndex()))) {
66132
continue;
67133
}
68134

69-
List<Token> modifierTokens = new ArrayList<>();
70-
List<Modifier> mods = new ArrayList<>();
135+
List<ModifierTokens> modifierTokens = new ArrayList<>();
71136

72-
int begin = token.getTok().getPosition();
73-
mods.add(mod);
74-
modifierTokens.add(token);
137+
int begin = tokens.startPosition();
138+
modifierTokens.add(tokens);
75139

76140
int end = -1;
77141
while (it.hasNext()) {
78-
token = it.next();
79-
mod = asModifier(token);
80-
if (mod == null) {
142+
tokens = getModifierTokens(it);
143+
if (tokens.isEmpty()) {
81144
break;
82145
}
83-
mods.add(mod);
84-
modifierTokens.add(token);
85-
end = token.getTok().getPosition() + token.getTok().length();
146+
modifierTokens.add(tokens);
147+
end = tokens.endPosition();
86148
}
87149

88-
if (!Ordering.natural().isOrdered(mods)) {
89-
Collections.sort(mods);
150+
if (!Ordering.natural().isOrdered(modifierTokens)) {
151+
List<ModifierTokens> sorted = Ordering.natural().sortedCopy(modifierTokens);
90152
StringBuilder replacement = new StringBuilder();
91-
for (int i = 0; i < mods.size(); i++) {
153+
for (int i = 0; i < sorted.size(); i++) {
92154
if (i > 0) {
93155
addTrivia(replacement, modifierTokens.get(i).getToksBefore());
94156
}
95-
replacement.append(mods.get(i));
96-
if (i < (modifierTokens.size() - 1)) {
157+
replacement.append(sorted.get(i).modifier());
158+
if (i < (sorted.size() - 1)) {
97159
addTrivia(replacement, modifierTokens.get(i).getToksAfter());
98160
}
99161
}
@@ -109,11 +171,41 @@ private static void addTrivia(StringBuilder replacement, ImmutableList<? extends
109171
}
110172
}
111173

174+
private static @Nullable ModifierTokens getModifierTokens(Iterator<? extends Token> it) {
175+
Token token = it.next();
176+
ImmutableList.Builder<Token> result = ImmutableList.builder();
177+
result.add(token);
178+
if (!token.getTok().getText().equals("non")) {
179+
return ModifierTokens.create(result.build());
180+
}
181+
if (!it.hasNext()) {
182+
return ModifierTokens.empty();
183+
}
184+
Token dash = it.next();
185+
result.add(dash);
186+
if (!dash.getTok().getText().equals("-") || !it.hasNext()) {
187+
return ModifierTokens.empty();
188+
}
189+
result.add(it.next());
190+
return ModifierTokens.create(result.build());
191+
}
192+
193+
private static @Nullable Modifier asModifier(ImmutableList<Token> tokens) {
194+
if (tokens.size() == 1) {
195+
return asModifier(tokens.get(0));
196+
}
197+
Modifier modifier = asModifier(getLast(tokens));
198+
if (modifier == null) {
199+
return null;
200+
}
201+
return Modifier.valueOf("NON_" + modifier.name());
202+
}
203+
112204
/**
113205
* Returns the given token as a {@link javax.lang.model.element.Modifier}, or {@code null} if it
114206
* is not a modifier.
115207
*/
116-
private static Modifier asModifier(Token token) {
208+
private static @Nullable Modifier asModifier(Token token) {
117209
TokenKind kind = ((JavaInput.Tok) token.getTok()).kind();
118210
if (kind != null) {
119211
switch (kind) {
@@ -145,8 +237,6 @@ private static Modifier asModifier(Token token) {
145237
}
146238
}
147239
switch (token.getTok().getText()) {
148-
case "non-sealed":
149-
return Modifier.valueOf("NON_SEALED");
150240
case "sealed":
151241
return Modifier.valueOf("SEALED");
152242
default:

core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class FormatterIntegrationTest {
5050
ImmutableMultimap.<Integer, String>builder()
5151
.putAll(14, "I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594")
5252
.putAll(15, "I603")
53-
.putAll(16, "I588")
53+
.putAll(16, "I588", "Sealed")
5454
.putAll(17, "I683", "I684", "I696")
5555
.putAll(
5656
21,

core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.googlejavaformat.java;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static com.google.common.truth.TruthJUnit.assume;
2021

2122
import com.google.common.base.Joiner;
2223
import com.google.common.collect.Range;
@@ -103,4 +104,11 @@ public void whitespace() throws FormatterException {
103104
.getText();
104105
assertThat(output).contains("public\n static int a;");
105106
}
107+
108+
@Test
109+
public void sealedClass() throws FormatterException {
110+
assume().that(Runtime.version().feature()).isAtLeast(16);
111+
assertThat(ModifierOrderer.reorderModifiers("non-sealed sealed public").getText())
112+
.isEqualTo("public sealed non-sealed");
113+
}
106114
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class T {
2+
sealed interface I extends A permits C, B {}
3+
final class C implements I {}
4+
sealed private interface A permits I {}
5+
non-sealed private interface B extends I {}
6+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class T {
2+
sealed interface I extends A permits C, B {}
3+
4+
final class C implements I {}
5+
6+
private sealed interface A permits I {}
7+
8+
private non-sealed interface B extends I {}
9+
}

0 commit comments

Comments
 (0)