Repository: eslint/eslint (js)

Issue:
no-extra-parens incorrectly reports returned IIFE
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).

    This template is for bug reports. If you are reporting a bug, please continue on. If you are here for another reason, please see below:

    1. To propose a new rule: http://eslint.org/docs/developer-guide/contributing/new-rules
    2. To request a change: http://eslint.org/docs/developer-guide/contributing/changes
    3. If you have any questions, please stop by our chatroom: https://gitter.im/eslint/eslint

    Note that leaving sections blank will make it difficult for us to troubleshoot and we may have to close the issue.
-->

**Tell us about your environment**

* **ESLint Version:** 4.5.0
* **Node Version:** 8.1.3
* **npm Version:** 5.3.0

**What parser (default, Babel-ESLint, etc.) are you using?**
default

**Please show your full configuration:**
<details>
<summary>Configuration</summary>

```sh
node node_modules/eslint/bin/eslint.js --no-eslintrc --rule '{"wrap-iife": [2, "any"], "no-extra-parens": 2, "no-trailing-spaces": 2, "quotes":[2, "double"]}' --fix test/compiled.js
```
</details>

**What did you do? Please include the actual source code causing the issue.**

<!-- Paste the source code below: -->
```js
(function() {
    // trailing spaces    
    console.log('quotes');

    // extra parens
    return (function() {
        // trailing spaces    
        console.log('quotes');
    }());
})();
```

**What did you expect to happen?**
Fix option to be executed on nested iife

**What actually happened? Please include the actual, raw output from ESLint.**
When applying the command line above on the code above, quotes fix is applied on the top iife but not on the neted iife.

Without the `wrap-iife` everything is fine.

I haven't tested if the fix is not applied only for those specified rules.

On v4.5.0 here is the output
```sh
  6:12  error  Gratuitous parentheses around expression  no-extra-parens
  8:21  error  Strings must use doublequote              quotes
```

On v3.19.0 here is the output
```sh
  6:12  error  Gratuitous parentheses around expression  no-extra-parens
  7:30  error  Trailing spaces not allowed               no-trailing-spaces
  8:21  error  Strings must use doublequote              quotes
```



