Repository: eslint/eslint (js)

Issue:
no-useless-rename should also report pattern properties with default values
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).

    This template is for requesting a rule change. If you are here for another reason, please see below:

    1. To report a bug: https://eslint.org/docs/developer-guide/contributing/reporting-bugs
    2. To propose a new rule: https://eslint.org/docs/developer-guide/contributing/new-rules
    3. To request a change that is not a bug fix, rule change, or new rule: https://eslint.org/docs/developer-guide/contributing/changes
    4. 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.
-->

**What rule do you want to change?**

no-useless-rename

**Does this change cause the rule to produce more or fewer warnings?**

more

**How will the change be implemented? (New option, new default behavior, etc.)?**

New default behavior, because I think this is more a bug than an enhancement.

But, there is a question would it be okay for a semver-minor release.

**Please provide some example code that this change will affect:**

<!-- Put your code examples here -->
```js
/* eslint no-useless-rename:error */

const { a: a = 1 } = foo;
```

**What does the rule currently do for this code?**

Nothing.

**What will the rule do after it's changed?**

Error, same as for `const { a: a } = foo;` and autofix to:

```js
/* eslint no-useless-rename:error */

const { a = 1 } = foo;
```

**Are you willing to submit a pull request to implement this change?**

Yes.

Pull request description:
Update: no-useless-rename also reports default values (fixes #12301)
<!--
    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)**

