diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 60208fe8ae3..5e83c5339c6 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1806,7 +1806,14 @@ namespace FourSlash { this.editScriptAndUpdateMarkers(fileName, offsetStart, offsetEnd, edit.newText); const editDelta = edit.newText.length - edit.span.length; if (offsetStart <= this.currentCaretPosition) { - this.currentCaretPosition += editDelta; + if (offsetEnd <= this.currentCaretPosition) { + // The entirety of the edit span falls before the caret position, shift the caret accordingly + this.currentCaretPosition += editDelta; + } + else { + // The span being replaced includes the caret position, place the caret at the beginning of the span + this.currentCaretPosition = offsetStart; + } } runningOffset += editDelta; // TODO: Consider doing this at least some of the time for higher fidelity. Currently causes a failure (bug 707150) @@ -2354,7 +2361,7 @@ namespace FourSlash { private applyCodeActions(actions: ts.CodeAction[], index?: number): void { if (index === undefined) { if (!(actions && actions.length === 1)) { - this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found.`); + this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found. ${actions ? actions.map(a => `${Harness.IO.newLine()} "${a.description}"`) : "" }`); } index = 0; } diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index cbe2ba5b1c5..18491aaba26 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -25,15 +25,15 @@ namespace ts.codefix { return [deleteNode(token.parent)]; default: - return [deleteDefault()]; + return deleteDefault(); } - function deleteDefault() { + function deleteDefault(): CodeAction[] | undefined { if (isDeclarationName(token)) { - return deleteNode(token.parent); + return [deleteNode(token.parent)]; } else if (isLiteralComputedPropertyDeclarationName(token)) { - return deleteNode(token.parent.parent); + return [deleteNode(token.parent.parent)]; } else { return undefined; @@ -87,20 +87,16 @@ namespace ts.codefix { case SyntaxKind.ImportSpecifier: const namedImports = parent.parent; if (namedImports.elements.length === 1) { - // Only 1 import and it is unused. So the entire declaration should be removed. - const importSpec = getAncestor(identifier, SyntaxKind.ImportDeclaration); - return [deleteNode(importSpec)]; + return deleteNamedImportBinding(namedImports); } else { // delete import specifier return [deleteNodeInList(parent)]; } - // handle case where "import d, * as ns from './file'" - // or "'import {a, b as ns} from './file'" case SyntaxKind.ImportClause: // this covers both 'import |d|' and 'import |d,| *' const importClause = parent; - if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'| + if (!importClause.namedBindings) { // |import d from './file'| const importDecl = getAncestor(importClause, SyntaxKind.ImportDeclaration); return [deleteNode(importDecl)]; } @@ -118,22 +114,30 @@ namespace ts.codefix { } case SyntaxKind.NamespaceImport: - const namespaceImport = parent; - if (namespaceImport.name === identifier && !(namespaceImport.parent).name) { - const importDecl = getAncestor(namespaceImport, SyntaxKind.ImportDeclaration); - return [deleteNode(importDecl)]; - } - else { - const previousToken = getTokenAtPosition(sourceFile, namespaceImport.pos - 1, /*includeJsDocComment*/ false); - if (previousToken && previousToken.kind === SyntaxKind.CommaToken) { - const startPosition = textChanges.getAdjustedStartPosition(sourceFile, previousToken, {}, textChanges.Position.FullStart); - return [deleteRange({ pos: startPosition, end: namespaceImport.end })]; - } - return [deleteRange(namespaceImport)]; - } + return deleteNamedImportBinding(parent); default: - return [deleteDefault()]; + return deleteDefault(); + } + } + + function deleteNamedImportBinding(namedBindings: NamedImportBindings): CodeAction[] | undefined { + if ((namedBindings.parent).name) { + // Delete named imports while preserving the default import + // import d|, * as ns| from './file' + // import d|, { a }| from './file' + const previousToken = getTokenAtPosition(sourceFile, namedBindings.pos - 1, /*includeJsDocComment*/ false); + if (previousToken && previousToken.kind === SyntaxKind.CommaToken) { + return [deleteRange({ pos: previousToken.getStart(), end: namedBindings.end })]; + } + return undefined; + } + else { + // Delete the entire import declaration + // |import * as ns from './file'| + // |import { a } from './file'| + const importDecl = getAncestor(namedBindings, SyntaxKind.ImportDeclaration); + return [deleteNode(importDecl)]; } } diff --git a/tests/cases/fourslash/formattingOnEnter.ts b/tests/cases/fourslash/formattingOnEnter.ts index 1c0915e839e..0e7d5af1a6d 100644 --- a/tests/cases/fourslash/formattingOnEnter.ts +++ b/tests/cases/fourslash/formattingOnEnter.ts @@ -6,4 +6,8 @@ goTo.marker(); edit.insertLine(""); -verify.currentLineContentIs('class bar {'); +verify.currentFileContentIs( +`class foo { } +class bar { +} +// new line here`); diff --git a/tests/cases/fourslash/unusedImports13FS.ts b/tests/cases/fourslash/unusedImports13FS.ts new file mode 100644 index 00000000000..4e19f4d7f1a --- /dev/null +++ b/tests/cases/fourslash/unusedImports13FS.ts @@ -0,0 +1,12 @@ +/// + +// @noUnusedLocals: true +// @Filename: file2.ts +//// [| import A, { x } from './a'; |] +//// console.log(A); + +// @Filename: file1.ts +//// export default 10; +//// export var x = 10; + +verify.rangeAfterCodeFix("import A from './a';"); \ No newline at end of file diff --git a/tests/cases/fourslash/unusedImports14FS.ts b/tests/cases/fourslash/unusedImports14FS.ts new file mode 100644 index 00000000000..75124802d35 --- /dev/null +++ b/tests/cases/fourslash/unusedImports14FS.ts @@ -0,0 +1,15 @@ +/// + +// @noUnusedLocals: true +// @Filename: file2.ts +//// [| import /* 1 */ A /* 2 */, /* 3 */ { /* 4 */ x /* 5 */ } /* 6 */ from './a'; |] +//// console.log(A); + +// @Filename: file1.ts +//// export default 10; +//// export var x = 10; + + +// It's ambiguous which token comment /* 6 */ applies to or whether it should be removed. +// In the current implementation the comment is left behind, but this behavior isn't a requirement. +verify.rangeAfterCodeFix("import /* 1 */ A /* 2 */ /* 6 */ from './a';"); \ No newline at end of file