Skip to content

Commit 75e0b84

Browse files
abashevabashev
committed
Recognize the value modifier of Valhalla value classes
Port of google-java-format 1959806; its first attempt, 5f7a569, was rolled back the same day. A modifier is no longer told by its text alone: nextIsModifier checks the token against the flags of the ModifiersTree being visited, so `value`, `sealed` and `non` stay identifiers where they are names. Before this a lambda parameter named `sealed` or `non` failed with "expected token: ')'". The orderer looks VALUE and NON_* up by name with Enums.getIfPresent, since neither VALUE nor NON_VALUE exists on JDK 21. The value class and record tests are skipped on a JDK without Modifier.VALUE, which is every GA release up to 27; they need a Valhalla early-access build. Closes #131
1 parent 197db87 commit 75e0b84

4 files changed

Lines changed: 142 additions & 26 deletions

File tree

‎open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java‎

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import static com.sun.source.tree.Tree.Kind.STRING_LITERAL;
3535
import static java.util.stream.Collectors.toList;
3636

37+
import com.google.common.base.Enums;
3738
import com.google.common.base.MoreObjects;
3839
import com.google.common.base.Predicate;
3940
import com.google.common.base.Throwables;
@@ -146,6 +147,7 @@
146147
import java.util.regex.Pattern;
147148
import java.util.stream.Stream;
148149
import javax.annotation.Nullable;
150+
import javax.lang.model.element.Modifier;
149151
import javax.lang.model.element.Name;
150152

151153
/** An AST visitor that builds a stream of {@link Op}s to format from the given {@link CompilationUnitTree}. */
@@ -1420,8 +1422,11 @@ public Void visitMethod(MethodTree node, Void unused) {
14201422
}
14211423
}
14221424
}
1423-
builder.addAll(
1424-
visitModifiers(annotations, Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty()));
1425+
builder.addAll(visitModifiers(
1426+
annotations,
1427+
node.getModifiers().getFlags(),
1428+
Direction.VERTICAL,
1429+
/* declarationAnnotationBreak= */ Optional.empty()));
14251430

14261431
Tree baseReturnType = null;
14271432
Deque<List<? extends AnnotationTree>> dims = null;
@@ -2236,30 +2241,36 @@ protected List<Op> visitModifiers(
22362241
Optional<BreakTag> declarationAnnotationBreak) {
22372242
boolean isRecordParameter = modifiersTree instanceof JCTree.JCModifiers && isInRecord(modifiersTree);
22382243
return visitModifiers(
2239-
modifiersTree.getAnnotations(), annotationsDirection, declarationAnnotationBreak, isRecordParameter);
2244+
modifiersTree.getAnnotations(),
2245+
modifiersTree.getFlags(),
2246+
annotationsDirection,
2247+
declarationAnnotationBreak,
2248+
isRecordParameter);
22402249
}
22412250

22422251
private List<Op> visitModifiers(
22432252
List<? extends AnnotationTree> annotationTrees,
2253+
Set<Modifier> flags,
22442254
Direction annotationsDirection,
22452255
Optional<BreakTag> declarationAnnotationBreak) {
2246-
return visitModifiers(annotationTrees, annotationsDirection, declarationAnnotationBreak, false);
2256+
return visitModifiers(annotationTrees, flags, annotationsDirection, declarationAnnotationBreak, false);
22472257
}
22482258

