Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression organize imports duplicates comments #38599

Conversation

jessetrinity
Copy link
Contributor

Fixes #38507

#37467 started preserving comments when deleting unused declarations. When organize imports creates a sorted list of imports, it deletes the old imports in the same way. This caused us to start duplicating comments when organizing imports.

@@ -286,6 +286,12 @@ namespace ts.textChanges {
this.deletedNodes.push({ sourceFile, node });
}

public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = { leadingTriviaOption: LeadingTriviaOption.IncludeAll }): void {
const startPosition = getAdjustedStartPosition(sourceFile, node, options);
const endPosition = getAdjustedEndPosition(sourceFile, node, options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use getAdjustedRange instead

@jessetrinity jessetrinity merged commit 1a15717 into microsoft:master May 15, 2020
@jessetrinity jessetrinity deleted the fixRegressionOrganizeImportsComments branch May 15, 2020 21:25
cangSDARM added a commit to cangSDARM/TypeScript that referenced this pull request May 18, 2020
* upstream/master:
  Use control flow analysis to check 'super(...)' call before 'this' access (microsoft#38612)
  LEGO: check in for master to temporary branch.
  Make `processTaggedTemplateExpression` visit a returned node
  goToDefinition: find only the value if it's the RHS of an assignment
  Fix regression organize imports duplicates comments (microsoft#38599)
  Fix crash in JS declaration emit (microsoft#38508)
@jessetrinity
Copy link
Contributor Author

@typescript-bot cherry-pick this to release-3.9

@andrewbranch
Copy link
Member

@typescript-bot cherry-pick this to release-3.9

I wonder if you need to be added to a list/role somewhere

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 19, 2020

Heya @andrewbranch, I've started to run the task to cherry-pick this into release-3.9 on this PR at 81d1732. You can monitor the build here.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request May 19, 2020
Component commits:
428f5a1 delete import comments on organize imports

8003791 add unit test

26eaf70 accept new baseline

81d1732 respond to review comment
@typescript-bot
Copy link
Collaborator

Hey @andrewbranch, I've opened #38668 for you.

DanielRosenwasser added a commit that referenced this pull request May 19, 2020
🤖 Pick PR #38599 (Fix regression organize imports dup...) into release-3.9
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

source.organizeImports is replicating last comment line
4 participants