Preserve trailing comments when removing an import#8010
Draft
MBoegers wants to merge 3 commits into
Draft
Conversation
An end-of-line comment on an import is stored in the LST as the prefix of the *next* import. RemoveImport carried a removed import's prefix forward to keep blank lines between groups (#3868) but only forwarded getLastWhitespace() — comments in the prefix were discarded. As a result the trailing comment of the line *before* a removed import (and standalone / commented-out lines) were dropped, while the removed import's *own* trailing comment dangled onto the previous line. Forward the previous line's trailing comment and standalone comments onto the next import (preserving blank lines), and drop only the removed import's own end-of-line comment, which described the now-removed import. Context: moderneinc/customer-requests#2437 Signed-off-by: Merlin Bögershausen <merlin.boegershausen@rwth-aachen.de>
64fc90c to
6ca21f8
Compare
When the last import is removed there is no following import to carry its prefix onto, so a preceding line's trailing comment or a commented-out line was lost, and the removed import's own trailing comment dangled onto the previous line. Forward the prefix to the first class declaration (or the EOF when there are none) instead, reusing the same merge logic. The first class keeps its own spacing so import-group blank lines are not pushed onto it. Addresses review feedback on the last-import edge case. Signed-off-by: Merlin Bögershausen <merlin.boegershausen@rwth-aachen.de>
sambsnyd
reviewed
Jun 16, 2026
sambsnyd
left a comment
Member
There was a problem hiding this comment.
If we're going to try and be smarter about this we should probably differentiate between "line ending" and "preceding line" comments .
import com.foo.Bar; // This comment is probably about Bar
// This comment is probably about Baz
import com.foo.Baz;In this example the two comments are both part of the import Baz prefix, but should probably be treated separately.
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
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.
Problem
An end-of-line comment on an import line is stored in the LST as the prefix of the next import, not as a suffix of its own.
RemoveImportcarries a removed import's prefix forward to the following import so blank lines between groups are preserved (#3868), but it only forwardedgetLastWhitespace()— any comments in that prefix were silently discarded.Because of where comments are stored, removing an import touched comments that belong to neighbouring lines:
The most visible casualty was a Kotlin
// ktlint-disablesuppression on a wildcard import being removed when the import on the next line was renamed, which then broke the build with a fresh lint failure.Fix
When removing an import:
Behaviour, by example
Each example removes
java.util.ArrayList. A comment is kept unless it described the removed line itself.1. The removed import's own end-of-line comment → dropped (it described the import that's gone):
import java.util.List; -import java.util.ArrayList; // explains the removed import import java.util.UUID;2. The previous line's end-of-line comment → preserved (e.g. a ktlint suppression on a wildcard import — the original bug):
import java.util.List; // ktlint-disable no-wildcard-imports -import java.util.ArrayList; import java.util.UUID;3. A standalone / commented-out line before the removed import → preserved:
import java.util.List; // import java.util.Date -import java.util.ArrayList; import java.util.UUID;4. The following line's end-of-line comment → preserved:
import java.util.List; -import java.util.ArrayList; import java.util.UUID; // explains UUIDLast import removed
When the last import is removed there is no following import to carry its prefix onto — the follower is the first class declaration (or the EOF when there are none). The same rules apply, except the class keeps its own spacing (an import-group blank line is not pushed in front of it). So a preceding line's comment is still preserved:
import java.util.List; // ktlint-disable no-wildcard-imports -import java.util.ArrayList; class A { }…and the removed import's own comment is still dropped (rather than dangling onto the previous line):
import java.util.List; -import java.util.ArrayList; // explains the removed import class A { }Tests
RemoveImportTest(rewrite-java-test):dropsTrailingCommentOfRemovedImport,preservesTrailingCommentOnLineBeforeRemovedImport,preservesTrailingCommentAndBlankLineAfterRemovedImport,preservesCommentedOutLineBeforeRemovedImport,preservesTrailingCommentOnLineAfterRemovedImport, and for the last-import casedropsTrailingCommentOfRemovedLastImport,preservesPreviousLineCommentWhenLastImportRemoved,preservesCommentedOutLineWhenLastImportRemoved.RemoveImportTest(rewrite-kotlin):preservesKtlintSuppressionCommentOnLineBeforeRemovedImport,preservesPreviousLineCommentWhenLastImportRemoved.Context: moderneinc/customer-requests#2437.