From c186e3dc91753bac0789d072e4e2310faac0e555 Mon Sep 17 00:00:00 2001 From: Nikolai Weibull Date: Fri, 23 Jul 2021 22:21:12 +0200 Subject: [PATCH 1/4] Support sealed classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implements support for JEP 409: Sealed Classes, see https://openjdk.java.net/jeps/409. The implementation handles the sealed and non-sealed keywords and sorts them according to their JLS order. This requires us to look at the actual text of the Tok, as there’s no TokenKind for sealed and non-sealed. The optional permits clause is handled in the same manner as the implements clause. A new private method, classDeclarationTypeList, has been added to facilitate this. This fixes #603. --- .../java/JavaInputAstVisitor.java | 43 +++++++++++-------- .../java/ModifierOrderer.java | 6 ++- .../java/FormatterIntegrationTest.java | 2 +- .../googlejavaformat/java/testdata/I603.input | 1 + .../java/testdata/I603.output | 1 + 5 files changed, 34 insertions(+), 19 deletions(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java index e22dae5d5..be0af73c7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -1963,6 +1963,7 @@ public void visitClassDeclaration(ClassTree node) { /* declarationAnnotationBreak= */ Optional.empty()); boolean hasSuperclassType = node.getExtendsClause() != null; boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty(); + boolean hasPermitsTypes = !node.getPermitsClause().isEmpty(); builder.addAll(breaks); token(node.getKind() == Tree.Kind.INTERFACE ? "interface" : "class"); builder.space(); @@ -1975,7 +1976,7 @@ 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(" "); @@ -1983,22 +1984,10 @@ public void visitClassDeclaration(ClassTree node) { 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", node.getPermitsClause()); } builder.close(); if (node.getMembers() == null) { @@ -2277,6 +2266,8 @@ boolean nextIsModifier() { case "native": case "strictfp": case "default": + case "sealed": + case "non-sealed": return true; default: return false; @@ -3549,6 +3540,24 @@ protected void addBodyDeclarations( } } + private void classDeclarationTypeList(String token, List 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 diff --git a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java index 42eaf45f3..1eea60c07 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -152,7 +152,11 @@ private static void addTrivia(StringBuilder replacement, ImmutableList JAVA14_TESTS = ImmutableSet.of("I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594"); - private static final ImmutableSet JAVA16_TESTS = ImmutableSet.of("I588"); + private static final ImmutableSet JAVA16_TESTS = ImmutableSet.of("I588", "I603"); @Parameters(name = "{index}: {0}") public static Iterable data() throws IOException { diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input new file mode 100644 index 000000000..5d4b99622 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input @@ -0,0 +1 @@ +sealed abstract class T permits D, E {} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output new file mode 100644 index 000000000..b9655c306 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output @@ -0,0 +1 @@ +abstract sealed class T permits D, E {} From 4631dd63b602cab2a6cb2a3421560cd957017f9a Mon Sep 17 00:00:00 2001 From: Nikolai Weibull Date: Sat, 24 Jul 2021 10:05:42 +0200 Subject: [PATCH 2/4] Break out Java 15-specific code to java15 package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit didn’t consider backwards compatibility with regards to compilation on a pre-Java 15 JDK. This patch breaks out all Java 15-specific code to the com.google.googlejavaformat.java.java15 package. This change requires us to make ModifierOrderer non-final and some of its methods non-static so that we can override asModifier to handle sealed and non-sealed in the new Java15ModifierOrderer. The new Java15InputAstVisitor overrides a new protected method, getPermitsClause, that reflectively invokes getPermitsClause. Extracting this method seems to make the most sense, as visitClassDeclaration is quite complex and overriding it as a whole didn’t seem like a good idea. Finally, creation of the InputAstVisitor was simplified, as creating it via reflection for Java14 and Java15 isn’t necessary, as Java14InputAstVisitor and Java15InputAstVisitor are built on earlier JDKs anyway. --- .../googlejavaformat/java/Formatter.java | 26 ++++++------- .../java/JavaInputAstVisitor.java | 10 ++++- .../java/ModifierOrderer.java | 14 +++---- .../java/java15/Java15InputAstVisitor.java | 38 +++++++++++++++++++ .../java/java15/Java15ModifierOrderer.java | 31 +++++++++++++++ .../java/ModifierOrdererTest.java | 19 ++++++---- 6 files changed, 105 insertions(+), 33 deletions(-) create mode 100644 core/src/main/java/com/google/googlejavaformat/java/java15/Java15InputAstVisitor.java create mode 100644 core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 3e973958d..962029760 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -33,6 +33,9 @@ import com.google.googlejavaformat.Newlines; import com.google.googlejavaformat.Op; import com.google.googlejavaformat.OpsBuilder; +import com.google.googlejavaformat.java.java14.Java14InputAstVisitor; +import com.google.googlejavaformat.java.java15.Java15InputAstVisitor; +import com.google.googlejavaformat.java.java15.Java15ModifierOrderer; import com.sun.tools.javac.file.JavacFileManager; import com.sun.tools.javac.parser.JavacParser; import com.sun.tools.javac.parser.ParserFactory; @@ -93,6 +96,9 @@ public final class Formatter { static final Range EMPTY_RANGE = Range.closedOpen(-1, -1); + static final ModifierOrderer MODIFIER_ORDERER = + getMajor() >= 15 ? new Java15ModifierOrderer() : new ModifierOrderer(); + private final JavaFormatterOptions options; /** A new Formatter instance with default options. */ @@ -154,19 +160,11 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept OpsBuilder builder = new OpsBuilder(javaInput, javaOutput); // Output the compilation unit. JavaInputAstVisitor visitor; - if (getMajor() >= 14) { - try { - visitor = - Class.forName("com.google.googlejavaformat.java.java14.Java14InputAstVisitor") - .asSubclass(JavaInputAstVisitor.class) - .getConstructor(OpsBuilder.class, int.class) - .newInstance(builder, options.indentationMultiplier()); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } - } else { - visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier()); - } + if (getMajor() >= 15) + visitor = new Java15InputAstVisitor(builder, options.indentationMultiplier()); + else if (getMajor() >= 14) + visitor = new Java14InputAstVisitor(builder, options.indentationMultiplier()); + else visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier()); visitor.scan(unit, null); builder.sync(javaInput.getText().length()); builder.drain(); @@ -282,7 +280,7 @@ public ImmutableList getFormatReplacements( // TODO(cushon): this is only safe because the modifier ordering doesn't affect whitespace, // and doesn't change the replacements that are output. This is not true in general for // 'de-linting' changes (e.g. import ordering). - javaInput = ModifierOrderer.reorderModifiers(javaInput, characterRanges); + javaInput = MODIFIER_ORDERER.reorderModifiers(javaInput, characterRanges); String lineSeparator = Newlines.guessLineSeparator(input); JavaOutput javaOutput = diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java index be0af73c7..f10493794 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -1961,9 +1961,10 @@ public void visitClassDeclaration(ClassTree node) { node.getModifiers(), Direction.VERTICAL, /* declarationAnnotationBreak= */ Optional.empty()); + List permitsTypes = getPermitsClause(node); boolean hasSuperclassType = node.getExtendsClause() != null; boolean hasSuperInterfaceTypes = !node.getImplementsClause().isEmpty(); - boolean hasPermitsTypes = !node.getPermitsClause().isEmpty(); + boolean hasPermitsTypes = !permitsTypes.isEmpty(); builder.addAll(breaks); token(node.getKind() == Tree.Kind.INTERFACE ? "interface" : "class"); builder.space(); @@ -1987,7 +1988,7 @@ public void visitClassDeclaration(ClassTree node) { classDeclarationTypeList( node.getKind() == Tree.Kind.INTERFACE ? "extends" : "implements", node.getImplementsClause()); - classDeclarationTypeList("permits", node.getPermitsClause()); + classDeclarationTypeList("permits", permitsTypes); } builder.close(); if (node.getMembers() == null) { @@ -3540,6 +3541,11 @@ protected void addBodyDeclarations( } } + /** Gets the permits clause for the given node. This is only available in Java 15 and later. */ + protected List getPermitsClause(ClassTree node) { + return ImmutableList.of(); + } + private void classDeclarationTypeList(String token, List types) { if (types.isEmpty()) return; builder.breakToFill(" "); diff --git a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java index 1eea60c07..49cdc24ca 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -34,7 +34,7 @@ import javax.lang.model.element.Modifier; /** Fixes sequences of modifiers to be in JLS order. */ -final class ModifierOrderer { +public class ModifierOrderer { /** * Returns the {@link javax.lang.model.element.Modifier} for the given token kind, or {@code @@ -75,7 +75,7 @@ private static Modifier getModifier(TokenKind kind) { } /** Reorders all modifiers in the given text to be in JLS order. */ - static JavaInput reorderModifiers(String text) throws FormatterException { + JavaInput reorderModifiers(String text) throws FormatterException { return reorderModifiers( new JavaInput(text), ImmutableList.of(Range.closedOpen(0, text.length()))); } @@ -84,7 +84,7 @@ static JavaInput reorderModifiers(String text) throws FormatterException { * 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) + public JavaInput reorderModifiers(JavaInput javaInput, Collection> characterRanges) throws FormatterException { if (javaInput.getTokens().isEmpty()) { // There weren't any tokens, possible because of a lexing error. @@ -151,12 +151,8 @@ private static void addTrivia(StringBuilder replacement, ImmutableList getPermitsClause(ClassTree node) { + try { + return (List) ClassTree.class.getMethod("getPermitsClause").invoke(node); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } +} diff --git a/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java new file mode 100644 index 000000000..de8a35a09 --- /dev/null +++ b/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java @@ -0,0 +1,31 @@ +/* + * Copyright 2021 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package com.google.googlejavaformat.java.java15; + +import com.google.googlejavaformat.Input.Token; +import com.google.googlejavaformat.java.ModifierOrderer; +import javax.lang.model.element.Modifier; + +public class Java15ModifierOrderer extends ModifierOrderer { + + @Override + protected Modifier asModifier(Token token) { + Modifier m = super.asModifier(token); + if (m != null) return m; + else if (token.getTok().getText().equals("sealed")) return Modifier.valueOf("SEALED"); + else if (token.getTok().getText().equals("non-sealed")) return Modifier.valueOf("NON_SEALED"); + else return null; + } +} diff --git a/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java b/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java index 1f8fc1721..580e7dd83 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java @@ -31,20 +31,22 @@ public class ModifierOrdererTest { @Test public void simple() throws FormatterException { - assertThat(ModifierOrderer.reorderModifiers("static abstract class InnerClass {}").getText()) + assertThat( + new ModifierOrderer().reorderModifiers("static abstract class InnerClass {}").getText()) .isEqualTo("abstract static class InnerClass {}"); } @Test public void comment() throws FormatterException { - assertThat(ModifierOrderer.reorderModifiers("static/*1*/abstract/*2*/public").getText()) + assertThat(new ModifierOrderer().reorderModifiers("static/*1*/abstract/*2*/public").getText()) .isEqualTo("public/*1*/abstract/*2*/static"); } @Test public void everything() throws FormatterException { assertThat( - ModifierOrderer.reorderModifiers( + new ModifierOrderer() + .reorderModifiers( "strictfp native synchronized volatile transient final static abstract" + " private protected public") .getText()) @@ -56,7 +58,8 @@ public void everything() throws FormatterException { @Test public void everythingIncludingDefault() throws FormatterException { assertThat( - ModifierOrderer.reorderModifiers( + new ModifierOrderer() + .reorderModifiers( "strictfp native synchronized volatile transient final static default abstract" + " private protected public") .getText()) @@ -78,8 +81,8 @@ public void subRange() throws FormatterException { int start = input.indexOf(substring); int end = start + substring.length(); String output = - ModifierOrderer.reorderModifiers( - new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) + new ModifierOrderer() + .reorderModifiers(new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) .getText(); assertThat(output).contains("public static int a;"); assertThat(output).contains("static public int b;"); @@ -98,8 +101,8 @@ public void whitespace() throws FormatterException { int start = input.indexOf(substring); int end = start + substring.length(); String output = - ModifierOrderer.reorderModifiers( - new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) + new ModifierOrderer() + .reorderModifiers(new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) .getText(); assertThat(output).contains("public\n static int a;"); } From d8b2c7d9b341b9f2973bd03cb16c3084359d81b8 Mon Sep 17 00:00:00 2001 From: Nikolai Weibull Date: Mon, 26 Jul 2021 15:51:58 +0200 Subject: [PATCH 3/4] Improvements after suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exclude Java15InputAstVisitor if we’re running on a JDK earlier than 15. Invoke getPermitsClause() directly. Load JAva15InputAstVisitor via reflection. Handle sealed and non-sealed modifers in ModifierOrderer directly. Add JAVA15_TESTS that consists of I603 for now. Add a couple more tests that checks that long lines are handled correctly, along with multiple permitted classes over multiple lines following implemented interfaces. --- core/pom.xml | 25 +++++++++++++++ .../googlejavaformat/java/Formatter.java | 29 ++++++++++------- .../java/ModifierOrderer.java | 18 ++++++++--- .../java/java15/Java15InputAstVisitor.java | 6 +--- .../java/java15/Java15ModifierOrderer.java | 31 ------------------- .../java/FormatterIntegrationTest.java | 7 ++++- .../java/ModifierOrdererTest.java | 19 +++++------- .../googlejavaformat/java/testdata/I603.input | 17 +++++++++- .../java/testdata/I603.output | 14 ++++++++- 9 files changed, 99 insertions(+), 67 deletions(-) delete mode 100644 core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java diff --git a/core/pom.xml b/core/pom.xml index faa94d627..bdd90ed62 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -280,5 +280,30 @@ + + jdk14 + + (,15) + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + **/Java15InputAstVisitor.java + + + + + maven-javadoc-plugin + + com.google.googlejavaformat.java.java14,com.google.googlejavaformat.java.java15 + + + + + diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 962029760..fbccfadff 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -33,9 +33,6 @@ import com.google.googlejavaformat.Newlines; import com.google.googlejavaformat.Op; import com.google.googlejavaformat.OpsBuilder; -import com.google.googlejavaformat.java.java14.Java14InputAstVisitor; -import com.google.googlejavaformat.java.java15.Java15InputAstVisitor; -import com.google.googlejavaformat.java.java15.Java15ModifierOrderer; import com.sun.tools.javac.file.JavacFileManager; import com.sun.tools.javac.parser.JavacParser; import com.sun.tools.javac.parser.ParserFactory; @@ -96,9 +93,6 @@ public final class Formatter { static final Range EMPTY_RANGE = Range.closedOpen(-1, -1); - static final ModifierOrderer MODIFIER_ORDERER = - getMajor() >= 15 ? new Java15ModifierOrderer() : new ModifierOrderer(); - private final JavaFormatterOptions options; /** A new Formatter instance with default options. */ @@ -160,11 +154,22 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept OpsBuilder builder = new OpsBuilder(javaInput, javaOutput); // Output the compilation unit. JavaInputAstVisitor visitor; - if (getMajor() >= 15) - visitor = new Java15InputAstVisitor(builder, options.indentationMultiplier()); - else if (getMajor() >= 14) - visitor = new Java14InputAstVisitor(builder, options.indentationMultiplier()); - else visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier()); + if (getMajor() >= 14) { + try { + visitor = + Class.forName( + getMajor() >= 15 ? + "com.google.googlejavaformat.java.java15.Java15InputAstVisitor" : + "com.google.googlejavaformat.java.java14.Java14InputAstVisitor") + .asSubclass(JavaInputAstVisitor.class) + .getConstructor(OpsBuilder.class, int.class) + .newInstance(builder, options.indentationMultiplier()); + } catch (ReflectiveOperationException e) { + throw new LinkageError(e.getMessage(), e); + } + } else { + visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier()); + } visitor.scan(unit, null); builder.sync(javaInput.getText().length()); builder.drain(); @@ -280,7 +285,7 @@ public ImmutableList getFormatReplacements( // TODO(cushon): this is only safe because the modifier ordering doesn't affect whitespace, // and doesn't change the replacements that are output. This is not true in general for // 'de-linting' changes (e.g. import ordering). - javaInput = MODIFIER_ORDERER.reorderModifiers(javaInput, characterRanges); + javaInput = ModifierOrderer.reorderModifiers(javaInput, characterRanges); String lineSeparator = Newlines.guessLineSeparator(input); JavaOutput javaOutput = diff --git a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java index 49cdc24ca..8f2816560 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -34,7 +34,7 @@ import javax.lang.model.element.Modifier; /** Fixes sequences of modifiers to be in JLS order. */ -public class ModifierOrderer { +final class ModifierOrderer { /** * Returns the {@link javax.lang.model.element.Modifier} for the given token kind, or {@code @@ -75,7 +75,7 @@ private static Modifier getModifier(TokenKind kind) { } /** Reorders all modifiers in the given text to be in JLS order. */ - JavaInput reorderModifiers(String text) throws FormatterException { + static JavaInput reorderModifiers(String text) throws FormatterException { return reorderModifiers( new JavaInput(text), ImmutableList.of(Range.closedOpen(0, text.length()))); } @@ -84,7 +84,7 @@ JavaInput reorderModifiers(String text) throws FormatterException { * Reorders all modifiers in the given text and within the given character ranges to be in JLS * order. */ - public JavaInput reorderModifiers(JavaInput javaInput, Collection> characterRanges) + static JavaInput reorderModifiers(JavaInput javaInput, Collection> characterRanges) throws FormatterException { if (javaInput.getTokens().isEmpty()) { // There weren't any tokens, possible because of a lexing error. @@ -151,8 +151,16 @@ private static void addTrivia(StringBuilder replacement, ImmutableList getPermitsClause(ClassTree node) { - try { - return (List) ClassTree.class.getMethod("getPermitsClause").invoke(node); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } + return node.getPermitsClause(); } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java deleted file mode 100644 index de8a35a09..000000000 --- a/core/src/main/java/com/google/googlejavaformat/java/java15/Java15ModifierOrderer.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2021 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -package com.google.googlejavaformat.java.java15; - -import com.google.googlejavaformat.Input.Token; -import com.google.googlejavaformat.java.ModifierOrderer; -import javax.lang.model.element.Modifier; - -public class Java15ModifierOrderer extends ModifierOrderer { - - @Override - protected Modifier asModifier(Token token) { - Modifier m = super.asModifier(token); - if (m != null) return m; - else if (token.getTok().getText().equals("sealed")) return Modifier.valueOf("SEALED"); - else if (token.getTok().getText().equals("non-sealed")) return Modifier.valueOf("NON_SEALED"); - else return null; - } -} diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java index f54f7582b..edbb12b6e 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java @@ -50,7 +50,9 @@ public class FormatterIntegrationTest { private static final ImmutableSet JAVA14_TESTS = ImmutableSet.of("I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594"); - private static final ImmutableSet JAVA16_TESTS = ImmutableSet.of("I588", "I603"); + private static final ImmutableSet JAVA15_TESTS = ImmutableSet.of("I603"); + + private static final ImmutableSet JAVA16_TESTS = ImmutableSet.of("I588"); @Parameters(name = "{index}: {0}") public static Iterable data() throws IOException { @@ -92,6 +94,9 @@ public static Iterable 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; } diff --git a/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java b/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java index 580e7dd83..1f8fc1721 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java @@ -31,22 +31,20 @@ public class ModifierOrdererTest { @Test public void simple() throws FormatterException { - assertThat( - new ModifierOrderer().reorderModifiers("static abstract class InnerClass {}").getText()) + assertThat(ModifierOrderer.reorderModifiers("static abstract class InnerClass {}").getText()) .isEqualTo("abstract static class InnerClass {}"); } @Test public void comment() throws FormatterException { - assertThat(new ModifierOrderer().reorderModifiers("static/*1*/abstract/*2*/public").getText()) + assertThat(ModifierOrderer.reorderModifiers("static/*1*/abstract/*2*/public").getText()) .isEqualTo("public/*1*/abstract/*2*/static"); } @Test public void everything() throws FormatterException { assertThat( - new ModifierOrderer() - .reorderModifiers( + ModifierOrderer.reorderModifiers( "strictfp native synchronized volatile transient final static abstract" + " private protected public") .getText()) @@ -58,8 +56,7 @@ public void everything() throws FormatterException { @Test public void everythingIncludingDefault() throws FormatterException { assertThat( - new ModifierOrderer() - .reorderModifiers( + ModifierOrderer.reorderModifiers( "strictfp native synchronized volatile transient final static default abstract" + " private protected public") .getText()) @@ -81,8 +78,8 @@ public void subRange() throws FormatterException { int start = input.indexOf(substring); int end = start + substring.length(); String output = - new ModifierOrderer() - .reorderModifiers(new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) + ModifierOrderer.reorderModifiers( + new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) .getText(); assertThat(output).contains("public static int a;"); assertThat(output).contains("static public int b;"); @@ -101,8 +98,8 @@ public void whitespace() throws FormatterException { int start = input.indexOf(substring); int end = start + substring.length(); String output = - new ModifierOrderer() - .reorderModifiers(new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) + ModifierOrderer.reorderModifiers( + new JavaInput(input), Arrays.asList(Range.closedOpen(start, end))) .getText(); assertThat(output).contains("public\n static int a;"); } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input index 5d4b99622..6cedc5300 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.input @@ -1 +1,16 @@ -sealed abstract class T permits D, E {} +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 {} +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output index b9655c306..22153b688 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I603.output @@ -1 +1,13 @@ -abstract sealed class T permits D, E {} +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 {} +} From 4825e3310e00a1145304d64b4b73e9d3d03d08b3 Mon Sep 17 00:00:00 2001 From: Nikolai Weibull Date: Tue, 27 Jul 2021 12:48:49 +0200 Subject: [PATCH 4/4] Move relevant code from Java15InputAstVisitor to Java14InputAstVisitor This commit moves the relevant code in Java15InputAstVisitor to Java14InputAstVisitor for possible future cleanup. This avoids a lot of seremony in pom.xml. ModifierOrderer is cleaned up a bit by inlining getModifier() in asModifier() and using a switch instead for checking for sealed and non-sealed. --- core/pom.xml | 25 ------ .../googlejavaformat/java/Formatter.java | 5 +- .../java/ModifierOrderer.java | 80 ++++++++----------- .../java/java14/Java14InputAstVisitor.java | 10 +++ .../java/java15/Java15InputAstVisitor.java | 34 -------- 5 files changed, 46 insertions(+), 108 deletions(-) delete mode 100644 core/src/main/java/com/google/googlejavaformat/java/java15/Java15InputAstVisitor.java diff --git a/core/pom.xml b/core/pom.xml index bdd90ed62..faa94d627 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -280,30 +280,5 @@ - - jdk14 - - (,15) - - - - - org.apache.maven.plugins - maven-compiler-plugin - - - **/Java15InputAstVisitor.java - - - - - maven-javadoc-plugin - - com.google.googlejavaformat.java.java14,com.google.googlejavaformat.java.java15 - - - - - diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index fbccfadff..3e973958d 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -157,10 +157,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept if (getMajor() >= 14) { try { visitor = - Class.forName( - getMajor() >= 15 ? - "com.google.googlejavaformat.java.java15.Java15InputAstVisitor" : - "com.google.googlejavaformat.java.java14.Java14InputAstVisitor") + Class.forName("com.google.googlejavaformat.java.java14.Java14InputAstVisitor") .asSubclass(JavaInputAstVisitor.class) .getConstructor(OpsBuilder.class, int.class) .newInstance(builder, options.indentationMultiplier()); diff --git a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java index 8f2816560..8647ced84 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -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( @@ -152,15 +114,43 @@ private static void addTrivia(StringBuilder replacement, ImmutableList getPermitsClause(ClassTree node) { + try { + return (List) 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); diff --git a/core/src/main/java/com/google/googlejavaformat/java/java15/Java15InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java15/Java15InputAstVisitor.java deleted file mode 100644 index 9028fb6cc..000000000 --- a/core/src/main/java/com/google/googlejavaformat/java/java15/Java15InputAstVisitor.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2021 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -package com.google.googlejavaformat.java.java15; - -import com.google.googlejavaformat.OpsBuilder; -import com.google.googlejavaformat.java.java14.Java14InputAstVisitor; -import com.sun.source.tree.ClassTree; -import com.sun.source.tree.Tree; -import java.util.List; - -/** Extends {@link Java14InputAstVisitor} with support for AST nodes that were added for Java 15. */ -public class Java15InputAstVisitor extends Java14InputAstVisitor { - - public Java15InputAstVisitor(OpsBuilder builder, int indentMultiplier) { - super(builder, indentMultiplier); - } - - @Override - protected List getPermitsClause(ClassTree node) { - return node.getPermitsClause(); - } -}