From c4b467d50bd6bb52935075b438f602ff902db167 Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Sat, 6 Apr 2024 03:02:26 -0700 Subject: [PATCH 01/13] Support overlapping --lines ranges in google-java-format. The strict behavior comes from the underlying ImmutableRangeSet.Builder class, which does not allow overlapping ranges to be added. Let's use TreeRangeSet instead. I have also updated the documentation for the --lines flag to make it clear that the line numbers are a closed-closed interval. Tested with: ``` blaze run :google_java_format_binary -- --lines=1:5 --lines=3:8 - < Foo.java ``` Before this CL the command fails with: ``` '--lines=1:5' '--lines=3:8' - Overlapping ranges not permitted but found [0..5) overlapping [2..8) ``` After this CL the command succeeds. PiperOrigin-RevId: 622410610 --- .../googlejavaformat/java/CommandLineOptions.java | 8 +++++--- .../java/CommandLineOptionsParser.java | 4 ++-- .../googlejavaformat/java/UsageException.java | 2 +- .../java/CommandLineOptionsParserTest.java | 15 ++++++--------- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java index 5a233284a..d5480a790 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java @@ -16,6 +16,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableRangeSet; +import com.google.common.collect.RangeSet; +import com.google.common.collect.TreeRangeSet; import java.util.Optional; /** @@ -178,7 +180,7 @@ static Builder builder() { static class Builder { private final ImmutableList.Builder files = ImmutableList.builder(); - private final ImmutableRangeSet.Builder lines = ImmutableRangeSet.builder(); + private final RangeSet lines = TreeRangeSet.create(); private final ImmutableList.Builder offsets = ImmutableList.builder(); private final ImmutableList.Builder lengths = ImmutableList.builder(); private boolean inPlace = false; @@ -204,7 +206,7 @@ Builder inPlace(boolean inPlace) { return this; } - ImmutableRangeSet.Builder linesBuilder() { + RangeSet linesBuilder() { return lines; } @@ -282,7 +284,7 @@ CommandLineOptions build() { return new CommandLineOptions( files.build(), inPlace, - lines.build(), + ImmutableRangeSet.copyOf(lines), offsets.build(), lengths.build(), aosp, diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java index f7c3dec95..52a5e05d4 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -18,8 +18,8 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableRangeSet; import com.google.common.collect.Range; +import com.google.common.collect.RangeSet; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; @@ -157,7 +157,7 @@ private static String getValue(String flag, Iterator it, String value) { * number. Line numbers are {@code 1}-based, but are converted to the {@code 0}-based numbering * used internally by google-java-format. */ - private static void parseRangeSet(ImmutableRangeSet.Builder result, String ranges) { + private static void parseRangeSet(RangeSet result, String ranges) { for (String range : COMMA_SPLITTER.split(ranges)) { result.add(parseRange(range)); } diff --git a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java index a10f2d076..50d55d4d4 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java +++ b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java @@ -56,7 +56,7 @@ final class UsageException extends Exception { " --set-exit-if-changed", " Return exit code 1 if there are any formatting changes.", " --lines, -lines, --line, -line", - " Line range(s) to format, like 5:10 (1-based; default is all).", + " Line range(s) to format, e.g. the first 5 lines are 1:5 (1-based; default is all).", " --offset, -offset", " Character offset to format (0-based; default is all).", " --length, -length", diff --git a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java index 08fbbbab3..93dfb79c2 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; @@ -151,15 +150,13 @@ public void setExitIfChanged() { .isTrue(); } - // TODO(cushon): consider handling this in the parser and reporting a more detailed error @Test - public void illegalLines() { - try { - CommandLineOptionsParser.parse(Arrays.asList("-lines=1:1", "-lines=1:1")); - fail(); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessageThat().contains("overlap"); - } + public void mergedLines() { + assertThat( + CommandLineOptionsParser.parse(Arrays.asList("-lines=1:5", "-lines=2:8")) + .lines() + .asRanges()) + .containsExactly(Range.closedOpen(0, 8)); } @Test From 6e3b852443363917504edd779e21c97f919ddf70 Mon Sep 17 00:00:00 2001 From: Michael Plump Date: Mon, 15 Apr 2024 09:56:07 -0700 Subject: [PATCH 02/13] update the IntelliJ plugin to 1.22.0 PiperOrigin-RevId: 624993575 --- idea_plugin/build.gradle.kts | 8 ++++++-- idea_plugin/src/main/resources/META-INF/plugin.xml | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/idea_plugin/build.gradle.kts b/idea_plugin/build.gradle.kts index 05010fe3c..0ba5032dc 100644 --- a/idea_plugin/build.gradle.kts +++ b/idea_plugin/build.gradle.kts @@ -14,7 +14,8 @@ * limitations under the License. */ -plugins { id("org.jetbrains.intellij") version "1.17.2" } +// https://github.com/JetBrains/intellij-platform-gradle-plugin/releases +plugins { id("org.jetbrains.intellij") version "1.17.3" } apply(plugin = "org.jetbrains.intellij") @@ -22,7 +23,8 @@ apply(plugin = "java") repositories { mavenCentral() } -val googleJavaFormatVersion = "1.21.0" +// https://github.com/google/google-java-format/releases +val googleJavaFormatVersion = "1.22.0" java { sourceCompatibility = JavaVersion.VERSION_11 @@ -61,6 +63,8 @@ tasks { dependencies { implementation("com.google.googlejavaformat:google-java-format:${googleJavaFormatVersion}") + // https://mvnrepository.com/artifact/junit/junit testImplementation("junit:junit:4.13.2") + // https://mvnrepository.com/artifact/com.google.truth/truth testImplementation("com.google.truth:truth:1.4.2") } diff --git a/idea_plugin/src/main/resources/META-INF/plugin.xml b/idea_plugin/src/main/resources/META-INF/plugin.xml index c426a1ff1..378c2529c 100644 --- a/idea_plugin/src/main/resources/META-INF/plugin.xml +++ b/idea_plugin/src/main/resources/META-INF/plugin.xml @@ -35,6 +35,10 @@ ]]> +
1.22.0.0
+
Updated to use google-java-format 1.22.0.
+
1.21.0.0
+
Updated to use google-java-format 1.21.0.
1.20.0.0
Updated to use google-java-format 1.20.0.
1.19.2.0
From fdf4b29a6e1206f451bd96ed7967f4061921881a Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Mon, 29 Apr 2024 14:11:05 -0700 Subject: [PATCH 03/13] Remove support for the String Templates preview feature Rollback of https://github.com/google/google-java-format/commit/b5feefe4ab4dbdbba393d769eb1815f26896104b, https://github.com/google/google-java-format/commit/38de9c4e05bc871496c232c1ccbfc4be06a580e2, https://github.com/google/google-java-format/commit/e946e82801eb5bbd52bea00355ba20450bc0725c See https://bugs.openjdk.org/browse/JDK-8329949 PiperOrigin-RevId: 629182379 --- .../googlejavaformat/java/JavaInput.java | 13 +- .../googlejavaformat/java/JavacTokens.java | 145 +++--------------- .../java/java21/Java21InputAstVisitor.java | 23 +-- .../java/FormatterIntegrationTest.java | 4 +- .../googlejavaformat/java/FormatterTest.java | 24 --- .../googlejavaformat/java/testdata/I981.input | 12 -- .../java/testdata/I981.output | 14 -- .../java/testdata/StringTemplate.input | 10 -- .../java/testdata/StringTemplate.output | 9 -- .../java/testdata/TextBlockTemplates.input | 11 -- .../java/testdata/TextBlockTemplates.output | 14 -- 11 files changed, 26 insertions(+), 253 deletions(-) delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.input delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.output delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.input delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.output delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.input delete mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.output diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java index b5290ab10..01c617776 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInput.java @@ -387,14 +387,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept final boolean isNumbered; // Is this tok numbered? (tokens and comments) String extraNewline = null; // Extra newline at end? List strings = new ArrayList<>(); - if (tokText.startsWith("'") - || tokText.startsWith("\"") - || JavacTokens.isStringFragment(t.kind())) { - // Perform this check first, STRINGFRAGMENT tokens can start with arbitrary characters. - isToken = true; - isNumbered = true; - strings.add(originalTokText); - } else if (Character.isWhitespace(tokText0)) { + if (Character.isWhitespace(tokText0)) { isToken = false; isNumbered = false; Iterator it = Newlines.lineIterator(originalTokText); @@ -411,6 +404,10 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept strings.add(line); } } + } else if (tokText.startsWith("'") || tokText.startsWith("\"")) { + isToken = true; + isNumbered = true; + strings.add(originalTokText); } else if (tokText.startsWith("//") || tokText.startsWith("/*")) { // For compatibility with an earlier lexer, the newline after a // comment is its own tok. if (tokText.startsWith("//") diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java index 986db3912..6daac33b9 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java @@ -15,8 +15,6 @@ package com.google.googlejavaformat.java; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkElementIndex; -import static java.util.Arrays.stream; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -29,15 +27,7 @@ import com.sun.tools.javac.parser.Tokens.TokenKind; import com.sun.tools.javac.parser.UnicodeReader; import com.sun.tools.javac.util.Context; -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; import java.util.Set; -import org.jspecify.annotations.Nullable; /** A wrapper around javac's lexer. */ final class JavacTokens { @@ -54,8 +44,6 @@ static class RawTok { private final int endPos; RawTok(String stringVal, TokenKind kind, int pos, int endPos) { - checkElementIndex(pos, endPos, "pos"); - checkArgument(pos < endPos, "expected pos (%s) < endPos (%s)", pos, endPos); this.stringVal = stringVal; this.kind = kind; this.pos = pos; @@ -83,18 +71,9 @@ public String stringVal() { } } - private static final TokenKind STRINGFRAGMENT = - stream(TokenKind.values()) - .filter(t -> t.name().contentEquals("STRINGFRAGMENT")) - .findFirst() - .orElse(null); - - static boolean isStringFragment(TokenKind kind) { - return STRINGFRAGMENT != null && Objects.equals(kind, STRINGFRAGMENT); - } - - private static ImmutableList readAllTokens( - String source, Context context, Set nonTerminalStringFragments) { + /** Lex the input and return a list of {@link RawTok}s. */ + public static ImmutableList getTokens( + String source, Context context, Set stopTokens) { if (source == null) { return ImmutableList.of(); } @@ -102,69 +81,20 @@ private static ImmutableList readAllTokens( char[] buffer = (source + EOF_COMMENT).toCharArray(); Scanner scanner = new AccessibleScanner(fac, new CommentSavingTokenizer(fac, buffer, buffer.length)); - List tokens = new ArrayList<>(); - do { - scanner.nextToken(); - tokens.add(scanner.token()); - } while (scanner.token().kind != TokenKind.EOF); - for (int i = 0; i < tokens.size(); i++) { - if (isStringFragment(tokens.get(i).kind)) { - int start = i; - while (isStringFragment(tokens.get(i).kind)) { - i++; - } - for (int j = start; j < i - 1; j++) { - nonTerminalStringFragments.add(tokens.get(j).pos); - } - } - } - // A string template is tokenized as a series of STRINGFRAGMENT tokens containing the string - // literal values, followed by the tokens for the template arguments. For the formatter, we - // want the stream of tokens to appear in order by their start position. - if (Runtime.version().feature() >= 21) { - Collections.sort(tokens, Comparator.comparingInt(t -> t.pos)); - } - return ImmutableList.copyOf(tokens); - } - - /** Lex the input and return a list of {@link RawTok}s. */ - public static ImmutableList getTokens( - String source, Context context, Set stopTokens) { - if (source == null) { - return ImmutableList.of(); - } - Set nonTerminalStringFragments = new HashSet<>(); - ImmutableList javacTokens = readAllTokens(source, context, nonTerminalStringFragments); - ImmutableList.Builder tokens = ImmutableList.builder(); int end = source.length(); int last = 0; - for (Token t : javacTokens) { + do { + scanner.nextToken(); + Token t = scanner.token(); if (t.comments != null) { - // javac accumulates comments in reverse order for (Comment c : Lists.reverse(t.comments)) { - int pos = c.getSourcePos(0); - int length; - if (pos == -1) { - // We've found a comment whose position hasn't been recorded. Deduce its position as the - // first `/` character after the end of the previous token. - // - // javac creates a new JavaTokenizer to process string template arguments, so - // CommentSavingTokenizer doesn't get a chance to preprocess those comments and save - // their text and positions. - // - // TODO: consider always using this approach once the minimum supported JDK is 16 and - // we can assume BasicComment#getRawCharacters is always available. - pos = source.indexOf('/', last); - length = CommentSavingTokenizer.commentLength(c); - } else { - length = c.getText().length(); + if (last < c.getSourcePos(0)) { + tokens.add(new RawTok(null, null, last, c.getSourcePos(0))); } - if (last < pos) { - tokens.add(new RawTok(null, null, last, pos)); - } - tokens.add(new RawTok(null, null, pos, pos + length)); - last = pos + length; + tokens.add( + new RawTok(null, null, c.getSourcePos(0), c.getSourcePos(0) + c.getText().length())); + last = c.getSourcePos(0) + c.getText().length(); } } if (stopTokens.contains(t.kind)) { @@ -176,25 +106,14 @@ public static ImmutableList getTokens( if (last < t.pos) { tokens.add(new RawTok(null, null, last, t.pos)); } - if (isStringFragment(t.kind)) { - int endPos = t.endPos; - int pos = t.pos; - if (nonTerminalStringFragments.contains(t.pos)) { - // Include the \ escape from \{...} in the preceding string fragment - endPos++; - } - tokens.add(new RawTok(source.substring(pos, endPos), t.kind, pos, endPos)); - last = endPos; - } else { - tokens.add( - new RawTok( - t.kind == TokenKind.STRINGLITERAL ? "\"" + t.stringVal() + "\"" : null, - t.kind, - t.pos, - t.endPos)); - last = t.endPos; - } - } + tokens.add( + new RawTok( + t.kind == TokenKind.STRINGLITERAL ? "\"" + t.stringVal() + "\"" : null, + t.kind, + t.pos, + t.endPos)); + last = t.endPos; + } while (scanner.token().kind != TokenKind.EOF); if (last < end) { tokens.add(new RawTok(null, null, last, end)); } @@ -203,32 +122,6 @@ public static ImmutableList getTokens( /** A {@link JavaTokenizer} that saves comments. */ static class CommentSavingTokenizer extends JavaTokenizer { - - private static final Method GET_RAW_CHARACTERS_METHOD = getRawCharactersMethod(); - - private static @Nullable Method getRawCharactersMethod() { - try { - // This is a method in PositionTrackingReader, but that class is not public. - return BasicComment.class.getMethod("getRawCharacters"); - } catch (NoSuchMethodException e) { - return null; - } - } - - static int commentLength(Comment comment) { - if (comment instanceof BasicComment && GET_RAW_CHARACTERS_METHOD != null) { - // If we've seen a BasicComment instead of a CommentWithTextAndPosition, getText() will - // be null, so we deduce the length using getRawCharacters. See also the comment at the - // usage of this method in getTokens. - try { - return ((char[]) GET_RAW_CHARACTERS_METHOD.invoke(((BasicComment) comment))).length; - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } - } - return comment.getText().length(); - } - CommentSavingTokenizer(ScannerFactory fac, char[] buffer, int length) { super(fac, buffer, length); } diff --git a/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java index 2192a32a9..859c9c0cf 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java21/Java21InputAstVisitor.java @@ -23,7 +23,6 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.PatternCaseLabelTree; import com.sun.source.tree.PatternTree; -import com.sun.source.tree.StringTemplateTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.tree.JCTree; import javax.lang.model.element.Name; @@ -63,7 +62,6 @@ public Void visitConstantCaseLabel(ConstantCaseLabelTree node, Void aVoid) { @Override public Void visitDeconstructionPattern(DeconstructionPatternTree node, Void unused) { - sync(node); scan(node.getDeconstructor(), null); builder.open(plusFour); token("("); @@ -82,25 +80,6 @@ public Void visitDeconstructionPattern(DeconstructionPatternTree node, Void unus return null; } - @SuppressWarnings("preview") - @Override - public Void visitStringTemplate(StringTemplateTree node, Void unused) { - sync(node); - builder.open(plusFour); - scan(node.getProcessor(), null); - token("."); - token(builder.peekToken().get()); - for (int i = 0; i < node.getFragments().size() - 1; i++) { - token("{"); - builder.breakOp(); - scan(node.getExpressions().get(i), null); - token("}"); - token(builder.peekToken().get()); - } - builder.close(); - return null; - } - @Override protected void variableName(Name name) { if (name.isEmpty()) { @@ -118,7 +97,7 @@ public Void scan(Tree tree, Void unused) { visitJcAnyPattern((JCTree.JCAnyPattern) tree); return null; } else { - return super.scan(tree, unused); + return super.scan(tree, 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 d31218e28..4a4cedc82 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java @@ -61,10 +61,8 @@ public class FormatterIntegrationTest { "I880", "Unnamed", "I981", - "StringTemplate", "I1020", - "I1037", - "TextBlockTemplates") + "I1037") .build(); @Parameters(name = "{index}: {0}") diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java index b0d7b4030..3835673d9 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertWithMessage; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeTrue; import com.google.common.base.Joiner; import com.google.common.io.CharStreams; @@ -511,27 +510,4 @@ public void removeTrailingTabsInComments() throws Exception { + " }\n" + "}\n"); } - - @Test - public void stringTemplateTests() throws Exception { - assumeTrue(Runtime.version().feature() >= 21); - assertThat( - new Formatter() - .formatSource( - "public class Foo {\n" - + " String test(){\n" - + " var simple = STR.\"mytemplate1XXXX \\{exampleXXXX.foo()}yyy\";\n" - + " var nested = STR.\"template \\{example. foo()+" - + " STR.\"templateInner\\{ example}\"}xxx }\";\n" - + " }\n" - + "}\n")) - .isEqualTo( - "public class Foo {\n" - + " String test() {\n" - + " var simple = STR.\"mytemplate1XXXX \\{exampleXXXX.foo()}yyy\";\n" - + " var nested = STR.\"template \\{example.foo() +" - + " STR.\"templateInner\\{example}\"}xxx }\";\n" - + " }\n" - + "}\n"); - } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.input deleted file mode 100644 index bba0b7267..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.input +++ /dev/null @@ -1,12 +0,0 @@ -class Foo { - private static final int X = 42; - private static final String A = STR."\{X} = \{X}"; - private static final String B = STR.""; - private static final String C = STR."\{X}"; - private static final String D = STR."\{X}\{X}"; - private static final String E = STR."\{X}\{X}\{X}"; - private static final String F = STR." \{X}"; - private static final String G = STR."\{X} "; - private static final String H = STR."\{X} one long incredibly unbroken sentence moving from "+"topic to topic so that no-one had a chance to interrupt"; - private static final String I = STR."\{X} \uD83D\uDCA9 "; -} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.output deleted file mode 100644 index ff173fb6a..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/I981.output +++ /dev/null @@ -1,14 +0,0 @@ -class Foo { - private static final int X = 42; - private static final String A = STR."\{X} = \{X}"; - private static final String B = STR.""; - private static final String C = STR."\{X}"; - private static final String D = STR."\{X}\{X}"; - private static final String E = STR."\{X}\{X}\{X}"; - private static final String F = STR." \{X}"; - private static final String G = STR."\{X} "; - private static final String H = - STR."\{X} one long incredibly unbroken sentence moving from " - + "topic to topic so that no-one had a chance to interrupt"; - private static final String I = STR."\{X} \uD83D\uDCA9 "; -} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.input deleted file mode 100644 index 98a19bba7..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.input +++ /dev/null @@ -1,10 +0,0 @@ -public class StringTemplates { - void test(){ - var m = STR."template \{example}xxx"; - var nested = STR."template \{example.foo()+ STR."templateInner\{example}"}xxx }"; - var nestNested = STR."template \{example0. - foo() + - STR."templateInner\{example1.test(STR."\{example2 - }")}"}xxx }"; - } -} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.output deleted file mode 100644 index e60bab70b..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/StringTemplate.output +++ /dev/null @@ -1,9 +0,0 @@ -public class StringTemplates { - void test() { - var m = STR."template \{example}xxx"; - var nested = STR."template \{example.foo() + STR."templateInner\{example}"}xxx }"; - var nestNested = - STR."template \{ - example0.foo() + STR."templateInner\{example1.test(STR."\{example2}")}"}xxx }"; - } -} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.input deleted file mode 100644 index 2d00d3300..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.input +++ /dev/null @@ -1,11 +0,0 @@ -abstract class RSLs { - abstract String f(); - - String a = STR.""" - lorem - foo\{ /** a method */ f( // line comment - /* TODO */ - )}bar - ipsum - """; -} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.output deleted file mode 100644 index 36736724f..000000000 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/TextBlockTemplates.output +++ /dev/null @@ -1,14 +0,0 @@ -abstract class RSLs { - abstract String f(); - - String a = - STR.""" - lorem - foo\{ - /** a method */ - f( // line comment - /* TODO */ - )}bar - ipsum - """; -} From bec248bb39e6abd8b79e8f9900135d263b6f7ad9 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 1 May 2024 11:39:18 -0700 Subject: [PATCH 04/13] Add a test for trailing unicode escaped whitespace in text blocks PiperOrigin-RevId: 629785894 --- .../java/StringWrapperTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java b/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java index 5ef7cb51a..fd176ed4e 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java @@ -119,6 +119,35 @@ public void textBlockTrailingWhitespace() throws Exception { assertThat(actual).isEqualTo(expected); } + // It would be neat if the formatter could remove the trailing whitespace here, but in general + // it preserves unicode escapes from the original text. + @Test + public void textBlockTrailingWhitespaceUnicodeEscape() throws Exception { + assumeTrue(Runtime.version().feature() >= 15); + // We want a unicode escape in the Java source being formatted, so it needs to be escaped + // in the string literal in this test. + String input = + lines( + "public class T {", + " String s =", + " \"\"\"", + " lorem\\u0020", + " ipsum", + " \"\"\";", + "}"); + String expected = + lines( + "public class T {", + " String s =", + " \"\"\"", + " lorem\\u0020", + " ipsum", + " \"\"\";", + "}"); + String actual = StringWrapper.wrap(100, input, new Formatter()); + assertThat(actual).isEqualTo(expected); + } + @Test public void textBlockSpaceTabMix() throws Exception { assumeTrue(Runtime.version().feature() >= 15); From b9b6730b80a16798da559f2d50112cec3d8538ad Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 29 May 2024 09:28:19 -0700 Subject: [PATCH 05/13] Handle classes with no members and semi-colons PiperOrigin-RevId: 638312117 --- .../google/googlejavaformat/java/JavaInputAstVisitor.java | 6 ++++++ .../googlejavaformat/java/testdata/SemicolonInClass.input | 3 +++ .../googlejavaformat/java/testdata/SemicolonInClass.output | 3 +++ 3 files changed, 12 insertions(+) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.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 1e0675ffd..e00877e96 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -3744,6 +3744,12 @@ protected void addBodyDeclarations( tokenBreakTrailingComment("{", plusTwo); builder.blankLineWanted(BlankLineWanted.NO); builder.open(ZERO); + if (builder.peekToken().equals(Optional.of(";"))) { + builder.open(plusTwo); + dropEmptyDeclarations(); + builder.close(); + builder.forcedBreak(); + } token("}", plusTwo); builder.close(); } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.input new file mode 100644 index 000000000..52d3c1226 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.input @@ -0,0 +1,3 @@ +class SemicolonInClass { + ; +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.output new file mode 100644 index 000000000..52d3c1226 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SemicolonInClass.output @@ -0,0 +1,3 @@ +class SemicolonInClass { + ; +} From 0b384e32b39117a8da2829d8543e7474338ddebe Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Thu, 30 May 2024 08:56:36 -0700 Subject: [PATCH 06/13] Prepare g-j-f for some internal javac API changes * Refactor comment handling in tokenization to avoid extending javac's internal Comment. This prepares for upcoming JDK changes to the Comment API, which break the current subclass approach by changing the return type of `Comment#getPos`, see also similar changes to Error Prone in unknown commit * `JCCompilationUnit#getImports` no longer returns `JCImport`, as part of the changes for modular imports. Supporting modular imports is future work, this change just avoids a crash formatting the existing syntax on the latest JDK versions. PiperOrigin-RevId: 638669955 --- .../googlejavaformat/java/JavacTokens.java | 77 ++++++++----------- .../java/RemoveUnusedImports.java | 8 +- 2 files changed, 36 insertions(+), 49 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java index 6daac33b9..793c6220c 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavacTokens.java @@ -15,9 +15,9 @@ package com.google.googlejavaformat.java; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.toImmutableList; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; import com.sun.tools.javac.parser.JavaTokenizer; import com.sun.tools.javac.parser.Scanner; import com.sun.tools.javac.parser.ScannerFactory; @@ -25,8 +25,9 @@ import com.sun.tools.javac.parser.Tokens.Comment.CommentStyle; import com.sun.tools.javac.parser.Tokens.Token; import com.sun.tools.javac.parser.Tokens.TokenKind; -import com.sun.tools.javac.parser.UnicodeReader; import com.sun.tools.javac.util.Context; +import java.util.HashMap; +import java.util.Map; import java.util.Set; /** A wrapper around javac's lexer. */ @@ -79,8 +80,8 @@ public static ImmutableList getTokens( } ScannerFactory fac = ScannerFactory.instance(context); char[] buffer = (source + EOF_COMMENT).toCharArray(); - Scanner scanner = - new AccessibleScanner(fac, new CommentSavingTokenizer(fac, buffer, buffer.length)); + CommentSavingTokenizer tokenizer = new CommentSavingTokenizer(fac, buffer, buffer.length); + Scanner scanner = new AccessibleScanner(fac, tokenizer); ImmutableList.Builder tokens = ImmutableList.builder(); int end = source.length(); int last = 0; @@ -88,7 +89,7 @@ public static ImmutableList getTokens( scanner.nextToken(); Token t = scanner.token(); if (t.comments != null) { - for (Comment c : Lists.reverse(t.comments)) { + for (CommentWithTextAndPosition c : getComments(t, tokenizer.comments())) { if (last < c.getSourcePos(0)) { tokens.add(new RawTok(null, null, last, c.getSourcePos(0))); } @@ -120,17 +121,36 @@ public static ImmutableList getTokens( return tokens.build(); } + private static ImmutableList getComments( + Token token, Map comments) { + if (token.comments == null) { + return ImmutableList.of(); + } + // javac stores the comments in reverse declaration order + return token.comments.stream().map(comments::get).collect(toImmutableList()).reverse(); + } + /** A {@link JavaTokenizer} that saves comments. */ static class CommentSavingTokenizer extends JavaTokenizer { + + private final Map comments = new HashMap<>(); + CommentSavingTokenizer(ScannerFactory fac, char[] buffer, int length) { super(fac, buffer, length); } + Map comments() { + return comments; + } + @Override protected Comment processComment(int pos, int endPos, CommentStyle style) { char[] buf = getRawCharactersReflectively(pos, endPos); - return new CommentWithTextAndPosition( - pos, endPos, new AccessibleReader(fac, buf, buf.length), style); + Comment comment = super.processComment(pos, endPos, style); + CommentWithTextAndPosition commentWithTextAndPosition = + new CommentWithTextAndPosition(pos, endPos, new String(buf)); + comments.put(comment, commentWithTextAndPosition); + return comment; } private char[] getRawCharactersReflectively(int beginIndex, int endIndex) { @@ -153,21 +173,16 @@ private char[] getRawCharactersReflectively(int beginIndex, int endIndex) { } /** A {@link Comment} that saves its text and start position. */ - static class CommentWithTextAndPosition implements Comment { + static class CommentWithTextAndPosition { private final int pos; private final int endPos; - private final AccessibleReader reader; - private final CommentStyle style; - - private String text = null; + private final String text; - public CommentWithTextAndPosition( - int pos, int endPos, AccessibleReader reader, CommentStyle style) { + public CommentWithTextAndPosition(int pos, int endPos, String text) { this.pos = pos; this.endPos = endPos; - this.reader = reader; - this.style = style; + this.text = text; } /** @@ -176,7 +191,6 @@ public CommentWithTextAndPosition( *

