From 2f453ce674c6f3f35610215ffc8944e0ca8b3281 Mon Sep 17 00:00:00 2001 From: Paul van Brenk Date: Thu, 3 Nov 2016 12:48:28 -0700 Subject: [PATCH] Handle most module cases --- src/harness/fourslash.ts | 2 +- .../codefixes/unusedIdentifierFixes.ts | 99 ++++++++++++++----- tests/cases/fourslash/unusedImports12FS.ts | 2 +- tests/cases/fourslash/unusedImports1FS.ts | 2 +- tests/cases/fourslash/unusedImports2FS.ts | 4 +- tests/cases/fourslash/unusedMethodInClass5.ts | 8 ++ tests/cases/fourslash/unusedMethodInClass6.ts | 8 ++ 7 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 tests/cases/fourslash/unusedMethodInClass5.ts create mode 100644 tests/cases/fourslash/unusedMethodInClass6.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index e58a189c923..fb1545d6aa4 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2055,7 +2055,7 @@ namespace FourSlash { const diagnostic = !errorCode ? diagnostics[0] : ts.find(diagnostics, d => d.code == errorCode); - return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.length, [diagnostic.code]); + return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.start + diagnostic.length, [diagnostic.code]); } public verifyCodeFixAtPosition(expectedText: string, errorCode?: number) { diff --git a/src/services/codefixes/unusedIdentifierFixes.ts b/src/services/codefixes/unusedIdentifierFixes.ts index 1e8220d4b7a..e563b78583f 100644 --- a/src/services/codefixes/unusedIdentifierFixes.ts +++ b/src/services/codefixes/unusedIdentifierFixes.ts @@ -8,7 +8,13 @@ namespace ts.codefix { getCodeActions: (context: CodeFixContext) => { const sourceFile = context.sourceFile; const start = context.span.start; - const token = getTokenAtPosition(sourceFile, start); + + let token = getTokenAtPosition(sourceFile, start); + + // this handles var ["computed"] = 12; + if (token.kind === SyntaxKind.OpenBracketToken) { + token = getTokenAtPosition(sourceFile, start + 1); + } switch (token.kind) { case ts.SyntaxKind.Identifier: @@ -72,34 +78,43 @@ namespace ts.codefix { return removeSingleItem(functionDeclaration.parameters, token); } - case SyntaxKind.ImportSpecifier: - const namedImports = token.parent.parent; - const elements = namedImports.elements; - if (elements.length === 1) { - // Only 1 import and it is unused. So the entire line could be removed. - return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); - } - else { - return removeSingleItem(elements, token); - } - // handle case where 'import a = A;' case SyntaxKind.ImportEqualsDeclaration: - return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); + let importEquals = findImportDeclaration(token); + return createCodeFix("", importEquals.pos, importEquals.end - importEquals.pos); - // handle case where 'import d from './file' - case SyntaxKind.ImportClause: - return createCodeFix("", token.parent.parent.pos, token.parent.parent.end - token.parent.parent.pos); - - // handle case where 'import * as a from './file' - case SyntaxKind.NamespaceImport: - return createCodeFix("", token.parent.parent.parent.pos, token.parent.parent.parent.end - token.parent.parent.parent.pos); - - default: - if (isDeclarationName(token)) { - return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); + case SyntaxKind.ImportSpecifier: + const namedImports = token.parent.parent; + if (namedImports.elements.length === 1) { + // Only 1 import and it is unused. So the entire declaration should be removed. + let importSpec = findImportDeclaration(token); + return createCodeFix("", importSpec.pos, importSpec.end - importSpec.pos); + } + else { + return removeSingleItem(namedImports.elements, token); + } + + // 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 = token.parent; + if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'| + const importDecl = findImportDeclaration(importClause); + return createCodeFix("", importDecl.pos, importDecl.end - importDecl.pos); + } + else { // import |d,| * as ns from './file' + return createCodeFix("", importClause.name.pos, importClause.namedBindings.pos - importClause.name.pos); + } + + case SyntaxKind.NamespaceImport: + const namespaceImport = token.parent; + if(namespaceImport.name == token && !(namespaceImport.parent).name){ + const importDecl = findImportDeclaration(namespaceImport); + return createCodeFix("", importDecl.pos, importDecl.end - importDecl.pos); + } else { + const start = (namespaceImport.parent).name.end; + return createCodeFix("", start, (namespaceImport.parent).namedBindings.end - start); } - break; } break; @@ -109,8 +124,24 @@ namespace ts.codefix { case SyntaxKind.NamespaceImport: return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); } + if (isDeclarationName(token)) { + return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); + } + else if (isLiteralComputedPropertyDeclarationName(token)) { + return createCodeFix("", token.parent.parent.pos, token.parent.parent.end - token.parent.parent.pos); + } + else { + return undefined; + } - return undefined; + function findImportDeclaration(token: Node): Node { + let importDecl = token; + while (importDecl.kind != SyntaxKind.ImportDeclaration && importDecl.parent) { + importDecl = importDecl.parent; + } + + return importDecl; + } function createCodeFix(newText: string, start: number, length: number): CodeAction[] { return [{ @@ -133,3 +164,19 @@ namespace ts.codefix { } }); } + +const s = "hello"; + +class C { + + + + private ["string"]: string; + private "b iz": string; + + bar() { + this + } +} + + diff --git a/tests/cases/fourslash/unusedImports12FS.ts b/tests/cases/fourslash/unusedImports12FS.ts index 647b7dfbe16..4d8243eb4ab 100644 --- a/tests/cases/fourslash/unusedImports12FS.ts +++ b/tests/cases/fourslash/unusedImports12FS.ts @@ -10,4 +10,4 @@ //// export function f2(s: string){}; //// export default f1; -verify.codeFixAtPosition('import f1 "./file1";'); \ No newline at end of file +verify.codeFixAtPosition('import f1 from "./file1";'); \ No newline at end of file diff --git a/tests/cases/fourslash/unusedImports1FS.ts b/tests/cases/fourslash/unusedImports1FS.ts index 411e9b3530b..d9fc17a2448 100644 --- a/tests/cases/fourslash/unusedImports1FS.ts +++ b/tests/cases/fourslash/unusedImports1FS.ts @@ -2,7 +2,7 @@ // @noUnusedLocals: true // @Filename: file2.ts -//// [| import { Calculator } from "./file1" |] +//// [|import { Calculator } from "./file1" |] // @Filename: file1.ts //// export class Calculator { diff --git a/tests/cases/fourslash/unusedImports2FS.ts b/tests/cases/fourslash/unusedImports2FS.ts index a27f568b0d2..6e3d9527c52 100644 --- a/tests/cases/fourslash/unusedImports2FS.ts +++ b/tests/cases/fourslash/unusedImports2FS.ts @@ -2,8 +2,8 @@ // @noUnusedLocals: true // @Filename: file2.ts -//// [| import {Calculator} from "./file1" -//// import {test} from "./file1" |] +//// [|import {Calculator} from "./file1" +//// import {test} from "./file1"|] //// var x = new Calculator(); //// x.handleChar(); diff --git a/tests/cases/fourslash/unusedMethodInClass5.ts b/tests/cases/fourslash/unusedMethodInClass5.ts new file mode 100644 index 00000000000..36ef3670cac --- /dev/null +++ b/tests/cases/fourslash/unusedMethodInClass5.ts @@ -0,0 +1,8 @@ +/// + +// @noUnusedLocals: true +//// [|class C { +//// private ["string"] (){} +//// }|] + +verify.codeFixAtPosition("class C { }"); \ No newline at end of file diff --git a/tests/cases/fourslash/unusedMethodInClass6.ts b/tests/cases/fourslash/unusedMethodInClass6.ts new file mode 100644 index 00000000000..71fcb92d037 --- /dev/null +++ b/tests/cases/fourslash/unusedMethodInClass6.ts @@ -0,0 +1,8 @@ +/// + +// @noUnusedLocals: true +//// [|class C { +//// private "string" (){} +//// }|] + +verify.codeFixAtPosition("class C { }"); \ No newline at end of file