Fix: no-this-before-super crash on unreachable paths (fixes #5894)

eslint · Apr 20, 2016 · f842d18 · f842d18
1 parent bd518d3
commit f842d18
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 25 deletions.
diff --git a/lib/rules/constructor-super.js b/lib/rules/constructor-super.js
@@ -10,6 +10,16 @@
 // Helpers
 //------------------------------------------------------------------------------
 
+/**
+ * Checks whether a given code path segment is reachable or not.
+ *
+ * @param {CodePathSegment} segment - A code path segment to check.
+ * @returns {boolean} `true` if the segment is reachable.
+ */
+function isReachable(segment) {
+    return segment.reachable;
+}
+
 /**
  * Checks whether or not a given node is a constructor.
  * @param {ASTNode} node - A node to check. This node type is one of
@@ -119,7 +129,7 @@ module.exports = {
          * @returns {boolean} The flag which shows `super()` is called in some paths
          */
         function isCalledInSomePath(segment) {
-            return segInfoMap[segment.id].calledInSomePaths;
+            return segment.reachable && segInfoMap[segment.id].calledInSomePaths;
         }
 
         /**
@@ -139,7 +149,7 @@ module.exports = {
             ) {
                 return true;
             }
-            return segInfoMap[segment.id].calledInEveryPaths;
+            return segment.reachable && segInfoMap[segment.id].calledInEveryPaths;
         }
 
         return {
@@ -297,29 +307,37 @@ module.exports = {
                 // Reports if needed.
                 if (funcInfo.hasExtends) {
                     var segments = funcInfo.codePath.currentSegments;
+                    var reachable = false;
                     var duplicate = false;
 
                     for (var i = 0; i < segments.length; ++i) {
-                        var info = segInfoMap[segments[i].id];
+                        var segment = segments[i];
 
-                        duplicate = duplicate || info.calledInSomePaths;
-                        info.calledInSomePaths = info.calledInEveryPaths = true;
+                        if (segment.reachable) {
+                            var info = segInfoMap[segment.id];
+
+                            reachable = true;
+                            duplicate = duplicate || info.calledInSomePaths;
+                            info.calledInSomePaths = info.calledInEveryPaths = true;
+                        }
                     }
 
-                    if (duplicate) {
-                        context.report({
-                            message: "Unexpected duplicate 'super()'.",
-                            node: node
-                        });
-                    } else if (!funcInfo.superIsConstructor) {
-                        context.report({
-                            message: "Unexpected 'super()' because 'super' is not a constructor.",
-                            node: node
-                        });
-                    } else {
-                        info.validNodes.push(node);
+                    if (reachable) {
+                        if (duplicate) {
+                            context.report({
+                                message: "Unexpected duplicate 'super()'.",
+                                node: node
+                            });
+                        } else if (!funcInfo.superIsConstructor) {
+                            context.report({
+                                message: "Unexpected 'super()' because 'super' is not a constructor.",
+                                node: node
+                            });
+                        } else {
+                            info.validNodes.push(node);
+                        }
                     }
-                } else {
+                } else if (funcInfo.codePath.currentSegments.some(isReachable)) {
                     context.report({
                         message: "Unexpected 'super()'.",
                         node: node
@@ -346,9 +364,13 @@ module.exports = {
                 var segments = funcInfo.codePath.currentSegments;
 
                 for (var i = 0; i < segments.length; ++i) {
-                    var info = segInfoMap[segments[i].id];
+                    var segment = segments[i];
 
-                    info.calledInSomePaths = info.calledInEveryPaths = true;
+                    if (segment.reachable) {
+                        var info = segInfoMap[segment.id];
+
+                        info.calledInSomePaths = info.calledInEveryPaths = true;
+                    }
                 }
             },
 

diff --git a/lib/rules/no-this-before-super.js b/lib/rules/no-this-before-super.js
@@ -62,7 +62,7 @@ module.exports = function(context) {
      * @returns {boolean} `true` if `super()` is called.
      */
     function isCalled(segment) {
-        return segInfoMap[segment.id].superCalled;
+        return !segment.reachable || segInfoMap[segment.id].superCalled;
     }
 
     /**
@@ -94,7 +94,11 @@ module.exports = function(context) {
         var segments = funcInfo.codePath.currentSegments;
 
         for (var i = 0; i < segments.length; ++i) {
-            segInfoMap[segments[i].id].invalidNodes.push(node);
+            var segment = segments[i];
+
+            if (segment.reachable) {
+                segInfoMap[segment.id].invalidNodes.push(node);
+            }
         }
     }
 
@@ -106,7 +110,11 @@ module.exports = function(context) {
         var segments = funcInfo.codePath.currentSegments;
 
         for (var i = 0; i < segments.length; ++i) {
-            segInfoMap[segments[i].id].superCalled = true;
+            var segment = segments[i];
+
+            if (segment.reachable) {
+                segInfoMap[segment.id].superCalled = true;
+            }
         }
     }
 

diff --git a/tests/lib/rules/constructor-super.js b/tests/lib/rules/constructor-super.js
@@ -78,7 +78,10 @@ ruleTester.run("constructor-super", rule, {
                 "}"
             ].join("\n"),
             parserOptions: {ecmaVersion: 6}
-        }
+        },
+
+        // https://github.com/eslint/eslint/issues/5894
+        { code: "class A { constructor() { return; super(); } }", parserOptions: {ecmaVersion: 6} }
     ],
     invalid: [
 
@@ -230,6 +233,13 @@ ruleTester.run("constructor-super", rule, {
                 { message: "Lacked a call of 'super()' in some code paths.", type: "MethodDefinition"},
                 { message: "Unexpected duplicate 'super()'.", type: "CallExpression", column: 48}
             ]
+        },
+
+        // ignores `super()` on unreachable paths.
+        {
+            code: "class A extends B { constructor() { return; super(); } }",
+            parserOptions: {ecmaVersion: 6},
+            errors: [{ message: "Expected to call 'super()'.", type: "MethodDefinition"}]
         }
     ]
 });
diff --git a/tests/lib/rules/no-this-before-super.js b/tests/lib/rules/no-this-before-super.js
@@ -77,7 +77,11 @@ ruleTester.run("no-this-before-super", rule, {
                 "}"
             ].join("\n"),
             parserOptions: {ecmaVersion: 6}
-        }
+        },
+
+        // https://github.com/eslint/eslint/issues/5894
+        { code: "class A { constructor() { return; this; } }", parserOptions: {ecmaVersion: 6} },
+        { code: "class A extends B { constructor() { return; this; } }", parserOptions: {ecmaVersion: 6} }
     ],
     invalid: [