The handling of javadoc comments in javac has more logic to skip over leading whitespace * and '*' characters when indexing into doc comments, but we don't need any of that. */ - @Override public int getSourcePos(int index) { checkArgument( 0 <= index && index < (endPos - pos), @@ -186,49 +200,22 @@ public int getSourcePos(int index) { return pos + index; } - @Override - public CommentStyle getStyle() { - return style; - } - - @Override public String getText() { - String text = this.text; - if (text == null) { - this.text = text = new String(reader.getRawCharacters()); - } return text; } - /** - * We don't care about {@code @deprecated} javadoc tags (see the DepAnn check). - * - * @return false - */ - @Override - public boolean isDeprecated() { - return false; - } - @Override public String toString() { return String.format("Comment: '%s'", getText()); } } - // Scanner(ScannerFactory, JavaTokenizer) is package-private + // Scanner(ScannerFactory, JavaTokenizer) is protected static class AccessibleScanner extends Scanner { protected AccessibleScanner(ScannerFactory fac, JavaTokenizer tokenizer) { super(fac, tokenizer); } } - // UnicodeReader(ScannerFactory, char[], int) is package-private - static class AccessibleReader extends UnicodeReader { - protected AccessibleReader(ScannerFactory fac, char[] buffer, int length) { - super(fac, buffer, length); - } - } - private JavacTokens() {} } diff --git a/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java index a0fc2f54a..8c3cae319 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java +++ b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java @@ -273,7 +273,7 @@ private static RangeMap buildReplacements( Set usedNames, Multimap> usedInJavadoc) { RangeMap replacements = TreeRangeMap.create(); - for (JCImport importTree : unit.getImports()) { + for (JCTree importTree : unit.getImports()) { String simpleName = getSimpleName(importTree); if (!isUnused(unit, usedNames, usedInJavadoc, importTree, simpleName)) { continue; @@ -291,7 +291,7 @@ private static RangeMap buildReplacements( return replacements; } - private static String getSimpleName(JCImport importTree) { + private static String getSimpleName(JCTree importTree) { return getQualifiedIdentifier(importTree).getIdentifier().toString(); } @@ -299,7 +299,7 @@ private static boolean isUnused( JCCompilationUnit unit, Set usedNames, Multimap> usedInJavadoc, - JCImport importTree, + JCTree importTree, String simpleName) { JCFieldAccess qualifiedIdentifier = getQualifiedIdentifier(importTree); String qualifier = qualifiedIdentifier.getExpression().toString(); @@ -322,7 +322,7 @@ private static boolean isUnused( return true; } - private static JCFieldAccess getQualifiedIdentifier(JCImport importTree) { + private static JCFieldAccess getQualifiedIdentifier(JCTree importTree) { // Use reflection because the return type is JCTree in some versions and JCFieldAccess in others try { return (JCFieldAccess) JCImport.class.getMethod("getQualifiedIdentifier").invoke(importTree); From db08589462600b26b69a2418b99d1de29026dcc9 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Fri, 31 May 2024 09:38:09 -0700 Subject: [PATCH 07/13] Automatic code cleanup. PiperOrigin-RevId: 639062534 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 152fdfde2..05a8fa9c5 100644 --- a/pom.xml +++ b/pom.xml @@ -89,7 +89,7 @@ 32.1.3-jre 1.4.0 0.3.0 - 2.16 + 2.28.0 1.9 1.0.1 3.6.3 From 7fd9300499228717bcfee75ff4f836507df832ee Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 5 Jun 2024 08:29:51 -0700 Subject: [PATCH 08/13] 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 --- .../java/ModifierOrderer.java | 142 ++++++++++++++---- .../java/FormatterIntegrationTest.java | 2 +- .../java/ModifierOrdererTest.java | 8 + .../java/testdata/Sealed.input | 6 + .../java/testdata/Sealed.output | 9 ++ 5 files changed, 140 insertions(+), 27 deletions(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.output 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 e14b29036..1b26f7d69 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ModifierOrderer.java @@ -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; @@ -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 { @@ -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 { + private final ImmutableList tokens; + private final Modifier modifier; + + static ModifierTokens create(ImmutableList tokens) { + return new ModifierTokens(tokens, asModifier(tokens)); + } + + static ModifierTokens empty() { + return new ModifierTokens(ImmutableList.of(), null); + } + + ModifierTokens(ImmutableList tokens, Modifier modifier) { + this.tokens = tokens; + this.modifier = modifier; + } + + boolean isEmpty() { + return tokens.isEmpty() || modifier == null; + } + + Modifier modifier() { + return modifier; + } + + ImmutableList 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 getToksBefore() { + return first().getToksBefore(); + } + + ImmutableList 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. @@ -57,43 +125,37 @@ static JavaInput reorderModifiers(JavaInput javaInput, Collection Iterator it = javaInput.getTokens().iterator(); TreeRangeMap 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 modifierTokens = new ArrayList<>(); - List mods = new ArrayList<>(); + List 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 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()); } } @@ -109,11 +171,41 @@ private static void addTrivia(StringBuilder replacement, ImmutableList it) { + Token token = it.next(); + ImmutableList.Builder 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 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) { @@ -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: 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 4a4cedc82..c7142b793 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,7 @@ public class FormatterIntegrationTest { ImmutableMultimap.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, 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..0f01e9d3f 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/ModifierOrdererTest.java @@ -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; @@ -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"); + } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.input new file mode 100644 index 000000000..f19165843 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.input @@ -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 {} +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.output new file mode 100644 index 000000000..4bafc6399 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/Sealed.output @@ -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 {} +} From 3affd633ce2e798051defe2e09090b46c0a49098 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Mon, 17 Jun 2024 16:05:35 -0700 Subject: [PATCH 09/13] Adjust indentation of line comments inside expression switches For statement switches there's some ambiguity about whether a comment documents the previous or next case: ``` case 1: // case 1 falls through to 2 case 2: doSomething() ``` ``` // this is information about case 1 case 1: // this is information about case 2 case 2: doSomething() ``` For expression switches there is no fall through, so assume that a line comments before a case label always apply to the case label after it. PiperOrigin-RevId: 644163145 --- .../java/java17/Java17InputAstVisitor.java | 6 ++-- .../java/FormatterIntegrationTest.java | 11 ++++++- .../java/testdata/SwitchComment.input | 31 +++++++++++++++++++ .../java/testdata/SwitchComment.output | 31 +++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.input create mode 100644 core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output diff --git a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java index e502f49a1..486324fb7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java @@ -19,6 +19,7 @@ import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; +import com.google.googlejavaformat.Indent; import com.google.googlejavaformat.OpsBuilder; import com.google.googlejavaformat.OpsBuilder.BlankLineWanted; import com.google.googlejavaformat.java.JavaInputAstVisitor; @@ -232,10 +233,11 @@ public Void visitCase(CaseTree node, Void unused) { && !node.getBody().getKind().equals(Tree.Kind.BLOCK) ? plusFour : ZERO); + Indent commentIndent = node.getCaseKind().equals(CaseTree.CaseKind.RULE) ? ZERO : plusTwo; if (isDefault) { - token("default", plusTwo); + token("default", commentIndent); } else { - token("case", plusTwo); + token("case", commentIndent); builder.open(labels.size() > 1 ? plusFour : ZERO); builder.space(); boolean afterFirstToken = false; 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 c7142b793..3e4e175e6 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java @@ -48,7 +48,16 @@ public class FormatterIntegrationTest { private static final ImmutableMultimap VERSIONED_TESTS = ImmutableMultimap.builder() - .putAll(14, "I477", "Records", "RSLs", "Var", "ExpressionSwitch", "I574", "I594") + .putAll( + 14, + "I477", + "Records", + "RSLs", + "Var", + "ExpressionSwitch", + "I574", + "I594", + "SwitchComment") .putAll(15, "I603") .putAll(16, "I588", "Sealed") .putAll(17, "I683", "I684", "I696") diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.input new file mode 100644 index 000000000..f43acd351 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.input @@ -0,0 +1,31 @@ +class T { + void f(String v) { + int x = + switch (v) { + // this is a line comment about "zero" + case "zero" -> 0; + case "one" -> + // this is a line comment about "one" + 1; + case "two" -> // this is a line comment about "two" + 2; + default -> -1; + }; + } + + void g(String v) { + int x = + switch (v) { + // this is a line comment about "zero" + case "zero": + return 0; + case "one": + // this is a line comment about "one" + return 1; + case "two": // this is a line comment about "two" + return 2; + default: + return -1; + }; + } +} \ No newline at end of file diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output new file mode 100644 index 000000000..ff4753b1e --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output @@ -0,0 +1,31 @@ +class T { + void f(String v) { + int x = + switch (v) { + // this is a line comment about "zero" + case "zero" -> 0; + case "one" -> + // this is a line comment about "one" + 1; + case "two" -> // this is a line comment about "two" + 2; + default -> -1; + }; + } + + void g(String v) { + int x = + switch (v) { + // this is a line comment about "zero" + case "zero": + return 0; + case "one": + // this is a line comment about "one" + return 1; + case "two": // this is a line comment about "two" + return 2; + default: + return -1; + }; + } +} From f7543b2a7d3b9c5b8214b33e8762e9550f5ab20f Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Mon, 17 Jun 2024 17:53:11 -0700 Subject: [PATCH 10/13] Adjust line comment indentation inside statement switches Follow-up to https://github.com/google/google-java-format/commit/3affd633ce2e798051defe2e09090b46c0a49098 Fixes https://github.com/google/google-java-format/issues/876 PiperOrigin-RevId: 644190687 --- .../googlejavaformat/java/java17/Java17InputAstVisitor.java | 6 ++---- .../googlejavaformat/java/testdata/SwitchComment.output | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java index 486324fb7..a0037edb7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/java17/Java17InputAstVisitor.java @@ -19,7 +19,6 @@ import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; -import com.google.googlejavaformat.Indent; import com.google.googlejavaformat.OpsBuilder; import com.google.googlejavaformat.OpsBuilder.BlankLineWanted; import com.google.googlejavaformat.java.JavaInputAstVisitor; @@ -233,11 +232,10 @@ public Void visitCase(CaseTree node, Void unused) { && !node.getBody().getKind().equals(Tree.Kind.BLOCK) ? plusFour : ZERO); - Indent commentIndent = node.getCaseKind().equals(CaseTree.CaseKind.RULE) ? ZERO : plusTwo; if (isDefault) { - token("default", commentIndent); + token("default", ZERO); } else { - token("case", commentIndent); + token("case", ZERO); builder.open(labels.size() > 1 ? plusFour : ZERO); builder.space(); boolean afterFirstToken = false; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output index ff4753b1e..f1afa6484 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/SwitchComment.output @@ -16,7 +16,7 @@ class T { void g(String v) { int x = switch (v) { - // this is a line comment about "zero" + // this is a line comment about "zero" case "zero": return 0; case "one": From 1296f1d90aef9bb96b2c859a834eb98dc7fb1d66 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 17 Jul 2024 12:56:57 -0700 Subject: [PATCH 11/13] Use JSpecify 1.0! PiperOrigin-RevId: 653332912 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 05a8fa9c5..96e613a0a 100644 --- a/pom.xml +++ b/pom.xml @@ -88,7 +88,7 @@ 1.8 32.1.3-jre 1.4.0 - 0.3.0 + 1.0.0 2.28.0 1.9 1.0.1 From de57db592b7e60cc4bab87a2e3f590d44e346675 Mon Sep 17 00:00:00 2001 From: google-java-format Team Date: Mon, 29 Jul 2024 11:13:54 -0700 Subject: [PATCH 12/13] Add customization for google-java-format arguments Setting google-java-format-arguments to `'("--aosp")` makes it easy to format on Android java files. PiperOrigin-RevId: 657263779 --- core/src/main/scripts/google-java-format.el | 28 +++++++++++++-------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/core/src/main/scripts/google-java-format.el b/core/src/main/scripts/google-java-format.el index f269ab361..5df8a1396 100644 --- a/core/src/main/scripts/google-java-format.el +++ b/core/src/main/scripts/google-java-format.el @@ -49,6 +49,13 @@ A string containing the name or the full path of the executable." :type '(file :must-match t :match (lambda (widget file) (file-executable-p file))) :risky t) +(defcustom google-java-format-arguments + '() + "Arguments to pass into google-java-format-executable" + :group 'google-java-format + :type '(repeat string) + :risky t) + ;;;###autoload (defun google-java-format-region (start end) "Use google-java-format to format the code between START and END. @@ -62,16 +69,17 @@ there is no region, then formats the current line." (temp-buffer (generate-new-buffer " *google-java-format-temp*")) (stderr-file (make-temp-file "google-java-format"))) (unwind-protect - (let ((status (call-process-region - ;; Note that emacs character positions are 1-indexed, - ;; and google-java-format is 0-indexed, so we have to - ;; subtract 1 from START to line it up correctly. - (point-min) (point-max) - google-java-format-executable - nil (list temp-buffer stderr-file) t - "--offset" (number-to-string (1- start)) - "--length" (number-to-string (- end start)) - "-")) + (let ((status (apply #'call-process-region + ;; Note that emacs character positions are 1-indexed, + ;; and google-java-format is 0-indexed, so we have to + ;; subtract 1 from START to line it up correctly. + (point-min) (point-max) + google-java-format-executable + nil (list temp-buffer stderr-file) t + (append google-java-format-arguments + `("--offset" ,(number-to-string (1- start)) + "--length" ,(number-to-string (- end start)) + "-")))) (stderr (with-temp-buffer (insert-file-contents stderr-file) From f1f68745375921b898a3b9f588ca94528b374e75 Mon Sep 17 00:00:00 2001 From: cushon Date: Tue, 30 Jul 2024 20:39:18 +0000 Subject: [PATCH 13/13] Release google-java-format 1.23.0 --- core/pom.xml | 2 +- eclipse_plugin/META-INF/MANIFEST.MF | 2 +- eclipse_plugin/pom.xml | 2 +- pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index c3753a150..03c4fe866 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-parent - HEAD-SNAPSHOT + 1.23.0 google-java-format diff --git a/eclipse_plugin/META-INF/MANIFEST.MF b/eclipse_plugin/META-INF/MANIFEST.MF index 913245393..12c9b58f2 100644 --- a/eclipse_plugin/META-INF/MANIFEST.MF +++ b/eclipse_plugin/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Bundle-Name: google-java-format Bundle-SymbolicName: google-java-format-eclipse-plugin;singleton:=true Bundle-Vendor: Google -Bundle-Version: 1.13.0 +Bundle-Version: 1.23.0 Bundle-RequiredExecutionEnvironment: JavaSE-11 Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0", org.eclipse.jface, diff --git a/eclipse_plugin/pom.xml b/eclipse_plugin/pom.xml index b2c6e368a..0efe5b90a 100644 --- a/eclipse_plugin/pom.xml +++ b/eclipse_plugin/pom.xml @@ -22,7 +22,7 @@ com.google.googlejavaformat google-java-format-eclipse-plugin eclipse-plugin - 1.13.0 + 1.23.0 Google Java Format Plugin for Eclipse 4.5+ diff --git a/pom.xml b/pom.xml index 96e613a0a..b300a328f 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ com.google.googlejavaformat google-java-format-parent pom - HEAD-SNAPSHOT + 1.23.0 core