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: add ignoreDestructuring option to id-match rule #10554

Merged
merged 1 commit into from Nov 9, 2018
Merged

Update: add ignoreDestructuring option to id-match rule #10554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 45 additions & 0 deletions docs/rules/id-match.md
View file
Open in desktop
Expand Up @@ -58,6 +58,9 @@ This rule has an object option:

* `"properties": true` requires object properties to match the specified regular expression
* `"onlyDeclarations": true` requires only `var`, `function`, and `class` declarations to match the specified regular expression
* `"onlyDeclarations": false` requires all variable names to match the specified regular expression
tinymins marked this conversation as resolved.
Show resolved Hide resolved
* `"ignoreDestructuring": false` (default) enforces `id-match` for destructured identifiers
* `"ignoreDestructuring": true` does not check destructured identifiers

### properties

Expand All @@ -81,6 +84,48 @@ Examples of **correct** code for this rule with the `"^[a-z]+([A-Z][a-z]+)*$", {
do_something(__dirname);
```

### ignoreDestructuring: false

Examples of **incorrect** code for this rule with the default `"^[^_]+$", { "ignoreDestructuring": false }` option:

```js
/*eslint id-match: [2, "^[^_]+$", { "ignoreDestructuring": false }]*/

var { category_id } = query;

var { category_id = 1 } = query;

var { category_id: category_id } = query;

var { category_id: category_alias } = query;

var { category_id: categoryId, ...other_props } = query;
```

### ignoreDestructuring: true

Examples of **incorrect** code for this rule with the `"^[^_]+$", { "ignoreDestructuring": true }` option:

```js
/*eslint id-match: [2, "^[^_]+$", { "ignoreDestructuring": true }]*/

var { category_id: category_alias } = query;

var { category_id, ...other_props } = query;
```

Examples of **correct** code for this rule with the `"^[^_]+$", { "ignoreDestructuring": true }` option:

```js
/*eslint id-match: [2, "^[^_]+$", { "ignoreDestructuring": true }]*/

var { category_id } = query;

var { category_id = 1 } = query;

var { category_id: category_id } = query;
```

## When Not To Use It

If you don't want to enforce any particular naming convention for all identifiers, or your naming convention is too complex to be enforced by configuring this rule, then you should not enable this rule.
126 changes: 99 additions & 27 deletions lib/rules/id-match.js
View file
Open in desktop
Expand Up @@ -27,6 +27,12 @@ module.exports = {
properties: {
properties: {
type: "boolean"
},
onlyDeclarations: {
tinymins marked this conversation as resolved.
Show resolved Hide resolved
type: "boolean"
},
ignoreDestructuring: {
type: "boolean"
}
}
}
Expand All @@ -36,15 +42,25 @@ module.exports = {
create(context) {

//--------------------------------------------------------------------------
// Helpers
// Options
//--------------------------------------------------------------------------

const pattern = context.options[0] || "^.+$",
regexp = new RegExp(pattern);

const options = context.options[1] || {},
properties = !!options.properties,
onlyDeclarations = !!options.onlyDeclarations;
onlyDeclarations = !!options.onlyDeclarations,
ignoreDestructuring = !!options.ignoreDestructuring;

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------

// contains reported nodes to avoid reporting twice on destructuring with shorthand notation
const reported = new Map();
const ALLOWED_PARENT_TYPES = new Set(["CallExpression", "NewExpression"]);
const DECLARATION_TYPES = new Set(["FunctionDeclaration", "VariableDeclarator"]);
const IMPORT_TYPES = new Set(["ImportSpecifier", "ImportNamespaceSpecifier", "ImportDefaultSpecifier"]);

/**
* Checks if a string matches the provided pattern
Expand All @@ -56,6 +72,26 @@ module.exports = {
return !regexp.test(name);
}

/**
* Checks if a parent of a node is an ObjectPattern.
* @param {ASTNode} node The node to check.
* @returns {boolean} if the node is inside an ObjectPattern
* @private
*/
function isInsideObjectPattern(node) {
let { parent } = node;

while (parent) {
if (parent.type === "ObjectPattern") {
return true;
}

parent = parent.parent;
}

return false;
}

/**
* Verifies if we should report an error or not based on the effective
* parent node and the identifier name.
Expand All @@ -64,9 +100,8 @@ module.exports = {
* @returns {boolean} whether an error should be reported or not
*/
function shouldReport(effectiveParent, name) {
return effectiveParent.type !== "CallExpression" &&
effectiveParent.type !== "NewExpression" &&
isInvalid(name);
return (!onlyDeclarations || DECLARATION_TYPES.has(effectiveParent.type)) &&
!ALLOWED_PARENT_TYPES.has(effectiveParent.type) && isInvalid(name);
}

/**
Expand All @@ -76,14 +111,17 @@ module.exports = {
* @private
*/
function report(node) {
context.report({
node,
message: "Identifier '{{name}}' does not match the pattern '{{pattern}}'.",
data: {
name: node.name,
pattern
}
});
if (!reported.has(node)) {
context.report({
node,
message: "Identifier '{{name}}' does not match the pattern '{{pattern}}'.",
data: {
name: node.name,
pattern
}
});
reported.set(node, true);
}
}

return {
Expand All @@ -106,36 +144,70 @@ module.exports = {
report(node);
}

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

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

} else if (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 (parent.type === "Property" || parent.type === "AssignmentPattern") {

if (parent.parent && parent.parent.type === "ObjectPattern") {
if (parent.shorthand && parent.value.left && isInvalid(name)) {

report(node);
}

const assignmentKeyEqualsValue = parent.key.name === parent.value.name;

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

const valueIsInvalid = parent.value.name && isInvalid(name);

// ignore destructuring if the option is set, unless a new identifier is created
if (valueIsInvalid && !(assignmentKeyEqualsValue && ignoreDestructuring)) {
report(node);
}
}

// never check properties or always ignore destructuring
if (!properties || (ignoreDestructuring && isInsideObjectPattern(node))) {
return;
}

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

} else {
const isDeclaration = effectiveParent.type === "FunctionDeclaration" || effectiveParent.type === "VariableDeclarator";
// Check if it's an import specifier
} else if (IMPORT_TYPES.has(parent.type)) {

if (onlyDeclarations && !isDeclaration) {
return;
}

if (shouldReport(effectiveParent, name)) {
// Report only if the local imported identifier is invalid
if (parent.local && parent.local.name === node.name && isInvalid(name)) {
report(node);
}

// Report anything that is invalid that isn't a CallExpression
} else if (shouldReport(effectiveParent, name)) {
report(node);
}
}

Expand Down