[ ] Documentation update
[ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[x] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    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 (https://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:
-->

fixes #12301 

**What changes did you make? (Give an overview)**

This updates `no-useless-rename` to check destructured assignments with default values. I think this should be considered a semver-minor bug fix, since it feels like an oversight that this rule doesn't already check this case.

**Is there anything you'd like reviewers to focus on?**

Are reviewers in agreement that this should be default behavior and not behind an option?



Patch:
diff --git a/lib/rules/new-parens.js b/lib/rules/new-parens.js
index 4b9958fe6..405ec1b51 100644
--- a/lib/rules/new-parens.js
+++ b/lib/rules/new-parens.js
@@ -65,7 +65,11 @@ module.exports = {
 
                 const lastToken = sourceCode.getLastToken(node);
                 const hasLastParen = lastToken && astUtils.isClosingParenToken(lastToken);
-                const hasParens = hasLastParen && astUtils.isOpeningParenToken(sourceCode.getTokenBefore(lastToken));
+
+                // `hasParens` is true only if the new expression ends with its own parens, e.g., new new foo() does not end with its own parens
+                const hasParens = hasLastParen &&
+                    astUtils.isOpeningParenToken(sourceCode.getTokenBefore(lastToken)) &&
+                    node.callee.range[1] < node.range[1];
 
                 if (always) {
                     if (!hasParens) {
diff --git a/lib/rules/no-useless-rename.js b/lib/rules/no-useless-rename.js
index 4ff65d4b8..67af97c8b 100644
--- a/lib/rules/no-useless-rename.js
+++ b/lib/rules/no-useless-rename.js
@@ -36,13 +36,12 @@ module.exports = {
     },
 
     create(context) {
-        const options = context.options[0] || {},
+        const sourceCode = context.getSourceCode(),
+            options = context.options[0] || {},
             ignoreDestructuring = options.ignoreDestructuring === true,
             ignoreImport = options.ignoreImport === true,
             ignoreExport = options.ignoreExport === true;
 
-        const sourceCode = context.getSourceCode();
-
         //--------------------------------------------------------------------------
         // Helpers
         //--------------------------------------------------------------------------
@@ -69,10 +68,15 @@ module.exports = {
                     if (sourceCode.commentsExistBetween(initial, result)) {
                         return null;
                     }
+
+                    const replacementText = result.type === "AssignmentPattern"
+                        ? sourceCode.getText(result)
+                        : name;
+
                     return fixer.replaceTextRange([
                         initial.range[0],
                         result.range[1]
-                    ], name);
+                    ], replacementText);
                 }
             });
         }
@@ -87,26 +91,21 @@ module.exports = {
                 return;
             }
 
-            const properties = node.properties;
-
-            for (let i = 0; i < properties.length; i++) {
-                if (properties[i].shorthand) {
-                    continue;
-                }
+            for (const property of node.properties) {
 
                 /**
-                 * If an ObjectPattern property is computed, we have no idea
-                 * if a rename is useless or not. If an ObjectPattern property
-                 * lacks a key, it is likely an ExperimentalRestProperty and
-                 * so there is no "renaming" occurring here.
+                 * Properties using shorthand syntax and rest elements can not be renamed.
+                 * If the property is computed, we have no idea if a rename is useless or not.
                  */
-                if (properties[i].computed || !properties[i].key) {
+                if (property.shorthand || property.type === "RestElement" || property.computed) {
                     continue;
                 }
 
-                if (properties[i].key.type === "Identifier" && properties[i].key.name === properties[i].value.name ||
-                        properties[i].key.type === "Literal" && properties[i].key.value === properties[i].value.name) {
-                    reportError(properties[i], properties[i].key, properties[i].value, "Destructuring assignment");
+                const key = (property.key.type === "Identifier" && property.key.name) || (property.key.type === "Literal" && property.key.value);
+                const renamedKey = property.value.type === "AssignmentPattern" ? property.value.left.name : property.value.name;
+
+                if (key === renamedKey) {
+                    reportError(property, property.key, property.value, "Destructuring assignment");
                 }
             }
         }


Test patch:
diff --git a/tests/lib/rules/new-parens.js b/tests/lib/rules/new-parens.js
index c13cc7150..3508ace8e 100644
--- a/tests/lib/rules/new-parens.js
+++ b/tests/lib/rules/new-parens.js
@@ -119,6 +119,12 @@ ruleTester.run("new-parens", rule, {
             options: ["always"],
             errors: [error]
         },
+        {
+            code: "var a = new new Foo()",
+            output: "var a = new new Foo()()",
+            options: ["always"],
+            errors: [error]
+        },
 
         // Never
         {
@@ -168,6 +174,12 @@ ruleTester.run("new-parens", rule, {
             output: "var a = ((new Foo)).bar;",
             options: ["never"],
             errors: [neverError]
+        },
+        {
+            code: "var a = new new Foo()",
+            output: "var a = new (new Foo)",
+            options: ["never"],
+            errors: [neverError]
         }
     ]
 });
diff --git a/tests/lib/rules/no-useless-rename.js b/tests/lib/rules/no-useless-rename.js
index 58796c625..c9be46d69 100644
--- a/tests/lib/rules/no-useless-rename.js
+++ b/tests/lib/rules/no-useless-rename.js
@@ -182,6 +182,21 @@ ruleTester.run("no-useless-rename", rule, {
             output: "let {'foo': {bar}, baz} = obj;",
             errors: ["Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
         },
+        {
+            code: "let {foo: foo = 1, 'bar': bar = 1, baz: baz} = obj;",
+            output: "let {foo = 1, bar = 1, baz} = obj;",
+            errors: ["Destructuring assignment foo unnecessarily renamed.", "Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "let {foo: {bar: bar = 1, 'baz': baz = 1}} = obj;",
+            output: "let {foo: {bar = 1, baz = 1}} = obj;",
+            errors: ["Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "let {foo: {bar: bar = {}} = {}} = obj;",
+            output: "let {foo: {bar = {}} = {}} = obj;",
+            errors: ["Destructuring assignment bar unnecessarily renamed."]
+        },
         {
             code: "function func({foo: foo}) {}",
             output: "function func({foo}) {}",
@@ -202,6 +217,21 @@ ruleTester.run("no-useless-rename", rule, {
             output: "function func({foo, bar}) {}",
             errors: ["Destructuring assignment foo unnecessarily renamed.", "Destructuring assignment bar unnecessarily renamed."]
         },
+        {
+            code: "function func({foo: foo = 1, 'bar': bar = 1, baz: baz}) {}",
+            output: "function func({foo = 1, bar = 1, baz}) {}",
+            errors: ["Destructuring assignment foo unnecessarily renamed.", "Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "function func({foo: {bar: bar = 1, 'baz': baz = 1}}) {}",
+            output: "function func({foo: {bar = 1, baz = 1}}) {}",
+            errors: ["Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "function func({foo: {bar: bar = {}} = {}}) {}",
+            output: "function func({foo: {bar = {}} = {}}) {}",
+            errors: ["Destructuring assignment bar unnecessarily renamed."]
+        },
         {
             code: "({foo: foo}) => {}",
             output: "({foo}) => {}",
@@ -222,6 +252,21 @@ ruleTester.run("no-useless-rename", rule, {
             output: "({foo, bar}) => {}",
             errors: ["Destructuring assignment foo unnecessarily renamed.", "Destructuring assignment bar unnecessarily renamed."]
         },
+        {
+            code: "({foo: foo = 1, 'bar': bar = 1, baz: baz}) => {}",
+            output: "({foo = 1, bar = 1, baz}) => {}",
+            errors: ["Destructuring assignment foo unnecessarily renamed.", "Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "({foo: {bar: bar = 1, 'baz': baz = 1}}) => {}",
+            output: "({foo: {bar = 1, baz = 1}}) => {}",
+            errors: ["Destructuring assignment bar unnecessarily renamed.", "Destructuring assignment baz unnecessarily renamed."]
+        },
+        {
+            code: "({foo: {bar: bar = {}} = {}}) => {}",
+            output: "({foo: {bar = {}} = {}}) => {}",
+            errors: ["Destructuring assignment bar unnecessarily renamed."]
+        },
         {
             code: "const {foo: foo, ...stuff} = myObject;",
             output: "const {foo, ...stuff} = myObject;",