Add code fix to remove unreachable code (#24028)

Andy · web-flow · commit 6ae4d3a516a5 · 2018-05-10T16:44:48.000-07:00
* Add code fix to remove unreachable code

* Code review

* Preserve more kinds of statements
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -4249,5 +4249,13 @@
     "Move to a new file": {
         "category": "Message",
         "code": 95049
+    },
+    "Remove unreachable code": {
+        "category": "Message",
+        "code": 95050
+    },
+    "Remove all unreachable code": {
+        "category": "Message",
+        "code": 95051
     }
 }
diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json
@@ -105,6 +105,7 @@
         "../services/codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "../services/codefixes/fixForgottenThisPropertyAccess.ts",
         "../services/codefixes/fixUnusedIdentifier.ts",
+        "../services/codefixes/fixUnreachableCode.ts",
         "../services/codefixes/fixJSDocTypes.ts",
         "../services/codefixes/fixAwaitInSyncFunction.ts",
         "../services/codefixes/disableJsDiagnostics.ts",
diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json
@@ -101,6 +101,7 @@
         "../services/codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "../services/codefixes/fixForgottenThisPropertyAccess.ts",
         "../services/codefixes/fixUnusedIdentifier.ts",
+        "../services/codefixes/fixUnreachableCode.ts",
         "../services/codefixes/fixJSDocTypes.ts",
         "../services/codefixes/fixAwaitInSyncFunction.ts",
         "../services/codefixes/disableJsDiagnostics.ts",
diff --git a/src/server/tsconfig.library.json b/src/server/tsconfig.library.json
@@ -107,6 +107,7 @@
         "../services/codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "../services/codefixes/fixForgottenThisPropertyAccess.ts",
         "../services/codefixes/fixUnusedIdentifier.ts",
+        "../services/codefixes/fixUnreachableCode.ts",
         "../services/codefixes/fixJSDocTypes.ts",
         "../services/codefixes/fixAwaitInSyncFunction.ts",
         "../services/codefixes/disableJsDiagnostics.ts",
diff --git a/src/services/codefixes/fixUnreachableCode.ts b/src/services/codefixes/fixUnreachableCode.ts
@@ -0,0 +1,89 @@
+/* @internal */
+namespace ts.codefix {
+    const fixId = "fixUnreachableCode";
+    const errorCodes = [Diagnostics.Unreachable_code_detected.code];
+    registerCodeFix({
+        errorCodes,
+        getCodeActions(context) {
+            const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start));
+            return [createCodeFixAction(fixId, changes, Diagnostics.Remove_unreachable_code, fixId, Diagnostics.Remove_all_unreachable_code)];
+        },
+        fixIds: [fixId],
+        getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start)),
+    });
+
+    function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, start: number): void {
+        const token = getTokenAtPosition(sourceFile, start, /*includeJsDocComment*/ false);
+        const statement = findAncestor(token, isStatement);
+        Debug.assert(statement.getStart(sourceFile) === token.getStart(sourceFile));
+
+        const container = (isBlock(statement.parent) ? statement.parent : statement).parent;
+        switch (container.kind) {
+            case SyntaxKind.IfStatement:
+                if ((container as IfStatement).elseStatement) {
+                    if (isBlock(statement.parent)) {
+                        changes.deleteNodeRange(sourceFile, first(statement.parent.statements), last(statement.parent.statements));
+                    }
+                    else {
+                        changes.replaceNode(sourceFile, statement, createBlock(emptyArray));
+                    }
+                    break;
+                }
+                // falls through
+            case SyntaxKind.WhileStatement:
+            case SyntaxKind.ForStatement:
+                changes.deleteNode(sourceFile, container);
+                break;
+            default:
+                if (isBlock(statement.parent)) {
+                    split(sliceAfter(statement.parent.statements, statement), shouldRemove, (start, end) => changes.deleteNodeRange(sourceFile, start, end));
+                }
+                else {
+                    changes.deleteNode(sourceFile, statement);
+                }
+        }
+    }
+
+    function shouldRemove(s: Statement): boolean {
+        // Don't remove statements that can validly be used before they appear.
+        return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) &&
+            // `var x;` may declare a variable used above
+            !(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
+    }
+
+    function isPurelyTypeDeclaration(s: Statement): boolean {
+        switch (s.kind) {
+            case SyntaxKind.InterfaceDeclaration:
+            case SyntaxKind.TypeAliasDeclaration:
+                return true;
+            case SyntaxKind.ModuleDeclaration:
+                return getModuleInstanceState(s as ModuleDeclaration) !== ModuleInstanceState.Instantiated;
+            case SyntaxKind.EnumDeclaration:
+                return hasModifier(s, ModifierFlags.Const);
+        }
+    }
+
+    function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T> {
+        const index = arr.indexOf(value);
+        Debug.assert(index !== -1);
+        return arr.slice(index);
+    }
+
+    // Calls 'cb' with the start and end of each range where 'pred' is true.
+    function split<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void {
+        let start: T | undefined;
+        for (let i = 0; i < arr.length; i++) {
+            const value = arr[i];
+            if (pred(value)) {
+                start = start || value;
+            }
+            else {
+                if (start) {
+                    cb(start, arr[i - 1]);
+                    start = undefined;
+                }
+            }
+        }
+        if (start) cb(start, arr[arr.length - 1]);
+    }
+}
diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json
@@ -98,6 +98,7 @@
         "codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "codefixes/fixForgottenThisPropertyAccess.ts",
         "codefixes/fixUnusedIdentifier.ts",
