8000 Codefix for removing Unused Identifiers by paulvanbrenk · Pull Request #11546 · microsoft/TypeScript · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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
Merged

Codefix for removing Unused Identifiers #11546

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
05ec512
Codefixes for unused variables, properties etc.
Oct 11, 2016
28c08fd
Merge branch 'pvb/codeaction/api' into unusedidentifier
Oct 12, 2016
03a6eeb
Major refactoring after PR feedback
Oct 21, 2016
79fa9db
Add tests for different constructor parameter cases.
Oct 21, 2016
1f94e14
Remove unused case, and fix for..in
Oct 22, 2016
4097874
Fix 'import a = A' case
Oct 24, 2016
d4b99d6
Simplifying handling import cases
Oct 25, 2016
b59714e
Add more testcases and simplify
Nov 1, 2016
7196018
Removed unused, confusing cases
Nov 1, 2016
2f453ce
Handle most module cases
Nov 3, 2016
40c0cbd
Clean up
Nov 3, 2016
78fdd44
Remove error-codes from fourslash tests
Nov 7, 2016
1924298
Merge branch 'master' into unusedidentifier
Nov 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Major refactoring after PR feedback
  • Loading branch information
Paul van Brenk committed Oct 21, 2016
commit 03a6eeb6436780c78026c1bbf453feec1d6ba422
203 changes: 92 additions & 111 deletions src/services/codefixes/unusedIdentifierFixes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,134 +10,106 @@ namespace ts.codefix {
const start = context.span.start;
const token = getTokenAtPosition(sourceFile, start);

if (token.kind === ts.SyntaxKind.Identifier) {
if (token.parent.kind === ts.SyntaxKind.VariableDeclaration) {
if (token.parent.parent.parent.kind === SyntaxKind.ForStatement) {
const forStatement = <ForStatement>token.parent.parent.parent;
const initializer = <VariableDeclarationList>forStatement.initializer;
if (initializer.declarations.length === 1) {
return createCodeFix("", initializer.pos, initializer.end - initializer.pos);
}
else {
if (initializer.declarations[0] === token.parent) {
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos + 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:
case SyntaxKind.ForInStatement:
const forOfStatement = <ForOfStatement | ForInStatement>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.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.FunctionDeclaration:
Copy link
Contributor

Choose a reason for hiding this comment

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

namespace, type alais, and enum as well

Copy link
Contributor

Choose a reason for hiding this comment

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

i would move this to the default clause and use isDeclarationName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.ArrowFunction:
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);

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 createCodeFix("", token.parent.pos - 1, token.parent.end - token.parent.pos + 1);
return removeSingleItem(typeParameters, token);
}
}
}
else if (token.parent.parent.parent.kind === SyntaxKind.ForInStatement) {
const forInStatement = <ForInStatement>token.parent.parent.parent;
const initializer = <VariableDeclarationList>forInStatement.initializer;
return createCodeFix("{}", initializer.declarations[0].pos, initializer.declarations[0].end - initializer.declarations[0].pos);
}
else if (token.parent.parent.parent.kind === SyntaxKind.ForOfStatement) {
const forOfStatement = <ForOfStatement>token.parent.parent.parent;
const initializer = <VariableDeclarationList>forOfStatement.initializer;
return createCodeFix("{}", initializer.declarations[0].pos, initializer.declarations[0].end - initializer.declarations[0].pos);
}
else if (token.parent.parent.kind === SyntaxKind.CatchClause) {
const catchClause = <CatchClause>token.parent.parent;
const parameter = catchClause.variableDeclaration.getChildren()[0];
return createCodeFix("", parameter.pos, parameter.end - parameter.pos);
}
else {
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;
if (declarations[0].name === token) {
return createCodeFix("", token.parent.pos + 1, token.parent.end - token.parent.pos);

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 createCodeFix("", token.parent.pos - 1, token.parent.end - token.parent.pos + 1);
return removeSingleItem(functionDeclaration.parameters, token);
}
}
}
}

if (token.parent.kind === SyntaxKind.FunctionDeclaration ||
token.parent.kind === SyntaxKind.ClassDeclaration ||
token.parent.kind === SyntaxKind.InterfaceDeclaration ||
token.parent.kind === SyntaxKind.MethodDeclaration ||
token.parent.kind === SyntaxKind.ModuleDeclaration ||
token.parent.kind === SyntaxKind.PropertyDeclaration ||
token.parent.kind === SyntaxKind.ArrowFunction) {
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
}
case SyntaxKind.ImportSpecifier:
const namedImports = <NamedImports>token.parent.parent;
const elements = namedImports.elements;
if (elements.length === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if no other imports in this list we need to remove the whole module import.

// 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);
}

if (token.parent.kind === SyntaxKind.TypeParameter) {
const typeParameters = (<ClassDeclaration>token.parent.parent).typeParameters;
if (typeParameters.length === 1) {
return createCodeFix("", token.parent.pos - 1, token.parent.end - token.parent.pos + 2);
}
else {
if (typeParameters[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);
}
}
}
case SyntaxKind.ImportEqualsDeclaration:
return createCodeFix("{}", token.pos, token.end - token.pos);

if (token.parent.kind === 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 {
if (functionDeclaration.parameters[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);
}
case SyntaxKind.EnumDeclaration:
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
}
}

if (token.parent.kind === SyntaxKind.ImportSpecifier) {
const namedImports = <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);
if (token.parent.parent.kind === SyntaxKind.ImportClause || token.parent.parent.kind === SyntaxKind.ImportDeclaration) {
return createCodeFix("{}", token.parent.pos, token.parent.end - token.parent.pos);
}
else {
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);
}
}
}

if (token.parent.parent.kind === SyntaxKind.ImportClause || token.parent.parent.kind === SyntaxKind.ImportDeclaration) {
return createCodeFix("{}", token.parent.pos, token.parent.end - token.parent.pos);
}

if (token.parent.kind === SyntaxKind.ImportEqualsDeclaration) {
return createCodeFix("{}", token.pos, token.end - token.pos);
}
break;

if (token.parent.kind === SyntaxKind.EnumDeclaration) {
case SyntaxKind.PrivateKeyword:
Copy link
Contributor

Choose a reason for hiding this comment

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

When is this ever used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

case SyntaxKind.PropertyDeclaration:
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
}
}

if (token.kind === SyntaxKind.PrivateKeyword && token.parent.kind === SyntaxKind.PropertyDeclaration) {
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
}

if (token.kind === SyntaxKind.AsteriskToken && token.parent.kind === SyntaxKind.NamespaceImport) {
return createCodeFix("{}", token.parent.pos, token.parent.end - token.parent.pos);
case SyntaxKind.AsteriskToken:
Copy link
Contributor

Choose a reason for hiding this comment

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

when do we get a * token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

case SyntaxKind.NamespaceImport:
return createCodeFix("{}", token.parent.pos, token.parent.end - token.parent.pos);
}

return undefined;
return [];

function createCodeFix(newText: string, start: number, length: number): CodeAction[] {
return [{
Expand All @@ -148,6 +120,15 @@ namespace ts.codefix {
}]
}];
}

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);
}
}
}
});
}
0