Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix: no-useless-rename invalid autofix with parenthesized identifiers (
  • Loading branch information
mdjermanovic committed Jan 30, 2021
1 parent 5800d92 commit d76e8f6
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 22 deletions.
44 changes: 22 additions & 22 deletions lib/rules/no-useless-rename.js
Expand Up @@ -5,6 +5,12 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -54,11 +60,10 @@ module.exports = {
* Reports error for unnecessarily renamed assignments
* @param {ASTNode} node node to report
* @param {ASTNode} initial node with initial name value
* @param {ASTNode} result node with new name value
* @param {string} type the type of the offending node
* @returns {void}
*/
function reportError(node, initial, result, type) {
function reportError(node, initial, type) {
const name = initial.type === "Identifier" ? initial.name : initial.value;

return context.report({
Expand All @@ -69,18 +74,21 @@ module.exports = {
type
},
fix(fixer) {
if (sourceCode.commentsExistBetween(initial, result)) {
const replacementNode = node.type === "Property" ? node.value : node.local;

if (sourceCode.getCommentsInside(node).length > sourceCode.getCommentsInside(replacementNode).length) {
return null;
}

const replacementText = result.type === "AssignmentPattern"
? sourceCode.getText(result)
: name;
// Don't autofix code such as `({foo: (foo) = a} = obj);`, parens are not allowed in shorthand properties.
if (
replacementNode.type === "AssignmentPattern" &&
astUtils.isParenthesised(sourceCode, replacementNode.left)
) {
return null;
}

return fixer.replaceTextRange([
initial.range[0],
result.range[1]
], replacementText);
return fixer.replaceText(node, sourceCode.getText(replacementNode));
}
});
}
Expand All @@ -97,27 +105,19 @@ module.exports = {

for (const property of node.properties) {

/*
* TODO: Remove after babel-eslint removes ExperimentalRestProperty
* https://github.com/eslint/eslint/issues/12335
*/
if (property.type === "ExperimentalRestProperty") {
continue;
}

/**
* 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 (property.shorthand || property.type === "RestElement" || property.computed) {
if (property.type !== "Property" || property.shorthand || property.computed) {
continue;
}

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");
reportError(property, property.key, "Destructuring assignment");
}
}
}
Expand All @@ -134,7 +134,7 @@ module.exports = {

if (node.imported.name === node.local.name &&
node.imported.range[0] !== node.local.range[0]) {
reportError(node, node.imported, node.local, "Import");
reportError(node, node.imported, "Import");
}
}

Expand All @@ -150,7 +150,7 @@ module.exports = {

if (node.local.name === node.exported.name &&
node.local.range[0] !== node.exported.range[0]) {
reportError(node, node.local, node.exported, "Export");
reportError(node, node.local, "Export");
}

}
Expand Down
150 changes: 150 additions & 0 deletions tests/lib/rules/no-useless-rename.js
Expand Up @@ -131,6 +131,26 @@ ruleTester.run("no-useless-rename", rule, {
output: "let {foo} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (foo)} = obj);",
output: "({foo} = obj);",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "let {\\u0061: a} = obj;",
output: "let {a} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "a" } }]
},
{
code: "let {a: \\u0061} = obj;",
output: "let {\\u0061} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "a" } }]
},
{
code: "let {\\u0061: \\u0061} = obj;",
output: "let {\\u0061} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "a" } }]
},
{
code: "let {a, foo: foo} = obj;",
output: "let {a, foo} = obj;",
Expand Down Expand Up @@ -225,6 +245,21 @@ ruleTester.run("no-useless-rename", rule, {
output: "let {foo: {bar = {}} = {}} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "bar" } }]
},
{
code: "({foo: (foo) = a} = obj);",
output: null, // The rule doesn't autofix this edge case. The correct fix would be without parens: `let {foo = a} = obj;`
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "let {foo: foo = (a)} = obj;",
output: "let {foo = (a)} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "let {foo: foo = (a, b)} = obj;",
output: "let {foo = (a, b)} = obj;",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "function func({foo: foo}) {}",
output: "function func({foo}) {}",
Expand Down Expand Up @@ -341,6 +376,21 @@ ruleTester.run("no-useless-rename", rule, {
output: "import {foo} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Import", name: "foo" } }]
},
{
code: "import {\\u0061 as a} from 'foo';",
output: "import {a} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Import", name: "a" } }]
},
{
code: "import {a as \\u0061} from 'foo';",
output: "import {\\u0061} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Import", name: "a" } }]
},
{
code: "import {\\u0061 as \\u0061} from 'foo';",
output: "import {\\u0061} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Import", name: "a" } }]
},
{
code: "import {foo as foo, bar as baz} from 'foo';",
output: "import {foo, bar as baz} from 'foo';",
Expand All @@ -364,6 +414,21 @@ ruleTester.run("no-useless-rename", rule, {
output: "var foo = 0; export {foo};",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "foo" } }]
},
{
code: "var a = 0; export {a as \\u0061};",
output: "var a = 0; export {a};",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "var \\u0061 = 0; export {\\u0061 as a};",
output: "var \\u0061 = 0; export {\\u0061};",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "var \\u0061 = 0; export {\\u0061 as \\u0061};",
output: "var \\u0061 = 0; export {\\u0061};",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "var foo = 0; var bar = 0; export {foo as foo, bar as baz};",
output: "var foo = 0; var bar = 0; export {foo, bar as baz};",
Expand All @@ -387,6 +452,21 @@ ruleTester.run("no-useless-rename", rule, {
output: "export {foo} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "foo" } }]
},
{
code: "export {a as \\u0061} from 'foo';",
output: "export {a} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "export {\\u0061 as a} from 'foo';",
output: "export {\\u0061} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "export {\\u0061 as \\u0061} from 'foo';",
output: "export {\\u0061} from 'foo';",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Export", name: "a" } }]
},
{
code: "export {foo as foo, bar as baz} from 'foo';",
output: "export {foo, bar as baz} from 'foo';",
Expand All @@ -412,6 +492,11 @@ ruleTester.run("no-useless-rename", rule, {
output: "({/* comment */foo} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({/* comment */foo: foo = 1} = {});",
output: "({/* comment */foo = 1} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo, /* comment */bar: bar} = {});",
output: "({foo, /* comment */bar} = {});",
Expand All @@ -422,11 +507,21 @@ ruleTester.run("no-useless-rename", rule, {
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo/**/ : foo = 1} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo /**/: foo} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo /**/: foo = 1} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo://\nfoo} = {});",
output: null,
Expand All @@ -437,6 +532,36 @@ ruleTester.run("no-useless-rename", rule, {
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (/**/foo)} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (foo/**/)} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (foo //\n)} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: /**/foo = 1} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (/**/foo) = 1} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: (foo/**/) = 1} = {});",
output: null,
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo/* comment */} = {});",
output: "({foo/* comment */} = {});",
Expand All @@ -447,6 +572,31 @@ ruleTester.run("no-useless-rename", rule, {
output: "({foo//comment\n,bar} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo/* comment */ = 1} = {});",
output: "({foo/* comment */ = 1} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo // comment\n = 1} = {});",
output: "({foo // comment\n = 1} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo = /* comment */ 1} = {});",
output: "({foo = /* comment */ 1} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo = // comment\n 1} = {});",
output: "({foo = // comment\n 1} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "({foo: foo = (1/* comment */)} = {});",
output: "({foo = (1/* comment */)} = {});",
errors: [{ messageId: "unnecessarilyRenamed", data: { type: "Destructuring assignment", name: "foo" } }]
},
{
code: "import {/* comment */foo as foo} from 'foo';",
output: "import {/* comment */foo} from 'foo';",
Expand Down

0 comments on commit d76e8f6

Please sign in to comment.