Skip to content

Commit

Permalink
Update: add ignoreDestructuring option to id-match rule (#10554)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinymins @nzakas
tinymins authored and nzakas committed Nov 9, 2018
1 parent 54687a8 commit c832cd5
Show file tree
Hide file tree
Showing 3 changed files with 481 additions and 28 deletions.
45 changes: 45 additions & 0 deletions docs/rules/id-match.md
View file
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
* `"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
Expand Up @@ -29,6 +29,12 @@ module.exports = {
properties: {
properties: {
type: "boolean"
},
onlyDeclarations: {
type: "boolean"
},
ignoreDestructuring: {
type: "boolean"
}
}
}
Expand All @@ -38,15 +44,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 @@ -58,6 +74,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 @@ -66,9 +102,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 @@ -78,14 +113,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 @@ -108,36 +146,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

0 comments on commit c832cd5

Please sign in to comment.