From 64d2e39afe23b80c3364dc11a4d3aab13954740c Mon Sep 17 00:00:00 2001 From: Emile Zhai Date: Wed, 10 Oct 2018 22:28:56 +0800 Subject: [PATCH] Update: add `ignoreDestructuring` option to `id-match` rule --- docs/rules/id-match.md | 45 +++++ lib/rules/id-match.js | 126 +++++++++++--- tests/lib/rules/id-match.js | 338 +++++++++++++++++++++++++++++++++++- 3 files changed, 481 insertions(+), 28 deletions(-) diff --git a/docs/rules/id-match.md b/docs/rules/id-match.md index bb56847f3ed..55f5e32539f 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 608ef17d114..51ad532c6d0 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,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 @@ -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. @@ -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); } /** @@ -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 { @@ -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); } } diff --git a/tests/lib/rules/id-match.js b/tests/lib/rules/id-match.js index 36d60e9dfe9..c3a12f6afb6 100644 --- a/tests/lib/rules/id-match.js +++ b/tests/lib/rules/id-match.js @@ -112,9 +112,41 @@ ruleTester.run("id-match", rule, { { code: "var obj = {key: no_under}", options: ["^[^_]+$", { - properties: true + properties: true, + onlyDeclarations: 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: ["^[^_]+$", { @@ -198,6 +230,18 @@ ruleTester.run("id-match", rule, { } ] }, + { + code: "var obj = {key: no_under}", + options: ["^[^_]+$", { + properties: true + }], + errors: [ + { + message: "Identifier 'no_under' does not match the pattern '^[^_]+$'.", + type: "Identifier" + } + ] + }, { code: "function no_under21(){}", options: ["^[^_]+$"], @@ -315,6 +359,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" + } + ] } ] });