- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.1k
Codefix for removing Unused Identifiers #11546
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
Changes from all commits
05ec512
              28c08fd
              03a6eeb
              79fa9db
              1f94e14
              4097874
              d4b99d6
              b59714e
              7196018
              2f453ce
              40c0cbd
              78fdd44
              1924298
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1 +1,2 @@ | ||
| ///<reference path='superFixes.ts' /> | ||
| ///<reference path='unusedIdentifierFixes.ts' /> | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| /* @internal */ | ||
| namespace ts.codefix { | ||
| registerCodeFix({ | ||
| errorCodes: [ | ||
| Diagnostics._0_is_declared_but_never_used.code, | ||
| Diagnostics.Property_0_is_declared_but_never_used.code | ||
| ], | ||
| getCodeActions: (context: CodeFixContext) => { | ||
| const sourceFile = context.sourceFile; | ||
| const start = context.span.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: | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also please add handeling for computed properties with non-identifier names: class C {
    private ["string"] (){}
    private [Symbol.Iterator]() {}
} | ||
| switch (token.parent.kind) { | ||
| case ts.SyntaxKind.VariableDeclaration: | ||
| switch (token.parent.parent.parent.kind) { | ||
| case SyntaxKind.ForStatement: | ||
| const forStatement = <ForStatement>token.parent.parent.parent; | ||
| const forInitializer = <VariableDeclarationList>forStatement.initializer; | ||
| if (forInitializer.declarations.length === 1) { | ||
| return createCodeFix("", forInitializer.pos, forInitializer.end - forInitializer.pos); | ||
| } | ||
| else { | ||
| return removeSingleItem(forInitializer.declarations, token); | ||
| } | ||
|  | ||
| case SyntaxKind.ForOfStatement: | ||
| const forOfStatement = <ForOfStatement>token.parent.parent.parent; | ||
| if (forOfStatement.initializer.kind === SyntaxKind.VariableDeclarationList) { | ||
| const forOfInitializer = <VariableDeclarationList>forOfStatement.initializer; | ||
| return createCodeFix("{}", forOfInitializer.declarations[0].pos, forOfInitializer.declarations[0].end - forOfInitializer.declarations[0].pos); | ||
| } | ||
| break; | ||
|  | ||
| case SyntaxKind.ForInStatement: | ||
| // There is no valid fix in the case of: | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rename the variable to start with  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of scope, will add for 2nd iteration of this fix | ||
| // for .. in | ||
| return undefined; | ||
|  | ||
| case SyntaxKind.CatchClause: | ||
| const catchClause = <CatchClause>token.parent.parent; | ||
| const parameter = catchClause.variableDeclaration.getChildren()[0]; | ||
| return createCodeFix("", parameter.pos, parameter.end - parameter.pos); | ||
|  | ||
| default: | ||
| const variableStatement = <VariableStatement>token.parent.parent.parent; | ||
| if (variableStatement.declarationList.declarations.length === 1) { | ||
| return createCodeFix("", variableStatement.pos, variableStatement.end - variableStatement.pos); | ||
| } | ||
| else { | ||
| const declarations = variableStatement.declarationList.declarations; | ||
| return removeSingleItem(declarations, token); | ||
| } | ||
| } | ||
|  | ||
| case SyntaxKind.TypeParameter: | ||
| const typeParameters = (<DeclarationWithTypeParameters>token.parent.parent).typeParameters; | ||
| if (typeParameters.length === 1) { | ||
| return createCodeFix("", token.parent.pos - 1, token.parent.end - token.parent.pos + 2); | ||
| } | ||
| else { | ||
| return removeSingleItem(typeParameters, token); | ||
| } | ||
|  | ||
| case ts.SyntaxKind.Parameter: | ||
| const functionDeclaration = <FunctionDeclaration>token.parent.parent; | ||
| if (functionDeclaration.parameters.length === 1) { | ||
| return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); | ||
| } | ||
| else { | ||
| return removeSingleItem(functionDeclaration.parameters, token); | ||
| } | ||
|  | ||
| // handle case where 'import a = A;' | ||
| case SyntaxKind.ImportEqualsDeclaration: | ||
| const importEquals = findImportDeclaration(token); | ||
| return createCodeFix("", importEquals.pos, importEquals.end - importEquals.pos); | ||
|  | ||
| case SyntaxKind.ImportSpecifier: | ||
| const namedImports = <NamedImports>token.parent.parent; | ||
| if (namedImports.elements.length === 1) { | ||
| // Only 1 import and it is unused. So the entire declaration should be removed. | ||
| const 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 = <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 = <NamespaceImport>token.parent; | ||
| if (namespaceImport.name == token && !(<ImportClause>namespaceImport.parent).name) { | ||
| const importDecl = findImportDeclaration(namespaceImport); | ||
| return createCodeFix("", importDecl.pos, importDecl.end - importDecl.pos); | ||
| } | ||
| else { | ||
| const start = (<ImportClause>namespaceImport.parent).name.end; | ||
| return createCodeFix("", start, (<ImportClause>namespaceImport.parent).namedBindings.end - start); | ||
| } | ||
| } | ||
| break; | ||
|  | ||
| case SyntaxKind.PropertyDeclaration: | ||
| return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos); | ||
|  | ||
| 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; | ||
| } | ||
|  | ||
| 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 [{ | ||
| description: getLocaleSpecificMessage(Diagnostics.Remove_unused_identifiers), | ||
| changes: [{ | ||
| fileName: sourceFile.fileName, | ||
| textChanges: [{ newText, span: { start, length } }] | ||
| }] | ||
| }]; | ||
| } | ||
|  | ||
| function removeSingleItem<T extends Node>(elements: NodeArray<T>, token: T): CodeAction[] { | ||
| if (elements[0] === token.parent) { | ||
| return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos + 1); | ||
| } | ||
| else { | ||
| return createCodeFix("", token.parent.pos - 1, token.parent.end - token.parent.pos + 1); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| { | ||
| { | ||
| "compilerOptions": { | ||
| "noImplicitAny": true, | ||
| "noImplicitThis": true, | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| namespace greeter { | ||
| //// class class1 { | ||
| //// } | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace greeter { | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| namespace greeter { | ||
| //// export class class2 { | ||
| //// } | ||
| //// class class1 { | ||
| //// } | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace greeter { | ||
| export class class2 { | ||
| } | ||
| }`); | ||
|  | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| // @noUnusedParameters:true | ||
| //// [| namespace Validation { | ||
| //// class c1 { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// export class c2 { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// class c3 extends c1 { | ||
| //// | ||
| //// } | ||
| ////} |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace Validation { | ||
| class c1 { | ||
| } | ||
|  | ||
| export class c2 { | ||
| } | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| // @noUnusedParameters:true | ||
| //// [| namespace Validation { | ||
| //// class c1 { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// export class c2 { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// class c3 { | ||
| //// public x: c1; | ||
| //// } | ||
| ////} |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace Validation { | ||
| class c1 { | ||
|  | ||
| } | ||
|  | ||
| export class c2 { | ||
|  | ||
| } | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| function f1 () { | ||
| //// const x: string = "x"; | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`function f1 () { | ||
| }`); | ||
|  | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| function f1 () { | ||
| //// enum Directions { Up, Down} | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`function f1 () { | ||
| } | ||
| `); | ||
|  | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| namespace greeter { | ||
| //// enum enum1 { | ||
| //// Monday | ||
| //// } | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace greeter { | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| namespace greeter { | ||
| //// function function1() { | ||
| //// }/*1*/ | ||
| //// } |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace greeter { | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| //// [| namespace greeter { | ||
| //// export function function2() { | ||
| //// } | ||
| //// function function1() { | ||
| //// } | ||
| ////} |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace greeter { | ||
| export function function2() { | ||
| } | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| // @noUnusedParameters:true | ||
|  | ||
| //// [| namespace Validation { | ||
| //// function function1() { | ||
| //// } | ||
| ////} |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace Validation { | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| // @noUnusedParameters:true | ||
| //// [| namespace Validation { | ||
| //// var function1 = function() { | ||
| //// } | ||
| ////} |] | ||
|  | ||
| verify.codeFixAtPosition(`namespace Validation { | ||
| }`); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /// <reference path='fourslash.ts' /> | ||
|  | ||
| // @noUnusedLocals: true | ||
| // @noUnusedParameters:true | ||
| ////namespace Validation { | ||
| //// var function1 = function() { | ||
| //// } | ||
| //// | ||
| //// export function function2() { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// [| function function3() { | ||
| //// function1(); | ||
| //// } | ||
| //// | ||
| //// function function4() { | ||
| //// | ||
| //// } | ||
| //// | ||
| //// export let a = function3; |] | ||
| ////} | ||
|  | ||
| verify.codeFixAtPosition(`function function3() { | ||
| function1(); | ||
| } | ||
|  | ||
| export let a = function3;`); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
binding patterns are not handled anywhere:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outside of scope, I think we can do a better job when we add the change signature refactoring