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

Commit

Permalink
Incorporated review comments and Added more testcases
Browse files Browse the repository at this point in the history
  • Loading branch information
@debsmita1
debsmita1 committed Feb 24, 2019
1 parent 0f7d798 commit 2e0825f
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 32 deletions.
35 changes: 26 additions & 9 deletions src/rules/unnecessaryElseRule.ts
View file
Expand Up @@ -53,24 +53,41 @@ export class Rule extends Lint.Rules.AbstractRule {
function walk(ctx: Lint.WalkContext<void>): void {
ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
if (utils.isIfStatement(node)) {
const jumpStatement = getJumpStatement(
getLastStatement(node.thenStatement as ts.Block),
);
if (jumpStatement !== undefined && node.elseStatement !== undefined) {
ctx.addFailureAtNode(
node.elseStatement.getChildAt(0),
Rule.FAILURE_STRING(jumpStatement),
let jumpStatement;
if (
node.thenStatement.kind !== ts.SyntaxKind.Block &&
!utils.isSameLine(ctx.sourceFile, node.thenStatement.pos, node.thenStatement.end)
) {
jumpStatement = getJumpStatement(
node.thenStatement.getChildAt(0, ctx.sourceFile).parent,
);
ts.forEachChild(node.elseStatement, cb);
} else if (
utils.isSameLine(ctx.sourceFile, node.thenStatement.pos, node.thenStatement.end)
) {
jumpStatement = getJumpStatement(node.thenStatement);
} else {
jumpStatement = getJumpStatement(getLastStatement(node.thenStatement as ts.Block));
}
if (jumpStatement !== undefined && node.elseStatement !== undefined) {
const elseKeyword = getPositionOfElseKeyword(node, ts.SyntaxKind.ElseKeyword);
ctx.addFailureAtNode(elseKeyword, Rule.FAILURE_STRING(jumpStatement));
}
ts.forEachChild(node.expression, cb);
ts.forEachChild(node.thenStatement, cb);
if (node.elseStatement !== undefined) {
ts.forEachChild(node.elseStatement, cb);
}
} else {
return ts.forEachChild(node, cb);
}
});
}

function getJumpStatement(node: ts.Statement): string | undefined {
function getPositionOfElseKeyword(node: ts.Node, kind: ts.SyntaxKind) {
return node.getChildren().filter(child => child.kind === kind)[0];
}

function getJumpStatement(node: ts.Statement | ts.Node): string | undefined {
switch (node.kind) {
case ts.SyntaxKind.BreakStatement:
return "break";
Expand Down
125 changes: 102 additions & 23 deletions test/rules/unnecessary-else/test.ts.lint
View file
Expand Up @@ -2,17 +2,31 @@ const testReturn = (a) => {
if (a===0) {
return 0;
} else {
~ [return]
~~~~ [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]
return "odd";
}
}
Expand All @@ -30,11 +44,11 @@ const testReturn = (a) => {
if (a<0) {
return;
} else if (a>0) {
~~ [return]
~~~~ [return]
if (a%2 === 0) {
return ;
} else {
~ [return]
~~~~ [return]
return ;
}
}
Expand All @@ -48,7 +62,7 @@ const testReturn = (a) => {
if (a===1) {
return ;
} else {
~ [return]
~~~~ [return]
return ;
}
}
Expand All @@ -58,7 +72,7 @@ const testReturn = (a) => {
if (a%3===0) {
return;
} else {
~ [return]
~~~~ [return]
console.log(a)
}
}
Expand All @@ -71,11 +85,25 @@ const testThrow = (a) => {
if ( a===0 ) {
throw "error";
} else {
~ [throw]
~~~~ [throw]
return 100/a;
}
}

const testThrow = (a) => {
if (a===0)
throw "error;
else
~~~~ [throw]
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:
Expand All @@ -84,7 +112,7 @@ const testThrow = (a) => {
if (true) {
throw "error";
} else {
~ [throw]
~~~~ [throw]
break;
}
default :
Expand All @@ -98,7 +126,7 @@ const testThrow = (a) => {
if (a-i === 0) {
throw "error;
} else {
~ [throw]
~~~~ [throw]
console.log(i/a-i);
}
++i;
Expand All @@ -112,17 +140,40 @@ const testThrow = (a) => {
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]
~~~~ [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) {
Expand All @@ -132,13 +183,21 @@ const testContinue = () => {
}
}

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]
~~~~ [break]
i++;
}
}
Expand All @@ -147,32 +206,52 @@ const testBreak = (a) => {

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

const testBreak = (a) => {
let i = 0;
while(i < 20) {
if (i === a) {
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` block is unnecessary.
[throw]: The preceding `if` block ends with a `throw` statement. This `else` block is unnecessary.
[break]: The preceding `if` block ends with a `break` statement. This `else` block is unnecessary.
Expand Down

0 comments on commit 2e0825f

Please sign in to comment.