Merge pull request microsoft#16309 from aozgaa/codeFixPrefixUnused2

aozgaa · web-flow · commit 9cd04e06fd0c · 2017-06-13T11:16:33.000-07:00
Code fix prefix unused2
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3625,6 +3625,10 @@
         "category": "Message",
         "code": 90024
     },
+    "Prefix '{0}' with an underscore.": {
+        "category": "Message",
+        "code": 90025
+    },
     
     "Convert function to an ES2015 class": {
         "category": "Message",
diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json
@@ -80,7 +80,7 @@
         "../services/codefixes/fixConstructorForDerivedNeedSuperCall.ts",
         "../services/codefixes/helpers.ts",
         "../services/codefixes/importFixes.ts",
-        "../services/codefixes/unusedIdentifierFixes.ts",
+        "../services/codefixes/fixUnusedIdentifier.ts",
         "../services/codefixes/disableJsDiagnostics.ts",
 
         "harness.ts",
diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts
@@ -18,14 +18,14 @@ namespace ts.codefix {
 
             switch (token.kind) {
                 case ts.SyntaxKind.Identifier:
-                    return deleteIdentifier();
+                    return deleteIdentifierOrPrefixWithUnderscore(<Identifier>token);
 
                 case SyntaxKind.PropertyDeclaration:
                 case SyntaxKind.NamespaceImport:
-                    return deleteNode(token.parent);
+                    return [deleteNode(token.parent)];
 
                 default:
-                    return deleteDefault();
+                    return [deleteDefault()];
             }
 
             function deleteDefault() {
@@ -40,126 +40,132 @@ namespace ts.codefix {
                 }
             }
 
-            function deleteIdentifier(): CodeAction[] | undefined {
-                switch (token.parent.kind) {
+            function prefixIdentifierWithUnderscore(identifier: Identifier): CodeAction {
+                const startPosition = identifier.getStart(sourceFile, /*includeJsDocComment*/ false);
+                return {
+                    description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Prefix_0_with_an_underscore), { 0: token.getText() }),
+                    changes: [{
+                        fileName: sourceFile.path,
+                        textChanges: [{
+                            span: { start: startPosition, length: 0 },
+                            newText: "_"
+                        }]
+                    }]
+                };
+            }
+
+            function deleteIdentifierOrPrefixWithUnderscore(identifier: Identifier): CodeAction[] | undefined {
+                const parent = identifier.parent;
+                switch (parent.kind) {
                     case ts.SyntaxKind.VariableDeclaration:
-                        return deleteVariableDeclaration(<ts.VariableDeclaration>token.parent);
+                        return deleteVariableDeclarationOrPrefixWithUnderscore(identifier, <ts.VariableDeclaration>parent);
 
                     case SyntaxKind.TypeParameter:
-                        const typeParameters = (<DeclarationWithTypeParameters>token.parent.parent).typeParameters;
+                        const typeParameters = (<DeclarationWithTypeParameters>parent.parent).typeParameters;
                         if (typeParameters.length === 1) {
                             const previousToken = getTokenAtPosition(sourceFile, typeParameters.pos - 1, /*includeJsDocComment*/ false);
-                            if (!previousToken || previousToken.kind !== SyntaxKind.LessThanToken) {
-                                return deleteRange(typeParameters);
-                            }
                             const nextToken = getTokenAtPosition(sourceFile, typeParameters.end, /*includeJsDocComment*/ false);
-                            if (!nextToken || nextToken.kind !== SyntaxKind.GreaterThanToken) {
-                                return deleteRange(typeParameters);
-                            }
-                            return deleteNodeRange(previousToken, nextToken);
+                            Debug.assert(previousToken.kind === SyntaxKind.LessThanToken);
+                            Debug.assert(nextToken.kind === SyntaxKind.GreaterThanToken);
+
+                            return [deleteNodeRange(previousToken, nextToken)];
                         }
                         else {
-                            return deleteNodeInList(token.parent);
+                            return [deleteNodeInList(parent)];
                         }
 
                     case ts.SyntaxKind.Parameter:
-                        const functionDeclaration = <FunctionDeclaration>token.parent.parent;
-                        if (functionDeclaration.parameters.length === 1) {
-                            return deleteNode(token.parent);
-                        }
-                        else {
-                            return deleteNodeInList(token.parent);
-                        }
+                        const functionDeclaration = <FunctionDeclaration>parent.parent;
+                        return [functionDeclaration.parameters.length === 1 ? deleteNode(parent) : deleteNodeInList(parent),
+                            prefixIdentifierWithUnderscore(identifier)];
 
                     // handle case where 'import a = A;'
                     case SyntaxKind.ImportEqualsDeclaration:
-                        const importEquals = getAncestor(token, SyntaxKind.ImportEqualsDeclaration);
-                        return deleteNode(importEquals);
+                        const importEquals = getAncestor(identifier, SyntaxKind.ImportEqualsDeclaration);
+                        return [deleteNode(importEquals)];
 
                     case SyntaxKind.ImportSpecifier:
-                        const namedImports = <NamedImports>token.parent.parent;
+                        const namedImports = <NamedImports>parent.parent;
                         if (namedImports.elements.length === 1) {
                             // Only 1 import and it is unused. So the entire declaration should be removed.
-                            const importSpec = getAncestor(token, SyntaxKind.ImportDeclaration);
-                            return deleteNode(importSpec);
+                            const importSpec = getAncestor(identifier, SyntaxKind.ImportDeclaration);
+                            return [deleteNode(importSpec)];
                         }
                         else {
                             // delete import specifier
-                            return deleteNodeInList(token.parent);
+                            return [deleteNodeInList(parent)];
                         }
 
                     // 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;
+                        const importClause = <ImportClause>parent;
                         if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'|
                             const importDecl = getAncestor(importClause, SyntaxKind.ImportDeclaration);
-                            return deleteNode(importDecl);
+                            return [deleteNode(importDecl)];
                         }
                         else {
                             // import |d,| * as ns from './file'
                             const start = importClause.name.getStart(sourceFile);
                             const nextToken = getTokenAtPosition(sourceFile, importClause.name.end, /*includeJsDocComment*/ false);
                             if (nextToken && nextToken.kind === SyntaxKind.CommaToken) {
                                 // shift first non-whitespace position after comma to the start position of the node
-                                return deleteRange({ pos: start, end: skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true) });
+                                return [deleteRange({ pos: start, end: skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true) })];
                             }
                             else {
-                                return deleteNode(importClause.name);
+                                return [deleteNode(importClause.name)];
                             }
                         }
 
                     case SyntaxKind.NamespaceImport:
-                        const namespaceImport = <NamespaceImport>token.parent;
-                        if (namespaceImport.name === token && !(<ImportClause>namespaceImport.parent).name) {
+                        const namespaceImport = <NamespaceImport>parent;
+                        if (namespaceImport.name === identifier && !(<ImportClause>namespaceImport.parent).name) {
                             const importDecl = getAncestor(namespaceImport, SyntaxKind.ImportDeclaration);
-                            return deleteNode(importDecl);
+                            return [deleteNode(importDecl)];
                         }
                         else {
                             const previousToken = getTokenAtPosition(sourceFile, namespaceImport.pos - 1, /*includeJsDocComment*/ false);
                             if (previousToken && previousToken.kind === SyntaxKind.CommaToken) {
                                 const startPosition = textChanges.getAdjustedStartPosition(sourceFile, previousToken, {}, textChanges.Position.FullStart);
-                                return deleteRange({ pos: startPosition, end: namespaceImport.end });
+                                return [deleteRange({ pos: startPosition, end: namespaceImport.end })];
                             }
-                            return deleteRange(namespaceImport);
+                            return [deleteRange(namespaceImport)];
                         }
 
                     default:
-                        return deleteDefault();
+                        return [deleteDefault()];
                 }
             }
 
             // token.parent is a variableDeclaration
