Stop an unused import from leaving two blank lines behind - #43
Merged
Merged
Conversation
The command line formats first and fixes imports afterwards. When RemoveUnusedImports deleted an import that sat between two blank lines, such as the only import between the package line and the class, it left both blank lines, and nothing ran after it to collapse them. A second run did, so --replace wrote files that --set-exit-if-changed then reported (#37, from google/google-java-format#598 and google/google-java-format#1436). The Gradle and Spotless entry point fixes imports before it formats and was not affected, so the two disagreed about these files. This ports google/google-java-format#1437 by arimu1: adjacent unused imports are deleted as one range, and a range that sits between blank lines, or at the start of the file, takes one of them along. The code is adapted to this codebase. The four RemoveUnusedImportsTest cases come from the upstream PR, and a MainTest checks that the command line and formatSourceAndFixImports now give the same file. Of the 15,747 files of the JDK 21 sources, 304 format differently, by 305 blank lines fewer in all and no other change. All 304 are files the old code changed again on a second run, and the new output is stable on every one of them.
abashev
enabled auto-merge
September 23, 2026 07:34
abashev
disabled auto-merge
September 23, 2026 08:25
abashev
added a commit
that referenced
this pull request
Sep 23, 2026
Conflicts with #43 and #44 in ImportOrderer, RemoveUnusedImports and their tests. ImportOrderer: an import now carries both the comments on the lines before it, from #44, and its declaration rendered from its toks, from this branch (palantir#1707). This branch's loop over same-line block comments already covers #44's single block comment after the `;`, so #44's copy of that step goes. Two interactions are decided here: - A javadoc comment right after an import's `;` is no longer an error. This branch rejected it because the formatter moves it onto a line of its own, which used to separate the imports. Since #44 a comment between imports goes with the import after it, and so does this one. - Two copies of an import that differ only in the comment on the lines before them no longer collapse into one, so that comment stays. This branch already kept copies that differ in a comment inside the declaration. RemoveUnusedImports: this branch's loop over JCTree with Trees.getEndPosition, and #43's coalesced ranges and blank-line cleanup. Tests: every case from both sides. This branch's case for a same-line block comment repeated #44's and is folded into it. Its javadoc case now expects the comment to move with the next import. A new case covers the duplicate. Checked: all 1493 formatter tests pass. On the JDK 21 sources (15,747 files, Temurin 21) and the JDK 25 sources (15,368 files, Corretto 25), the merge formats every file exactly as main does, with no errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #37. Upstream reports: google/google-java-format#598, google/google-java-format#1436.
From upstream, with the author kept
google/google-java-format#1437 by arimu1 (open upstream), ported as one commit authored by them and adapted to this codebase:
RemoveUnusedImportsdeletes adjacent unused imports as one range (putCoalescing);Why
The command line formats first and fixes imports afterwards. An unused import between two blank lines, such as the only import between the package line and the class, left both of them behind, and only a second run collapsed them. So
--replacewrote files that--dry-run --set-exit-if-changedthen reported as unformatted. The Gradle and Spotless entry point fixes imports before it formats, so it was not affected, and the two entry points disagreed about these files.Checked
RemoveUnusedImportsTestcases from the upstream PR: three fail without the change; the fourth, a partial deletion that keeps its blank line, passes either way and guards against deleting too much.MainTest.unusedImportRemovalLeavesOneBlankLineruns the file from unused import removal leaves extra blank line between package declaration and class Javadoc / declaration google/google-java-format#1436 through the command line and throughformatSourceAndFixImportsand expects the same result from both. It fails without the change../gradlew :open-java-format:teston JDK 21: 1422 tests, all green.