diff --git a/lib/rules/no-useless-rename.js b/lib/rules/no-useless-rename.js index fa88f37f50b..a7cec025da7 100644 --- a/lib/rules/no-useless-rename.js +++ b/lib/rules/no-useless-rename.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -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({ @@ -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)); } }); } @@ -97,19 +105,11 @@ 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; } @@ -117,7 +117,7 @@ module.exports = { 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"); } } } @@ -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"); } } @@ -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"); } } diff --git a/tests/lib/rules/no-useless-rename.js b/tests/lib/rules/no-useless-rename.js index af8f6b8a520..f09a9d448ca 100644 --- a/tests/lib/rules/no-useless-rename.js +++ b/tests/lib/rules/no-useless-rename.js @@ -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;", @@ -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}) {}", @@ -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';", @@ -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};", @@ -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';", @@ -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} = {});", @@ -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, @@ -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 */} = {});", @@ -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';",