From 31e51b0b6d20eedc3d0c18c6d997d33d55e78974 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 18:00:30 +0300 Subject: [PATCH 1/9] Run the tests on JDK 25, 26 and 27 as well The formatter parses with the javac of the JDK it runs on, so a change can pass on 21 and break on a newer JDK. A new `jdk` job runs the tests of `build` on 25, 26 and 27, in parallel with the other jobs; nothing waits for it. -PjavaRuntime sets baseline's javaVersions.runtime, which moves only the test JVMs: the code is still compiled for Java 21 by JDK 21. The IntelliJ plugin's tests keep the runtime of the IDE they start. `build` keeps JDK 21 and its name, which the ruleset on main requires. --- .github/workflows/ci.yml | 43 ++++++++++++++++++++++++++++++++++++++++ README.md | 4 ++++ build.gradle | 4 +++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c62b6298..bc984d182 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,49 @@ jobs: annotate_only: true job_summary: true + # The tests of `build` on the newer JDKs, in parallel with everything else: nothing waits for these + # jobs and nothing they build is kept. The code is still compiled for Java 21 by JDK 21, as in the + # release; -PjavaRuntime moves only the test JVMs, and with them the javac whose internals the + # formatter parses with. The IntelliJ plugin's tests stay on the runtime of the IDE they start. + # JDK 21 itself is `build`, the check the main branch requires. + jdk: + name: build (JDK ${{ matrix.jdk }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + jdk: [25, 26, 27] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Install JDK ${{ matrix.jdk }} for the tests + id: test-jdk + uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6 # v6.0.1 + with: + distribution: temurin + java-version: ${{ matrix.jdk }} + + # Installed last, so that it is the JAVA_HOME Gradle runs and compiles on. + - name: Install JDK 21 + uses: actions/setup-java@de7274f081f381c8f8158605e0321c36c376e2e6 # v6.0.1 + with: + distribution: temurin + java-version: '21' + + # Gradle does not look into the runner's tool cache, so it is told where the test JDK is. + - name: Build + run: >- + ./gradlew test -PjavaRuntime=${{ matrix.jdk }} + -Porg.gradle.java.installations.paths=${{ steps.test-jdk.outputs.path }} + + - name: Publish Test Report + uses: mikepenz/action-junit-report@a9170d5795813c01ab4901ffb045b52bab4ab09d # v6.5.0 + if: ${{ !cancelled() }} + with: + report_paths: '**/build/test-results/test/*.xml' + annotate_only: true + job_summary: true + # One explicit job per target, so every binary we ship is traceable to a named run. native: name: native (${{ matrix.platform }}) diff --git a/README.md b/README.md index 1be167feb..ad4eef38c 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,10 @@ mise trust && mise install ./gradlew test # what the CI build job runs ``` +`-PjavaRuntime=25` runs the same tests on JDK 25, as CI's `jdk` jobs do for 25, 26 and 27. The code +is compiled for Java 21 either way. Gradle has to find that JDK: installed with mise, or named with +`-Porg.gradle.java.installations.paths=/path/to/jdk`. + Nothing inside the build downloads a JDK. `gradle.properties` turns toolchain auto-download off and reads the installations from `JDK21_HOME` and `GRAALVM_HOME`, so a missing JDK is an error you can read rather than a silent download. diff --git a/build.gradle b/build.gradle index a73a45132..2093235e1 100644 --- a/build.gradle +++ b/build.gradle @@ -129,5 +129,7 @@ subprojects { javaVersions { libraryTarget = 21 - runtime = 21 + // The JDK the tests run on, and with it the javac whose internals the formatter parses with. The code is + // compiled for 21 whatever this says. CI's `jdk` jobs pass -PjavaRuntime=25 and so on. + runtime = providers.gradleProperty('javaRuntime').getOrElse('21') } From 4a89e1e892e543f6897a2489a38d9b7b64e435ec Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Wed, 16 Sep 2026 00:41:57 +0200 Subject: [PATCH 2/9] Support JDK 27 end positions (JDK-8372948) JDK-8372948 removed EndPosTable and JCCompilationUnit.endPositions and re-ordered the four-argument ParserFactory.newParser. Trees now resolves the end-position API once through a method handle and all three call sites go through it; Trees.newParser passes the parser flags in the order each JDK generation expects. From palantir/palantir-java-format#1786 by Sylwester Lachiewicz, commit 81e3fff463, without its changelog entry. --- .../palantir/javaformat/java/Formatter.java | 4 +- .../javaformat/java/RemoveUnusedImports.java | 2 +- .../javaformat/java/StringWrapper.java | 2 +- .../com/palantir/javaformat/java/Trees.java | 77 ++++++++++++++++++- 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java b/open-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java index ef8dab408..b6b78bfda 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java @@ -184,8 +184,8 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept }; Log.instance(context).useSource(source); ParserFactory parserFactory = ParserFactory.instance(context); - JavacParser parser = parserFactory.newParser( - sourceText, /*keepDocComments=*/ true, /*keepEndPos=*/ true, /*keepLineMap=*/ true); + JavacParser parser = + Trees.newParser(parserFactory, sourceText, /*keepDocComments=*/ true, /*keepLineMap=*/ true); unit = parser.parseCompilationUnit(); unit.sourcefile = source; diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java b/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java index a66b6b559..c5e3be2a1 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java @@ -232,7 +232,7 @@ private static RangeMap buildReplacements( continue; } // delete the import - int endPosition = importTree.getEndPosition(unit.endPositions); + int endPosition = Trees.getEndPosition(importTree, unit); endPosition = Math.max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition); String sep = Newlines.guessLineSeparator(contents); if (endPosition + sep.length() < contents.length() diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java b/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java index 39f97316f..429d8a4eb 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java @@ -576,7 +576,7 @@ private static boolean noComments(String input, JCTree.JCCompilationUnit unit, T CharMatcher.whitespace().or(CharMatcher.anyOf("\"+")); private static int getEndPosition(JCTree.JCCompilationUnit unit, Tree tree) { - return ((JCTree) tree).getEndPosition(unit.endPositions); + return Trees.getEndPosition(tree, unit); } private static int getStartPosition(Tree tree) { diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/Trees.java b/open-java-format/src/main/java/com/palantir/javaformat/java/Trees.java index ead97b908..501561fc6 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/Trees.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/Trees.java @@ -23,11 +23,19 @@ import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; +import com.sun.tools.javac.parser.JavacParser; +import com.sun.tools.javac.parser.ParserFactory; import com.sun.tools.javac.tree.JCTree; +import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; import com.sun.tools.javac.tree.Pretty; import com.sun.tools.javac.tree.TreeInfo; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.invoke.VarHandle; +import javax.annotation.Nullable; import javax.lang.model.element.Name; /** Utilities for working with {@link Tree}s. */ @@ -44,8 +52,73 @@ static int getStartPosition(Tree expression) { /** Returns the source end position of the node. */ static int getEndPosition(Tree expression, TreePath path) { - return ((JCTree) expression) - .getEndPosition(((JCTree.JCCompilationUnit) path.getCompilationUnit()).endPositions); + return getEndPosition(expression, (JCCompilationUnit) path.getCompilationUnit()); + } + + /** Returns the source end position of the node. */ + static int getEndPosition(Tree tree, JCCompilationUnit unit) { + try { + return (int) GET_END_POSITION.invokeExact((JCTree) tree, unit); + } catch (RuntimeException | Error e) { + throw e; + } catch (Throwable e) { + throw new IllegalStateException(e); + } + } + + /** + * Creates a parser that records end positions. JDK-8372948 (JDK 27) dropped the {@code keepEndPos} argument + * from {@link ParserFactory#newParser}, so the remaining four-argument overload now reads + * {@code (input, keepDocComments, keepLineMap, parseModuleInfo)}. + */ + static JavacParser newParser( + ParserFactory parserFactory, CharSequence input, boolean keepDocComments, boolean keepLineMap) { + if (END_POS_TABLE_CLASS != null) { + return parserFactory.newParser(input, keepDocComments, /* keepEndPos= */ true, keepLineMap); + } + // The last argument is parseModuleInfo on these JDKs. + return parserFactory.newParser(input, keepDocComments, keepLineMap, false); + } + + /** + * {@code com.sun.tools.javac.tree.EndPosTable}, or null on JDKs that store end positions directly in the tree + * (JDK-8372948, JDK 27 and later). + */ + @Nullable + private static final Class END_POS_TABLE_CLASS = endPosTableClass(); + + /** {@code (JCTree, JCCompilationUnit) -> int}, bound to whichever end position API this JDK has. */ + private static final MethodHandle GET_END_POSITION = getEndPositionHandle(); + + @Nullable + private static Class endPosTableClass() { + try { + return Class.forName("com.sun.tools.javac.tree.EndPosTable"); + } catch (ClassNotFoundException e) { + return null; + } + } + + private static MethodHandle getEndPositionHandle() { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + try { + if (END_POS_TABLE_CLASS == null) { + // (tree, unit) -> tree.getEndPosition() + return MethodHandles.dropArguments( + lookup.findVirtual(JCTree.class, "getEndPosition", MethodType.methodType(int.class)), + 1, + JCCompilationUnit.class); + } + // (tree, unit) -> tree.getEndPosition(unit.endPositions) + return MethodHandles.filterArguments( + lookup.findVirtual( + JCTree.class, "getEndPosition", MethodType.methodType(int.class, END_POS_TABLE_CLASS)), + 1, + lookup.findVarHandle(JCCompilationUnit.class, "endPositions", END_POS_TABLE_CLASS) + .toMethodHandle(VarHandle.AccessMode.GET)); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException("Unsupported javac end position API", e); + } } /** Returns the source text for the node. */ From b2e270b69f084b5ddeab942e9f383a6b1ad11bb0 Mon Sep 17 00:00:00 2001 From: asm0dey Date: Fri, 11 Sep 2026 21:24:36 +0200 Subject: [PATCH 3/9] Format module imports, compact source files and unnamed patterns From palantir/palantir-java-format#1707 by Pavel Finkelshtein (asm0dey), its 15 commits up to 53bea7f8c7 squashed into one. The formatter failed on `import module ...;` (JEP 511), on compact source files with an instance main (JEP 512) and on unnamed patterns such as `case Box(_, _)` (JEP 456). ImportOrderer now renders each import from its tokens instead of rebuilding it, so whitespace and comments inside an import declaration are accepted. Adapted while bringing it over: - paths moved from palantir-java-format/ to open-java-format/; - left out: the changelog entry, the gradle/jdks/25 files, and the build changes for palantir's gradle-jdks with the testJdk23 and testJdk25 tasks. Here the JDK 25 and later runs come from CI's jdk jobs; - left out: the README changes, written for upstream's README; - RemoveUnusedImports takes the end position through Trees.getEndPosition, from palantir/palantir-java-format#1786. Co-authored-by: Claude Opus 5 (1M context) --- .../native-image/reachability-metadata.json | 9 + .../javaformat/java/ImportOrderer.java | 180 +++++++++++++--- .../javaformat/java/JavaInputAstVisitor.java | 71 ++++++- .../javaformat/java/RemoveUnusedImports.java | 25 ++- .../java/java14/Java14InputAstVisitor.java | 17 -- .../java/java21/Java21InputAstVisitor.java | 16 ++ .../javaformat/java/AospImportStyleTest.java | 77 +++++++ .../javaformat/java/FileBasedTests.java | 7 +- .../javaformat/java/FormatterVersionTest.java | 36 ++++ .../java/GoogleImportStyleTest.java | 192 +++++++++++++++++- .../javaformat/java/ModuleImportTest.java | 123 +++++++++++ .../java/RemoveUnusedImportsTest.java | 20 ++ .../java/testdata/CompactSource.input | 15 ++ .../java/testdata/CompactSource.output | 15 ++ .../java/testdata/FlexibleConstructor.input | 12 ++ .../java/testdata/FlexibleConstructor.output | 12 ++ .../java/testdata/MarkdownDoc.input | 10 + .../java/testdata/MarkdownDoc.output | 10 + .../java/testdata/ModuleImport.input | 8 + .../java/testdata/ModuleImport.output | 8 + .../java/testdata/UnnamedPattern.input | 34 ++++ .../java/testdata/UnnamedPattern.output | 35 ++++ 22 files changed, 873 insertions(+), 59 deletions(-) create mode 100644 open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java create mode 100644 open-java-format/src/test/java/com/palantir/javaformat/java/ModuleImportTest.java create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.input create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.output create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.input create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.output create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.input create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.output create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.input create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.output create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.input create mode 100644 open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.output diff --git a/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json b/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json index c40f18794..956ece11c 100644 --- a/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json +++ b/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json @@ -131,6 +131,15 @@ } ] }, + { + "type": "com.sun.source.tree.ImportTree", + "methods": [ + { + "name": "isModule", + "parameterTypes": [] + } + ] + }, { "type": "com.sun.tools.javac.parser.JavaTokenizer", "fields": [ diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/ImportOrderer.java b/open-java-format/src/main/java/com/palantir/javaformat/java/ImportOrderer.java index df69a51f0..58545510d 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/ImportOrderer.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/ImportOrderer.java @@ -123,27 +123,39 @@ private String reorderImports() throws FormatterException { /** * A {@link Comparator} that orders {@link Import}s by Google Style, defined at * https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing. + * + *

