Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1961,8 +1961,10 @@ public void visitClassDeclaration(ClassTree node) {
node.getModifiers(),
Direction.VERTICAL,
/* declarationAnnotationBreak= */ Optional.empty());
List<? extends Tree> permitsTypes = getPermitsClause(node);
boolean hasSuperclassType = node.getExtendsClause() != null;
boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty();
boolean hasPermitsTypes = !permitsTypes.isEmpty();
builder.addAll(breaks);
token(node.getKind() == Tree.Kind.INTERFACE ? "interface" : "class");
builder.space();
Expand All @@ -1975,30 +1977,18 @@ public void visitClassDeclaration(ClassTree node) {
if (!node.getTypeParameters().isEmpty()) {
typeParametersRest(
node.getTypeParameters(),
hasSuperclassType || hasSuperInterfaceTypes ? plusFour : ZERO);
hasSuperclassType || hasSuperInterfaceTypes || hasPermitsTypes ? plusFour : ZERO);
}
if (hasSuperclassType) {
builder.breakToFill(" ");
token("extends");
builder.space();
scan(node.getExtendsClause(), null);
}
if (hasSuperInterfaceTypes) {
builder.breakToFill(" ");
builder.open(node.getImplementsClause().size() > 1 ? plusFour : ZERO);
token(node.getKind() == Tree.Kind.INTERFACE ? "extends" : "implements");
builder.space();
boolean first = true;
for (Tree superInterfaceType : node.getImplementsClause()) {
if (!first) {
token(",");
builder.breakOp(" ");
}
scan(superInterfaceType, null);
first = false;
}
builder.close();
}
classDeclarationTypeList(
node.getKind() == Tree.Kind.INTERFACE ? "extends" : "implements",
node.getImplementsClause());
classDeclarationTypeList("permits", permitsTypes);
}
builder.close();
if (node.getMembers() == null) {
Expand Down Expand Up @@ -2277,6 +2267,8 @@ boolean nextIsModifier() {
case "native":
case "strictfp":
case "default":
case "sealed":
case "non-sealed":
return true;
default:
return false;
Expand Down Expand Up @@ -3549,6 +3541,29 @@ protected void addBodyDeclarations(
}
}

/** Gets the permits clause for the given node. This is only available in Java 15 and later. */
protected List<? extends Tree> getPermitsClause(ClassTree node) {
return ImmutableList.of();
}

private void classDeclarationTypeList(String token, List<? extends Tree> types) {
if (types.isEmpty()) return;
builder.breakToFill(" ");
builder.open(types.size() > 1 ? plusFour : ZERO);
token(token);
builder.space();
boolean first = true;
for (Tree type : types) {
if (!first) {
token(",");
builder.breakOp(" ");
}
scan(type, null);
first = false;
}
builder.close();
}

/**
* The parser expands multi-variable declarations into separate single-variable declarations. All
* of the fragments in the original declaration have the same start position, so we use that as a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,44 +36,6 @@
/** Fixes sequences of modifiers to be in JLS order. */
final class ModifierOrderer {

/**
* Returns the {@link javax.lang.model.element.Modifier} for the given token kind, or {@code
* null}.
*/
private static Modifier getModifier(TokenKind kind) {
if (kind == null) {
return null;
}
switch (kind) {
case PUBLIC:
return Modifier.PUBLIC;
case PROTECTED:
return Modifier.PROTECTED;
case PRIVATE:
return Modifier.PRIVATE;
case ABSTRACT:
return Modifier.ABSTRACT;
case STATIC:
return Modifier.STATIC;
case DEFAULT:
return Modifier.DEFAULT;
case FINAL:
return Modifier.FINAL;
case TRANSIENT:
return Modifier.TRANSIENT;
case VOLATILE:
return Modifier.VOLATILE;
case SYNCHRONIZED:
return Modifier.SYNCHRONIZED;
case NATIVE:
return Modifier.NATIVE;
case STRICTFP:
return Modifier.STRICTFP;
default:
return null;
}
}

/** Reorders all modifiers in the given text to be in JLS order. */
static JavaInput reorderModifiers(String text) throws FormatterException {
return reorderModifiers(
Expand Down Expand Up @@ -152,7 +114,43 @@ private static void addTrivia(StringBuilder replacement, ImmutableList<? extends
* is not a modifier.
*/
private static Modifier asModifier(Token token) {
return getModifier(((JavaInput.Tok) token.getTok()).kind());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about putting this in the default: in getModifier? This is currently the only call of getModifier, but putting the logic there would prevent a bug if another call of getModifier is added.

Also I'd consider using a switch instead of the if/else chain:

switch (token.getTok().getText()) {
  case "sealed":
     return Modifier.valueOf("SEALED");

Copy link
Contributor Author

@now now Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn’t do it like that initially, as getModifier takes a TokenKind (and returns early if that’s null) and I didn’t want to change the signature or the null check, but I agree that it’s not optimal. Would it be OK if I inline getModifier in asModifier and handle both cases using switches there? (I agree that a switch is much better than an if/else chain.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inlining getModifier SGTM

TokenKind kind = ((JavaInput.Tok) token.getTok()).kind();
if (kind != null) {
switch (kind) {
case PUBLIC:
return Modifier.PUBLIC;
case PROTECTED:
return Modifier.PROTECTED;
case PRIVATE:
return Modifier.PRIVATE;
case ABSTRACT:
return Modifier.ABSTRACT;
case STATIC:
return Modifier.STATIC;
case DEFAULT:
return Modifier.DEFAULT;
case FINAL:
return Modifier.FINAL;
case TRANSIENT:
return Modifier.TRANSIENT;
case VOLATILE:
return Modifier.VOLATILE;
case SYNCHRONIZED:
return Modifier.SYNCHRONIZED;
case NATIVE:
return Modifier.NATIVE;
case STRICTFP:
return Modifier.STRICTFP;
}
}
switch (token.getTok().getText()) {
case "non-sealed":
return Modifier.valueOf("NON_SEALED");
case "sealed":
return Modifier.valueOf("SEALED");
default:
return null;
}
}

/** Applies replacements to the given string. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ protected void handleModule(boolean first, CompilationUnitTree node) {
}
}

@Override
protected List<? extends Tree> getPermitsClause(ClassTree node) {
try {
return (List<? extends Tree>) ClassTree.class.getMethod("getPermitsClause").invoke(node);
} catch (ReflectiveOperationException e) {
// Java < 15
return super.getPermitsClause(node);
}
}

@Override
public Void visitBindingPattern(BindingPatternTree node, Void unused) {
sync(node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public class FormatterIntegrationTest {
private static final ImmutableSet<String> JAVA14_TESTS =
ImmutableSet.of("I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594");

private static final ImmutableSet<String> JAVA15_TESTS = ImmutableSet.of("I603");

private static final ImmutableSet<String> JAVA16_TESTS = ImmutableSet.of("I588");

@Parameters(name = "{index}: {0}")
Expand Down Expand Up @@ -92,6 +94,9 @@ public static Iterable<Object[]> data() throws IOException {
if (JAVA14_TESTS.contains(fileName) && getMajor() < 14) {
continue;
}
if (JAVA15_TESTS.contains(fileName) && getMajor() < 15) {
continue;
}
if (JAVA16_TESTS.contains(fileName) && getMajor() < 16) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class I603 {
sealed abstract class T1 {}

sealed class T2 extends X implements Y permits Z {}

sealed class T3
permits
Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {}

sealed class T4
implements
Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
permits
Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx,
Yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class I603 {
abstract sealed class T1 {}

sealed class T2 extends X implements Y permits Z {}

sealed class T3
permits Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {}

sealed class T4
implements Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
permits Xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx,
Yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy {}
}