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: update handling of destructuring in camelcase (fixes #8511) #9468

Merged
merged 2 commits into from
Dec 8, 2017
Merged

Update: update handling of destructuring in camelcase (fixes #8511) #9468

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
8d9ed81
Fix: handle destructuring with defaults in camelcase rule (fixes #8511)
vitorbal Apr 29, 2017
1a4e72f
Update: checking right-side of default value assignment
Oct 17, 2017
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
37 changes: 37 additions & 0 deletions docs/rules/camelcase.md
View file
Open in desktop
Original file line number Diff line number Diff line change
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:
Copy link
Member

Choose a reason for hiding this comment

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

We should probably add the correct examples here now (i.e. the fixed versions of the examples in the incorrect section).

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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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"
}
]
}
]
});