-            function deleteVariableDeclaration(varDecl: ts.VariableDeclaration): CodeAction[] | undefined {
+            function deleteVariableDeclarationOrPrefixWithUnderscore(identifier: Identifier, varDecl: ts.VariableDeclaration): CodeAction[] | undefined {
                 switch (varDecl.parent.parent.kind) {
                     case SyntaxKind.ForStatement:
                         const forStatement = <ForStatement>varDecl.parent.parent;
                         const forInitializer = <VariableDeclarationList>forStatement.initializer;
-                        if (forInitializer.declarations.length === 1) {
-                            return deleteNode(forInitializer);
-                        }
-                        else {
-                            return deleteNodeInList(varDecl);
-                        }
+                        return [forInitializer.declarations.length === 1 ? deleteNode(forInitializer) : deleteNodeInList(varDecl)];
 
                     case SyntaxKind.ForOfStatement:
                         const forOfStatement = <ForOfStatement>varDecl.parent.parent;
                         Debug.assert(forOfStatement.initializer.kind === SyntaxKind.VariableDeclarationList);
                         const forOfInitializer = <VariableDeclarationList>forOfStatement.initializer;
-                        return replaceNode(forOfInitializer.declarations[0], createObjectLiteral());
+                        return [
+                            replaceNode(forOfInitializer.declarations[0], createObjectLiteral()),
+                            prefixIdentifierWithUnderscore(identifier)
+                        ];
 
                     case SyntaxKind.ForInStatement:
                         // There is no valid fix in the case of:
                         //  for .. in
-                        return undefined;
+                        return [prefixIdentifierWithUnderscore(identifier)];
 
                     default:
                         const variableStatement = <VariableStatement>varDecl.parent.parent;
                         if (variableStatement.declarationList.declarations.length === 1) {
-                            return deleteNode(variableStatement);
+                            return [deleteNode(variableStatement)];
                         }
                         else {
-                            return deleteNodeInList(varDecl);
+                            return [deleteNodeInList(varDecl)];
                         }
                 }
             }
@@ -184,11 +190,11 @@ namespace ts.codefix {
                 return makeChange(textChanges.ChangeTracker.fromCodeFixContext(context).replaceNode(sourceFile, n, newNode));
             }
 
-            function makeChange(changeTracker: textChanges.ChangeTracker) {
-                return [{
+            function makeChange(changeTracker: textChanges.ChangeTracker): CodeAction {
+                return {
                     description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Remove_declaration_for_Colon_0), { 0: token.getText() }),
                     changes: changeTracker.getChanges()
-                }];
+                };
             }
         }
     });