+        "codefixes/fixUnreachableCode.ts",
         "codefixes/fixJSDocTypes.ts",
         "codefixes/fixAwaitInSyncFunction.ts",
         "codefixes/disableJsDiagnostics.ts",
diff --git a/tests/cases/fourslash/codeFixUnreachableCode.ts b/tests/cases/fourslash/codeFixUnreachableCode.ts
@@ -0,0 +1,31 @@
+/// <reference path='fourslash.ts' />
+
+////function f() {
+////    return f();
+////    return 1;
+////    function f() {}
+////    return 2;
+////    type T = number;
+////    interface I {}
+////    const enum E {}
+////    enum E {}
+////    namespace N { export type T = number; }
+////    namespace N { export const x = 0; }
+////    var x;
+////    var y = 0;
+////}
+
+verify.codeFix({
+    description: "Remove unreachable code",
+    index: 0,
+    newFileContent:
+`function f() {
+    return f();
+    function f() {}
+    type T = number;
+    interface I {}
+    const enum E {}
+    namespace N { export type T = number; }
+    var x;
+}`,
+});
diff --git a/tests/cases/fourslash/codeFixUnreachableCode_if.ts b/tests/cases/fourslash/codeFixUnreachableCode_if.ts
@@ -0,0 +1,40 @@
+/// <reference path='fourslash.ts' />
+
+////if (false) a;
+////if (false) {
+////    a;
+////}
+////
+////// No good way to delete just the 'if' part
+////if (false) a; else b;
+////if (false) {
+////    a;
+////} else {
+////    b;
+////}
+////
+////while (false) a;
+////while (false) {
+////    a;
+////}
+////
+////for (let x = 0; false; ++x) a;
+////for (let x = 0; false; ++x) {
+////    a;
+////}
+
+verify.codeFixAll({
+    fixId: "fixUnreachableCode",
+    fixAllDescription: "Remove all unreachable code",
+    newFileContent:
+`
+// No good way to delete just the 'if' part
+if (false) { } else b;
+if (false) {
+} else {
+    b;
+}
+
+
+`,
+});

Original file line number	Diff line number	Diff line change
`@@ -4249,5 +4249,13 @@`
`4249`	`4249`	`"Move to a new file": {`
`4250`	`4250`	`"category": "Message",`
`4251`	`4251`	`"code": 95049`
	`4252`	`+ },`
	`4253`	`+ "Remove unreachable code": {`
	`4254`	`+ "category": "Message",`
	`4255`	`+ "code": 95050`
	`4256`	`+ },`
	`4257`	`+ "Remove all unreachable code": {`
	`4258`	`+ "category": "Message",`
	`4259`	`+ "code": 95051`
`4252`	`4260`	`}`
`4253`	`4261`	`}`