Skip to content

Commit

Permalink
Update: update handling of destructuring in camelcase (fixes #8511) (#…
Browse files Browse the repository at this point in the history
…9468)

* Fix: handle destructuring with defaults in camelcase rule (fixes #8511)

* Update: checking right-side of default value assignment

adding to docs, tests, and fixing logic
  • Loading branch information
erindepew authored and kaicataldo committed Dec 8, 2017
1 parent d067ae1 commit 256481b
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 21 deletions.
37 changes: 37 additions & 0 deletions docs/rules/camelcase.md
Expand Up @@ -32,9 +32,27 @@ obj.do_something = function() {
// ...
};

function foo({ no_camelcased }) {
// ...
};

function foo({ isCamelcased: no_camelcased }) {
// ...
}

function foo({ no_camelcased = 'default value' }) {
// ...
};

var obj = {
my_pref: 1
};

var { category_id = 1 } = query;

var { foo: no_camelcased } = bar;

var { foo: bar_baz = 1 } = quz;
```

Examples of **correct** code for this rule with the default `{ "properties": "always" }` option:
Expand All @@ -56,6 +74,25 @@ do_something();
new do_something();

var { category_id: category } = query;

function foo({ isCamelCased }) {
// ...
};

function foo({ isCamelCased: isAlsoCamelCased }) {
// ...
}

function foo({ isCamelCased = 'default value' }) {
// ...
};

var { categoryId = 1 } = query;

var { foo: isCamelCased } = bar;

var { foo: isCamelCased = 1 } = quz;

```

### never
Expand Down
43 changes: 27 additions & 16 deletions lib/rules/camelcase.js
Expand Up @@ -92,34 +92,45 @@ module.exports = {
}

// Always report underscored object names
if (node.parent.object.type === "Identifier" &&
node.parent.object.name === node.name &&
isUnderscored(name)) {
if (node.parent.object.type === "Identifier" && node.parent.object.name === node.name && isUnderscored(name)) {
report(node);

// Report AssignmentExpressions only if they are the left side of the assignment
} else if (effectiveParent.type === "AssignmentExpression" &&
isUnderscored(name) &&
(effectiveParent.right.type !== "MemberExpression" ||
effectiveParent.left.type === "MemberExpression" &&
effectiveParent.left.property.name === node.name)) {
} else if (effectiveParent.type === "AssignmentExpression" && isUnderscored(name) && (effectiveParent.right.type !== "MemberExpression" || effectiveParent.left.type === "MemberExpression" && effectiveParent.left.property.name === node.name)) {
report(node);
}

// Properties have their own rules
} else if (node.parent.type === "Property") {
/*
* Properties have their own rules, and
* AssignmentPattern nodes can be treated like Properties:
* e.g.: const { no_camelcased = false } = bar;
*/
} else if (node.parent.type === "Property" || node.parent.type === "AssignmentPattern") {

// "never" check properties
if (properties === "never") {
return;
if (node.parent.parent && node.parent.parent.type === "ObjectPattern") {

if (node.parent.shorthand && node.parent.value.left && isUnderscored(node.parent.value.left.name)) {

report(node);
}

// prevent checking righthand side of destructured object
if (node.parent.key === node && node.parent.value !== node) {
return;
}

if (node.parent.value.name && isUnderscored(node.parent.value.name)) {
report(node);
}
}

if (node.parent.parent && node.parent.parent.type === "ObjectPattern" &&
node.parent.key === node && node.parent.value !== node) {
// "never" check properties
if (properties === "never") {
return;
}

if (isUnderscored(name) && !ALLOWED_PARENT_TYPES.has(effectiveParent.type)) {
// don't check right hand side of AssignmentExpression to prevent duplicate warnings
if (isUnderscored(name) && !ALLOWED_PARENT_TYPES.has(effectiveParent.type) && !(node.parent.right === node)) {
report(node);
}

Expand Down
111 changes: 106 additions & 5 deletions tests/lib/rules/camelcase.js
Expand Up @@ -63,11 +63,6 @@ ruleTester.run("camelcase", rule, {
code: "var { category_id: category } = query;",
parserOptions: { ecmaVersion: 6 }
},
{
code: "var { category_id: category } = query;",
options: [{ properties: "never" }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "import { camelCased } from \"external module\";",
parserOptions: { ecmaVersion: 6, sourceType: "module" }
Expand All @@ -79,6 +74,18 @@ ruleTester.run("camelcase", rule, {
{
code: "import { no_camelcased as camelCased, anoterCamelCased } from \"external-module\";",
parserOptions: { ecmaVersion: 6, sourceType: "module" }
},
{
code: "function foo({ no_camelcased: camelCased }) {};",
parserOptions: { ecmaVersion: 6 }
},
{
code: "function foo({ camelCased = 'default value' }) {};",
parserOptions: { ecmaVersion: 6 }
},
{
code: "function foo({ camelCased }) {};",
parserOptions: { ecmaVersion: 6 }
}
],
invalid: [
Expand Down Expand Up @@ -212,6 +219,16 @@ ruleTester.run("camelcase", rule, {
}
]
},
{
code: "var { category_id = 1 } = query;",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'category_id' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "import no_camelcased from \"external-module\";",
parserOptions: { ecmaVersion: 6, sourceType: "module" },
Expand Down Expand Up @@ -301,6 +318,90 @@ ruleTester.run("camelcase", rule, {
type: "Identifier"
}
]
},
{
code: "function foo({ no_camelcased }) {};",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "function foo({ no_camelcased = 'default value' }) {};",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "const no_camelcased = 0; function foo({ camelcased_value = no_camelcased}) {}",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
},
{
message: "Identifier 'camelcased_value' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "const { bar: no_camelcased } = foo;",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "function foo({ value_1: my_default }) {}",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'my_default' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "function foo({ isCamelcased: no_camelcased }) {};",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "var { foo: bar_baz = 1 } = quz;",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'bar_baz' is not in camel case.",
type: "Identifier"
}
]
},
{
code: "const { no_camelcased = false } = bar;",
parserOptions: { ecmaVersion: 6 },
errors: [
{
message: "Identifier 'no_camelcased' is not in camel case.",
type: "Identifier"
}
]
}
]
});

0 comments on commit 256481b

Please sign in to comment.