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: check renaming identifier in object destructuring (fixes 12827) #12881

Merged
merged 11 commits into from Feb 17, 2020
Merged

Update: check renaming identifier in object destructuring (fixes 12827) #12881

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4ad9e98
Update: check renaming identifier in object destructuring (fixes 12827)
yeonjuan Feb 6, 2020
213b236
Fix wrong error type
yeonjuan Feb 6, 2020
ce40f92
ignore properties options on object pattern, ignore computed prop
yeonjuan Feb 7, 2020
6c235de
modify docs, add test cases
yeonjuan Feb 7, 2020
ed2485d
add test cases
yeonjuan Feb 7, 2020
c22fe9c
remove wrong test cases
yeonjuan Feb 7, 2020
81ee5d7
change to ignore destructuring on properties never
yeonjuan Feb 10, 2020
54ee384
update documentation
yeonjuan Feb 11, 2020
a644b11
fix typo
yeonjuan Feb 11, 2020
81dc86c
fix documentation
yeonjuan Feb 11, 2020
486b642
include error loc, data info
yeonjuan Feb 16, 2020
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
9 changes: 6 additions & 3 deletions docs/rules/id-length.md
View file
Open in desktop
Expand Up @@ -31,8 +31,9 @@ var myObj = { a: 1 };
class x { }
class Foo { x() {} }
function foo(...x) { }
function foo({x}) { }
var { x } = {};
var { x: a} = {};
var { prop: a} = {};
var { a: [x]} = {};
({ prop: obj.x } = {});
```
Expand All @@ -59,8 +60,10 @@ function foo(num = 0) { }
class MyClass { }
class Foo { method() {} }
function foo(...args) { }
function foo({ prop }) { }
function foo({ a: prop }) { }
var { prop } = {};
var { prop: a } = {};
var { a: prop } = {};
var { prop: [x] } = {};
({ prop: obj.longName } = {});
var data = { "x": 1 }; // excused because of quotes
Expand Down Expand Up @@ -116,7 +119,7 @@ class MyClass { }
class Foobar { method() {} }
function foobar(...args) { }
var { prop } = {};
var { prop: a } = {};
var { a: longName } = {};
var { prop: [x] } = {};
({ prop: obj.name } = {});
var data = { "x": 1 }; // excused because of quotes
Expand Down
15 changes: 12 additions & 3 deletions lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -63,6 +63,7 @@ module.exports = {

return obj;
}, {});
const reportedNode = new Set();

const SUPPORTED_EXPRESSIONS = {
MemberExpression: properties && function(parent) {
Expand All @@ -82,8 +83,15 @@ module.exports = {
VariableDeclarator(parent, node) {
return parent.id === node;
},
Property: properties && function(parent, node) {
return parent.key === node;
Property(parent, node) {

if (parent.parent.type === "ObjectPattern") {
return (
parent.value !== parent.key && parent.value === node ||
parent.value === parent.key && parent.key === node
);
}
return properties && !parent.computed && parent.key === node;
},
ImportDefaultSpecifier: true,
RestElement: true,
Expand All @@ -109,7 +117,8 @@ module.exports = {

const isValidExpression = SUPPORTED_EXPRESSIONS[parent.type];

if (isValidExpression && (isValidExpression === true || isValidExpression(parent, node))) {
if (isValidExpression && !reportedNode.has(node) && (isValidExpression === true || isValidExpression(parent, node))) {
reportedNode.add(node);
context.report({
node,
messageId: isShort ? "tooShort" : "tooLong",
Expand Down
177 changes: 169 additions & 8 deletions tests/lib/rules/id-length.js
View file
Open in desktop
Expand Up @@ -39,6 +39,9 @@ ruleTester.run("id-length", rule, {
"var obj = { 'a': 1, bc: 2 }; obj.tk = obj.a;",
"var query = location.query.q || '';",
"var query = location.query.q ? location.query.q : ''",
{ code: "let {a: foo} = bar;", parserOptions: { ecmaVersion: 6 } },
{ code: "let foo = { [a]: 1 };", parserOptions: { ecmaVersion: 6 } },
{ code: "let foo = { [a + b]: 1 };", parserOptions: { ecmaVersion: 6 } },
{ code: "var x = Foo(42)", options: [{ min: 1 }] },
{ code: "var x = Foo(42)", options: [{ min: 0 }] },
{ code: "foo.$x = Foo(42)", options: [{ min: 1 }] },
Expand All @@ -50,20 +53,27 @@ ruleTester.run("id-length", rule, {
{ code: "class Foo { method() {} }", parserOptions: { ecmaVersion: 6 } },
{ code: "function foo(...args) { }", parserOptions: { ecmaVersion: 6 } },
{ code: "var { prop } = {};", parserOptions: { ecmaVersion: 6 } },
{ code: "var { prop: a } = {};", parserOptions: { ecmaVersion: 6 } },
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
{ code: "var { prop: [x] } = {};", parserOptions: { ecmaVersion: 6 } },
{ code: "var { [a]: prop } = {};", parserOptions: { ecmaVersion: 6 } },
{ code: "var { a: foo } = {};", options: [{ min: 3 }], parserOptions: { ecmaVersion: 6 } },
{ code: "var { prop: foo } = {};", options: [{ max: 3 }], parserOptions: { ecmaVersion: 6 } },
{ code: "var { longName: foo } = {};", options: [{ min: 3, max: 5 }], parserOptions: { ecmaVersion: 6 } },
{ code: "var { foo: a } = {};", options: [{ exceptions: ["a"] }], parserOptions: { ecmaVersion: 6 } },
{ code: "var { a: { b: { c: longName } } } = {};", parserOptions: { ecmaVersion: 6 } },
{ code: "({ a: obj.x.y.z } = {});", options: [{ properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "import something from 'y';", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "export var num = 0;", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "({ prop: obj.x.y.something } = {});", parserOptions: { ecmaVersion: 6 } },
{ code: "({ prop: obj.longName } = {});", parserOptions: { ecmaVersion: 6 } },
{ code: "var obj = { a: 1, bc: 2 };", options: [{ properties: "never" }] },
{ code: "var obj = { [a]: 2 };", options: [{ properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "var obj = {}; obj.a = 1; obj.bc = 2;", options: [{ properties: "never" }] },
{ code: "({ a: obj.x.y.z } = {});", options: [{ properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "({ prop: obj.x } = {});", options: [{ properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "var obj = { aaaaa: 1 };", options: [{ max: 4, properties: "never" }] },
{ code: "var obj = {}; obj.aaaaa = 1;", options: [{ max: 4, properties: "never" }] },
{ code: "({ a: obj.x.y.z } = {});", options: [{ max: 4, properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "({ prop: obj.xxxxx } = {});", options: [{ max: 4, properties: "never" }], parserOptions: { ecmaVersion: 6 } }
{ code: "({ prop: obj.xxxxx } = {});", options: [{ max: 4, properties: "never" }], parserOptions: { ecmaVersion: 6 } },
{ code: "let foo = { [a]: 1 };", options: [{ properties: "always" }], parserOptions: { ecmaVersion: 6 } },
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
{ code: "let foo = { [a + b]: 1 };", options: [{ properties: "always" }], parserOptions: { ecmaVersion: 6 } }
],
invalid: [
{ code: "var x = 1;", errors: [tooShortError] },
Expand Down Expand Up @@ -126,27 +136,155 @@ ruleTester.run("id-length", rule, {
]
},
{
code: "var { x} = {};",
code: "function foo({x}) { }",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "function foo({x: a}) { }",
parserOptions: { ecmaVersion: 6 },
errors: [
{
messageId: "tooShort",
data: { name: "a", min: 2 },
type: "Identifier"
}
]
},
{
code: "function foo({x: a, longName}) { }",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "function foo({ longName: a }) {}",
options: [{ min: 3, max: 5 }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "function foo({ prop: longName }) {};",
options: [{ min: 3, max: 5 }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooLongError
]
},
{
code: "function foo({ a: b }) {};",
options: [{ exceptions: ["a"] }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "function foo({ a: { b: { c: d, e } } }) { }",
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError,
tooShortError
]
},
{
code: "var { x} = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { x: a} = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
{
messageId: "tooShort",
data: { name: "a", min: 2 },
type: "Identifier"
}
]
},
{
code: "var { a: a} = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
{
code: "var { a: [x]} = {};",
code: "var { prop: a } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { longName: a } = {};",
options: [{ min: 3, max: 5 }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { prop: longName } = {};",
options: [{ min: 3, max: 5 }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooLongError
]
},
{
code: "var { x: a} = {};",
options: [{ exceptions: ["x"] }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { a: { b: { c: d } } } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { a: { b: { c: d, e } } } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError,
tooShortError
]
},
{
code: "var { a: { b: { c, e: longName } } } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { a: { b: { c: d, e: longName } } } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
{
code: "var { a, b: { c: d, e: longName } } = {};",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError,
tooShortError
]
},
{
code: "import x from 'y';",
parserOptions: { ecmaVersion: 6, sourceType: "module" },
Expand All @@ -165,7 +303,6 @@ ruleTester.run("id-length", rule, {
code: "({ a: obj.x.y.z } = {});",
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError,
tooShortError
]
},
Expand All @@ -176,6 +313,30 @@ ruleTester.run("id-length", rule, {
tooShortError
]
},
{ code: "var x = 1;", options: [{ properties: "never" }], errors: [tooShortError] }
{ code: "var x = 1;", options: [{ properties: "never" }], errors: [tooShortError] },
{
code: "var {x} = foo;",
options: [{ properties: "never" }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
{
code: "var {prop: x} = foo;",
options: [{ properties: "never" }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
},
yeonjuan marked this conversation as resolved.
Show resolved Hide resolved
{
code: "var foo = {x: prop};",
options: [{ properties: "always" }],
parserOptions: { ecmaVersion: 6 },
errors: [
tooShortError
]
}
]
});