Skip to content

Commit a2451a4

Browse files
zhengblizhengbli
authored
Merge pull request #13851 from zhengbli/fixCommentsForUnusedLocal
avoid removing comments when removing unused locals
2 parents 12e8f91 + 62f716a commit a2451a4

File tree

3 files changed

+36
-18
lines changed

3 files changed

+36
-18
lines changed

‎src/services/codefixes/unusedIdentifierFixes.ts

+32-16
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace ts.codefix {
2525
const forStatement = <ForStatement>token.parent.parent.parent;
2626
const forInitializer = <VariableDeclarationList>forStatement.initializer;
2727
if (forInitializer.declarations.length === 1) {
28-
return createCodeFix("", forInitializer.pos, forInitializer.end - forInitializer.pos);
28+
return createCodeFixToRemoveNode(forInitializer);
2929
}
3030
else {
3131
return removeSingleItem(forInitializer.declarations, token);
@@ -35,7 +35,7 @@ namespace ts.codefix {
3535
const forOfStatement = <ForOfStatement>token.parent.parent.parent;
3636
if (forOfStatement.initializer.kind === SyntaxKind.VariableDeclarationList) {
3737
const forOfInitializer = <VariableDeclarationList>forOfStatement.initializer;
38-
return createCodeFix("{}", forOfInitializer.declarations[0].pos, forOfInitializer.declarations[0].end - forOfInitializer.declarations[0].pos);
38+
return createCodeFix("{}", forOfInitializer.declarations[0].getStart(), forOfInitializer.declarations[0].getWidth());
3939
}
4040
break;
4141

@@ -47,12 +47,12 @@ namespace ts.codefix {
4747
case SyntaxKind.CatchClause:
4848
const catchClause = <CatchClause>token.parent.parent;
4949
const parameter = catchClause.variableDeclaration.getChildren()[0];
50-
return createCodeFix("", parameter.pos, parameter.end - parameter.pos);
50+
return createCodeFixToRemoveNode(parameter);
5151

5252
default:
5353
const variableStatement = <VariableStatement>token.parent.parent.parent;
5454
if (variableStatement.declarationList.declarations.length === 1) {
55-
return createCodeFix("", variableStatement.pos, variableStatement.end - variableStatement.pos);
55+
return createCodeFixToRemoveNode(variableStatement);
5656
}
5757
else {
5858
const declarations = variableStatement.declarationList.declarations;
@@ -72,7 +72,7 @@ namespace ts.codefix {
7272
case ts.SyntaxKind.Parameter:
7373
const functionDeclaration = <FunctionDeclaration>token.parent.parent;
7474
if (functionDeclaration.parameters.length === 1) {
75-
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
75+
return createCodeFixToRemoveNode(token.parent);
7676
}
7777
else {
7878
return removeSingleItem(functionDeclaration.parameters, token);
@@ -81,14 +81,14 @@ namespace ts.codefix {
8181
// handle case where 'import a = A;'
8282
case SyntaxKind.ImportEqualsDeclaration:
8383
const importEquals = findImportDeclaration(token);
84-
return createCodeFix("", importEquals.pos, importEquals.end - importEquals.pos);
84+
return createCodeFixToRemoveNode(importEquals);
8585

8686
case SyntaxKind.ImportSpecifier:
8787
const namedImports = <NamedImports>token.parent.parent;
8888
if (namedImports.elements.length === 1) {
8989
// Only 1 import and it is unused. So the entire declaration should be removed.
9090
const importSpec = findImportDeclaration(token);
91-
return createCodeFix("", importSpec.pos, importSpec.end - importSpec.pos);
91+
return createCodeFixToRemoveNode(importSpec);
9292
}
9393
else {
9494
return removeSingleItem(namedImports.elements, token);
@@ -100,17 +100,24 @@ namespace ts.codefix {
100100
const importClause = <ImportClause>token.parent;
101101
if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'|
102102
const importDecl = findImportDeclaration(importClause);
103-
return createCodeFix("", importDecl.pos, importDecl.end - importDecl.pos);
103+
return createCodeFixToRemoveNode(importDecl);
104104
}
105-
else { // import |d,| * as ns from './file'
106-
return createCodeFix("", importClause.name.pos, importClause.namedBindings.pos - importClause.name.pos);
105+
else {
106+
// import |d,| * as ns from './file'
107+
const start = importClause.name.getStart();
108+
let end = findFirstNonSpaceCharPosStarting(importClause.name.end);
109+
if (sourceFile.text.charCodeAt(end) === CharacterCodes.comma) {
110+
end = findFirstNonSpaceCharPosStarting(end + 1);
111+
}
112+
113+
return createCodeFix("", start, end - start);
107114
}
108115

109116
case SyntaxKind.NamespaceImport:
110117
const namespaceImport = <NamespaceImport>token.parent;
111118
if (namespaceImport.name == token && !(<ImportClause>namespaceImport.parent).name) {
112119
const importDecl = findImportDeclaration(namespaceImport);
113-
return createCodeFix("", importDecl.pos, importDecl.end - importDecl.pos);
120+
return createCodeFixToRemoveNode(importDecl);
114121
}
115122
else {
116123
const start = (<ImportClause>namespaceImport.parent).name.end;
@@ -120,16 +127,14 @@ namespace ts.codefix {
120127
break;
121128

122129
case SyntaxKind.PropertyDeclaration:
123-
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
124-
125130
case SyntaxKind.NamespaceImport:
126-
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
131+
return createCodeFixToRemoveNode(token.parent);
127132
}
128133
if (isDeclarationName(token)) {
129-
return createCodeFix("", token.parent.pos, token.parent.end - token.parent.pos);
134+
return createCodeFixToRemoveNode(token.parent);
130135
}
131136
else if (isLiteralComputedPropertyDeclarationName(token)) {
132-
return createCodeFix("", token.parent.parent.pos, token.parent.parent.end - token.parent.parent.pos);
137+
return createCodeFixToRemoveNode(token.parent.parent);
133138
}
134139
else {
135140
return undefined;
@@ -144,6 +149,17 @@ namespace ts.codefix {
144149
return importDecl;
145150
}
146151

152+
function createCodeFixToRemoveNode(node: Node) {
153+
return createCodeFix("", node.getStart(), node.getWidth());
154+
}
155+
156+
function findFirstNonSpaceCharPosStarting(start: number) {
157+
while (isWhiteSpace(sourceFile.text.charCodeAt(start))) {
158+
start += 1;
159+
}
160+
return start;
161+
}
162+
147163
function createCodeFix(newText: string, start: number, length: number): CodeAction[] {
148164
return [{
149165
description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Remove_declaration_for_Colon_0), { 0: token.getText() }),

‎tests/cases/fourslash/unusedFunctionInNamespace1.ts

+2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
// @noUnusedLocals: true
44
//// [| namespace greeter {
5+
//// // some legit comments
56
//// function function1() {
67
//// }/*1*/
78
//// } |]
89

910
verify.rangeAfterCodeFix(`namespace greeter {
11+
// some legit comments
1012
}`);

‎tests/cases/fourslash/unusedImports3FS.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
// @noUnusedLocals: true
44
// @Filename: file2.ts
5-
////[| import {Calculator, test, test2} from "./file1" |]
5+
////[| import {Calculator, /*some comments*/ test, test2} from "./file1" |]
66

77
//// test();
88
//// test2();
@@ -20,5 +20,5 @@
2020
////
2121
//// }
2222

23-
verify.rangeAfterCodeFix(`import {test, test2} from "./file1"`);
23+
verify.rangeAfterCodeFix(`import {/*some comments*/ test, test2} from "./file1"`);
2424

0 commit comments

Comments
 (0)