Added fixer for switch-final-break (#3615)

palantir · May 30, 2018 · db5fa51 · db5fa51
1 parent 2cd3dee
commit db5fa51
Show file tree

Hide file tree

Showing 3 changed files with 152 additions and 13 deletions.
diff --git a/src/rules/switchFinalBreakRule.ts b/src/rules/switchFinalBreakRule.ts
@@ -27,6 +27,7 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "switch-final-break",
         description: "Checks whether the final clause of a switch statement ends in \`break;\`.",
+        hasFix: true,
         optionsDescription: Lint.Utils.dedent`
             If no options are passed, a final 'break;' is forbidden.
             If the "always" option is passed this will require a 'break;' to always be present
@@ -70,26 +71,54 @@ function walk(ctx: Lint.WalkContext<Options>): void {
 
         if (always) {
             if (!endsControlFlow(clause)) {
-                ctx.addFailureAtNode(clause.getChildAt(0), Rule.FAILURE_STRING_ALWAYS);
+                ctx.addFailureAtNode(clause.getChildAt(0), Rule.FAILURE_STRING_ALWAYS, createAddFix(clause));
             }
             return;
         }
 
-        if (clause.statements.length === 0) { return; }
-        const block = clause.statements[0];
-        const statements = clause.statements.length === 1 && isBlock(block) ? block.statements : clause.statements;
-        const lastStatement = last(statements);
-        if (lastStatement !== undefined && isBreakStatement(lastStatement)) {
-            if (lastStatement.label !== undefined) {
-                const parent = node.parent!;
-                if (!isLabeledStatement(parent) || parent.label === lastStatement.label) {
-                    // break jumps somewhere else, don't complain
-                    return;
-                }
+        const lastStatement = getLastStatement(clause);
+        if (lastStatement === undefined || !isBreakStatement(lastStatement)) {
+            return;
+        }
+
+        if (lastStatement.label !== undefined) {
+            const parent = node.parent!;
+            if (!isLabeledStatement(parent) || parent.label === lastStatement.label) {
+                // break jumps somewhere else, don't complain
+                return;
             }
-            ctx.addFailureAtNode(lastStatement, Rule.FAILURE_STRING_NEVER);
         }
+
+        ctx.addFailureAtNode(lastStatement, Rule.FAILURE_STRING_NEVER, createRemoveFix(lastStatement));
+    }
+
+    function createAddFix(clause: ts.CaseClause | ts.DefaultClause) {
+        const lastStatement = getLastStatement(clause);
+        if (lastStatement === undefined) {
+            return Lint.Replacement.appendText(clause.end, " break;");
+        }
+
+        const fullText = lastStatement.getFullText(ctx.sourceFile);
+        const indentation = fullText.slice(0, fullText.search(/\S+/));
+
+        return Lint.Replacement.appendText(lastStatement.end, `${indentation}break;`);
     }
+
+    function createRemoveFix(lastStatement: ts.BreakStatement) {
+        return Lint.Replacement.replaceFromTo(lastStatement.getFullStart(), lastStatement.end, "");
+    }
+
+}
+
+function getLastStatement(clause: ts.CaseClause | ts.DefaultClause): ts.Statement | undefined {
+    if (clause.statements.length === 0) {
+        return undefined;
+    }
+
+    const block = clause.statements[0];
+    const statements = clause.statements.length === 1 && isBlock(block) ? block.statements : clause.statements;
+
+    return last(statements);
 }
 
 function last<T>(arr: ReadonlyArray<T>): T | undefined {

diff --git a/test/rules/switch-final-break/always/test.ts.fix b/test/rules/switch-final-break/always/test.ts.fix
@@ -0,0 +1,49 @@
+switch (x) {
+    case 0:
+        foo();
+        break;
+}
+
+switch (x) {
+    case 0: {
+        foo();
+        break;
+    }
+}
+
+switch (x) {
+    case 0:
+        foo();
+        break;
+}
+
+switch (x) {
+    case 0: {
+        foo();
+        break;
+    }
+}
+
+switch (x) {
+    default:
+        foo();
+        break;
+}
+
+switch (x) {
+    default:
+        foo();
+        break;
+}
+
+switch (x) {
+    default: {
+        foo();
+        break;
+    }
+}
+
+switch (x) {
+    case 0: break;
+}
+
diff --git a/test/rules/switch-final-break/default/test.ts.fix b/test/rules/switch-final-break/default/test.ts.fix
@@ -0,0 +1,61 @@
+switch (x) {
+    case 0:
+        foo();
+}
+
+switch (x) {
+    case 0: {
+        foo();
+    }
+}
+
+switch (x) {
+    case 0:
+        foo();
+}
+
+switch (x) {
+    case 0: {
+        foo();
+    }
+}
+
+switch (x) {
+    default:
+        foo();
+}
+
+switch (x) {
+    default:
+        foo();
+}
+
+switch (x) {
+    default: {
+        foo();
+    }
+}
+
+switch (x) {
+    case 0:
+}
+
+outer: while (true) {
+    switch (x) {
+        case 0:
+            x++;
+            break;
+        default:
+            break outer;
+    }
+}
+
+outer2: while (true) {
+    inner: switch (x) {
+        case 0:
+            ++x;
+            break;
+        default:
+    }
+}
+