Skip to content

Commit

Permalink
Update: add ignoreDestructuring option to id-match rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinymins
tinymins committed Jul 5, 2018
1 parent 076a6b6 commit 202ac79
Show file tree
Hide file tree
Showing 3 changed files with 467 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.
125 changes: 98 additions & 27 deletions lib/rules/id-match.js
View file
Expand Up @@ -27,6 +27,12 @@ module.exports = {
properties: {
properties: {
type: "boolean"
},
onlyDeclarations: {
type: "boolean"
},
ignoreDestructuring: {
type: "boolean"
}
}
}
Expand All @@ -36,15 +42,24 @@ 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 = [];
const ALLOWED_PARENT_TYPES = new Set(["CallExpression", "NewExpression"]);
const DECLARATION_TYPES = new Set(["FunctionDeclaration", "VariableDeclarator"]);

/**
* Checks if a string matches the provided pattern
Expand All @@ -56,6 +71,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 +99,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 +110,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.indexOf(node) < 0) {
context.report({
node,
message: "Identifier '{{name}}' does not match the pattern '{{pattern}}'.",
data: {
name: node.name,
pattern
}
});
reported.push(node);
}
}

return {
Expand All @@ -106,36 +143,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" || node.parent.type === "AssignmentPattern") {

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

report(node);
}

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

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

const valueIsInvalid = node.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 (node.parent.right !== node && shouldReport(effectiveParent, name, true)) {
report(node);
}

} else {
const isDeclaration = effectiveParent.type === "FunctionDeclaration" || effectiveParent.type === "VariableDeclarator";
// Check if it's an import specifier
} else if (["ImportSpecifier", "ImportNamespaceSpecifier", "ImportDefaultSpecifier"].indexOf(node.parent.type) >= 0) {

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

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

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

Expand Down

0 comments on commit 202ac79

Please sign in to comment.