prefer-conditional-expression: don't check else-if by default (#2963)

src/rules/preferConditionalExpressionRule.ts

@@ -20,6 +20,12 @@ import * as ts from "typescript";
 
 import * as Lint from "../index";
 
+const OPTION_CHECK_ELSE_IF = "check-else-if";
+
+interface Options {
+    checkElseIf: boolean;
+}
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
@@ -28,9 +34,12 @@ export class Rule extends Lint.Rules.AbstractRule {
             Recommends to use a conditional expression instead of assigning to the same thing in each branch of an if statement.`,
         rationale: Lint.Utils.dedent`
             This reduces duplication and can eliminate an unnecessary variable declaration.`,
-        optionsDescription: "Not configurable.",
-        options: null,
-        optionExamples: [true],
+        optionsDescription: `If \`${OPTION_CHECK_ELSE_IF}\` is specified, the rule also checks nested if-else-if statements.`,
+        options: {
+            type: "string",
+            enum: [OPTION_CHECK_ELSE_IF],
+        },
+        optionExamples: [true, [true, OPTION_CHECK_ELSE_IF]],
         type: "functionality",
         typescriptOnly: false,
     };
@@ -41,19 +50,23 @@ export class Rule extends Lint.Rules.AbstractRule {
     }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithFunction(sourceFile, walk);
+        return this.applyWithFunction(sourceFile, walk, {
+            checkElseIf: this.ruleArguments.indexOf(OPTION_CHECK_ELSE_IF) !== -1,
+        });
     }
 }
 
-function walk(ctx: Lint.WalkContext<void>): void {
-    const { sourceFile } = ctx;
+function walk(ctx: Lint.WalkContext<Options>): void {
+    const { sourceFile, options: { checkElseIf } } = ctx;
     return ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
         if (isIfStatement(node)) {
-            const assigned = detect(node, sourceFile);
+            const assigned = detect(node, sourceFile, checkElseIf);
             if (assigned !== undefined) {
                 ctx.addFailureAtNode(
-                    Lint.childOfKind(node, ts.SyntaxKind.IfKeyword)!,
+                    node.getChildAt(0, sourceFile),
                     Rule.FAILURE_STRING(assigned.getText(sourceFile)));
+            }
+            if (assigned !== undefined || !checkElseIf) {
                 // Be careful not to fail again for the "else if"
                 ts.forEachChild(node.expression, cb);
                 ts.forEachChild(node.thenStatement, cb);
@@ -67,11 +80,11 @@ function walk(ctx: Lint.WalkContext<void>): void {
     });
 }
 
-function detect({ thenStatement, elseStatement }: ts.IfStatement, sourceFile: ts.SourceFile): ts.Expression | undefined {
-    if (elseStatement === undefined) {
+function detect({ thenStatement, elseStatement }: ts.IfStatement, sourceFile: ts.SourceFile, elseIf: boolean): ts.Expression | undefined {
+    if (elseStatement === undefined || !elseIf && elseStatement.kind === ts.SyntaxKind.IfStatement) {
         return undefined;
     }
-    const elze = isIfStatement(elseStatement) ? detect(elseStatement, sourceFile) : getAssigned(elseStatement, sourceFile);
+    const elze = isIfStatement(elseStatement) ? detect(elseStatement, sourceFile, elseIf) : getAssigned(elseStatement, sourceFile);
     if (elze === undefined) {
         return undefined;
     }

test/rules/prefer-conditional-expression/test.ts.lint → test/rules/prefer-conditional-expression/check-else-if/test.ts.lint

@@ -1,13 +1,13 @@
 let x;
 if (true) {
-~~ [X]
+~~ [err % ('x')]
     x = 1;
 } else {
     x = 2;
 }
 
 if (true)
-~~ [X]
+~~ [err % ('x')]
     x = "TRUE";
 else if (false)
     x = "FALSE";
@@ -51,5 +51,4 @@ if (true) {
     foo(bar).baz = 1;
 }
 
-[X]: Use a conditional expression instead of assigning to 'x' in multiple places.
 [err]: Use a conditional expression instead of assigning to '%s' in multiple places.

test/rules/prefer-conditional-expression/check-else-if/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "prefer-conditional-expression": [true, "check-else-if"]
+  }
+}

test/rules/prefer-conditional-expression/default/test.ts.lint

@@ -0,0 +1,53 @@
+let x;
+if (true) {
+~~ [err % ('x')]
+    x = 1;
+} else {
+    x = 2;
+}
+
+if (true)
+    x = "TRUE";
+else if (false)
+    x = "FALSE";
+else
+    x = "FILE_NOT_FOUND";
+
+// Must assign same variable
+if (true) {
+    x = 1;
+} else {
+    y = 2;
+}
+
+// All branches must be present
+if (true) {
+    x = 1;
+}
+
+// Must not be a multi-statement block.
+if (true) {
+    x = 1;
+} else {
+    x = 2;
+    y = 3;
+}
+
+// Or even multi-line.
+if (true)
+    x = [
+        1,
+        2
+    ];
+else
+    x = 3;
+
+// Works for complex left hand side.
+if (true) {
+~~ [err % ('foo(bar).baz')]
+    foo(bar).baz = 0;
+} else {
+    foo(bar).baz = 1;
+}
+
+[err]: Use a conditional expression instead of assigning to '%s' in multiple places.