Remove spurious leading newline in AttributeRemover#3335
Open
broken-circle wants to merge 1 commit into
Open
Conversation
When the removed attribute's leading trivia contains the newline that ends its line, the existing backward-trim drops the newline and the trailing indentation. But when the attribute has no leading trivia of its own (e.g. it's the first token of the file), the line-terminating newline lives on the following node, out of reach of the backward-trim, and is preserved as a spurious leading newline. Detect that case and record that the next call to attach accumulated trivia should also drop one leading newline from the combined result, whether it ends up on the next kept attribute or the next token.
broken-circle
requested review from
ahoppen,
hamishknight,
hborla and
rintaro
as code owners
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
1 participant
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 #2269.
AttributeRemoverleft behind a leading newline when the removed attribute had no leading trivia of its own. The existing cleanup logic trims back through the attribute's leading trivia up to its last newline, which works when the attribute carries the line-terminating newline (e.g. removing@Testfrom@State\n@Test\nvar x), but does nothing when the newline lives downstream on a following node (e.g. removing@Testfrom@Test\nvar x, where the newline lives on the leading trivia ofvar).This PR adds a complementary forward-trim signal. When an attribute that occupies its own line is removed, but it contributes no line-terminating newline of its own, a flag is set that causes the next
prependAndClearAccumulatedTrivia(to:)call to drop one leading newline from the combinedtriviaToAttachToNextToken + node.leadingTrivia. A kept attribute encountered before the next token consumes the flag, so the newline is dropped from whichever node the trivia attaches to.Testing
This PR updates 5 existing tests in
AttributeRemoverTeststo expect the correct reduction.