Skip to content

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

Merged
merged 13 commits into from
Nov 15, 2016
4 changes: 2 additions & 2 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2013,13 +2013,13 @@ namespace FourSlash {
this.raiseError("Errors expected.");
}

if (diagnostics.length > 1 && errorCode !== undefined) {
if (diagnostics.length > 1 && errorCode === undefined) {
this.raiseError("When there's more than one error, you must specify the errror to fix.");
}

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) {
Expand Down
1 change: 1 addition & 0 deletions src/services/codefixes/fixes.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
///<reference path='superFixes.ts' />
///<reference path='unusedIdentifierFixes.ts' />
167 changes: 167 additions & 0 deletions src/services/codefixes/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:
Copy link
Contributor

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:

function f({a, b}) {}

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

The 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:
Copy link
Contributor

Choose a reason for hiding this comment

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

rename the variable to start with _.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
}
}
}
});
}
2 changes: 1 addition & 1 deletion src/services/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
{
"compilerOptions": {
"noImplicitAny": true,
"noImplicitThis": true,
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/fourslash/unusedClassInNamespace1.ts
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 {
}`);
15 changes: 15 additions & 0 deletions tests/cases/fourslash/unusedClassInNamespace2.ts
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 {
}
}`);

25 changes: 25 additions & 0 deletions tests/cases/fourslash/unusedClassInNamespace3.ts
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 {
}
}`);
27 changes: 27 additions & 0 deletions tests/cases/fourslash/unusedClassInNamespace4.ts
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 {

}
}`);
10 changes: 10 additions & 0 deletions tests/cases/fourslash/unusedConstantInFunction1.ts
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 () {
}`);

11 changes: 11 additions & 0 deletions tests/cases/fourslash/unusedEnumInFunction1.ts
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 () {
}
`);

11 changes: 11 additions & 0 deletions tests/cases/fourslash/unusedEnumInNamespace1.ts
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 {
}`);
10 changes: 10 additions & 0 deletions tests/cases/fourslash/unusedFunctionInNamespace1.ts
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 {
}`);
14 changes: 14 additions & 0 deletions tests/cases/fourslash/unusedFunctionInNamespace2.ts
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() {
}
}`);
12 changes: 12 additions & 0 deletions tests/cases/fourslash/unusedFunctionInNamespace3.ts
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 {
}`);
11 changes: 11 additions & 0 deletions tests/cases/fourslash/unusedFunctionInNamespace4.ts
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 {
}`);
28 changes: 28 additions & 0 deletions tests/cases/fourslash/unusedFunctionInNamespace5.ts
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;`);
Loading