From 792206bcad534d6f76ee0eadcca6be66d84417e5 Mon Sep 17 00:00:00 2001 From: Klaus Meinhardt Date: Mon, 7 Aug 2017 22:08:39 +0200 Subject: [PATCH] prefer-conditional-expression: don't check else-if by default (#2963) --- src/rules/preferConditionalExpressionRule.ts | 35 ++++++++---- .../{ => check-else-if}/test.ts.lint | 5 +- .../check-else-if/tslint.json | 5 ++ .../default/test.ts.lint | 53 +++++++++++++++++++ .../{ => default}/tslint.json | 0 5 files changed, 84 insertions(+), 14 deletions(-) rename test/rules/prefer-conditional-expression/{ => check-else-if}/test.ts.lint (87%) create mode 100644 test/rules/prefer-conditional-expression/check-else-if/tslint.json create mode 100644 test/rules/prefer-conditional-expression/default/test.ts.lint rename test/rules/prefer-conditional-expression/{ => default}/tslint.json (100%) diff --git a/src/rules/preferConditionalExpressionRule.ts b/src/rules/preferConditionalExpressionRule.ts index 5e9af1e118e..08b93ba0c11 100644 --- a/src/rules/preferConditionalExpressionRule.ts +++ b/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 { - const { sourceFile } = ctx; +function walk(ctx: Lint.WalkContext): 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 { }); } -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; } diff --git a/test/rules/prefer-conditional-expression/test.ts.lint b/test/rules/prefer-conditional-expression/check-else-if/test.ts.lint similarity index 87% rename from test/rules/prefer-conditional-expression/test.ts.lint rename to test/rules/prefer-conditional-expression/check-else-if/test.ts.lint index 3ffbf69025a..f4bd6934c2e 100644 --- a/test/rules/prefer-conditional-expression/test.ts.lint +++ b/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. diff --git a/test/rules/prefer-conditional-expression/check-else-if/tslint.json b/test/rules/prefer-conditional-expression/check-else-if/tslint.json new file mode 100644 index 00000000000..07c97a9338a --- /dev/null +++ b/test/rules/prefer-conditional-expression/check-else-if/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "prefer-conditional-expression": [true, "check-else-if"] + } +} diff --git a/test/rules/prefer-conditional-expression/default/test.ts.lint b/test/rules/prefer-conditional-expression/default/test.ts.lint new file mode 100644 index 00000000000..23cd02a4086 --- /dev/null +++ b/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. diff --git a/test/rules/prefer-conditional-expression/tslint.json b/test/rules/prefer-conditional-expression/default/tslint.json similarity index 100% rename from test/rules/prefer-conditional-expression/tslint.json rename to test/rules/prefer-conditional-expression/default/tslint.json