Google Style does not use module imports ({@code import module foo.bar;}, JEP 511); when present, they + * sort between static and non-static type imports, matching google-java-format. */ - private static final Comparator GOOGLE_IMPORT_COMPARATOR = - Comparator.comparing(Import::isStatic, trueFirst()).thenComparing(Import::imported); + private static final Comparator GOOGLE_IMPORT_COMPARATOR = Comparator.comparing( + Import::isStatic, trueFirst()) + .thenComparing(Import::isModule, trueFirst()) + .thenComparing(Import::imported) + // Imports that compare equal collapse into one, so two that are written differently -- one of them + // carrying a comment, say -- must not compare equal, or that comment disappears with it. + .thenComparing(Import::declaration); /** * A {@link Comparator} that orders {@link Import}s by AOSP Style, defined at * https://source.android.com/setup/contribute/code-style#order-import-statements and implemented in IntelliJ at * https://android.googlesource.com/platform/development/+/master/ide/intellij/codestyles/AndroidStyle.xml. + * + *

As with {@link #GOOGLE_IMPORT_COMPARATOR}, module imports sort after static imports. */ private static final Comparator AOSP_IMPORT_COMPARATOR = Comparator.comparing(Import::isStatic, trueFirst()) + .thenComparing(Import::isModule, trueFirst()) .thenComparing(Import::isAndroid, trueFirst()) .thenComparing(Import::isThirdParty, trueFirst()) .thenComparing(Import::isJava, trueFirst()) - .thenComparing(Import::imported); + .thenComparing(Import::imported) + .thenComparing(Import::declaration); /** * Determines whether to insert a blank line between the {@code prev} and {@code curr} {@link Import}s based on * Google style. */ private static boolean shouldInsertBlankLineGoogle(Import prev, Import curr) { - return prev.isStatic() && !curr.isStatic(); + return prev.isStatic() != curr.isStatic() || prev.isModule() != curr.isModule(); } /** @@ -151,7 +163,7 @@ private static boolean shouldInsertBlankLineGoogle(Import prev, Import curr) { * style. */ private static boolean shouldInsertBlankLineAosp(Import prev, Import curr) { - if (prev.isStatic() && !curr.isStatic()) { + if (prev.isStatic() != curr.isStatic() || prev.isModule() != curr.isModule()) { return true; } // insert blank line between "com.android" from "com.anythingelse" @@ -186,12 +198,24 @@ private ImportOrderer(String text, ImmutableList toks, Style style) { class Import { private final String imported; private final boolean isStatic; + private final boolean isModule; private final String trailing; + private final String declaration; - Import(String imported, String trailing, boolean isStatic) { + Import(String imported, String trailing, boolean isStatic, boolean isModule, String declaration) { this.imported = imported; this.trailing = trailing; this.isStatic = isStatic; + this.isModule = isModule; + this.declaration = declaration; + } + + /** + * The declaration as it will be written: the {@code import} keyword through the semicolon, with whitespace + * normalized and any comments left in the slot the author wrote them in. + */ + String declaration() { + return declaration; } /** The name being imported, for example {@code java.util.List}. */ @@ -204,6 +228,11 @@ boolean isStatic() { return isStatic; } + /** True if this is {@code import module} (JEP 511). */ + boolean isModule() { + return isModule; + } + /** The top-level package of the import. */ String topLevel() { return DOT_SPLITTER.split(imported()).iterator().next(); @@ -245,11 +274,7 @@ public boolean isThirdParty() { @Override public String toString() { StringBuilder sb = new StringBuilder(); - sb.append("import "); - if (isStatic()) { - sb.append("static "); - } - sb.append(imported()).append(';'); + sb.append(declaration()); if (trailing().trim().isEmpty()) { sb.append(lineSeparator); } else { @@ -259,6 +284,42 @@ public String toString() { } } + /** + * Renders one import declaration from the toks it is made of. Whitespace between the toks is normalized, since the + * style guide puts one import on a line of its own, but a comment stays in the slot the author wrote it in rather + * than being moved to wherever the rendered declaration can accommodate it. + */ + private final class Declaration { + private final StringBuilder text = new StringBuilder(); + private boolean atLineStart = false; + + /** Appends one tok: a keyword, an identifier, {@code .}, {@code *}, {@code ;}, or a comment. */ + void append(String piece) { + if (text.length() > 0 && !atLineStart && needsSpaceBefore(piece)) { + text.append(' '); + } + text.append(piece); + atLineStart = false; + if (piece.startsWith("//")) { + // A // comment swallows the rest of its line, so the declaration continues on the next one. + text.append(lineSeparator); + atLineStart = true; + } + } + + private boolean needsSpaceBefore(String piece) { + if (piece.equals(".") || piece.equals(";") || piece.equals("*")) { + return false; + } + return text.charAt(text.length() - 1) != '.'; + } + + @Override + public String toString() { + return text.toString(); + } + } + private String tokString(int start, int end) { StringBuilder sb = new StringBuilder(); for (int i = start; i < end; i++) { @@ -282,9 +343,10 @@ private static class ImportsAndIndex { * * 

{@code
      *  -> ( | )*
-     *  -> "import"  ("static" )?
-     *     ("." )* ("." "*")? ? ";"
+     *  -> "import"  (("static" | "module") )?
+     *     ("." )* ("." "*")? ? ";"
      *    ? ? ( )*
+     *  -> ( |  | )+
      * }
* * @param i the index to start parsing at. @@ -298,35 +360,43 @@ private ImportsAndIndex scanImports(int i) throws FormatterException { // of our tests here and protects us from running off the end of the toks list. Since it is // zero-width it doesn't matter if we include it in our string concatenation at the end. while (i < toks.size() && tokenAt(i).equals("import")) { + Declaration declaration = new Declaration(); + declaration.append(tokenAt(i)); i++; - if (isSpaceToken(i)) { + i = skipIgnored(i, declaration); + boolean isModule = isModuleKeyword(i); + if (isModule) { + declaration.append(tokenAt(i)); i++; + i = skipIgnored(i, declaration); } - boolean isStatic = tokenAt(i).equals("static"); + boolean isStatic = !isModule && tokenAt(i).equals("static"); if (isStatic) { + declaration.append(tokenAt(i)); i++; - if (isSpaceToken(i)) { - i++; - } + i = skipIgnored(i, declaration); } if (!isIdentifierToken(i)) { throw new FormatterException("Unexpected token after import: " + tokenAt(i)); } - StringAndIndex imported = scanImported(i); + StringAndIndex imported = scanImported(i, declaration); String importedName = imported.string; i = imported.index; - if (isSpaceToken(i)) { - i++; - } + i = skipIgnored(i, declaration); if (!tokenAt(i).equals(";")) { throw new FormatterException("Expected ; after import"); } + declaration.append(";"); while (tokenAt(i).equals(";")) { // Extra semicolons are not allowed by the JLS but are accepted by javac. i++; } StringBuilder trailing = new StringBuilder(); - if (isSpaceToken(i)) { + // A block comment on the same line as the `;` trails this import; one on a later line + // belongs to whatever follows it, so only same-line toks are absorbed here. Javadoc is + // excluded: the formatter moves a javadoc comment onto a line of its own, which would + // separate the imports. + while (isSpaceToken(i) || isBlockCommentToken(i)) { trailing.append(tokenAt(i)); i++; } @@ -344,7 +414,7 @@ private ImportsAndIndex scanImports(int i) throws FormatterException { i++; } } - imports.add(new Import(importedName, trailing.toString(), isStatic)); + imports.add(new Import(importedName, trailing.toString(), isStatic, isModule, declaration.toString())); // Remember the position just after the import we just saw, before skipping blank lines. // If the next thing after the blank lines is not another import then we don't want to // include those blank lines in the text to be replaced. @@ -387,17 +457,19 @@ private static class StringAndIndex { } /** - * Scans the imported thing, the dot-separated name that comes after import [static] and before the semicolon. We - * don't allow spaces inside the dot-separated name. Wildcard imports are supported: if the input is {@code import - * java.util.*;} then the returned string will be {@code java.util.*}. + * Scans the imported thing, the dot-separated name that comes after import [static] and before the semicolon. + * Whitespace, line terminators and comments may appear between its parts, as they may anywhere else in the + * declaration; the returned name contains none of them. Wildcard imports are supported: if the input is + * {@code import java.util.*;} then the returned string will be {@code java.util.*}. * * @param start the index of the start of the identifier. If the import is {@code import java.util.List;} then this * index points to the token {@code java}. + * @param declaration collects the toks scanned, so a comment between the parts of the name keeps its place * @return the parsed import ({@code java.util.List} in the example) and the index of the first token after the * imported thing ({@code ;} in the example). * @throws FormatterException if the imported name could not be parsed. */ - private StringAndIndex scanImported(int start) throws FormatterException { + private StringAndIndex scanImported(int start, Declaration declaration) throws FormatterException { int i = start; StringBuilder imported = new StringBuilder(); // At the start of each iteration of this loop, i points to an identifier. @@ -405,14 +477,19 @@ private StringAndIndex scanImported(int start) throws FormatterException { while (true) { Preconditions.checkState(isIdentifierToken(i)); imported.append(tokenAt(i)); + declaration.append(tokenAt(i)); i++; + i = skipIgnored(i, declaration); if (!tokenAt(i).equals(".")) { return new StringAndIndex(imported.toString(), i); } imported.append('.'); + declaration.append("."); i++; + i = skipIgnored(i, declaration); if (tokenAt(i).equals("*")) { imported.append('*'); + declaration.append("*"); return new StringAndIndex(imported.toString(), i + 1); } else if (!isIdentifierToken(i)) { throw new FormatterException("Could not parse imported name, at: " + tokenAt(i)); @@ -452,6 +529,41 @@ private String tokenAt(int i) { return toks.get(i).getOriginalText(); } + /** + * Returns true if the token at {@code i} is the {@code module} contextual keyword introducing a module import + * declaration ({@code import module foo.bar;}, JEP 511), as opposed to an ordinary import whose first segment + * happens to be an identifier literally named {@code module} (for example {@code import module.Foo;}). As with + * other contextual keywords ({@code var}, {@code yield}, ...), this is disambiguated by lookahead: {@code + * module} only introduces a module import when the next token is another identifier, rather than {@code .} or + * {@code ;}. Whitespace, line terminators and comments are skipped, as they are by the parsing that follows. + */ + private boolean isModuleKeyword(int i) { + if (!tokenAt(i).equals("module")) { + return false; + } + return isIdentifierToken(skipIgnored(i + 1, new Declaration())); + } + + /** + * Skips whitespace, line terminators and comments starting at {@code i}, appending each comment to + * {@code declaration}, and returns the index of the first token that is none of those. Javadoc comments are not + * skipped: the formatter moves them onto a line of their own, which would separate the imports, so an import + * carrying one is rejected, as it was before module imports were supported. + */ + private int skipIgnored(int i, Declaration declaration) { + while (i < toks.size()) { + if (isSpaceToken(i) || isNewlineToken(i)) { + i++; + } else if (isSlashSlashCommentToken(i) || isBlockCommentToken(i)) { + declaration.append(tokenAt(i).trim()); + i++; + } else { + break; + } + } + return i; + } + private boolean isIdentifierToken(int i) { String s = tokenAt(i); return !s.isEmpty() && Character.isJavaIdentifierStart(s.codePointAt(0)); @@ -470,6 +582,18 @@ private boolean isSlashSlashCommentToken(int i) { return toks.get(i).isSlashSlashComment(); } + /** True if the tok is a {@code /* *}{@code /} comment that is not javadoc. */ + private boolean isBlockCommentToken(int i) { + return toks.get(i).isSlashStarComment() && !toks.get(i).isJavadocComment(); + } + + /** True if {@code text} ends in a line terminator. */ + private static boolean endsInNewline(CharSequence text) { + return text.length() > 0 + && Newlines.isNewline( + text.subSequence(text.length() - 1, text.length()).toString()); + } + private boolean isNewlineToken(int i) { return toks.get(i).isNewline(); } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index 3917911ee..66ba31f44 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -132,6 +132,7 @@ import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.TreeScanner; +import java.lang.reflect.Method; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; @@ -376,8 +377,15 @@ public Void visitCompilationUnit(CompilationUnitTree node, Void unused) { builder.blankLineWanted(BlankLineWanted.YES); } markForPartialFormat(); - scan(type, null); - builder.forcedBreak(); + if (isCompactSourceFile(type)) { + // JEP 512 (Java 25): a "compact source file" has top-level fields/methods and no + // explicit class declaration. javac models this by synthesizing an implicit wrapper + // class; format its member list directly, at column zero, with no enclosing braces. + addBodyDeclarations(((ClassTree) type).getMembers(), BracesOrNot.NO, FirstDeclarationsOrNot.YES, ZERO); + } else { + scan(type, null); + builder.forcedBreak(); + } first = false; dropEmptyDeclarations(); } @@ -1123,12 +1131,19 @@ public Void visitIf(IfTree node, Void unused) { return null; } + // ImportTree#isModule() (JEP 511) exists from JDK 23 on; this module compiles with a JDK 21 + // compiler, so it can't be referenced directly. + private static final Method IMPORT_TREE_IS_MODULE = maybeGetMethod(ImportTree.class, "isModule"); + @Override public Void visitImport(ImportTree node, Void unused) { sync(node); token("import"); builder.space(); - if (node.isStatic()) { + if (IMPORT_TREE_IS_MODULE != null && Boolean.TRUE.equals(invoke(IMPORT_TREE_IS_MODULE, node))) { + token("module"); + builder.space(); + } else if (node.isStatic()) { token("static"); builder.space(); } @@ -1381,6 +1396,16 @@ public Void visitAnnotatedType(AnnotatedTypeTree node, Void unused) { protected static final long RECORD = 1L << 61; + // TODO: use Flags.IMPLICIT_CLASS once this module compiles with JDK 22 or later (JDK 21 calls + // bit 19 UNNAMED_CLASS). javac sets it on the class it synthesizes for a compact source file. + private static final long IMPLICIT_CLASS = 1L << 19; + + /** Is {@code type} the implicit wrapper class javac synthesizes for a compact source file? */ + private static boolean isCompactSourceFile(Tree type) { + return type instanceof JCTree.JCClassDecl + && (((JCTree.JCClassDecl) type).mods.flags & IMPLICIT_CLASS) == IMPLICIT_CLASS; + } + @SuppressWarnings("for-rollout:NullAway") @Override public Void visitMethod(MethodTree node, Void unused) { @@ -3741,9 +3766,22 @@ private void declareMany(List fragments, Direction annotationDirec } /** Add a list of declarations. */ - @SuppressWarnings("for-rollout:NullAway") protected void addBodyDeclarations( List bodyDeclarations, BracesOrNot braces, FirstDeclarationsOrNot first0) { + addBodyDeclarations(bodyDeclarations, braces, first0, plusTwo); + } + + /** + * Add a list of declarations, indenting the member list by {@code memberIndent}. Used both for ordinary + * class/interface/enum bodies (indented by {@link #plusTwo}) and for the bare top-level member list of a + * compact source file (JEP 512), which is indented by {@link Indent.Const#ZERO}. + */ + @SuppressWarnings("for-rollout:NullAway") + private void addBodyDeclarations( + List bodyDeclarations, + BracesOrNot braces, + FirstDeclarationsOrNot first0, + Indent memberIndent) { if (bodyDeclarations.isEmpty()) { if (braces.isYes()) { builder.space(); @@ -3759,7 +3797,7 @@ protected void addBodyDeclarations( tokenBreakTrailingComment("{", plusTwo); builder.open(ZERO, BreakBehaviours.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN); } - builder.open(plusTwo); + builder.open(memberIndent); boolean first = first0.isYes(); boolean lastOneGotBlankLineBefore = false; PeekingIterator it = Iterators.peekingIterator(bodyDeclarations.iterator()); @@ -3958,6 +3996,29 @@ protected void sync(Tree node) { builder.sync(((JCTree) node).getStartPosition()); } + /** + * Returns the public no-argument method {@code name} of {@code c}, or {@code null} if the running JDK does not + * have it. Used with {@link #invoke} to reach AST accessors that are newer than the compiler this module builds + * with, such as {@code ImportTree#isModule()} (JDK 23). + */ + @SuppressWarnings("for-rollout:NullAway") + protected static Method maybeGetMethod(Class c, String name) { + try { + return c.getMethod(name); + } catch (ReflectiveOperationException e) { + return null; + } + } + + /** Invokes {@code m} on {@code target}, wrapping any {@link ReflectiveOperationException} in a runtime one. */ + protected static Object invoke(Method m, Object target) { + try { + return m.invoke(target); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e.getMessage(), e); + } + } + @Override public String toString() { return MoreObjects.toStringHelper(this).add("builder", builder).toString(); diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java b/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java index c5e3be2a1..9a021ae52 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java @@ -42,7 +42,6 @@ import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; import com.sun.tools.javac.tree.JCTree.JCFieldAccess; import com.sun.tools.javac.tree.JCTree.JCIdent; -import com.sun.tools.javac.tree.JCTree.JCImport; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.Options; import java.lang.reflect.Method; @@ -226,13 +225,16 @@ private static RangeMap buildReplacements( Set usedNames, Multimap> usedInJavadoc) { RangeMap replacements = TreeRangeMap.create(); - for (JCImport importTree : unit.getImports()) { + // From JDK 23 on, getImports() also returns JCModuleImport, which is not a JCImport, so iterate + // over their common supertype and use ImportTree, which both implement. + for (JCTree importDecl : unit.getImports()) { + ImportTree importTree = (ImportTree) importDecl; String simpleName = getSimpleName(importTree); if (!isUnused(unit, usedNames, usedInJavadoc, importTree, simpleName)) { continue; } // delete the import - int endPosition = Trees.getEndPosition(importTree, unit); + int endPosition = Trees.getEndPosition(importDecl, unit); endPosition = Math.max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition); String sep = Newlines.guessLineSeparator(contents); if (endPosition + sep.length() < contents.length() @@ -241,11 +243,21 @@ private static RangeMap buildReplacements( .equals(sep)) { endPosition += sep.length(); } - replacements.put(Range.closedOpen(importTree.getStartPosition(), endPosition), ""); + replacements.put(Range.closedOpen(importDecl.getStartPosition(), endPosition), ""); } return replacements; } + // ImportTree#isModule() (JEP 511) exists from JDK 23 on; this module compiles with a JDK 21 + // compiler, so it can't be referenced directly. Same idiom as CASE_TREE_GET_LABELS above. + private static final Method IMPORT_TREE_IS_MODULE = + JavaInputAstVisitor.maybeGetMethod(ImportTree.class, "isModule"); + + private static boolean isModuleImport(ImportTree importTree) { + return IMPORT_TREE_IS_MODULE != null + && Boolean.TRUE.equals(JavaInputAstVisitor.invoke(IMPORT_TREE_IS_MODULE, importTree)); + } + private static String getSimpleName(ImportTree importTree) { return importTree.getQualifiedIdentifier() instanceof JCIdent ? ((JCIdent) importTree.getQualifiedIdentifier()).getName().toString() @@ -260,6 +272,11 @@ private static boolean isUnused( Multimap> usedInJavadoc, ImportTree importTree, String simpleName) { + if (isModuleImport(importTree)) { + // A module import binds every exported package of the module, so this scanner can't tell + // whether it's needed - same as the `.*` wildcard imports below. Never remove it. + return false; + } String qualifier = ((JCFieldAccess) importTree.getQualifiedIdentifier()) .getExpression() .toString(); diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java index 76bb0d578..24ab861a0 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java @@ -351,23 +351,6 @@ public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) { return null; } - @SuppressWarnings("for-rollout:NullAway") - private static Method maybeGetMethod(Class c, String name) { - try { - return c.getMethod(name); - } catch (ReflectiveOperationException e) { - return null; - } - } - - private static Object invoke(Method m, Object target) { - try { - return m.invoke(target); - } catch (ReflectiveOperationException e) { - throw new RuntimeException(e.getMessage(), e); - } - } - @SuppressWarnings({"NullableProblems", "for-rollout:NullAway"}) protected ExpressionTree getGuard(final CaseTree node) { return null; diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/java21/Java21InputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/java21/Java21InputAstVisitor.java index 5f089c4e6..f6f586ee7 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/java21/Java21InputAstVisitor.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/java21/Java21InputAstVisitor.java @@ -25,6 +25,7 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.PatternCaseLabelTree; import com.sun.source.tree.PatternTree; +import com.sun.source.tree.Tree; import javax.lang.model.element.Name; /** @@ -33,10 +34,25 @@ */ @SuppressWarnings("Since21") public class Java21InputAstVisitor extends Java14InputAstVisitor { + // AnyPatternTree (the unnamed pattern `_`, JEP 456) is still a preview API on JDK 21, which this + // module compiles with, so match it by Tree.Kind name instead of by type. + private static final String ANY_PATTERN_KIND_NAME = "ANY_PATTERN"; + public Java21InputAstVisitor(OpsBuilder builder, int indentMultiplier) { super(builder, indentMultiplier); } + @Override + public Void scan(Tree tree, Void unused) { + if (tree != null && tree.getKind().name().equals(ANY_PATTERN_KIND_NAME)) { + // No sync(tree): javac records the start position one past the `_`, which makes sync() + // think a token was skipped and throw. This branch emits one leaf token and never recurses. + token("_"); + return null; + } + return super.scan(tree, null); + } + @Override protected ExpressionTree getGuard(final CaseTree node) { return node.getGuard(); diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/AospImportStyleTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/AospImportStyleTest.java index 9401b1299..f568f8d9e 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/AospImportStyleTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/AospImportStyleTest.java @@ -277,6 +277,79 @@ public static List parameters() { "public class Blim {}", }, }, + + // Module imports (JEP 511) sort between static imports and the android/third-party/java + // groups, as in google-java-format, each group separated by a blank line. + { + { + "package foo;", + "", + "import java.util.List;", + "import static android.Bar.baz;", + "import module java.desktop;", + "import android.Bar;", + "import module java.base;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import static android.Bar.baz;", + "", + "import module java.base;", + "import module java.desktop;", + "", + "import android.Bar;", + "", + "import java.util.List;", + "", + "public class Blim {}", + }, + }, + + // A module import and a non-module import that share a top level package: the blank line + // comes from the module boundary, not from the top-level-package rule. + { + { + "package foo;", + "", + "import java.util.List;", + "import module java.base;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import module java.base;", + "", + "import java.util.List;", + "", + "public class Blim {}", + }, + }, + + // Module imports sort before third-party imports, which would otherwise come first. + { + { + "package foo;", + "", + "import org.example.Bar;", + "import module java.base;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import module java.base;", + "", + "import org.example.Bar;", + "", + "public class Blim {}", + }, + }, }; ImmutableList.Builder builder = ImmutableList.builder(); Arrays.stream(inputsOutputs).forEach(input -> builder.add(ImportOrdererUtils.createRow(input))); @@ -289,6 +362,10 @@ public void reorder() throws FormatterException { String output = ImportOrderer.reorderImports(input, JavaFormatterOptions.Style.AOSP); assertWithMessage("Expected exception").that(reordered).doesNotMatch("^!!"); assertWithMessage(input).that(output).isEqualTo(reordered); + // Reordering must be a fixed point: a formatted file has to survive being formatted again. + assertWithMessage("not idempotent: %s", output) + .that(ImportOrderer.reorderImports(output, JavaFormatterOptions.Style.AOSP)) + .isEqualTo(output); } catch (FormatterException e) { if (!reordered.startsWith("!!")) { throw e; diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java b/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java index 4edb8fa12..5305d7c47 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/FileBasedTests.java @@ -59,7 +59,12 @@ public final class FileBasedTests { "SwitchUnderscore", "I880", "I1309", - "Unnamed") + "Unnamed", + "UnnamedPattern", + "CompactSource", + "MarkdownDoc", + "FlexibleConstructor") + .putAll(23, "ModuleImport") .build(); private final Class testClass; diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java new file mode 100644 index 000000000..4671a0fe2 --- /dev/null +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java @@ -0,0 +1,36 @@ +/* + * (c) Copyright 2026 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.javaformat.java; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +/** + * Guards the {@code testJdkNN} legs: without this, a leg that resolves to a JDK older than the one it asked for skips + * the {@code ModuleImport} golden and the module import tests, and still passes. + */ +public class FormatterVersionTest { + + @Test + public void runsOnTheJdkTheTestTaskAskedFor() { + String expected = System.getProperty("expectedJavaVersion"); + Assumptions.assumeTrue(expected != null, "expectedJavaVersion is set only by the testJdkNN tasks"); + assertThat(Formatter.getRuntimeVersion()).isEqualTo(Integer.parseInt(expected)); + } +} diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/GoogleImportStyleTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/GoogleImportStyleTest.java index e3b0cb412..65f0b6615 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/GoogleImportStyleTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/GoogleImportStyleTest.java @@ -368,7 +368,9 @@ public static List parameters() { "import", }, { - "!!Unexpected token after import: \n", + // The line break after `import` is skipped, so the token we report is the + // zero-width EOF tok rather than the newline. + "!!Unexpected token after import: ", } }, { @@ -403,15 +405,37 @@ public static List parameters() { "!!Imports not contiguous (perhaps a comment separates them?)", } }, + // A block comment on the same line as the `;` trails its import and travels with it. + { + { + "import com.foo.Second; /* stays with Second */", // + "import com.foo.First;", + }, + { + "import com.foo.First;", // + "import com.foo.Second; /* stays with Second */", + } + }, + // Javadoc is the exception: the formatter moves it onto a line of its own, which would + // separate the imports, so an import carrying one is still rejected. { { - "import com.foo.Second; /* no block comments after imports */", // + "import com.foo.Second; /** javadoc after an import */", // "import com.foo.First;", }, { "!!Imports not contiguous (perhaps a comment separates them?)", } }, + { + { + "import /** javadoc inside an import */ com.foo.Second;", // + "import com.foo.First;", + }, + { + "!!Unexpected token after import: /** javadoc inside an import */", + } + }, { { "import com.foo.Second;", @@ -429,13 +453,15 @@ public static List parameters() { "*/", } }, + // Whitespace may appear between the parts of a qualified name; it is normalized away. { { - "import com . foo . Second ;", // syntactically valid, but we don't support it + "import com . foo . Second ;", // "import com.foo.First;", }, { - "!!Expected ; after import", + "import com.foo.First;", // + "import com.foo.Second;", } }, { @@ -524,6 +550,160 @@ public static List parameters() { "class Test {}", } }, + + // Module imports (JEP 511) sort between static and non-static type imports, as in + // google-java-format, each group separated by a blank line. + { + { + "package foo;", + "", + "import java.util.List;", + "import static com.google.truth.Truth.assertThat;", + "import module java.desktop;", + "import module java.base;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import static com.google.truth.Truth.assertThat;", + "", + "import module java.base;", + "import module java.desktop;", + "", + "import java.util.List;", + "", + "public class Blim {}", + }, + }, + + // A module whose name sorts after a non-module import: the module still comes first, and a + // blank line separates the two groups. + { + { + "package foo;", + "", + "import java.util.List;", + "import module org.example.api;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import module org.example.api;", + "", + "import java.util.List;", + "", + "public class Blim {}", + }, + }, + + // Whitespace, line breaks and comments may appear between the tokens of an import. The + // comments are re-emitted after the semicolon so that nothing is dropped. + { + { + "package foo;", + "", + "import module /* the base module */ java.base;", + "import module", + " java.desktop;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import module /* the base module */ java.base;", + "import module java.desktop;", + "", + "public class Blim {}", + }, + }, + + // The same holds for ordinary and static imports, which had this limitation before. + { + { + "package foo;", + "", + "import /* a type */ com.foo.Second;", + "import static /* a member */ com.foo.First.first;", + "import", + " com.foo.Third;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import static /* a member */ com.foo.First.first;", + "", + "import /* a type */ com.foo.Second;", + "import com.foo.Third;", + "", + "public class Blim {}", + }, + }, + + // A comment may also sit between the parts of the name, and stays there. Only the + // whitespace around it is normalized. + { + { + "package foo;", "", "import com.foo./* the second one */Second;", "", "public class Blim {}", + }, + { + "package foo;", "", "import com.foo./* the second one */ Second;", "", "public class Blim {}", + }, + }, + + // Identical declarations collapse into one; ones that differ are both kept, so that a + // comment does not disappear with the copy that goes. + { + { + "package foo;", + "", + "import com.foo.First;", + "import com.foo.First;", + "import /* explanation A */ com.foo.Second;", + "import /* explanation B */ com.foo.Second;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import com.foo.First;", + "import /* explanation A */ com.foo.Second;", + "import /* explanation B */ com.foo.Second;", + "", + "public class Blim {}", + }, + }, + + // A package literally named `module` is an ordinary import, not a module import: + // `module` only introduces one when another identifier follows it. + { + { + "package foo;", + "", + "import module.Foo;", + "import java.util.List;", + "import module java.base;", + "", + "public class Blim {}", + }, + { + "package foo;", + "", + "import module java.base;", + "", + "import java.util.List;", + "import module.Foo;", + "", + "public class Blim {}", + }, + }, }; ImmutableList.Builder builder = ImmutableList.builder(); @@ -537,6 +717,10 @@ public void reorder() throws FormatterException { String output = ImportOrderer.reorderImports(input, JavaFormatterOptions.Style.GOOGLE); assertWithMessage("Expected exception").that(reordered).doesNotMatch("^!!"); assertWithMessage(input).that(output).isEqualTo(reordered); + // Reordering must be a fixed point: a formatted file has to survive being formatted again. + assertWithMessage("not idempotent: %s", output) + .that(ImportOrderer.reorderImports(output, JavaFormatterOptions.Style.GOOGLE)) + .isEqualTo(output); } catch (FormatterException e) { if (!reordered.startsWith("!!")) { throw e; diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/ModuleImportTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/ModuleImportTest.java new file mode 100644 index 000000000..269c98b07 --- /dev/null +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/ModuleImportTest.java @@ -0,0 +1,123 @@ +/* + * (c) Copyright 2026 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.javaformat.java; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayInputStream; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.nio.charset.StandardCharsets; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * End-to-end tests for module import declarations (JEP 511) through the pipeline the Gradle plugin and Spotless use, + * which reorders imports before formatting. The {@code ModuleImport} golden only exercises {@code formatSource}. + */ +public class ModuleImportTest { + + @BeforeAll + public static void requiresAParserThatProducesModuleImports() { + // Module imports parse from JDK 23 on, as a preview feature there; the formatter enables preview. + Assumptions.assumeTrue( + Formatter.getRuntimeVersion() >= 23, "import module requires running on JDK 23 or later"); + } + + /** Asserts both entry points that reorder imports, and that their output survives a second pass. */ + private static void assertFormats(String input, String expected) throws FormatterException { + assertThat(Formatter.create().formatSourceAndFixImports(input)).isEqualTo(expected); + assertThat(Formatter.create().fixImports(input)).isEqualTo(expected); + assertThat(Formatter.create().formatSourceAndFixImports(expected)).isEqualTo(expected); + assertThat(Formatter.create().fixImports(expected)).isEqualTo(expected); + } + + @Test + public void formatsAndFixesImports() throws FormatterException { + String input = "import module java.base;\n" + "class Example {}\n"; + String expected = "import module java.base;\n" + "\n" + "class Example {}\n"; + assertFormats(input, expected); + } + + @Test + public void fixesImportsOnlyFromTheCommandLine() throws Exception { + // The flag combination from the #1506 report, which failed with `Expected ; after import`. + String input = "import module java.base;\n" + "class Example {}\n"; + // Reordering puts a blank line after the import block; nothing else changes. + String expected = "import module java.base;\n" + "\n" + "class Example {}\n"; + StringWriter out = new StringWriter(); + StringWriter err = new StringWriter(); + Main main = new Main( + new PrintWriter(out, true), + new PrintWriter(err, true), + new ByteArrayInputStream(input.getBytes(StandardCharsets.UTF_8))); + int exitCode = main.format("-", "--fix-imports-only", "--skip-removing-unused-imports"); + assertThat(err.toString()).isEmpty(); + assertThat(exitCode).isZero(); + assertThat(out.toString()).isEqualTo(expected); + } + + @Test + public void keepsACommentBetweenModuleAndTheModuleName() throws FormatterException { + String input = "import module /* comment */ java.base;\n" + "class Example {}\n"; + String expected = "import module /* comment */ java.base;\n" + "\n" + "class Example {}\n"; + assertFormats(input, expected); + } + + @Test + public void keepsACommentBetweenThePartsOfTheModuleName() throws FormatterException { + String input = "import module java./* comment */base;\n" + "class Example {}\n"; + // Reordering normalizes the whitespace around the comment and leaves it between the parts. + String reordered = "import module java./* comment */ base;\n" + "\n" + "class Example {}\n"; + assertThat(Formatter.create().fixImports(input)).isEqualTo(reordered); + assertThat(Formatter.create().fixImports(reordered)).isEqualTo(reordered); + + // Formatting then breaks the line after the dot, which is where the formatter puts a comment + // in a qualified name; that output is stable too. + String formatted = "import module java.\n" + "/* comment */ base;\n" + "\n" + "class Example {}\n"; + assertThat(Formatter.create().formatSourceAndFixImports(input)).isEqualTo(formatted); + assertThat(Formatter.create().formatSourceAndFixImports(formatted)).isEqualTo(formatted); + } + + @Test + public void normalizesWhitespaceInsideTheModuleName() throws FormatterException { + String input = "import module java . base;\n" + "class Example {}\n"; + String expected = "import module java.base;\n" + "\n" + "class Example {}\n"; + assertFormats(input, expected); + } + + @Test + public void keepsBothCopiesOfADuplicateThatCarriesAComment() throws FormatterException { + // Identical declarations collapse; ones that differ are both kept, so no comment is dropped. + String input = "import module /* explanation A */ java.base;\n" + + "import module /* explanation B */ java.base;\n" + + "class Example {}\n"; + String expected = "import module /* explanation A */ java.base;\n" + + "import module /* explanation B */ java.base;\n" + + "\n" + + "class Example {}\n"; + assertFormats(input, expected); + } + + @Test + public void acceptsALineBreakAfterModule() throws FormatterException { + String input = "import module\n" + " java.base;\n" + "class Example {}\n"; + String expected = "import module java.base;\n" + "\n" + "class Example {}\n"; + assertFormats(input, expected); + } +} diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedImportsTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedImportsTest.java index cdea6fe04..2823e47a9 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedImportsTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedImportsTest.java @@ -21,6 +21,7 @@ import com.google.common.truth.Truth; import com.palantir.javaformat.jupiter.ParameterizedClass; import java.util.List; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.parallel.Execution; @@ -254,6 +255,21 @@ public static List parameters() { "interface Test { private static void foo() {} }", }, }, + { + // Module imports (JEP 511, `import module foo.bar;`) parse to JCModuleImport, a + // sibling of JCImport rather than a subtype. They must never be reported as unused + // (see isUnused), and must not crash buildReplacements when mixed with ordinary + // imports, some used and some not. + { + "import module java.base;", + "import java.util.List;", + "import java.util.Map;", + "class T { List xs; }", + }, + { + "import module java.base;", "import java.util.List;", "class T { List xs; }", + }, + }, }; ImmutableList.Builder builder = ImmutableList.builder(); for (String[][] inputAndOutput : inputsOutputs) { @@ -278,6 +294,10 @@ public RemoveUnusedImportsTest(String input, String expected) { @TestTemplate public void removeUnused() throws FormatterException { + // Module imports (JEP 511) parse from JDK 23 on, as preview there; the formatter enables preview. + Assumptions.assumeTrue( + !input.contains("import module") || Formatter.getRuntimeVersion() >= 23, + "import module requires running on JDK 23 or later"); Truth.assertThat(RemoveUnusedImports.removeUnusedImports(input)).isEqualTo(expected); } } diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.input new file mode 100644 index 000000000..8ab254240 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.input @@ -0,0 +1,15 @@ +import java.util.List; + +/// A field with a markdown doc comment. +String greeting = "hello"; + +record Pair(String first, String second) {} + +/** Javadoc on a top-level member of a compact source file. */ +List names() { + return List.of(greeting, new Pair(greeting, greeting).second()); +} + +void main() { + System.out.println(names()); +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.output new file mode 100644 index 000000000..26ef3e586 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/CompactSource.output @@ -0,0 +1,15 @@ +import java.util.List; + +/// A field with a markdown doc comment. +String greeting = "hello"; + +record Pair(String first, String second) {} + +/** Javadoc on a top-level member of a compact source file. */ +List names() { + return List.of(greeting, new Pair(greeting, greeting).second()); +} + +void main() { + System.out.println(names()); +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.input new file mode 100644 index 000000000..e01857a19 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.input @@ -0,0 +1,12 @@ +class FlexibleConstructor { + private final int value; + + FlexibleConstructor(int raw) { + if (raw < 0) { + throw new IllegalArgumentException("negative"); + } + var normalized = Math.max(raw, 1); + super(); + this.value = normalized; + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.output new file mode 100644 index 000000000..798bb786c --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/FlexibleConstructor.output @@ -0,0 +1,12 @@ +class FlexibleConstructor { + private final int value; + + FlexibleConstructor(int raw) { + if (raw < 0) { + throw new IllegalArgumentException("negative"); + } + var normalized = Math.max(raw, 1); + super(); + this.value = normalized; + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.input new file mode 100644 index 000000000..557fd468e --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.input @@ -0,0 +1,10 @@ +/// A greeter. +/// +/// Says hello. See [String]. +class MarkdownDoc { + /// Returns the greeting. + /// @return the text + String greeting() { + return "hi"; + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.output new file mode 100644 index 000000000..2b5d30db2 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/MarkdownDoc.output @@ -0,0 +1,10 @@ +/// A greeter. +/// +/// Says hello. See [String]. +class MarkdownDoc { + /// Returns the greeting. + /// @return the text + String greeting() { + return "hi"; + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.input new file mode 100644 index 000000000..63d020f52 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.input @@ -0,0 +1,8 @@ +import module java.base; +import module java.sql; + +import java.util.List; + +class ModuleImport { + List xs; +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.output new file mode 100644 index 000000000..72e62d733 --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/ModuleImport.output @@ -0,0 +1,8 @@ +import module java.base; +import module java.sql; + +import java.util.List; + +class ModuleImport { + List xs; +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.input b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.input new file mode 100644 index 000000000..79c12d94a --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.input @@ -0,0 +1,34 @@ +class UnnamedPattern { + sealed interface Shape permits Box {} + record Box(Object a, Object b) implements Shape {} + + record Pair(Box first, Object second) {} + + int classify(Shape s) { + return switch (s) { + case Box(Integer i, _) when i > 0 -> i; + case Box(Integer i, _) -> i; + case Box(_, _) -> 0; + }; + } + + int nested(Object o) { + return switch (o) { + case Pair(Box(var _, Integer i), _) -> i; + case Pair(Box(_, _), _) -> 0; + default -> -1; + }; + } + + boolean isBox(Object o) { + return o instanceof Box(Integer _, _); + } + + void swallow(Runnable r) { + try { + r.run(); + } catch (Exception _) { + // ignored + } + } +} diff --git a/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.output b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.output new file mode 100644 index 000000000..21a53185f --- /dev/null +++ b/open-java-format/src/test/resources/com/palantir/javaformat/java/testdata/UnnamedPattern.output @@ -0,0 +1,35 @@ +class UnnamedPattern { + sealed interface Shape permits Box {} + + record Box(Object a, Object b) implements Shape {} + + record Pair(Box first, Object second) {} + + int classify(Shape s) { + return switch (s) { + case Box(Integer i, _) when i > 0 -> i; + case Box(Integer i, _) -> i; + case Box(_, _) -> 0; + }; + } + + int nested(Object o) { + return switch (o) { + case Pair(Box(var _, Integer i), _) -> i; + case Pair(Box(_, _), _) -> 0; + default -> -1; + }; + } + + boolean isBox(Object o) { + return o instanceof Box(Integer _, _); + } + + void swallow(Runnable r) { + try { + r.run(); + } catch (Exception _) { + // ignored + } + } +} From 60f39af190834435c7f795b7b742e1f052a7d7db Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:00:59 +0300 Subject: [PATCH 4/9] Register the javac end-position API for the native image Since palantir/palantir-java-format#1786, Trees reaches JCTree.getEndPosition(EndPosTable) and JCCompilationUnit.endPositions through method handles, so that the same code runs on JDK 27, where neither exists. The image is built with --exact-reachability-metadata, so without these entries every file failed, even `class A{int x;}`: "Cannot reflectively access method 'com.sun.tools.javac.tree.JCTree#getEndPosition(com.sun.tools.javac.tree.EndPosTable)'". --- .../native-image/reachability-metadata.json | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json b/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json index 956ece11c..5fde3bc33 100644 --- a/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json +++ b/open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json @@ -160,6 +160,28 @@ } ] }, + { + "type": "com.sun.tools.javac.tree.EndPosTable" + }, + { + "type": "com.sun.tools.javac.tree.JCTree", + "methods": [ + { + "name": "getEndPosition", + "parameterTypes": [ + "com.sun.tools.javac.tree.EndPosTable" + ] + } + ] + }, + { + "type": "com.sun.tools.javac.tree.JCTree$JCCompilationUnit", + "fields": [ + { + "name": "endPositions" + } + ] + }, { "type": "java.io.Serializable" }, From 5e998682c7d9605b5d927ce541d64a2e87832c29 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:01:00 +0300 Subject: [PATCH 5/9] Check that the tests run on the JDK the jdk job asked for FormatterVersionTest, from palantir/palantir-java-format#1707, compares the test JVM with the expectedJavaVersion system property, which upstream's testJdkNN tasks set. Here the build passes the -PjavaRuntime value, so a CI leg that runs on another JDK fails instead of skipping the tests that need a newer parser and still passing. --- build.gradle | 8 +++++--- open-java-format/build.gradle | 3 +++ .../palantir/javaformat/java/FormatterVersionTest.java | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 2093235e1..2eced690c 100644 --- a/build.gradle +++ b/build.gradle @@ -127,9 +127,11 @@ subprojects { } } +// The JDK the tests run on, and with it the javac whose internals the formatter parses with. The code is +// compiled for 21 whatever this says. CI's `jdk` jobs pass -PjavaRuntime=25 and so on. +ext.javaRuntime = providers.gradleProperty('javaRuntime').getOrElse('21') + javaVersions { libraryTarget = 21 - // The JDK the tests run on, and with it the javac whose internals the formatter parses with. The code is - // compiled for 21 whatever this says. CI's `jdk` jobs pass -PjavaRuntime=25 and so on. - runtime = providers.gradleProperty('javaRuntime').getOrElse('21') + runtime = javaRuntime } diff --git a/open-java-format/build.gradle b/open-java-format/build.gradle index 12eb94e2f..d78260236 100644 --- a/open-java-format/build.gradle +++ b/open-java-format/build.gradle @@ -76,6 +76,9 @@ tasks.named("test") { // Run all classes and tests in parallel // https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution systemProperty 'junit.jupiter.execution.parallel.mode.default', 'concurrent' + // FormatterVersionTest fails when the tests run on another JDK than the one asked for, so a CI leg + // cannot pass on the wrong JDK and skip the tests that need a newer parser. + systemProperty 'expectedJavaVersion', rootProject.ext.javaRuntime } javaVersion { diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java index 4671a0fe2..b04e99446 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java +++ b/open-java-format/src/test/java/com/palantir/javaformat/java/FormatterVersionTest.java @@ -22,15 +22,15 @@ import org.junit.jupiter.api.Test; /** - * Guards the {@code testJdkNN} legs: without this, a leg that resolves to a JDK older than the one it asked for skips - * the {@code ModuleImport} golden and the module import tests, and still passes. + * Guards CI's {@code jdk} legs: without this, a leg that runs on an older JDK than the one it asked for with + * {@code -PjavaRuntime} skips the {@code ModuleImport} golden and the module import tests, and still passes. */ public class FormatterVersionTest { @Test public void runsOnTheJdkTheTestTaskAskedFor() { String expected = System.getProperty("expectedJavaVersion"); - Assumptions.assumeTrue(expected != null, "expectedJavaVersion is set only by the testJdkNN tasks"); + Assumptions.assumeTrue(expected != null, "expectedJavaVersion is set by the Gradle build"); assertThat(Formatter.getRuntimeVersion()).isEqualTo(Integer.parseInt(expected)); } } From 97358ba757a9f7396d8947619832fc2becde60d0 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:02:11 +0300 Subject: [PATCH 6/9] Smoke-test the native binary on Java 25 syntax A compact source file with a module import and an unnamed pattern, formatted by the image itself: this is the syntax palantir/palantir-java-format#1707 brings, and the image also needs its ImportTree#isModule metadata for it. --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc984d182..fa1053292 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -130,7 +130,8 @@ jobs: env: JDK21_HOME: ${{ steps.jdk21.outputs.path }} - # The binary itself on a file that needs formatting, a formatted one and one that does not parse. + # The binary itself on a file that needs formatting, a formatted one, one that does not parse and + # one in Java 25 syntax. - name: Smoke-test the binary run: | binary="$PWD/$(ls open-java-format-native/build/native/nativeCompile/open-java-format-* | grep -v '\.txt$')" @@ -143,6 +144,10 @@ jobs: printf 'class B {\n' > B.java set +e; "$binary" B.java; status=$?; set -e test "$status" -eq 2 + # Java 25 syntax: a compact source file with a module import and an unnamed pattern. + printf 'import module java.base;\nrecord Box(int a,int b){}\nvoid main(){Object o=new Box(1,2);if(o instanceof Box(_,_)){IO.println(List.of(1));}}\n' > C.java + "$binary" --replace C.java + printf 'import module java.base;\n\nrecord Box(int a, int b) {}\n\nvoid main() {\n Object o = new Box(1, 2);\n if (o instanceof Box(_, _)) {\n IO.println(List.of(1));\n }\n}\n' | diff - C.java - name: Test the plugins against the image run: ./gradlew -PnativeImage=true :open-java-format-jdk-bootstrap:test :gradle-open-java-format:test From 9c315d600748ba33de40dcdddd198f9160757566 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:15:36 +0300 Subject: [PATCH 7/9] Print `var` from its token, now that JDK 27 gives it a type node JDK 27 (JDK-8268850) gives a variable declared with `var` a VarTypeTree as its type, where earlier JDKs have none. This visitor compiles against JDK 21 and has no visitVarType, so it scanned the new node to nothing and every `var` declaration failed on JDK 27: six golden files in CI, among them FlexibleConstructor, I959 and B380299722. declareOne now looks for the `var` token before the type node, the same reordering google-java-format made in google/google-java-format@075e025c94. A binding pattern such as `Nested(var i)` has a method of its own, which now treats a VAR_TYPE node like the missing type of the older JDKs. --- .../palantir/javaformat/java/JavaInputAstVisitor.java | 8 +++++--- .../javaformat/java/java14/Java14InputAstVisitor.java | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index 66ba31f44..0fd3569d8 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -3576,15 +3576,17 @@ int declareOne( { builder.open(ZERO); { - if (typeWithDims.isPresent() && typeWithDims.get().node != null) { + // `var` first: from JDK 27 (JDK-8268850) its type is a VarTypeTree rather than null, + // and this visitor, compiled against JDK 21, has no visitVarType to print it. + if (isVar) { + token("var"); + } else if (typeWithDims.isPresent() && typeWithDims.get().node != null) { scan(typeWithDims.get().node, null); int totalDims = dims.size(); builder.open(plusFour); maybeAddDims(dims); builder.close(); baseDims = totalDims - dims.size(); - } else if (isVar) { - token("var"); } else { scan(type, null); } diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java index 24ab861a0..4566c0fbc 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java +++ b/open-java-format/src/main/java/com/palantir/javaformat/java/java14/Java14InputAstVisitor.java @@ -113,12 +113,20 @@ public Void visitBindingPattern(BindingPatternTree node, Void unused) { return null; } + /** + * Is {@code type} the {@code var} of a binding pattern? Older JDKs leave such a pattern without a type, JDK 27 + * (JDK-8268850) gives it a VarTypeTree, which does not exist on JDK 21, so it is matched by {@link Tree.Kind} name. + */ + private static boolean isVarType(Tree type) { + return type.getKind().name().equals("VAR_TYPE"); + } + private void visitBindingPattern(ModifiersTree modifiers, Tree type, Name name) { builder.open(plusFour); if (modifiers != null) { builder.addAll(visitModifiers(modifiers, Direction.HORIZONTAL, Optional.empty())); } - if (type == null) { + if (type == null || isVarType(type)) { token("var"); } else { scan(type, null); From 4e6ca5e791a81a50635c42cd51b23dbd6b26992e Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:15:37 +0300 Subject: [PATCH 8/9] Show a failing test's whole exception in the log In CI the short form gave only "FormatterException at FormatterIntegrationTest.java:76", not what the formatter complained about, and nothing of the output of a TestKit build that failed. --- build.gradle | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build.gradle b/build.gradle index 2eced690c..566cb7961 100644 --- a/build.gradle +++ b/build.gradle @@ -87,6 +87,9 @@ allprojects { tasks.withType(Test).configureEach { jvmArgs(javacInternalExports.collect { "--add-exports=${it}=ALL-UNNAMED".toString() }) + // The short form gave CI logs only "FormatterException at FormatterIntegrationTest.java:76", never what + // the formatter complained about, nor the output of a TestKit build that failed. + testLogging.exceptionFormat = 'full' } tasks.withType(Javadoc).configureEach { From aed6a78e670ab59021f6b8d1a587a79401514d4c Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Tue, 22 Sep 2026 19:22:49 +0300 Subject: [PATCH 9/9] Leave the Gradle plugin's tests out of the JDK 27 job On JDK 27 every TestKit build failed with "BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 71": Gradle 9.7.1 does not run on Java 27, and TestKit runs the builds on the test JVM. The formatter's own tests all pass there. Each leg of the matrix now names its extra Gradle arguments and the reason for them, empty for 25 and 26. The 27 leg passes -x :gradle-open-java-format:test, and the job shows the reason as a notice, so it is visible in the run and not only in a YAML comment. --- .github/workflows/ci.yml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa1053292..e6fa6fe02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,19 @@ jobs: strategy: fail-fast: false matrix: - jdk: [25, 26, 27] + include: + - jdk: 25 + gradle_args: '' + gradle_args_reason: '' + - jdk: 26 + gradle_args: '' + gradle_args_reason: '' + - jdk: 27 + gradle_args: -x :gradle-open-java-format:test + gradle_args_reason: >- + Gradle 9.7.1 does not run on Java 27 yet, and the Gradle plugin's tests run their TestKit builds on + the test JVM, where they fail with "Unsupported class file major version 71". Those tests are left + out until the wrapper's Gradle supports Java 27. steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -66,10 +78,17 @@ jobs: distribution: temurin java-version: '21' + - name: Explain the extra Gradle arguments + if: ${{ matrix.gradle_args != '' }} + env: + GRADLE_ARGS: ${{ matrix.gradle_args }} + REASON: ${{ matrix.gradle_args_reason }} + run: echo "::notice title=JDK ${{ matrix.jdk }} runs with $GRADLE_ARGS::$REASON" + # Gradle does not look into the runner's tool cache, so it is told where the test JDK is. - name: Build run: >- - ./gradlew test -PjavaRuntime=${{ matrix.jdk }} + ./gradlew test -PjavaRuntime=${{ matrix.jdk }} ${{ matrix.gradle_args }} -Porg.gradle.java.installations.paths=${{ steps.test-jdk.outputs.path }} - name: Publish Test Report