Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: no-useless-rename also reports default values (fixes #12301) #12322

Merged
merged 2 commits into from Sep 29, 2019
Merged

Update: no-useless-rename also reports default values (fixes #12301) #12322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6da1e4
Update: no-useless-rename also reports default values (fixes #12301)
kaicataldo Sep 27, 2019
64be074
Refactor
kaicataldo Sep 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 17 additions & 18 deletions lib/rules/no-useless-rename.js
View file
Open in desktop
Expand Up @@ -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
//--------------------------------------------------------------------------
Expand All @@ -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);
}
});
}
Expand All @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are already tests covering the property.type === "RestElement" change on this line.

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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some change here seems to break!

CI Error: https://circleci.com/gh/nuxt/nuxt.js/55219?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Failing line: https://github.com/nuxt/nuxt.js/blob/13331a02b4d5ef37932f42a845ca1754fb834243/packages/webpack/src/builder.js#L169

(just informational! I'll try to reproduce it better and make PR or Issue)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please track this issue: #12335 .

const renamedKey = property.value.type === "AssignmentPattern" ? property.value.left.name : property.value.name;

if (key === renamedKey) {
reportError(property, property.key, property.value, "Destructuring assignment");
}
}
}
Expand Down
45 changes: 45 additions & 0 deletions tests/lib/rules/no-useless-rename.js
View file
Open in desktop
Expand Up @@ -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}) {}",
Expand All @@ -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}) => {}",
Expand All @@ -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;",
Expand Down