Incorporated review comments and Added more testcases

debsmita1 · Feb 24, 2019 · 2e0825f · 2e0825f
1 parent 0f7d798
commit 2e0825f
Show file tree

Hide file tree

Showing 2 changed files with 128 additions and 32 deletions.
diff --git a/src/rules/unnecessaryElseRule.ts b/src/rules/unnecessaryElseRule.ts
@@ -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";

diff --git a/test/rules/unnecessary-else/test.ts.lint b/test/rules/unnecessary-else/test.ts.lint
@@ -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";
         }
     }
@@ -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 ;
         }
     }
@@ -48,7 +62,7 @@ const testReturn = (a) => {
     if (a===1) {
         return ;
     } else {
-           ~  [return]
+      ~~~~  [return]
         return ;
     }
 }
@@ -58,7 +72,7 @@ const testReturn = (a) => {
        if (a%3===0) {
            return;
         } else {
-               ~  [return]
+          ~~~~  [return]
             console.log(a)
         }
     }
@@ -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:
@@ -84,7 +112,7 @@ const testThrow = (a) => {
             if (true) {
                 throw "error";
             } else {
-                   ~  [throw]
+              ~~~~  [throw]
                 break;
             }
         default :
@@ -98,7 +126,7 @@ const testThrow = (a) => {
             if (a-i === 0) {
                 throw "error;
             } else {
-                   ~  [throw]
+              ~~~~  [throw]
                 console.log(i/a-i);
             }
         ++i;
@@ -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) {
@@ -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++;
         }
     }
@@ -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.