GeezerVisonThe World Wide Web, restored to 800 pixels of glory. |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites |
GeezerVison is showing github.com — Keep comments between imports with the import after them by abashev · Pull Request #44 · openjavaformat/open-java-format Conversation
Collaborator
Closes #39. Upstream reports: google/google-java-format#424, google/google-java-format#546, and a sample in palantir#789. What changesA comment between two imports made
In the JDK 21 sources, for example: import java.io.Serializable; // Javadoc imports: import java.lang.management.MemoryUsage; Checked
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsImportOrderer.scanImports read imports while the next token was "import" and took along only the rest of an import's line and the // comments right under it. A blank line followed by a comment, or any block comment, ended the scan, and the import after it made reorderImports throw "Imports not contiguous", so the whole file went unformatted (#39, from google/google-java-format#424 and google/google-java-format#546). 18 files of the JDK 21 sources failed this way, with group headings such as "// Javadoc imports:", commented-out imports, and a block comment around an import. Comments between two imports now go with the import after them and move with it when the imports are sorted. A block comment on an import's own line stays with that import, as a // comment there already did. Comments after the last import still belong to what follows. Only files that failed before change: of the 15,747 JDK 21 files, the 18 now format and no other file changes. Two of the 18 change again on a second run, in string concatenations away from the imports, as other files of the JDK already do. The two import tests that expected the error now expect the sorted imports.
abashev
enabled auto-merge
abashev
disabled auto-merge
Uh oh!
There was an error while loading. Please reload this page.
abashev
added a commit
that referenced
this pull request
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersNo reviewsUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
None yet
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. A comment between imports fails the whole file: "Imports not contiguous"Uh oh!
There was an error while loading. Please reload this page. 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. |
| You are visitor 01402520 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |