Update: add allowParens option to no-confusing-arrow (fixes #5332)

docs/rules/no-confusing-arrow.md

@@ -1,6 +1,6 @@
 # Disallow arrow functions where they could be confused with comparisons (no-confusing-arrow)
 
-Arrow functions (`=>`) are similar in syntax to some comparison operators (`>`, `<`, `<=`, and `>=`). This rule warns against using the arrow function syntax in places where it could be confused with a comparison operator. Even if the arguments of the arrow function are wrapped with parens, this rule still warns about it.
+Arrow functions (`=>`) are similar in syntax to some comparison operators (`>`, `<`, `<=`, and `>=`). This rule warns against using the arrow function syntax in places where it could be confused with a comparison operator. Even if the arguments of the arrow function are wrapped with parens, this rule still warns about it unless `allowParens` is set to `true`.
 
 Here's an example where the usage of `=>` could be confusing:
 
@@ -21,8 +21,9 @@ The following patterns are considered warnings:
 /*eslint no-confusing-arrow: 2*/
 /*eslint-env es6*/
 
-var x = a => 1 ? 2 : 3
-var x = (a) => 1 ? 2 : 3
+var x = a => 1 ? 2 : 3;
+var x = (a) => 1 ? 2 : 3;
+var x = (a) => (1 ? 2 : 3);
 ```
 
 The following patterns are not considered warnings:
@@ -31,8 +32,34 @@ The following patterns are not considered warnings:
 /*eslint no-confusing-arrow: 2*/
 /*eslint-env es6*/
 
-var x = a => { return 1 ? 2 : 3; }
-var x = (a) => { return 1 ? 2 : 3; }
+var x = a => { return 1 ? 2 : 3; };
+var x = (a) => { return 1 ? 2 : 3; };
+```
+
+## Options
+
+This rule accepts a single options argument with the following defaults:
+
+```json
+{
+    "rules": {
+        "no-confusing-arrow": [2, {"allowParens": false}]
+    }
+}
+```
+
+`allowParens` is a boolean setting that can be `true` or `false`:
+
+1. `true` relaxes the rule and accepts parenthesis as a valid "confusion-preventing" syntax.
+2. `false` warns even if the expression is wrapped in parenthesis
+
+When `allowParens` is set to `true` following patterns are no longer considered as warnings:
+
+```js
+/*eslint no-confusing-arrow: [2, {allowParens: true}]*/
+/*eslint-env es6*/
+var x = a => (1 ? 2 : 3);
+var x = (a) => (1 ? 2 : 3);
 ```
 
 ## Related Rules

lib/ast-utils.js

@@ -168,6 +168,22 @@ function hasJSDocThisTag(node, sourceCode) {
     });
 }
 
+/**
+ * Determines if a node is surrounded by parentheses.
+ * @param {RuleContext} context The context object passed to the rule
+ * @param {ASTNode} node The node to be checked.
+ * @returns {boolean} True if the node is parenthesised.
+ * @private
+ */
+function isParenthesised(context, node) {
+    var previousToken = context.getTokenBefore(node),
+        nextToken = context.getTokenAfter(node);
+
+    return Boolean(previousToken && nextToken) &&
+        previousToken.value === "(" && previousToken.range[1] <= node.range[0] &&
+        nextToken.value === ")" && nextToken.range[0] >= node.range[1];
+}
+
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------
@@ -190,6 +206,7 @@ module.exports = {
     isES5Constructor: isES5Constructor,
     getUpperFunction: getUpperFunction,
     isArrayFromMethod: isArrayFromMethod,
+    isParenthesised: isParenthesised,
 
     /**
      * Checks whether or not a given node is a string literal.

lib/rules/no-confusing-arrow.js

@@ -28,6 +28,8 @@
 
 "use strict";
 
+var astUtils = require("../ast-utils.js");
+
 //------------------------------------------------------------------------------
 // Helpers
 //------------------------------------------------------------------------------
@@ -38,21 +40,24 @@
  * @returns {boolean} `true` if the node is a conditional expression.
  */
 function isConditional(node) {
-    return node.body && node.body.type === "ConditionalExpression";
+    return node && node.type === "ConditionalExpression";
 }
 
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
 
 module.exports = function(context) {
+    var config = context.options[0] || {};
+
     /**
      * Reports if an arrow function contains an ambiguous conditional.
      * @param {ASTNode} node - A node to check and report.
      * @returns {void}
      */
     function checkArrowFunc(node) {
-        if (isConditional(node)) {
+        var body = node.body;
+        if (isConditional(body) && !(config.allowParens && astUtils.isParenthesised(context, body))) {
             context.report(node, "Arrow function used ambiguously with a conditional expression.");
         }
     }
@@ -62,4 +67,10 @@ module.exports = function(context) {
     };
 };
 
-module.exports.schema = [];
+module.exports.schema = [{
+    type: "object",
+    properties: {
+        allowParens: {type: "boolean"}
+    },
+    additionalProperties: false
+}];

lib/rules/no-extra-parens.js

@@ -10,8 +10,10 @@
 // Rule Definition
 //------------------------------------------------------------------------------
 
-module.exports = function(context) {
+var astUtils = require("../ast-utils.js");
 
+module.exports = function(context) {
+    var isParenthesised = astUtils.isParenthesised.bind(astUtils, context);
     var ALL_NODES = context.options[0] !== "functions";
     var EXCEPT_COND_ASSIGN = ALL_NODES && context.options[1] && context.options[1].conditionalAssign === false;
     var sourceCode = context.getSourceCode();
@@ -26,21 +28,6 @@ module.exports = function(context) {
         return ALL_NODES || node.type === "FunctionExpression" || node.type === "ArrowFunctionExpression";
     }
 
-    /**
-     * Determines if a node is surrounded by parentheses.
-     * @param {ASTNode} node - The node to be checked.
-     * @returns {boolean} True if the node is parenthesised.
-     * @private
-     */
-    function isParenthesised(node) {
-        var previousToken = context.getTokenBefore(node),
-            nextToken = context.getTokenAfter(node);
-
-        return previousToken && nextToken &&
-            previousToken.value === "(" && previousToken.range[1] <= node.range[0] &&
-            nextToken.value === ")" && nextToken.range[0] >= node.range[1];
-    }
-
     /**
      * Determines if a node is surrounded by parentheses twice.
      * @param {ASTNode} node - The node to be checked.

tests/lib/ast-utils.js

@@ -11,8 +11,10 @@
 
 var assert = require("chai").assert,
     sinon = require("sinon"),
+    espree = require("espree"),
     astUtils = require("../../lib/ast-utils"),
-    eslint = require("../../lib/eslint");
+    eslint = require("../../lib/eslint"),
+    SourceCode = require("../../lib/util/source-code");
 
 //------------------------------------------------------------------------------
 // Tests
@@ -297,4 +299,29 @@ describe("ast-utils", function() {
             eslint.verify("/*eslint bar", {}, filename, true);
         });
     });
+
+    describe("isParenthesised", function() {
+        var ESPREE_CONFIG = {
+            ecmaVersion: 6,
+            comment: true,
+            tokens: true,
+            range: true,
+            loc: true
+        };
+        it("should return false for not parenthesised nodes", function() {
+            var code = "condition ? 1 : 2";
+            var ast = espree.parse(code, ESPREE_CONFIG);
+            var sourceCode = new SourceCode(code, ast);
+
+            assert.isFalse(astUtils.isParenthesised(sourceCode, ast.body[0].expression));
+        });
+
+        it("should return true for not parenthesised nodes", function() {
+            var code = "(condition ? 1 : 2)";
+            var ast = espree.parse(code, ESPREE_CONFIG);
+            var sourceCode = new SourceCode(code, ast);
+
+            assert.isTrue(astUtils.isParenthesised(sourceCode, ast.body[0].expression));
+        });
+    });
 });

tests/lib/rules/no-confusing-arrow.js

@@ -57,7 +57,8 @@ ruleTester.run("no-confusing-arrow", rule, {
     valid: [
         { code: "a => { return 1 ? 2 : 3; }" },
         { code: "var x = a => { return 1 ? 2 : 3; }" },
-        { code: "var x = (a) => { return 1 ? 2 : 3; }" }
+        { code: "var x = (a) => { return 1 ? 2 : 3; }" },
+        { code: "var x = a => (1 ? 2 : 3)", options: [{ allowParens: true }]}
     ].map(addArrowFunctions),
     invalid: [
         {
@@ -71,6 +72,10 @@ ruleTester.run("no-confusing-arrow", rule, {
         {
             code: "var x = (a) => 1 ? 2 : 3",
             errors: [{ message: "Arrow function used ambiguously with a conditional expression." }]
+        },
+        {
+            code: "var x = a => (1 ? 2 : 3)",
+            errors: [{ message: "Arrow function used ambiguously with a conditional expression." }]
         }
     ].map(addArrowFunctions)
 });