diff --git a/src/services/codefixes/fixes.ts b/src/services/codefixes/fixes.ts
@@ -6,7 +6,7 @@
 /// <reference path="fixConstructorForDerivedNeedSuperCall.ts" />
 /// <reference path="fixExtendsInterfaceBecomesImplements.ts" />
 /// <reference path="fixForgottenThisPropertyAccess.ts" />
-/// <reference path='unusedIdentifierFixes.ts' />
+/// <reference path='fixUnusedIdentifier.ts' />
 /// <reference path='importFixes.ts' />
 /// <reference path='disableJsDiagnostics.ts' />
 /// <reference path='helpers.ts' />
diff --git a/tests/cases/fourslash/unusedClassInNamespaceWithTrivia.ts b/tests/cases/fourslash/unusedClassInNamespaceWithTrivia.ts
@@ -0,0 +1,11 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+//// [| namespace greeter {
+////   /* comment1 */
+////   class /* comment2 */ class1 {
+////   }
+//// } |]
+
+verify.rangeAfterCodeFix(`namespace greeter {
+}`);
diff --git a/tests/cases/fourslash/unusedLocalsInFunction3.ts b/tests/cases/fourslash/unusedLocalsInFunction3.ts
@@ -7,4 +7,4 @@
 ////    z+1;
 ////}
 
-verify.rangeAfterCodeFix("var x,z = 1;", /*includeWhiteSpace*/ undefined, 6133);
+verify.rangeAfterCodeFix("var x,z = 1;", /*includeWhiteSpace*/ undefined, /*errorCode*/ 6133);
diff --git a/tests/cases/fourslash/unusedParameterInConstructor1.ts b/tests/cases/fourslash/unusedParameterInConstructor1.ts
@@ -5,4 +5,4 @@
 ////    [|constructor(private p1: string, public p2: boolean, public p3: any, p5)|] { p5; }
 //// }
 