22492259
private List<Op> visitModifiers(
22502260
List<? extends AnnotationTree> annotationTrees,
2261+
Set<Modifier> flags,
22512262
Direction annotationsDirection,
22522263
Optional<BreakTag> declarationAnnotationBreak,
22532264
boolean isRecordParameter) {
2254-
if (annotationTrees.isEmpty() && !nextIsModifier()) {
2265+
if (annotationTrees.isEmpty() && !nextIsModifier(flags)) {
22552266
return EMPTY_LIST;
22562267
}
22572268
Deque<AnnotationTree> annotations = new ArrayDeque<>(annotationTrees);
22582269
builder.open(ZERO);
22592270
boolean first = true;
22602271
boolean lastWasAnnotation = false;
22612272
while (!annotations.isEmpty()) {
2262-
if (nextIsModifier()) {
2273+
if (nextIsModifier(flags)) {
22632274
break;
22642275
}
22652276
if (!first) {
@@ -2286,7 +2297,7 @@ private List<Op> visitModifiers(
22862297
: annotationsDirection.isVertical()
22872298
? forceBreakList(declarationAnnotationBreak)
22882299
: breakList(declarationAnnotationBreak);
2289-
if (annotations.isEmpty() && !nextIsModifier()) {
2300+
if (annotations.isEmpty() && !nextIsModifier(flags)) {
22902301
return trailingBreak;
22912302
}
22922303
if (lastWasAnnotation) {
@@ -2295,11 +2306,11 @@ private List<Op> visitModifiers(
22952306

22962307
builder.open(ZERO);
22972308
first = true;
2298-
while (nextIsModifier() || !annotations.isEmpty()) {
2309+
while (nextIsModifier(flags) || !annotations.isEmpty()) {
22992310
if (!first) {
23002311
builder.addAll(breakFillList(Optional.empty()));
23012312
}
2302-
if (nextIsModifier()) {
2313+
if (nextIsModifier(flags)) {
23032314
String currentToken = builder.peekToken().get();
23042315
token(currentToken);
23052316
if (currentToken.equals("non")) {
@@ -2316,23 +2327,39 @@ private List<Op> visitModifiers(
23162327
return breakFillList(Optional.empty());
23172328
}
23182329

2319-
boolean nextIsModifier() {
2330+
/**
2331+
* The {@code value} modifier of Project Valhalla's value classes, on a JDK that has it. It is looked up by name so
2332+
* that the formatter still compiles for JDK 21.
2333+
*/
2334+
@Nullable
2335+
private static final Modifier VALUE_MODIFIER =
2336+
Enums.getIfPresent(Modifier.class, "VALUE").orNull();
2337+
2338+
/**
2339+
* Whether the next token is one of {@code flags}, the modifiers of the declaration being visited. The text alone
2340+
* does not tell the contextual modifiers {@code sealed} and {@code value} from identifiers of that name, and a
2341+
* lambda parameter named so starts its declaration with it.
2342+
*/
2343+
boolean nextIsModifier(Set<Modifier> flags) {
2344+
if (flags.isEmpty()) {
2345+
return false;
2346+
}
23202347
return switch (builder.peekToken().get()) {
2321-
case "public",
2322-
"protected",
2323-
"private",
2324-
"abstract",
2325-
"static",
2326-
"final",
2327-
"transient",
2328-
"volatile",
2329-
"synchronized",
2330-
"native",
2331-
"strictfp",
2332-
"default",
2333-
"sealed",
2334-
"non",
2335-
"-" -> true;
2348+
case "public" -> flags.contains(Modifier.PUBLIC);
2349+
case "protected" -> flags.contains(Modifier.PROTECTED);
2350+
case "private" -> flags.contains(Modifier.PRIVATE);
2351+
case "abstract" -> flags.contains(Modifier.ABSTRACT);
2352+
case "static" -> flags.contains(Modifier.STATIC);
2353+
case "final" -> flags.contains(Modifier.FINAL);
2354+
case "transient" -> flags.contains(Modifier.TRANSIENT);
2355+
case "volatile" -> flags.contains(Modifier.VOLATILE);
2356+
case "synchronized" -> flags.contains(Modifier.SYNCHRONIZED);
2357+
case "native" -> flags.contains(Modifier.NATIVE);
2358+
case "strictfp" -> flags.contains(Modifier.STRICTFP);
2359+
case "default" -> flags.contains(Modifier.DEFAULT);
2360+
case "sealed" -> flags.contains(Modifier.SEALED) || flags.contains(Modifier.NON_SEALED);
2361+
case "non", "-" -> flags.contains(Modifier.NON_SEALED);
2362+
case "value" -> VALUE_MODIFIER != null && flags.contains(VALUE_MODIFIER);
23362363
default -> false;
23372364
};
23382365
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkState;
2020
import static com.google.common.collect.Iterables.getLast;
2121

22+
import com.google.common.base.Enums;
2223
import com.google.common.collect.ImmutableList;
2324
import com.google.common.collect.Ordering;
2425
import com.google.common.collect.Range;
@@ -201,7 +202,7 @@ private static Modifier asModifier(ImmutableList<Token> tokens) {
201202
if (modifier == null) {
202203
return null;
203204
}
204-
return Modifier.valueOf("NON_" + modifier.name());
205+
return Enums.getIfPresent(Modifier.class, "NON_" + modifier.name()).orNull();
205206
}
206207

207208
/**
@@ -254,6 +255,7 @@ private static Modifier asModifier(Token token) {
254255
}
255256
return switch (token.getTok().getText()) {
256257
case "sealed" -> Modifier.valueOf("SEALED");
258+
case "value" -> Enums.getIfPresent(Modifier.class, "VALUE").orNull();
257259
default -> null;
258260
};
259261
}

‎open-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,16 @@
3434
import java.nio.file.Files;
3535
import java.nio.file.Path;
3636
import java.time.Duration;
37+
import java.util.Arrays;
3738
import java.util.List;
39+
import javax.lang.model.element.Modifier;
3840
import org.junit.jupiter.api.Assumptions;
3941
import org.junit.jupiter.api.Test;
4042
import org.junit.jupiter.api.io.TempDir;
4143
import org.junit.jupiter.api.parallel.Execution;
4244
import org.junit.jupiter.api.parallel.ExecutionMode;
45+
import org.junit.jupiter.params.ParameterizedTest;
46+
import org.junit.jupiter.params.provider.ValueSource;
4347

4448
@Execution(ExecutionMode.CONCURRENT)
4549
final class FormatterTest {
@@ -568,4 +572,68 @@ void removeTrailingTabsInComments() throws FormatterException {
568572

569573
assertThat(formatter.formatSource(input)).isEqualTo(expected);
570574
}
575+
576+
@Test
577+
void valueClassAndRecord() throws FormatterException {
578+
Assumptions.assumeTrue(
579+
Arrays.stream(Modifier.values())
580+
.anyMatch(modifier -> modifier.name().equals("VALUE")),
581+
"`value` is a modifier only on a JDK with Project Valhalla's value classes");
582+
String input = """
583+
value final public class PointValueClass {
584+
final int x;
585+
final int y;
586+
587+
PointValueClass(int x, int y) {
588+
this.x = x;
589+
this.y = y;
590+
}
591+
}
592+
593+
value public record PointValueRecord(int x, int y) {}
594+
""";
595+
String expected = """
596+
public final value class PointValueClass {
597+
final int x;
598+
final int y;
599+
600+
PointValueClass(int x, int y) {
601+
this.x = x;
602+
this.y = y;
603+
}
604+
}
605+
606+
public value record PointValueRecord(int x, int y) {}
607+
""";
608+
Formatter formatter = Formatter.createFormatter(
609+
JavaFormatterOptions.builder().style(Style.OJF).build());
610+
611+
assertThat(formatter.formatSource(input)).isEqualTo(expected);
612+
}
613+
614+
@ParameterizedTest
615+
@ValueSource(strings = {"non", "sealed", "value"})
616+
void contextualKeywordAsIdentifier(String name) throws FormatterException {
617+
// `value`, `sealed` and `non-sealed` are modifiers only in front of a declaration that carries them. Anywhere
618+
// else they are identifiers, and a lambda parameter is the one declaration that starts with its name.
619+
String input = """
620+
import java.util.Map;
621+
622+
class T {
623+
private final int NAME;
624+
625+
T(int NAME) {
626+
this.NAME = NAME;
627+
}
628+
629+
void f(Map<String, Integer> map) {
630+
map.forEach((key, NAME) -> System.err.println(key + NAME));
631+
}
632+
}
633+
""".replace("NAME", name);
634+
Formatter formatter = Formatter.createFormatter(
635+
JavaFormatterOptions.builder().style(Style.OJF).build());
636+
637+
assertThat(formatter.formatSource(input)).isEqualTo(input);
638+
}
571639
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import com.google.common.base.Joiner;
2222
import com.google.common.collect.Range;
2323
import java.util.Arrays;
24+
import javax.lang.model.element.Modifier;
25+
import org.junit.jupiter.api.Assumptions;
2426
import org.junit.jupiter.api.Test;
2527
import org.junit.jupiter.api.parallel.Execution;
2628
import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -112,4 +114,21 @@ void nonSealedBeforeAccessModifier() throws FormatterException {
112114
.getText())
113115
.isEqualTo("private non-sealed interface B extends I {}");
114116
}
117+
118+
@Test
119+
void valueClass() throws FormatterException {
120+
Assumptions.assumeTrue(
121+
Arrays.stream(Modifier.values())
122+
.anyMatch(modifier -> modifier.name().equals("VALUE")),
123+
"`value` is a modifier only on a JDK with Project Valhalla's value classes");
124+
assertThat(ModifierOrderer.reorderModifiers("value final public").getText())
125+
.isEqualTo("public final value");
126+
}
127+
128+
@Test
129+
void nonMinusValueIsAnExpression() throws FormatterException {
130+
// `non - value` lexes like `non-sealed`, and there is no NON_VALUE modifier to look up for it
131+
assertThat(ModifierOrderer.reorderModifiers("int x = non - value;").getText())
132+
.isEqualTo("int x = non - value;");
133+
}
115134
}

0 commit comments

Comments
 (0)