Skip to content

Commit b2e270b

Browse files
asm0deyclaude
authored andcommitted
Format module imports, compact source files and unnamed patterns
From palantir#1707 by Pavel Finkelshtein (asm0dey), its 15 commits up to 53bea7f 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#1786. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4a89e1e commit b2e270b

22 files changed

Lines changed: 873 additions & 59 deletions

‎open-java-format-native/src/main/resources/META-INF/native-image/reachability-metadata.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,15 @@
131131
}
132132
]
133133
},
134+
{
135+
"type": "com.sun.source.tree.ImportTree",
136+
"methods": [
137+
{
138+
"name": "isModule",
139+
"parameterTypes": []
140+
}
141+
]
142+
},
134143
{
135144
"type": "com.sun.tools.javac.parser.JavaTokenizer",
136145
"fields": [

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

Lines changed: 152 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -123,35 +123,47 @@ private String reorderImports() throws FormatterException {
123123
/**
124124
* A {@link Comparator} that orders {@link Import}s by Google Style, defined at
125125
* https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing.
126+
*
127+
* <p>Google Style does not use module imports ({@code import module foo.bar;}, JEP 511); when present, they
128+
* sort between static and non-static type imports, matching google-java-format.
126129
*/
127-
private static final Comparator<Import> GOOGLE_IMPORT_COMPARATOR =
128-
Comparator.comparing(Import::isStatic, trueFirst()).thenComparing(Import::imported);
130+
private static final Comparator<Import> GOOGLE_IMPORT_COMPARATOR = Comparator.comparing(
131+
Import::isStatic, trueFirst())
132+
.thenComparing(Import::isModule, trueFirst())
133+
.thenComparing(Import::imported)
134+
// Imports that compare equal collapse into one, so two that are written differently -- one of them
135+
// carrying a comment, say -- must not compare equal, or that comment disappears with it.
136+
.thenComparing(Import::declaration);
129137

130138
/**
131139
* A {@link Comparator} that orders {@link Import}s by AOSP Style, defined at
132140
* https://source.android.com/setup/contribute/code-style#order-import-statements and implemented in IntelliJ at
133141
* https://android.googlesource.com/platform/development/+/master/ide/intellij/codestyles/AndroidStyle.xml.
142+
*
143+
* <p>As with {@link #GOOGLE_IMPORT_COMPARATOR}, module imports sort after static imports.
134144
*/
135145
private static final Comparator<Import> AOSP_IMPORT_COMPARATOR = Comparator.comparing(Import::isStatic, trueFirst())
146+
.thenComparing(Import::isModule, trueFirst())
136147
.thenComparing(Import::isAndroid, trueFirst())
137148
.thenComparing(Import::isThirdParty, trueFirst())
138149
.thenComparing(Import::isJava, trueFirst())
139-
.thenComparing(Import::imported);
150+
.thenComparing(Import::imported)
151+
.thenComparing(Import::declaration);
140152

141153
/**
142154
* Determines whether to insert a blank line between the {@code prev} and {@code curr} {@link Import}s based on
143155
* Google style.
144156
*/
145157
private static boolean shouldInsertBlankLineGoogle(Import prev, Import curr) {
146-
return prev.isStatic() && !curr.isStatic();
158+
return prev.isStatic() != curr.isStatic() || prev.isModule() != curr.isModule();
147159
}
148160

149161
/**
150162
* Determines whether to insert a blank line between the {@code prev} and {@code curr} {@link Import}s based on AOSP
151163
* style.
152164
*/
153165
private static boolean shouldInsertBlankLineAosp(Import prev, Import curr) {
154-
if (prev.isStatic() && !curr.isStatic()) {
166+
if (prev.isStatic() != curr.isStatic() || prev.isModule() != curr.isModule()) {
155167
return true;
156168
}
157169
// insert blank line between "com.android" from "com.anythingelse"
@@ -186,12 +198,24 @@ private ImportOrderer(String text, ImmutableList<Tok> toks, Style style) {
186198
class Import {
187199
private final String imported;
188200
private final boolean isStatic;
201+
private final boolean isModule;
189202
private final String trailing;
203+
private final String declaration;
190204

191-
Import(String imported, String trailing, boolean isStatic) {
205+
Import(String imported, String trailing, boolean isStatic, boolean isModule, String declaration) {
192206
this.imported = imported;
193207
this.trailing = trailing;
194208
this.isStatic = isStatic;
209+
this.isModule = isModule;
210+
this.declaration = declaration;
211+
}
212+
213+
/**
214+
* The declaration as it will be written: the {@code import} keyword through the semicolon, with whitespace
215+
* normalized and any comments left in the slot the author wrote them in.
216+
*/
217+
String declaration() {
218+
return declaration;
195219
}
196220

197221
/** The name being imported, for example {@code java.util.List}. */
@@ -204,6 +228,11 @@ boolean isStatic() {
204228
return isStatic;
205229
}
206230

231+
/** True if this is {@code import module} (JEP 511). */
232+
boolean isModule() {
233+
return isModule;
234+
}
235+
207236
/** The top-level package of the import. */
208237
String topLevel() {
209238
return DOT_SPLITTER.split(imported()).iterator().next();
@@ -245,11 +274,7 @@ public boolean isThirdParty() {
245274
@Override
246275
public String toString() {
247276
StringBuilder sb = new StringBuilder();
248-
sb.append("import ");
249-
if (isStatic()) {
250-
sb.append("static ");
251-
}
252-
sb.append(imported()).append(';');
277+
sb.append(declaration());
253278
if (trailing().trim().isEmpty()) {
254279
sb.append(lineSeparator);
255280
} else {
@@ -259,6 +284,42 @@ public String toString() {
259284
}
260285
}
261286

287+
/**
288+
* Renders one import declaration from the toks it is made of. Whitespace between the toks is normalized, since the
289+
* style guide puts one import on a line of its own, but a comment stays in the slot the author wrote it in rather
290+
* than being moved to wherever the rendered declaration can accommodate it.
291+
*/
292+
private final class Declaration {
293+
private final StringBuilder text = new StringBuilder();
294+
private boolean atLineStart = false;
295+
296+
/** Appends one tok: a keyword, an identifier, {@code .}, {@code *}, {@code ;}, or a comment. */
297+
void append(String piece) {
298+
if (text.length() > 0 && !atLineStart && needsSpaceBefore(piece)) {
299+
text.append(' ');
300+
}
301+
text.append(piece);
302+
atLineStart = false;
303+
if (piece.startsWith("//")) {
304+
// A // comment swallows the rest of its line, so the declaration continues on the next one.
305+
text.append(lineSeparator);
306+
atLineStart = true;
307+
}
308+
}
309+
310+
private boolean needsSpaceBefore(String piece) {
311+
if (piece.equals(".") || piece.equals(";") || piece.equals("*")) {
312+
return false;
313+
}
314+
return text.charAt(text.length() - 1) != '.';
315+
}
316+
317+
@Override
318+
public String toString() {
319+
return text.toString();
320+
}
321+
}
322+
262323
private String tokString(int start, int end) {
263324
StringBuilder sb = new StringBuilder();
264325
for (int i = start; i < end; i++) {
@@ -282,9 +343,10 @@ private static class ImportsAndIndex {
282343
*
283344
* <pre>{@code
284345
* <imports> -> (<end-of-line> | <import>)*
285-
* <import> -> "import" <whitespace> ("static" <whitespace>)?
286-
* <identifier> ("." <identifier>)* ("." "*")? <whitespace>? ";"
346+
* <import> -> "import" <ignorable> (("static" | "module") <ignorable>)?
347+
* <identifier> ("." <identifier>)* ("." "*")? <ignorable>? ";"
287348
* <whitespace>? <end-of-line>? (<line-comment> <end-of-line>)*
349+
* <ignorable> -> (<whitespace> | <end-of-line> | <comment>)+
288350
* }</pre>
289351
*
290352
* @param i the index to start parsing at.
@@ -298,35 +360,43 @@ private ImportsAndIndex scanImports(int i) throws FormatterException {
298360
// of our tests here and protects us from running off the end of the toks list. Since it is
299361
// zero-width it doesn't matter if we include it in our string concatenation at the end.
300362
while (i < toks.size() && tokenAt(i).equals("import")) {
363+
Declaration declaration = new Declaration();
364+
declaration.append(tokenAt(i));
301365
i++;
302-
if (isSpaceToken(i)) {
366+
i = skipIgnored(i, declaration);
367+
boolean isModule = isModuleKeyword(i);
368+
if (isModule) {
369+
declaration.append(tokenAt(i));
303370
i++;
371+
i = skipIgnored(i, declaration);
304372
}
305-
boolean isStatic = tokenAt(i).equals("static");
373+
boolean isStatic = !isModule && tokenAt(i).equals("static");
306374
if (isStatic) {
375+
declaration.append(tokenAt(i));
307376
i++;
308-
if (isSpaceToken(i)) {
309-
i++;
310-
}
377+
i = skipIgnored(i, declaration);
311378
}
312379
if (!isIdentifierToken(i)) {
313380
throw new FormatterException("Unexpected token after import: " + tokenAt(i));
314381
}
315-
StringAndIndex imported = scanImported(i);
382+
StringAndIndex imported = scanImported(i, declaration);
316383
String importedName = imported.string;
317384
i = imported.index;
318-
if (isSpaceToken(i)) {
319-
i++;
320-
}
385+
i = skipIgnored(i, declaration);
321386
if (!tokenAt(i).equals(";")) {
322387
throw new FormatterException("Expected ; after import");
323388
}
389+
declaration.append(";");
324390
while (tokenAt(i).equals(";")) {
325391
// Extra semicolons are not allowed by the JLS but are accepted by javac.
326392
i++;
327393
}
328394
StringBuilder trailing = new StringBuilder();
329-
if (isSpaceToken(i)) {
395+
// A block comment on the same line as the `;` trails this import; one on a later line
396+
// belongs to whatever follows it, so only same-line toks are absorbed here. Javadoc is
397+
// excluded: the formatter moves a javadoc comment onto a line of its own, which would
398+
// separate the imports.
399+
while (isSpaceToken(i) || isBlockCommentToken(i)) {
330400
trailing.append(tokenAt(i));
331401
i++;
332402
}
@@ -344,7 +414,7 @@ private ImportsAndIndex scanImports(int i) throws FormatterException {
344414
i++;
345415
}
346416
}
347-
imports.add(new Import(importedName, trailing.toString(), isStatic));
417+
imports.add(new Import(importedName, trailing.toString(), isStatic, isModule, declaration.toString()));
348418
// Remember the position just after the import we just saw, before skipping blank lines.
349419
// If the next thing after the blank lines is not another import then we don't want to
350420
// include those blank lines in the text to be replaced.
@@ -387,32 +457,39 @@ private static class StringAndIndex {
387457
}
388458

389459
/**
390-
* Scans the imported thing, the dot-separated name that comes after import [static] and before the semicolon. We
391-
* don't allow spaces inside the dot-separated name. Wildcard imports are supported: if the input is {@code import
392-
* java.util.*;} then the returned string will be {@code java.util.*}.
460+
* Scans the imported thing, the dot-separated name that comes after import [static] and before the semicolon.
461+
* Whitespace, line terminators and comments may appear between its parts, as they may anywhere else in the
462+
* declaration; the returned name contains none of them. Wildcard imports are supported: if the input is
463+
* {@code import java.util.*;} then the returned string will be {@code java.util.*}.
393464
*
394465
* @param start the index of the start of the identifier. If the import is {@code import java.util.List;} then this
395466
* index points to the token {@code java}.
467+
* @param declaration collects the toks scanned, so a comment between the parts of the name keeps its place
396468
* @return the parsed import ({@code java.util.List} in the example) and the index of the first token after the
397469
* imported thing ({@code ;} in the example).
398470
* @throws FormatterException if the imported name could not be parsed.
399471
*/
400-
private StringAndIndex scanImported(int start) throws FormatterException {
472+
private StringAndIndex scanImported(int start, Declaration declaration) throws FormatterException {
401473
int i = start;
402474
StringBuilder imported = new StringBuilder();
403475
// At the start of each iteration of this loop, i points to an identifier.
404476
// On exit from the loop, i points to a token after an identifier or after *.
405477
while (true) {
406478
Preconditions.checkState(isIdentifierToken(i));
407479
imported.append(tokenAt(i));
480+
declaration.append(tokenAt(i));
408481
i++;
482+
i = skipIgnored(i, declaration);
409483
if (!tokenAt(i).equals(".")) {
410484
return new StringAndIndex(imported.toString(), i);
411485
}
412486
imported.append('.');
487+
declaration.append(".");
413488
i++;
489+
i = skipIgnored(i, declaration);
414490
if (tokenAt(i).equals("*")) {
415491
imported.append('*');
492+
declaration.append("*");
416493
return new StringAndIndex(imported.toString(), i + 1);
417494
} else if (!isIdentifierToken(i)) {
418495
throw new FormatterException("Could not parse imported name, at: " + tokenAt(i));
@@ -452,6 +529,41 @@ private String tokenAt(int i) {
452529
return toks.get(i).getOriginalText();
453530
}
454531

532+
/**
533+
* Returns true if the token at {@code i} is the {@code module} contextual keyword introducing a module import
534+
* declaration ({@code import module foo.bar;}, JEP 511), as opposed to an ordinary import whose first segment
535+
* happens to be an identifier literally named {@code module} (for example {@code import module.Foo;}). As with
536+
* other contextual keywords ({@code var}, {@code yield}, ...), this is disambiguated by lookahead: {@code
537+
* module} only introduces a module import when the next token is another identifier, rather than {@code .} or
538+
* {@code ;}. Whitespace, line terminators and comments are skipped, as they are by the parsing that follows.
539+
*/
540+
private boolean isModuleKeyword(int i) {
541+
if (!tokenAt(i).equals("module")) {
542+
return false;
543+
}
544+
return isIdentifierToken(skipIgnored(i + 1, new Declaration()));
545+
}
546+
547+
/**
548+
* Skips whitespace, line terminators and comments starting at {@code i}, appending each comment to
549+
* {@code declaration}, and returns the index of the first token that is none of those. Javadoc comments are not
550+
* skipped: the formatter moves them onto a line of their own, which would separate the imports, so an import
551+
* carrying one is rejected, as it was before module imports were supported.
552+
*/
553+
private int skipIgnored(int i, Declaration declaration) {
554+
while (i < toks.size()) {
555+
if (isSpaceToken(i) || isNewlineToken(i)) {
556+
i++;
557+
} else if (isSlashSlashCommentToken(i) || isBlockCommentToken(i)) {
558+
declaration.append(tokenAt(i).trim());
559+
i++;
560+
} else {
561+
break;
562+
}
563+
}
564+
return i;
565+
}
566+
455567
private boolean isIdentifierToken(int i) {
456568
String s = tokenAt(i);
457569
return !s.isEmpty() && Character.isJavaIdentifierStart(s.codePointAt(0));
@@ -470,6 +582,18 @@ private boolean isSlashSlashCommentToken(int i) {
470582
return toks.get(i).isSlashSlashComment();
471583
}
472584

585+
/** True if the tok is a {@code /* *}{@code /} comment that is not javadoc. */
586+
private boolean isBlockCommentToken(int i) {
587+
return toks.get(i).isSlashStarComment() && !toks.get(i).isJavadocComment();
588+
}
589+
590+
/** True if {@code text} ends in a line terminator. */
591+
private static boolean endsInNewline(CharSequence text) {
592+
return text.length() > 0
593+
&& Newlines.isNewline(
594+
text.subSequence(text.length() - 1, text.length()).toString());
595+
}
596+
473597
private boolean isNewlineToken(int i) {
474598
return toks.get(i).isNewline();
475599
}

0 commit comments

Comments
 (0)