diff --git a/src/services/codefixes/convertFunctionToEs6Class.ts b/src/services/codefixes/convertFunctionToEs6Class.ts index b98f2abb708fc..c204117988855 100644 --- a/src/services/codefixes/convertFunctionToEs6Class.ts +++ b/src/services/codefixes/convertFunctionToEs6Class.ts @@ -13,7 +13,6 @@ namespace ts.codefix { }); function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, checker: TypeChecker): void { - const deletedNodes: { node: Node, inList: boolean }[] = []; const ctorSymbol = checker.getSymbolAtLocation(getTokenAtPosition(sourceFile, position))!; if (!ctorSymbol || !(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) { @@ -28,7 +27,7 @@ namespace ts.codefix { switch (ctorDeclaration.kind) { case SyntaxKind.FunctionDeclaration: precedingNode = ctorDeclaration; - deleteNode(ctorDeclaration); + changes.delete(sourceFile, ctorDeclaration); newClassDeclaration = createClassFromFunctionDeclaration(ctorDeclaration as FunctionDeclaration); break; @@ -37,10 +36,10 @@ namespace ts.codefix { newClassDeclaration = createClassFromVariableDeclaration(ctorDeclaration as VariableDeclaration); if ((ctorDeclaration.parent).declarations.length === 1) { copyComments(precedingNode, newClassDeclaration!, sourceFile); // TODO: GH#18217 - deleteNode(precedingNode); + changes.delete(sourceFile, precedingNode); } else { - deleteNode(ctorDeclaration, /*inList*/ true); + changes.delete(sourceFile, ctorDeclaration); } break; } @@ -53,21 +52,6 @@ namespace ts.codefix { // Because the preceding node could be touched, we need to insert nodes before delete nodes. changes.insertNodeAfter(sourceFile, precedingNode!, newClassDeclaration); - for (const { node, inList } of deletedNodes) { - if (inList) { - changes.deleteNodeInList(sourceFile, node); - } - else { - changes.deleteNode(sourceFile, node); - } - } - - function deleteNode(node: Node, inList = false) { - // If parent node has already been deleted, do nothing - if (!deletedNodes.some(n => isNodeDescendantOf(node, n.node))) { - deletedNodes.push({ node, inList }); - } - } function createClassElementsFromSymbol(symbol: Symbol) { const memberElements: ClassElement[] = []; @@ -115,7 +99,7 @@ namespace ts.codefix { // delete the entire statement if this expression is the sole expression to take care of the semicolon at the end const nodeToDelete = assignmentBinaryExpression.parent && assignmentBinaryExpression.parent.kind === SyntaxKind.ExpressionStatement ? assignmentBinaryExpression.parent : assignmentBinaryExpression; - deleteNode(nodeToDelete); + changes.delete(sourceFile, nodeToDelete); if (!assignmentBinaryExpression.right) { return createProperty([], modifiers, symbol.name, /*questionToken*/ undefined, diff --git a/src/services/codefixes/convertToEs6Module.ts b/src/services/codefixes/convertToEs6Module.ts index bd1e02845cd86..87a1f49cc4864 100644 --- a/src/services/codefixes/convertToEs6Module.ts +++ b/src/services/codefixes/convertToEs6Module.ts @@ -197,7 +197,7 @@ namespace ts.codefix { if (isExportsOrModuleExportsOrAlias(sourceFile, left)) { if (isExportsOrModuleExportsOrAlias(sourceFile, right)) { // `const alias = module.exports;` or `module.exports = alias;` can be removed. - changes.deleteNode(sourceFile, assignment.parent); + changes.delete(sourceFile, assignment.parent); } else { const replacement = isObjectLiteralExpression(right) ? tryChangeModuleExportsObject(right) @@ -297,7 +297,7 @@ namespace ts.codefix { if (!right.name) changes.insertName(sourceFile, right, name); const semi = findChildOfKind(parent, SyntaxKind.SemicolonToken, sourceFile); - if (semi) changes.deleteNode(sourceFile, semi, { useNonAdjustedEndPosition: true }); + if (semi) changes.delete(sourceFile, semi); } else { // `exports.f = function g() {}` -> `export const f = function g() {}` -- just replace `exports.` with `export const ` diff --git a/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts b/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts index 8ee9b60051f84..1c26c39b10c1a 100644 --- a/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts +++ b/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts @@ -29,7 +29,7 @@ namespace ts.codefix { function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, constructor: ConstructorDeclaration, superCall: ExpressionStatement): void { changes.insertNodeAtConstructorStart(sourceFile, constructor, superCall); - changes.deleteNode(sourceFile, superCall); + changes.delete(sourceFile, superCall); } function getNodes(sourceFile: SourceFile, pos: number): { readonly constructor: ConstructorDeclaration, readonly superCall: ExpressionStatement } | undefined { diff --git a/src/services/codefixes/fixUnreachableCode.ts b/src/services/codefixes/fixUnreachableCode.ts index 6c279d75ab393..1c6c53efe8e60 100644 --- a/src/services/codefixes/fixUnreachableCode.ts +++ b/src/services/codefixes/fixUnreachableCode.ts @@ -32,14 +32,14 @@ namespace ts.codefix { // falls through case SyntaxKind.WhileStatement: case SyntaxKind.ForStatement: - changes.deleteNode(sourceFile, container); + changes.delete(sourceFile, container); break; default: if (isBlock(statement.parent)) { split(sliceAfter(statement.parent.statements, statement), shouldRemove, (start, end) => changes.deleteNodeRange(sourceFile, start, end)); } else { - changes.deleteNode(sourceFile, statement); + changes.delete(sourceFile, statement); } } } diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index d164d4eee9fdf..f70c9ce648222 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -22,7 +22,7 @@ namespace ts.codefix { const importDecl = tryGetFullImport(token); if (importDecl) { - const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, importDecl)); + const changes = textChanges.ChangeTracker.with(context, t => t.delete(sourceFile, importDecl)); return [createCodeFixAction(fixName, changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)], fixIdDelete, Diagnostics.Delete_all_unused_declarations)]; } const delDestructure = textChanges.ChangeTracker.with(context, t => @@ -67,7 +67,7 @@ namespace ts.codefix { case fixIdDelete: { const importDecl = tryGetFullImport(token); if (importDecl) { - changes.deleteDeclaration(sourceFile, importDecl); + changes.delete(sourceFile, importDecl); } else if (!tryDeleteFullDestructure(token, changes, sourceFile, checker, sourceFiles, /*isFixAll*/ true) && !tryDeleteFullVariableStatement(sourceFile, token, changes)) { @@ -94,7 +94,7 @@ namespace ts.codefix { tryDeleteParameter(changes, sourceFile, decl, checker, sourceFiles, isFixAll); } else { - changes.deleteDeclaration(sourceFile, decl); + changes.delete(sourceFile, decl); } return true; } @@ -102,7 +102,7 @@ namespace ts.codefix { function tryDeleteFullVariableStatement(sourceFile: SourceFile, token: Node, changes: textChanges.ChangeTracker): boolean { const declarationList = tryCast(token.parent, isVariableDeclarationList); if (declarationList && declarationList.getChildren(sourceFile)[0] === token) { - changes.deleteDeclaration(sourceFile, declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList); + changes.delete(sourceFile, declarationList.parent.kind === SyntaxKind.VariableStatement ? declarationList.parent : declarationList); return true; } return false; @@ -140,7 +140,7 @@ namespace ts.codefix { FindAllReferences.Core.eachSymbolReferenceInFile(token, checker, sourceFile, (ref: Node) => { if (ref.parent.kind === SyntaxKind.PropertyAccessExpression) ref = ref.parent; if (ref.parent.kind === SyntaxKind.BinaryExpression && ref.parent.parent.kind === SyntaxKind.ExpressionStatement) { - changes.deleteDeclaration(sourceFile, ref.parent.parent); + changes.delete(sourceFile, ref.parent.parent); } }); } @@ -151,13 +151,13 @@ namespace ts.codefix { tryDeleteParameter(changes, sourceFile, parent, checker, sourceFiles, isFixAll); } else { - changes.deleteDeclaration(sourceFile, isImportClause(parent) ? token : isComputedPropertyName(parent) ? parent.parent : parent); + changes.delete(sourceFile, isImportClause(parent) ? token : isComputedPropertyName(parent) ? parent.parent : parent); } } function tryDeleteParameter(changes: textChanges.ChangeTracker, sourceFile: SourceFile, p: ParameterDeclaration, checker: TypeChecker, sourceFiles: ReadonlyArray, isFixAll: boolean): void { if (mayDeleteParameter(p, checker, isFixAll)) { - changes.deleteDeclaration(sourceFile, p); + changes.delete(sourceFile, p); deleteUnusedArguments(changes, sourceFile, p, sourceFiles, checker); } } @@ -199,7 +199,7 @@ namespace ts.codefix { FindAllReferences.Core.eachSignatureCall(deletedParameter.parent, sourceFiles, checker, call => { const index = deletedParameter.parent.parameters.indexOf(deletedParameter); if (call.arguments.length > index) { // Just in case the call didn't provide enough arguments. - changes.deleteDeclaration(sourceFile, call.arguments[index]); + changes.delete(sourceFile, call.arguments[index]); } }); } diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 7e4affb14aa6b..460cb3dd5a282 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -63,10 +63,7 @@ namespace ts.OrganizeImports { // Delete or replace the first import. if (newImportDecls.length === 0) { - changeTracker.deleteNode(sourceFile, oldImportDecls[0], { - useNonAdjustedStartPosition: true, // Leave header comment in place - useNonAdjustedEndPosition: false, - }); + changeTracker.delete(sourceFile, oldImportDecls[0]); } else { // Note: Delete the surrounding trivia because it will have been retained in newImportDecls. @@ -79,7 +76,7 @@ namespace ts.OrganizeImports { // Delete any subsequent imports. for (let i = 1; i < oldImportDecls.length; i++) { - changeTracker.deleteNode(sourceFile, oldImportDecls[i]); + changeTracker.delete(sourceFile, oldImportDecls[i]); } } } diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index 9ce9fcf264b47..ca31ef5810044 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -78,7 +78,7 @@ namespace ts.refactor { function changeExport(exportingSourceFile: SourceFile, { wasDefault, exportNode, exportName }: Info, changes: textChanges.ChangeTracker, checker: TypeChecker): void { if (wasDefault) { - changes.deleteNode(exportingSourceFile, Debug.assertDefined(findModifier(exportNode, SyntaxKind.DefaultKeyword))); + changes.delete(exportingSourceFile, Debug.assertDefined(findModifier(exportNode, SyntaxKind.DefaultKeyword))); } else { const exportKeyword = Debug.assertDefined(findModifier(exportNode, SyntaxKind.ExportKeyword)); @@ -155,7 +155,7 @@ namespace ts.refactor { } else { // `import foo, { bar } from "./a"` --> `import { bar, foo } from "./a";` - changes.deleteNode(importingSourceFile, ref); + changes.delete(importingSourceFile, ref); changes.insertNodeAtEndOfList(importingSourceFile, namedBindings.elements, spec); } break; @@ -183,7 +183,7 @@ namespace ts.refactor { changes.replaceNode(importingSourceFile, spec.parent, defaultImport); } else { - changes.deleteNodeInList(importingSourceFile, spec); + changes.delete(importingSourceFile, spec); changes.insertNodeBefore(importingSourceFile, spec.parent, defaultImport); } } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index ade704e36404b..d57ab6e478abb 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -1090,7 +1090,7 @@ namespace ts.refactor.extractSymbol { // Consume if (node.parent.kind === SyntaxKind.ExpressionStatement) { // If the parent is an expression statement, delete it. - changeTracker.deleteNode(context.file, node.parent, textChanges.useNonAdjustedPositions); + changeTracker.delete(context.file, node.parent); } else { const localReference = createIdentifier(localNameText); diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index f050e8fb174f7..b10f2969906cb 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -325,7 +325,7 @@ namespace ts.refactor { break; case SyntaxKind.ImportEqualsDeclaration: if (isUnused(importDecl.name)) { - changes.deleteNode(sourceFile, importDecl); + changes.delete(sourceFile, importDecl); } break; case SyntaxKind.VariableDeclaration: @@ -342,19 +342,19 @@ namespace ts.refactor { const namedBindingsUnused = !namedBindings || (namedBindings.kind === SyntaxKind.NamespaceImport ? isUnused(namedBindings.name) : namedBindings.elements.every(e => isUnused(e.name))); if (defaultUnused && namedBindingsUnused) { - changes.deleteNode(sourceFile, importDecl); + changes.delete(sourceFile, importDecl); } else { if (name && defaultUnused) { - changes.deleteNode(sourceFile, name); + changes.delete(sourceFile, name); } if (namedBindings) { if (namedBindingsUnused) { - changes.deleteNode(sourceFile, namedBindings); + changes.delete(sourceFile, namedBindings); } else if (namedBindings.kind === SyntaxKind.NamedImports) { for (const element of namedBindings.elements) { - if (isUnused(element.name)) changes.deleteNodeInList(sourceFile, element); + if (isUnused(element.name)) changes.delete(sourceFile, element); } } } @@ -365,20 +365,20 @@ namespace ts.refactor { switch (name.kind) { case SyntaxKind.Identifier: if (isUnused(name)) { - changes.deleteNode(sourceFile, name); + changes.delete(sourceFile, name); } break; case SyntaxKind.ArrayBindingPattern: break; case SyntaxKind.ObjectBindingPattern: if (name.elements.every(e => isIdentifier(e.name) && isUnused(e.name))) { - changes.deleteNode(sourceFile, + changes.delete(sourceFile, isVariableDeclarationList(varDecl.parent) && varDecl.parent.declarations.length === 1 ? varDecl.parent.parent : varDecl); } else { for (const element of name.elements) { if (isIdentifier(element.name) && isUnused(element.name)) { - changes.deleteNode(sourceFile, element.name); + changes.delete(sourceFile, element.name); } } } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index ba2b8bfc96af8..374893830a72c 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -212,9 +212,8 @@ namespace ts.textChanges { export class ChangeTracker { private readonly changes: Change[] = []; private readonly newFiles: { readonly oldFile: SourceFile, readonly fileName: string, readonly statements: ReadonlyArray }[] = []; - private readonly deletedNodesInLists = new NodeSet(); // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`. private readonly classesWithNodesInsertedAtStart = createMap(); // Set implemented as Map - private readonly deletedDeclarations: { readonly sourceFile: SourceFile, readonly node: Node }[] = []; + private readonly deletedNodes: { readonly sourceFile: SourceFile, readonly node: Node }[] = []; public static fromContext(context: TextChangesContext): ChangeTracker { return new ChangeTracker(getNewLineOrDefaultFromHost(context.host, context.formatContext.options), context.formatContext); @@ -234,16 +233,8 @@ namespace ts.textChanges { return this; } - deleteDeclaration(sourceFile: SourceFile, node: Node): void { - this.deletedDeclarations.push({ sourceFile, node }); - } - - /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */ - public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}) { - const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart); - const endPosition = getAdjustedEndPosition(sourceFile, node, options); - this.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); - return this; + delete(sourceFile: SourceFile, node: Node): void { + this.deletedNodes.push({ sourceFile, node, }); } public deleteModifier(sourceFile: SourceFile, modifier: Modifier): void { @@ -263,32 +254,6 @@ namespace ts.textChanges { this.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); } - public deleteNodeInList(sourceFile: SourceFile, node: Node) { - const containingList = formatting.SmartIndenter.getContainingList(node, sourceFile); - if (!containingList) { - Debug.fail("node is not a list element"); - return this; - } - const index = indexOfNode(containingList, node); - if (index < 0) { - return this; - } - if (containingList.length === 1) { - this.deleteNode(sourceFile, node); - return this; - } - - // Note: We will only delete a comma *after* a node. This will leave a trailing comma if we delete the last node. - // That's handled in the end by `finishTrailingCommaAfterDeletingNodesInList`. - Debug.assert(!this.deletedNodesInLists.has(node), "Deleting a node twice"); - this.deletedNodesInLists.add(node); - this.deleteRange(sourceFile, { - pos: startPositionToDeleteNodeInList(sourceFile, node), - end: index === containingList.length - 1 ? getAdjustedEndPosition(sourceFile, node, {}) : startPositionToDeleteNodeInList(sourceFile, containingList[index + 1]), - }); - return this; - } - public replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options: InsertNodeOptions = {}) { this.changes.push({ kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range, options, node: newNode }); return this; @@ -538,7 +503,7 @@ namespace ts.textChanges { if (lparen) { // `() => {}` --> `function f() {}` this.insertNodesAt(sourceFile, lparen.getStart(sourceFile), [createToken(SyntaxKind.FunctionKeyword), createIdentifier(name)], { joiner: " " }); - this.deleteNode(sourceFile, arrow); + deleteNode(this, sourceFile, arrow); } else { // `x => {}` -> `function f(x) {}` @@ -691,27 +656,26 @@ namespace ts.textChanges { }); } - private finishTrailingCommaAfterDeletingNodesInList() { - this.deletedNodesInLists.forEach(node => { + private finishDeleteDeclarations(): void { + const deletedNodesInLists = new NodeSet(); // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`. + for (const { sourceFile, node } of this.deletedNodes) { + if (!this.deletedNodes.some(d => d.sourceFile === sourceFile && rangeContainsRangeExclusive(d.node, node))) { + deleteDeclaration.deleteDeclaration(this, deletedNodesInLists, sourceFile, node); + } + } + + deletedNodesInLists.forEach(node => { const sourceFile = node.getSourceFile(); const list = formatting.SmartIndenter.getContainingList(node, sourceFile)!; if (node !== last(list)) return; - const lastNonDeletedIndex = findLastIndex(list, n => !this.deletedNodesInLists.has(n), list.length - 2); + const lastNonDeletedIndex = findLastIndex(list, n => !deletedNodesInLists.has(n), list.length - 2); if (lastNonDeletedIndex !== -1) { this.deleteRange(sourceFile, { pos: list[lastNonDeletedIndex].end, end: startPositionToDeleteNodeInList(sourceFile, list[lastNonDeletedIndex + 1]) }); } }); } - private finishDeleteDeclarations(): void { - for (const { sourceFile, node } of this.deletedDeclarations) { - if (!this.deletedDeclarations.some(d => d.sourceFile === sourceFile && rangeContainsRangeExclusive(d.node, node))) { - deleteDeclaration.deleteDeclaration(this, sourceFile, node); - } - } - } - /** * Note: after calling this, the TextChanges object must be discarded! * @param validate only for tests @@ -721,7 +685,6 @@ namespace ts.textChanges { public getChanges(validate?: ValidateNonFormattedText): FileTextChanges[] { this.finishDeleteDeclarations(); this.finishClassesWithNodesInsertedAtStart(); - this.finishTrailingCommaAfterDeletingNodesInList(); const changes = changesToText.getTextChangesFromChanges(this.changes, this.newLineCharacter, this.formatContext, validate); for (const { oldFile, fileName, statements } of this.newFiles) { changes.push(changesToText.newFileChanges(oldFile, fileName, statements, this.newLineCharacter, this.formatContext)); @@ -1037,7 +1000,7 @@ namespace ts.textChanges { } namespace deleteDeclaration { - export function deleteDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: Node): void { + export function deleteDeclaration(changes: ChangeTracker, deletedNodesInLists: NodeSet, sourceFile: SourceFile, node: Node): void { switch (node.kind) { case SyntaxKind.Parameter: { const oldFunction = node.parent; @@ -1062,28 +1025,30 @@ namespace ts.textChanges { changes.replaceNode(sourceFile, oldFunction, newFunction); } else { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } break; } case SyntaxKind.ImportDeclaration: - changes.deleteNode(sourceFile, node); + deleteNode(changes, sourceFile, node, + // For first import, leave header comment in place + node === sourceFile.imports[0].parent ? { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: false } : undefined); break; case SyntaxKind.BindingElement: const pattern = (node as BindingElement).parent; const preserveComma = pattern.kind === SyntaxKind.ArrayBindingPattern && node !== last(pattern.elements); if (preserveComma) { - changes.deleteNode(sourceFile, node); + deleteNode(changes, sourceFile, node); } else { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } break; case SyntaxKind.VariableDeclaration: - deleteVariableDeclaration(changes, sourceFile, node as VariableDeclaration); + deleteVariableDeclaration(changes, deletedNodesInLists, sourceFile, node as VariableDeclaration); break; case SyntaxKind.TypeParameter: { @@ -1098,7 +1063,7 @@ namespace ts.textChanges { changes.deleteNodeRange(sourceFile, previousToken, nextToken); } else { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } break; } @@ -1109,7 +1074,7 @@ namespace ts.textChanges { deleteImportBinding(changes, sourceFile, namedImports); } else { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } break; @@ -1122,10 +1087,10 @@ namespace ts.textChanges { deleteDefaultImport(changes, sourceFile, node.parent); } else if (isCallLikeExpression(node.parent)) { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } else { - changes.deleteNode(sourceFile, node); + deleteNode(changes, sourceFile, node, node.kind === SyntaxKind.SemicolonToken ? { useNonAdjustedEndPosition: true } : undefined); } } } @@ -1133,7 +1098,7 @@ namespace ts.textChanges { function deleteDefaultImport(changes: ChangeTracker, sourceFile: SourceFile, importClause: ImportClause): void { if (!importClause.namedBindings) { // Delete the whole import - changes.deleteNode(sourceFile, importClause.parent); + deleteNode(changes, sourceFile, importClause.parent); } else { // import |d,| * as ns from './file' @@ -1145,7 +1110,7 @@ namespace ts.textChanges { changes.deleteRange(sourceFile, { pos: start, end }); } else { - changes.deleteNode(sourceFile, importClause.name!); + deleteNode(changes, sourceFile, importClause.name!); } } } @@ -1163,11 +1128,11 @@ namespace ts.textChanges { // |import * as ns from './file'| // |import { a } from './file'| const importDecl = getAncestor(node, SyntaxKind.ImportDeclaration)!; - changes.deleteNode(sourceFile, importDecl); + deleteNode(changes, sourceFile, importDecl); } } - function deleteVariableDeclaration(changes: ChangeTracker, sourceFile: SourceFile, node: VariableDeclaration): void { + function deleteVariableDeclaration(changes: ChangeTracker, deletedNodesInLists: NodeSet, sourceFile: SourceFile, node: VariableDeclaration): void { const { parent } = node; if (parent.kind === SyntaxKind.CatchClause) { @@ -1177,7 +1142,7 @@ namespace ts.textChanges { } if (parent.declarations.length !== 1) { - changes.deleteNodeInList(sourceFile, node); + deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); return; } @@ -1189,11 +1154,11 @@ namespace ts.textChanges { break; case SyntaxKind.ForStatement: - changes.deleteNode(sourceFile, parent); + deleteNode(changes, sourceFile, parent); break; case SyntaxKind.VariableStatement: - changes.deleteNode(sourceFile, gp); + deleteNode(changes, sourceFile, gp); break; default: @@ -1201,4 +1166,31 @@ namespace ts.textChanges { } } } + + /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */ + // Exported for tests only! (TODO: improve tests to not need this) + export function deleteNode(changes: ChangeTracker, sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}): void { + const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart); + const endPosition = getAdjustedEndPosition(sourceFile, node, options); + changes.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); + } + + function deleteNodeInList(changes: ChangeTracker, deletedNodesInLists: NodeSet, sourceFile: SourceFile, node: Node): void { + const containingList = Debug.assertDefined(formatting.SmartIndenter.getContainingList(node, sourceFile)); + const index = indexOfNode(containingList, node); + Debug.assert(index !== -1); + if (containingList.length === 1) { + deleteNode(changes, sourceFile, node); + return; + } + + // Note: We will only delete a comma *after* a node. This will leave a trailing comma if we delete the last node. + // That's handled in the end by `finishTrailingCommaAfterDeletingNodesInList`. + Debug.assert(!deletedNodesInLists.has(node), "Deleting a node twice"); + deletedNodesInLists.add(node); + changes.deleteRange(sourceFile, { + pos: startPositionToDeleteNodeInList(sourceFile, node), + end: index === containingList.length - 1 ? getAdjustedEndPosition(sourceFile, node, {}) : startPositionToDeleteNodeInList(sourceFile, containingList[index + 1]), + }); + } } diff --git a/src/testRunner/unittests/textChanges.ts b/src/testRunner/unittests/textChanges.ts index 0c06eecb51d31..a2ba91f04a2fa 100644 --- a/src/testRunner/unittests/textChanges.ts +++ b/src/testRunner/unittests/textChanges.ts @@ -128,6 +128,7 @@ function bar() { function findVariableDeclarationContaining(name: string, sourceFile: SourceFile): VariableDeclaration { return cast(findChild(name, sourceFile), isVariableDeclaration); } + const { deleteNode } = textChanges; { const text = ` var x = 1; // some comment - 1 @@ -138,19 +139,19 @@ var y = 2; // comment 3 var z = 3; // comment 4 `; runSingleFileTest("deleteNode1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findVariableStatementContaining("y", sourceFile)); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile)); }); runSingleFileTest("deleteNode2", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true }); }); runSingleFileTest("deleteNode3", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedEndPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedEndPosition: true }); }); runSingleFileTest("deleteNode4", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); }); runSingleFileTest("deleteNode5", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findVariableStatementContaining("x", sourceFile)); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("x", sourceFile)); }); } { @@ -377,25 +378,25 @@ class A { { const text = `var a = 1, b = 2, c = 3;`; runSingleFileTest("deleteNodeInList1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList2", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList3", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { const text = `var a = 1,b = 2,c = 3;`; runSingleFileTest("deleteNodeInList1_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList2_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList3_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -406,13 +407,13 @@ namespace M { c = 3; }`; runSingleFileTest("deleteNodeInList4", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList5", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList6", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -425,13 +426,13 @@ namespace M { c = 3; // comment 5 }`; runSingleFileTest("deleteNodeInList4_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList5_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList6_1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -440,13 +441,13 @@ function foo(a: number, b: string, c = true) { return 1; }`; runSingleFileTest("deleteNodeInList7", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList8", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList9", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -455,13 +456,13 @@ function foo(a: number,b: string,c = true) { return 1; }`; runSingleFileTest("deleteNodeInList10", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList11", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList12", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -473,13 +474,13 @@ function foo( return 1; }`; runSingleFileTest("deleteNodeInList13", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("a", sourceFile)); + changeTracker.delete(sourceFile, findChild("a", sourceFile)); }); runSingleFileTest("deleteNodeInList14", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("b", sourceFile)); + changeTracker.delete(sourceFile, findChild("b", sourceFile)); }); runSingleFileTest("deleteNodeInList15", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNodeInList(sourceFile, findChild("c", sourceFile)); + changeTracker.delete(sourceFile, findChild("c", sourceFile)); }); } { @@ -661,7 +662,7 @@ class A { } `; runSingleFileTest("deleteNodeAfterInClass1", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findChild("x", sourceFile)); + deleteNode(changeTracker, sourceFile, findChild("x", sourceFile)); }); } { @@ -672,7 +673,7 @@ class A { } `; runSingleFileTest("deleteNodeAfterInClass2", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - changeTracker.deleteNode(sourceFile, findChild("x", sourceFile)); + deleteNode(changeTracker, sourceFile, findChild("x", sourceFile)); }); } { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index a3b6164245c9c..ca0cd34b9ab0b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -11510,21 +11510,17 @@ declare namespace ts.textChanges { private readonly formatContext; private readonly changes; private readonly newFiles; - private readonly deletedNodesInLists; private readonly classesWithNodesInsertedAtStart; - private readonly deletedDeclarations; + private readonly deletedNodes; static fromContext(context: TextChangesContext): ChangeTracker; static with(context: TextChangesContext, cb: (tracker: ChangeTracker) => void): FileTextChanges[]; /** Public for tests only. Other callers should use `ChangeTracker.with`. */ constructor(newLineCharacter: string, formatContext: formatting.FormatContext); deleteRange(sourceFile: SourceFile, range: TextRange): this; - deleteDeclaration(sourceFile: SourceFile, node: Node): void; - /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */ - deleteNode(sourceFile: SourceFile, node: Node, options?: ConfigurableStartEnd): this; + delete(sourceFile: SourceFile, node: Node): void; deleteModifier(sourceFile: SourceFile, modifier: Modifier): void; deleteNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options?: ConfigurableStartEnd): this; deleteNodeRangeExcludingEnd(sourceFile: SourceFile, startNode: Node, afterEndNode: Node | undefined, options?: ConfigurableStartEnd): void; - deleteNodeInList(sourceFile: SourceFile, node: Node): this; replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options?: InsertNodeOptions): this; replaceNode(sourceFile: SourceFile, oldNode: Node, newNode: Node, options?: ChangeNodeOptions): this; replaceNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, newNode: Node, options?: ChangeNodeOptions): void; @@ -11567,7 +11563,6 @@ declare namespace ts.textChanges { */ insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node, containingList?: NodeArray | undefined): this; private finishClassesWithNodesInsertedAtStart; - private finishTrailingCommaAfterDeletingNodesInList; private finishDeleteDeclarations; /** * Note: after calling this, the TextChanges object must be discarded! @@ -11581,6 +11576,8 @@ declare namespace ts.textChanges { type ValidateNonFormattedText = (node: Node, text: string) => void; function applyChanges(text: string, changes: ReadonlyArray): string; function isValidLocationToAddComment(sourceFile: SourceFile, position: number): boolean; + /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */ + function deleteNode(changes: ChangeTracker, sourceFile: SourceFile, node: Node, options?: ConfigurableStartEnd): void; } declare namespace ts { interface CodeFixRegistration { diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js index 23cdfee82812a..2813287690332 100644 --- a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.js @@ -17,6 +17,5 @@ function F() { let i = 0; const /*RENAME*/newLocal = i++; function F() { - } \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts index 23cdfee82812a..2813287690332 100644 --- a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts +++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementInNestedScope.ts @@ -17,6 +17,5 @@ function F() { let i = 0; const /*RENAME*/newLocal = i++; function F() { - } \ No newline at end of file diff --git a/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts b/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts index a75ef81b1ed39..f8993a2a72548 100644 --- a/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts +++ b/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts @@ -7,5 +7,7 @@ verify.codeFix({ description: "Remove import from 'mod'", - newFileContent: " // trailing trivia", + newFileContent: +`// leading trivia + // trailing trivia`, });