Pull request:
Fix: no-extra-parens reported some parenthesized IIFEs (fixes #9140)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/9140)

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

This updates `no-extra-parens` to handle IIFEs more consistently by doing an IIFE check as part of the `report` call, rather than overloading the precedence calculation with special logic for IIFEs.

The bug occurred because the rule was assuming that the precedence calculation would always return -1 for an IIFE, preventing it from ever being reported (since any surrounding parens would always be considered necessary). However, the `ReturnStatement` listener doesn't check the precedence of its argument (since it correctly assumes that `return` statements have the lowest precedence), so it incorrectly caused the IIFE to be reported.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


Patch:
diff --git a/Makefile.js b/Makefile.js
index bf175d261..0c1424aa6 100644
--- a/Makefile.js
+++ b/Makefile.js
@@ -631,11 +631,13 @@ target.gensite = function(prereleaseVersion) {
     }
 
     // 1. create temp and build directory
+    echo("> Creating a temporary directory (Step 1)");
     if (!test("-d", TEMP_DIR)) {
         mkdir(TEMP_DIR);
     }
 
     // 2. remove old files from the site
+    echo("> Removing old files (Step 2)");
     docFiles.forEach(filePath => {
         const fullPath = path.join(DOCS_DIR, filePath),
             htmlFullPath = fullPath.replace(".md", ".html");
@@ -651,6 +653,7 @@ target.gensite = function(prereleaseVersion) {
     });
 
     // 3. Copy docs folder to a temporary directory
+    echo("> Copying the docs folder (Step 3)");
     cp("-rf", "docs/*", TEMP_DIR);
 
     let versions = test("-f", "./versions.json") ? JSON.parse(cat("./versions.json")) : {};
@@ -668,7 +671,11 @@ target.gensite = function(prereleaseVersion) {
     const FIXABLE_TEXT = "\n\n(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) can automatically fix some of the problems reported by this rule.";
 
     // 4. Loop through all files in temporary directory
-    find(TEMP_DIR).forEach(filename => {
+    process.stdout.write("> Updating files (Steps 4-9): 0/... - ...\r");
+    const tempFiles = find(TEMP_DIR);
+    const length = tempFiles.length;
+
+    tempFiles.forEach((filename, i) => {
         if (test("-f", filename) && path.extname(filename) === ".md") {
 
             const rulesUrl = "https://github.com/eslint/eslint/tree/master/lib/rules/",
@@ -676,10 +683,13 @@ target.gensite = function(prereleaseVersion) {
                 baseName = path.basename(filename),
                 sourceBaseName = `${path.basename(filename, ".md")}.js`,
                 sourcePath = path.join("lib/rules", sourceBaseName),
-                ruleName = path.basename(filename, ".md");
+                ruleName = path.basename(filename, ".md"),
+                filePath = path.join("docs", path.relative("tmp", filename));
             let text = cat(filename),
                 title;
 
+            process.stdout.write(`> Updating files (Steps 4-9): ${i}/${length} - ${filePath + " ".repeat(30)}\r`);
+
             // 5. Prepend page title and layout variables at the top of rules
             if (path.dirname(filename).indexOf("rules") >= 0) {
 
@@ -695,7 +705,7 @@ target.gensite = function(prereleaseVersion) {
 
                 text = `${ruleHeading}${isRecommended ? RECOMMENDED_TEXT : ""}${isFixable ? FIXABLE_TEXT : ""}\n${ruleDocsContent}`;
 
-                text = `---\ntitle: ${ruleName} - Rules\nlayout: doc\n---\n<!-- Note: No pull requests accepted for this file. See README.md in the root directory for details. -->\n\n${text}`;
+                title = `${ruleName} - Rules`;
             } else {
 
                 // extract the title from the file itself
@@ -705,9 +715,19 @@ target.gensite = function(prereleaseVersion) {
                 } else {
                     title = "Documentation";
                 }
-                text = `---\ntitle: ${title}\nlayout: doc\n---\n<!-- Note: No pull requests accepted for this file. See README.md in the root directory for details. -->\n\n${text}`;
             }
 
+            text = [
+                "---",
+                `title: ${title}`,
+                "layout: doc",
+                `https://github.com/eslint/eslint/edit/master/${filePath}`,
+                "---",
+                "<!-- Note: No pull requests accepted for this file. See README.md in the root directory for details. -->",
+                "",
+                text
+            ].join("\n");
+
             // 6. Remove .md extension for relative links and change README to empty string
             text = text.replace(/\((?!https?:\/\/)([^)]*?)\.md.*?\)/g, "($1)").replace("README.html", "");
 
@@ -745,8 +765,10 @@ target.gensite = function(prereleaseVersion) {
         }
     });
     JSON.stringify(versions).to("./versions.json");
+    echo(`> Updating files (Steps 4-9)${" ".repeat(50)}`);
 
     // 10. Copy temporary directory to site's docs folder
+    echo("> Copying the temporary directory the site (Step 10)");
     let outputDir = DOCS_DIR;
 
     if (prereleaseVersion) {
@@ -755,18 +777,26 @@ target.gensite = function(prereleaseVersion) {
     cp("-rf", `${TEMP_DIR}*`, outputDir);
 
     // 11. Generate rule listing page
+    echo("> Generating the rule listing (Step 11)");
     generateRuleIndexPage(process.cwd());
 
     // 12. Delete temporary directory
+    echo("> Removing the temporary directory (Step 12)");
     rm("-r", TEMP_DIR);
 
     // 13. Update demos, but only for non-prereleases
     if (!prereleaseVersion) {
+        echo("> Updating the demos (Step 13)");
         cp("-f", "build/eslint.js", `${SITE_DIR}js/app/eslint.js`);
+    } else {
+        echo("> Skipped updating the demos (Step 13)");
     }
 
     // 14. Create Example Formatter Output Page
+    echo("> Creating the formatter examples (Step 14)");
     generateFormatterExamples(getFormatterResults(), prereleaseVersion);
+
+    echo("Done generating eslint.org");
 };
 
 target.browserify = function() {
diff --git a/lib/ast-utils.js b/lib/ast-utils.js
index c88e34967..e65284ff7 100644
--- a/lib/ast-utils.js
+++ b/lib/ast-utils.js
@@ -808,19 +808,14 @@ module.exports = {
                 return 17;
 
             case "CallExpression":
-
-                // IIFE is allowed to have parens in any position (#655)
-                if (node.callee.type === "FunctionExpression") {
-                    return -1;
-                }
                 return 18;
 
             case "NewExpression":
                 return 19;
 
-            // no default
+            default:
+                return 20;
         }
-        return 20;
     },
 
     /**
diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js
index 879529bf0..7e350d29e 100644
--- a/lib/rules/no-extra-parens.js
+++ b/lib/rules/no-extra-parens.js
@@ -276,6 +276,15 @@ module.exports = {
                 !astUtils.canTokensBeAdjacent(tokenBeforeRightParen, tokenAfterRightParen);
         }
 
+        /**
+         * Determines if a given expression node is an IIFE
+         * @param {ASTNode} node The node to check
+         * @returns {boolean} `true` if the given node is an IIFE
+         */
+        function isIIFE(node) {
+            return node.type === "CallExpression" && node.callee.type === "FunctionExpression";
+        }
+
         /**
          * Report the node
          * @param {ASTNode} node node to evaluate
@@ -286,8 +295,14 @@ module.exports = {
             const leftParenToken = sourceCode.getTokenBefore(node);
             const rightParenToken = sourceCode.getTokenAfter(node);
 
-            if (tokensToIgnore.has(sourceCode.getFirstToken(node)) && !isParenthesisedTwice(node)) {
-                return;
+            if (!isParenthesisedTwice(node)) {
+                if (tokensToIgnore.has(sourceCode.getFirstToken(node))) {
+                    return;
+                }
+
+                if (isIIFE(node) && !isParenthesised(node.callee)) {
+                    return;
+                }
             }
 
             context.report({
@@ -328,26 +343,21 @@ module.exports = {
          * @private
          */
         function checkCallNew(node) {
-            if (hasExcessParens(node.callee) && precedence(node.callee) >= precedence(node) && !(
-                node.type === "CallExpression" &&
-                (node.callee.type === "FunctionExpression" ||
-                  node.callee.type === "NewExpression" && !isNewExpressionWithParens(node.callee)) &&
-
-                // One set of parentheses are allowed for a function expression
-                !hasDoubleExcessParens(node.callee)
-            )) {
-                report(node.callee);
+            if (hasExcessParens(node.callee) && precedence(node.callee) >= precedence(node)) {
+                const hasNewParensException = node.callee.type === "NewExpression" && !isNewExpressionWithParens(node.callee);
+
+                if (hasDoubleExcessParens(node.callee) || !isIIFE(node) && !hasNewParensException) {
+                    report(node.callee);
+                }
             }
             if (node.arguments.length === 1) {
                 if (hasDoubleExcessParens(node.arguments[0]) && precedence(node.arguments[0]) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                     report(node.arguments[0]);
                 }
             } else {
-                [].forEach.call(node.arguments, arg => {
-                    if (hasExcessParens(arg) && precedence(arg) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
-                        report(arg);
-                    }
-                });
+                node.arguments
+                    .filter(arg => hasExcessParens(arg) && precedence(arg) >= PRECEDENCE_OF_ASSIGNMENT_EXPR)
+                    .forEach(report);
             }
         }
 
@@ -442,11 +452,9 @@ module.exports = {
 
         return {
             ArrayExpression(node) {
-                [].forEach.call(node.elements, e => {
-                    if (e && hasExcessParens(e) && precedence(e) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
-                        report(e);
-                    }
-                });
+                node.elements
+                    .filter(e => e && hasExcessParens(e) && precedence(e) >= PRECEDENCE_OF_ASSIGNMENT_EXPR)
+                    .forEach(report);
             },
 
             ArrowFunctionExpression(node) {
@@ -589,13 +597,12 @@ module.exports = {
             NewExpression: checkCallNew,
 
             ObjectExpression(node) {
-                [].forEach.call(node.properties, e => {
-                    const v = e.value;
+                node.properties
+                    .filter(property => {
+                        const value = property.value;
 
-                    if (v && hasExcessParens(v) && precedence(v) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
-                        report(v);
-                    }
-                });
+                        return value && hasExcessParens(value) && precedence(value) >= PRECEDENCE_OF_ASSIGNMENT_EXPR;
+                    }).forEach(property => report(property.value));
             },
 
             ReturnStatement(node) {
@@ -615,11 +622,9 @@ module.exports = {
             },
 
             SequenceExpression(node) {
-                [].forEach.call(node.expressions, e => {
-                    if (hasExcessParens(e) && precedence(e) >= precedence(node)) {
-                        report(e);
-                    }
-                });
+                node.expressions
+                    .filter(e => hasExcessParens(e) && precedence(e) >= precedence(node))
+                    .forEach(report);
             },
 
             SwitchCase(node) {


Test patch:
diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js
index d9345960e..4958fb6ac 100644
--- a/tests/lib/rules/no-extra-parens.js
+++ b/tests/lib/rules/no-extra-parens.js
@@ -175,6 +175,7 @@ ruleTester.run("no-extra-parens", rule, {
         "var o = { foo: (function() { return bar(); })() };",
         "o.foo = (function(){ return bar(); })();",
         "(function(){ return bar(); })(), (function(){ return bar(); })()",
+        "function foo() { return (function(){}()); }",
 
         // parens are required around yield
         "var foo = (function*() { if ((yield foo()) + 1) { return; } }())",