Skip to content

Commit

Permalink
Fix modifier order handling for non-sealed
Browse files Browse the repository at this point in the history
`non-sealed` is tokenized as three tokens, the modifier sorting logic was assuming it would show up as a single token.

PiperOrigin-RevId: 640534518
  • Loading branch information
cushon authored and google-java-format Team committed Jun 5, 2024
1 parent db08589 commit 7fd9300
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 27 deletions.
142 changes: 116 additions & 26 deletions core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.google.googlejavaformat.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;
Expand All @@ -26,12 +29,12 @@
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 java.util.Map.Entry;
import javax.lang.model.element.Modifier;
import org.jspecify.annotations.Nullable;

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

/**
* A class that contains the tokens corresponding to a modifier. This is usually a single token
* (e.g. for {@code public}), but may be multiple tokens for modifiers containing {@code -} (e.g.
* {@code non-sealed}).
*/
static class ModifierTokens implements Comparable<ModifierTokens> {
private final ImmutableList<Token> tokens;
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);
}

ModifierTokens(ImmutableList<Token> tokens, Modifier modifier) {
this.tokens = tokens;
this.modifier = modifier;
}

boolean isEmpty() {
return tokens.isEmpty() || modifier == null;
}

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().getText().length();
}

ImmutableList<? extends Tok> getToksBefore() {
return first().getToksBefore();
}

ImmutableList<? extends Tok> getToksAfter() {
return last().getToksAfter();
}

@Override
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.
Expand All @@ -57,43 +125,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));
if (i < (modifierTokens.size() - 1)) {
replacement.append(sorted.get(i).modifier());
if (i < (sorted.size() - 1)) {
addTrivia(replacement, modifierTokens.get(i).getToksAfter());
}
}
Expand All @@ -109,11 +171,41 @@ private static void addTrivia(StringBuilder replacement, ImmutableList<? extends
}
}

private static @Nullable 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());
}

private static @Nullable 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.
*/
private static Modifier asModifier(Token token) {
private static @Nullable Modifier asModifier(Token token) {
TokenKind kind = ((JavaInput.Tok) token.getTok()).kind();
if (kind != null) {
switch (kind) {
Expand Down Expand Up @@ -145,8 +237,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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class FormatterIntegrationTest {
ImmutableMultimap.<Integer, String>builder()
.putAll(14, "I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594")
.putAll(15, "I603")
.putAll(16, "I588")
.putAll(16, "I588", "Sealed")
.putAll(17, "I683", "I684", "I696")
.putAll(
21,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.googlejavaformat.java;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;

import com.google.common.base.Joiner;
import com.google.common.collect.Range;
Expand Down Expand Up @@ -103,4 +104,11 @@ public void whitespace() throws FormatterException {
.getText();
assertThat(output).contains("public\n static int a;");
}

@Test
public void sealedClass() throws FormatterException {
assume().that(Runtime.version().feature()).isAtLeast(16);
assertThat(ModifierOrderer.reorderModifiers("non-sealed sealed public").getText())
.isEqualTo("public sealed non-sealed");
}
}
Original file line number Diff line number Diff line change
@@ -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 {}
}
Original file line number Diff line number Diff line change
@@ -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 {}
}

0 comments on commit 7fd9300

Please sign in to comment.