Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[unnecessary-else] Allowed skipping else if statements via options #4599

Merged
merged 3 commits into from Apr 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 31 additions & 6 deletions src/rules/unnecessaryElseRule.ts
Expand Up @@ -21,15 +21,24 @@ import * as Lint from "../index";

import { codeExamples } from "./code-examples/unnecessaryElse.examples";

const OPTION_ALLOW_ELSE_IF = "allow-else-if";

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
description: Lint.Utils.dedent`
Disallows \`else\` blocks following \`if\` blocks ending with a \`break\`, \`continue\`, \`return\`, or \`throw\` statement.`,
descriptionDetails: "",
optionExamples: [true],
options: null,
optionsDescription: "Not configurable.",
optionExamples: [true, [true, { [OPTION_ALLOW_ELSE_IF]: true }]],
options: {
type: "object",
properties: {
[OPTION_ALLOW_ELSE_IF]: { type: "boolean" },
},
},
optionsDescription: Lint.Utils.dedent`
You can optionally specify the option \`"${OPTION_ALLOW_ELSE_IF}"\` to allow "else if" statements.
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
`,
rationale: Lint.Utils.dedent`
When an \`if\` block is guaranteed to exit control flow when entered,
it is unnecessary to add an \`else\` statement.
Expand All @@ -46,7 +55,11 @@ export class Rule extends Lint.Rules.AbstractRule {
}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
return this.applyWithFunction(
sourceFile,
walk,
parseOptions(this.ruleArguments[0] as Partial<Options> | undefined),
);
}
}

Expand All @@ -55,7 +68,18 @@ interface IJumpAndIfStatement {
node: ts.IfStatement;
}

function walk(ctx: Lint.WalkContext): void {
interface Options {
[OPTION_ALLOW_ELSE_IF]: boolean;
}

function parseOptions(option: Partial<Options> | undefined): Options {
return {
[OPTION_ALLOW_ELSE_IF]: false,
...option,
};
}

function walk(ctx: Lint.WalkContext<Options>): void {
const ifStatementStack: IJumpAndIfStatement[] = [];

function visitIfStatement(node: ts.IfStatement) {
Expand All @@ -68,7 +92,8 @@ function walk(ctx: Lint.WalkContext): void {
if (
jumpStatement !== undefined &&
node.elseStatement !== undefined &&
!recentStackParentMissingJumpStatement()
!recentStackParentMissingJumpStatement() &&
(!utils.isIfStatement(node.elseStatement) || !ctx.options[OPTION_ALLOW_ELSE_IF])
) {
const elseKeyword = getPositionOfElseKeyword(node, ts.SyntaxKind.ElseKeyword);
ctx.addFailureAtNode(elseKeyword, Rule.FAILURE_STRING(jumpStatement));
Expand Down
255 changes: 255 additions & 0 deletions test/rules/unnecessary-else/allow-else-if/test.ts.lint
@@ -0,0 +1,255 @@
const testReturn = (a) => {
if (a===0) {
return 0;
} else {
~~~~ [return]
return a;
}
}

const testReturn = (a) => {
if (a===0) return 0;
else return a;
~~~~ [return]
}

const testReturn = (a) => {
if (a===0)
return 0;
else
~~~~ [return]
return a;
}

const testReturn = (a) => {
if (a>0) {
if (a%2 ===0) {
return "even" ;
} else {
~~~~ [return]
return "odd";
}
}
return "negative";
}

const testReturn = (a) => {
if (a===0) {
return 0;
}
return a;
}

const testReturn = (a) => {
if (a<0) {
return;
} else if (a>0) {
if (a%2 === 0) {
return ;
} else if (a === 0) {
return ;
}
}
return;
}

const testReturn = (a) => {
if (a<0) {
return;
}
if (a===1) {
return ;
} else {
~~~~ [return]
return ;
}
}

const testReturn = (a) => {
if (a>0) {
if (a%3===0) {
return;
} else {
~~~~ [return]
console.log(a)
}
}
else {
console.log("negative");
}
}

const testThrow = (a) => {
if ( a===0 ) {
throw "error";
} else {
~~~~ [throw]
return 100/a;
}
}

const testThrow = (a) => {
if (a===0)
throw "error;
else if (a < 0)
console.log(100/a);
}

const testThrow = (a) => {
if (a===0) throw "error;
else console.log(100/a);
~~~~ [throw]
}

const testThrow = (a) => {
switch (a) {
case 1:
break;
case 2:
if (true) {
throw "error";
} else {
~~~~ [throw]
break;
}
default :
break;
}
}

const testThrow = (a) => {
let i = 1;
do {
if (a-i === 0) {
throw "error;
} else {
~~~~ [throw]
console.log(i/a-i);
}
++i;
}
}

const testThrow = (a) => {
if (a===0) {
throw "error";
}
return 100/a;
}

const testThrow = (a) => {
if (a===0) throw "error";
return 100/a;
}

const testContinue = () => {
for (let i = 1; i < 10; i++) {
if (i===8) {
continue ;
} else {
~~~~ [continue]
console.log(i);
}
}
}

const testContinue = () => {
for (let i = 1; i < 10; i++) {
if (i===8) continue ;
else console.log(i);
~~~~ [continue]
}
}

const testContinue = () => {
for (let i = 1; i < 10; i++) {
if (i===8)
continue ;
else
~~~~ [continue]
console.log(i);
}
}

const testContinue = () => {
for (let i = 1; i < 10; i++) {
if (i===4) {
continue ;
}
console.log(i);
}
}

const testContinue = () => {
for (let i = 1; i < 10; i++) {
if (i===4)
continue ;
console.log(i);
}
}

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a) {
break ;
} else {
~~~~ [break]
i++;
}
}
return i-1;
}

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a) {
break ;
}
i++;
}
return i-1;
}

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a)
break ;
i++;
}
return i-1;
}

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a) break ;
i++;
}
return i-1;
}

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a) break ;
else i++;
~~~~ [break]
}
return i-1;
}

const testNoJump = (a) => {
if (a % 2 === 0) {
console.log(a);
} else {
console.log(a * 2);
}
}

[return]: The preceding `if` block ends with a `return` statement. This `else` is unnecessary.
[throw]: The preceding `if` block ends with a `throw` statement. This `else` is unnecessary.
[break]: The preceding `if` block ends with a `break` statement. This `else` is unnecessary.
[continue]: The preceding `if` block ends with a `continue` statement. This `else` is unnecessary.
5 changes: 5 additions & 0 deletions test/rules/unnecessary-else/allow-else-if/tslint.json
@@ -0,0 +1,5 @@
{
"rules": {
"unnecessary-else": [true, { "allow-else-if": true }]
}
}