New: Rule - 'no-unsafe-finally' (fixes eslint#5808)

conf/eslint.json

@@ -109,6 +109,7 @@
         "no-unmodified-loop-condition": "off",
         "no-unneeded-ternary": "off",
         "no-unreachable": "error",
+        "no-unsafe-finally": "off",
         "no-unused-expressions": "off",
         "no-unused-labels": "error",
         "no-unused-vars": "error",

docs/rules/README.md

@@ -35,6 +35,7 @@ These rules relate to possible syntax or logic errors in JavaScript code:
 * [no-sparse-arrays](no-sparse-arrays.md): disallow sparse arrays (recommended)
 * [no-unexpected-multiline](no-unexpected-multiline.md): disallow confusing multiline expressions (recommended)
 * [no-unreachable](no-unreachable.md): disallow unreachable code after `return`, `throw`, `continue`, and `break` statements (recommended)
+* [no-unsafe-finally](no-unsafe-finally.md): disallow control flow statements in `finally` blocks
 * [use-isnan](use-isnan.md): require calls to `isNaN()` when checking for `NaN` (recommended)
 * [valid-jsdoc](valid-jsdoc.md): enforce valid JSDoc comments
 * [valid-typeof](valid-typeof.md): enforce comparing `typeof` expressions against valid strings (recommended)

docs/rules/no-unsafe-finally.md

@@ -0,0 +1,134 @@
+# disallow control flow statements in `finally` blocks (no-unsafe-finally)
+
+Javascript suspends the control flow statements of `try` and `catch` blocks until the execution of `finally` block finishes. So, when a control flow statement is used in a `finally` block, it overwrites the previous statements, which is considered as unexpected behavior. Examples of this situation:
+
+When `return` is used:
+
+```js
+(() => {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        return 3;
+    }
+})();
+
+// > 3
+```
+
+When `throw` is used:
+
+```js
+(() => {
+    try {
+        throw new Error("Try");
+    } catch(err) {
+        throw err;
+    } finally {
+        throw new Error("Finally");
+    }
+})();
+
+// > Uncaught Error: Finally(...)
+```
+
+When `break` is used:
+
+```js
+(() => {
+  label: try {
+    return 0;
+  } finally {
+    break label;
+  }
+  return 1;
+})();
+
+// > 1
+```
+
+## Rule Details
+
+This rule disallows `return`, `throw`, `break` and `continue` statements inside `finally` blocks. It allows indirect usages such as in `function` or `class` declarations / expressions etc.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint no-unsafe-finally: "error"*/
+let foo = function() {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        return 3;
+    }
+};
+```
+
+```js
+/*eslint no-unsafe-finally: "error"*/
+let foo = function() {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        throw new Error;
+    }
+};
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint no-unsafe-finally: "error"*/
+let foo = function() {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        console.log("hola!");
+    }
+};
+```
+
+```js
+/*eslint no-unsafe-finally: "error"*/
+let foo = function() {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        let a = function() {
+            return "hola!";
+        }
+    }
+};
+```
+
+```js
+/*eslint no-unsafe-finally: "error"*/
+let foo = function(a) {
+    try {
+        return 1;
+    } catch(err) {
+        return 2;
+    } finally {
+        switch(a) {
+            case 1: {
+                console.log("hola!")
+                break;
+            }
+        }
+    }
+};
+```
+
+## When Not To Use It
+
+If you want to allow control flow operations in finally blocks, you can turn this rule off.

lib/rules/no-unsafe-finally.js

@@ -0,0 +1,78 @@
+/**
+ * @fileoverview Rule to flag unsafe statements in finally block
+ * @author Onur Temizkan
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+var SENTINEL_NODE_TYPE = /^(?:Program|(?:Function|Class)(?:Declaration|Expression)|ArrowFunctionExpression)$/;
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "disallow control flow statements in finally blocks",
+            category: "Possible Errors",
+            recommended: false
+        }
+    },
+    create: function(context) {
+
+        /**
+         * Checks if the node is the finalizer of a TryStatement
+         *
+         * @param {ASTNode} node - node to check.
+         * @returns {Boolean} - true if the node is the finalizer of a TryStatement
+         */
+        function isFinallyBlock(node) {
+            return node.parent.type === "TryStatement" && node.parent.finalizer === node;
+        }
+
+        /**
+         * Climbs up the tree if the node is not a sentinel node
+         *
+         * @param {ASTNode} node - node to check.
+         * @returns {Boolean} - return whether the node is a finally block or a sentinel node
+         */
+        function isInFinallyBlock(node) {
+            while (node && !SENTINEL_NODE_TYPE.test(node.type)) {
+                if (isFinallyBlock(node)) {
+                    return true;
+                }
+                node = node.parent;
+            }
+            return false;
+        }
+
+        /**
+         * Checks whether the possibly-unsafe statement is inside a finally block.
+         *
+         * @param {ASTNode} node - node to check.
+         * @returns {void}
+         */
+        function check(node) {
+            if (isInFinallyBlock(node)) {
+                context.report({
+                    message: "Unsafe usage of " + node.type,
+                    node: node,
+                    line: node.loc.line,
+                    column: node.loc.column
+                });
+            }
+        }
+
+        return {
+            ReturnStatement: check,
+            ThrowStatement: check,
+            BreakStatement: check,
+            ContinueStatement: check
+        };
+    }
+};

tests/lib/rules/no-unsafe-finally.js

@@ -0,0 +1,87 @@
+/**
+ * @fileoverview Tests for no-unpredictable-finally
+ * @author Onur Temizkan
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var rule = require("../../../lib/rules/no-unsafe-finally"),
+    RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+var ruleTester = new RuleTester();
+
+ruleTester.run("no-unsafe-finally", rule, {
+    valid: [
+        "var foo = function() {\n try { \n return 1; \n } catch(err) { \n return 2; \n } finally { \n console.log('hola!') \n } \n }",
+        "var foo = function() { try { return 1 } catch(err) { return 2 } finally { console.log('hola!') } }",
+        "var foo = function() { try { return 1 } catch(err) { return 2 } finally { function a(x) { return x } } }",
+        "var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = function(x) { if(!x) { throw new Error() } } } }",
+        "var foo = function() { try { return 1 } catch(err) { return 2 } finally { var a = function(x) { while(true) { if(x) { break } else { continue } } } } }",
+        {
+            code: "var foo = function() { try { return 1; } catch(err) { return 2; } finally { var bar = () => { throw new Error(); }; } };",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var foo = function() { try { return 1; } catch(err) { return 2 } finally { (x) => x } }",
+            parserOptions: { ecmaVersion: 6 }
+        },
+
+        {
+            code: "var foo = function() { try { return 1; } finally { class bar { constructor() {} static ehm() { return 'Hola!'; } } } };",
+            parserOptions: { ecmaVersion: 6 }
+        }
+
+    ],
+    invalid: [
+        {
+            code: "var foo = function() { \n try { \n return 1; \n } catch(err) { \n return 2; \n } finally { \n return 3; \n } \n }",
+            errors: [{ message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 7, column: 2 }]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { if(true) { return 3 } else { return 2 } } }",
+            errors: [
+                { message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 86 },
+                { message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 104 }
+            ]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { return 3 } }",
+            errors: [{ message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 75 }]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { return function(x) { return y } } }",
+            errors: [{ message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 75 }]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { return { x: function(c) { return c } } } }",
+            errors: [{ message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 75 }]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { throw new Error() } }",
+            errors: [{ message: "Unsafe usage of ThrowStatement", type: "ThrowStatement", line: 1, column: 75 }]
+        },
+        {
+            code: "var foo = function() { try { return 1 } catch(err) { return 2 } finally { while(true) { if(x) { break } else { continue } } } }",
+            errors: [
+                { message: "Unsafe usage of BreakStatement", type: "BreakStatement", line: 1, column: 97 },
+                { message: "Unsafe usage of ContinueStatement", type: "ContinueStatement", line: 1, column: 112 }
+            ]
+        },
+        {
+            code: "var foo = function() { try { foo(); } finally { try { bar(); } finally { return; } } };",
+            errors: [{ message: "Unsafe usage of ReturnStatement", type: "ReturnStatement", line: 1, column: 74 }]
+        },
+        {
+            code: "var foo = function() { label: try { return 0; } finally { break label; } return 1; }",
+            errors: [{ message: "Unsafe usage of BreakStatement", type: "BreakStatement", line: 1, column: 59}]
+        }
+    ]
+});