-verify.rangeAfterCodeFix("constructor(public p2: boolean, public p3: any, p5)");
+verify.rangeAfterCodeFix("constructor(public p2: boolean, public p3: any, p5)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInConstructor1AddUnderscore.ts b/tests/cases/fourslash/unusedParameterInConstructor1AddUnderscore.ts
@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+//// class C1 {
+////    [|constructor(private p1: string, public p2: boolean, public p3: any, p5) |] { p5; }
+//// }
+
+verify.rangeAfterCodeFix("constructor(private _p1: string, public p2: boolean, public p3: any, p5)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);
diff --git a/tests/cases/fourslash/unusedParameterInConstructor2.ts b/tests/cases/fourslash/unusedParameterInConstructor2.ts
@@ -5,4 +5,4 @@
 ////    [|constructor(public p1: string, private p2: boolean, public p3: any, p5)|] { p5; }
 //// }
 
-verify.rangeAfterCodeFix("constructor(public p1: string, public p3: any, p5)");
+verify.rangeAfterCodeFix("constructor(public p1: string, public p3: any, p5)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInConstructor3.ts b/tests/cases/fourslash/unusedParameterInConstructor3.ts
@@ -5,4 +5,4 @@
 ////    [|constructor(public p1: string, public p2: boolean, private p3: any, p5)|] { p5; }
 //// }
 
-verify.rangeAfterCodeFix("constructor(public p1: string, public p2: boolean, p5)");
+verify.rangeAfterCodeFix("constructor(public p1: string, public p2: boolean, p5)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInConstructor4.ts b/tests/cases/fourslash/unusedParameterInConstructor4.ts
@@ -5,4 +5,4 @@
 ////    [|constructor(private readonly p2: boolean, p5)|] { p5; }
 //// }
 
-verify.rangeAfterCodeFix("constructor(p5)");
+verify.rangeAfterCodeFix("constructor(p5)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInFunction1.ts b/tests/cases/fourslash/unusedParameterInFunction1.ts
@@ -4,4 +4,4 @@
 ////function [|greeter( x)|] {
 ////}
 
-verify.rangeAfterCodeFix("greeter()");
+verify.rangeAfterCodeFix("greeter()", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInFunction1AddUnderscore.ts b/tests/cases/fourslash/unusedParameterInFunction1AddUnderscore.ts
@@ -0,0 +1,7 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedParameters: true
+////function [|greeter( x) |] {
+////}
+
+verify.rangeAfterCodeFix("greeter( _x)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);
diff --git a/tests/cases/fourslash/unusedParameterInFunction2.ts b/tests/cases/fourslash/unusedParameterInFunction2.ts
@@ -5,4 +5,4 @@
 ////    x++;
 ////}
 
-verify.rangeAfterCodeFix("greeter(x)");
+verify.rangeAfterCodeFix("greeter(x)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInFunction3.ts b/tests/cases/fourslash/unusedParameterInFunction3.ts
@@ -5,4 +5,4 @@
 ////    y++;
 ////}
 
-verify.rangeAfterCodeFix("greeter(y)");
+verify.rangeAfterCodeFix("greeter(y)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInFunction4.ts b/tests/cases/fourslash/unusedParameterInFunction4.ts
@@ -6,4 +6,4 @@
 ////    z++;
 ////}
 
-verify.rangeAfterCodeFix("function greeter(x,z)");
+verify.rangeAfterCodeFix("function greeter(x,z)", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInLambda1.ts b/tests/cases/fourslash/unusedParameterInLambda1.ts
@@ -6,4 +6,4 @@
 ////     [|return (x:number) => {}|]
 //// }
 
-verify.rangeAfterCodeFix("return () => {}");
+verify.rangeAfterCodeFix("return () => {}", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
diff --git a/tests/cases/fourslash/unusedParameterInLambda1AddUnderscore.ts b/tests/cases/fourslash/unusedParameterInLambda1AddUnderscore.ts
@@ -0,0 +1,9 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+//// function f1() {
+////     [|return (x:number) => {} |]
+//// }
+
+verify.rangeAfterCodeFix("return (_x:number) => {}", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);
diff --git a/tests/cases/fourslash/unusedVariableInForLoop5FS.ts b/tests/cases/fourslash/unusedVariableInForLoop5FS.ts
diff --git a/tests/cases/fourslash/unusedVariableInForLoop5FSAddUnderscore.ts b/tests/cases/fourslash/unusedVariableInForLoop5FSAddUnderscore.ts
@@ -0,0 +1,10 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+//// function f1 () {
+////     [|for (const elem in ["a", "b", "c"]) |]{
+////
+////     }
+//// }
+
+verify.rangeAfterCodeFix(`for (const _elem in ["a", "b", "c"])`, /*includeWhiteSpace*/ true, /*errorCode*/ 0);
diff --git a/tests/cases/fourslash/unusedVariableInForLoop6FS.ts b/tests/cases/fourslash/unusedVariableInForLoop6FS.ts
@@ -7,5 +7,5 @@
 ////     }
 //// }
 
-verify.rangeAfterCodeFix("const {} of ");
+verify.rangeAfterCodeFix("const {} of ", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
 
diff --git a/tests/cases/fourslash/unusedVariableInForLoop6FSAddUnderscore.ts b/tests/cases/fourslash/unusedVariableInForLoop6FSAddUnderscore.ts
@@ -0,0 +1,11 @@
+/// <reference path='fourslash.ts' />
+
+// @noUnusedLocals: true
+//// function f1 () {
+////     for ([|const elem of |]["a", "b", "c"]) {
+////
+////     }
+//// }
+
+verify.rangeAfterCodeFix("const _elem of", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);
+
diff --git a/tests/cases/fourslash/unusedVariableInModule4.ts b/tests/cases/fourslash/unusedVariableInModule4.ts
