From 00011286a7f51424b640cf70dd42c3608c7be72b Mon Sep 17 00:00:00 2001 From: Emile Zhai Date: Thu, 5 Jul 2018 12:31:17 +0800 Subject: [PATCH] Update: add `ignoreDestructuring` option to `id-match` rule --- docs/rules/id-match.md | 45 +++++ lib/rules/id-match.js | 125 +++++++++++--- tests/lib/rules/id-match.js | 325 +++++++++++++++++++++++++++++++++++- 3 files changed, 467 insertions(+), 28 deletions(-) diff --git a/docs/rules/id-match.md b/docs/rules/id-match.md index bb56847f3edd..55f5e32539f7 100644 --- a/docs/rules/id-match.md +++ b/docs/rules/id-match.md @@ -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 @@ -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. diff --git a/lib/rules/id-match.js b/lib/rules/id-match.js index 608ef17d1148..39c99fb5a510 100644 --- a/lib/rules/id-match.js +++ b/lib/rules/id-match.js @@ -27,6 +27,12 @@ module.exports = { properties: { properties: { type: "boolean" + }, + onlyDeclarations: { + type: "boolean" + }, + ignoreDestructuring: { + type: "boolean" } } } @@ -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 @@ -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. @@ -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); } /** @@ -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 { @@ -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); } } diff --git a/tests/lib/rules/id-match.js b/tests/lib/rules/id-match.js index 36d60e9dfe91..5b5d11c261ab 100644 --- a/tests/lib/rules/id-match.js +++ b/tests/lib/rules/id-match.js @@ -110,11 +110,42 @@ ruleTester.run("id-match", rule, { options: ["^[^_]+$"] }, { - code: "var obj = {key: no_under}", + code: "var obj = {key: 'no_under'}", options: ["^[^_]+$", { properties: true }] }, + { + code: "var {key_no_under: key} = {}", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "var { category_id } = query;", + options: ["^[^_]+$", { + properties: true, + ignoreDestructuring: true + }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "var { category_id: category_id } = query;", + options: ["^[^_]+$", { + properties: true, + ignoreDestructuring: true + }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "var { category_id = 1 } = query;", + options: ["^[^_]+$", { + properties: true, + ignoreDestructuring: true + }], + parserOptions: { ecmaVersion: 6 } + }, { code: "var o = {key: 1}", options: ["^[^_]+$", { @@ -315,6 +346,298 @@ ruleTester.run("id-match", rule, { type: "Identifier" } ] + }, + { + code: "var { category_id: category_alias } = query;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'category_alias' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "var { category_id: category_alias } = query;", + options: ["^[^_]+$", { + properties: true, + ignoreDestructuring: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'category_alias' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "var { category_id: categoryId, ...other_props } = query;", + options: ["^[^_]+$", { + properties: true, + ignoreDestructuring: true + }], + parserOptions: { ecmaVersion: 2018 }, + errors: [ + { + message: "Identifier 'other_props' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "var { category_id } = query;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'category_id' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "var { category_id = 1 } = query;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'category_id' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import no_camelcased from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import * as no_camelcased from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import { no_camelcased } from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import { no_camelcased as no_camel_cased } from \"external module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camel_cased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import { camelCased as no_camel_cased } from \"external module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camel_cased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import { camelCased, no_camelcased } from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import { no_camelcased as camelCased, another_no_camelcased } from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'another_no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import camelCased, { no_camelcased } from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "import no_camelcased, { another_no_camelcased as camelCased } from \"external-module\";", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "function foo({ no_camelcased }) {};", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "function foo({ no_camelcased = 'default value' }) {};", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "const no_camelcased = 0; function foo({ camelcased_value = no_camelcased }) {}", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + }, + { + message: "Identifier 'camelcased_value' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "const { bar: no_camelcased } = foo;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "function foo({ value_1: my_default }) {}", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'my_default' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "function foo({ isCamelcased: no_camelcased }) {};", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "var { foo: bar_baz = 1 } = quz;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'bar_baz' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, + { + code: "const { no_camelcased = false } = bar;", + options: ["^[^_]+$", { + properties: true + }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + message: "Identifier 'no_camelcased' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] } ] });