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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions src/rules/unnecessaryElseRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,16 @@ export class Rule extends Lint.Rules.AbstractRule {
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, { allowElseIf: true }]],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowElseIf

TSLint rules use dash-case, so this should be allow-else-if. For example, noAnyRule.ts has const OPTION_IGNORE_REST_ARGS = "ignore-rest-args";.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 03b2414

options: {
type: "object",
properties: {
allowElseIf: { type: "boolean" },
},
},
optionsDescription: Lint.Utils.dedent`
You can optionally specify the option \`"allowElseIf"\` to allow "else if" statements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`"allowElseIf"`

Rules typically create an all-caps const for these option names to reduce the chance of mispelling them anywhere. For example, curlyRule.ts declares OPTION_AS_NEEDED and OPTION_IGNORE_SAME_LINE. Let's switch to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 03b2414

`,
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 +53,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 +66,18 @@ interface IJumpAndIfStatement {
node: ts.IfStatement;
}

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

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

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

function visitIfStatement(node: ts.IfStatement) {
Expand All @@ -68,7 +90,8 @@ function walk(ctx: Lint.WalkContext): void {
if (
jumpStatement !== undefined &&
node.elseStatement !== undefined &&
!recentStackParentMissingJumpStatement()
!recentStackParentMissingJumpStatement() &&
(!utils.isIfStatement(node.elseStatement) || !ctx.options.allowElseIf)
) {
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"unnecessary-else": [true, { "allowElseIf": true }]
}
}