From 7a7580b860e5d2ea38f9b1ec6860ea6f3c744d42 Mon Sep 17 00:00:00 2001 From: Dieter Luypaert Date: Fri, 15 Jun 2018 16:28:48 +0200 Subject: [PATCH] Update: Add considerPropertyDescriptor option to func-name-matching (#9078) * Update: Add considerPropertyDescriptor option to func-name-matching * Update: Include Reflect.defineProperty in considerPropertyDescriptor --- docs/rules/func-name-matching.md | 58 ++++++++--- lib/rules/func-name-matching.js | 56 +++++++++- tests/lib/rules/func-name-matching.js | 141 ++++++++++++++++++++++++++ 3 files changed, 239 insertions(+), 16 deletions(-) diff --git a/docs/rules/func-name-matching.md b/docs/rules/func-name-matching.md index 564671d571d..4d502869c99 100644 --- a/docs/rules/func-name-matching.md +++ b/docs/rules/func-name-matching.md @@ -4,10 +4,6 @@ This rule requires function names to match the name of the variable or property to which they are assigned. The rule will ignore property assignments where the property name is a literal that is not a valid identifier in the ECMAScript version specified in your configuration (default ES5). -## Options - -This rule takes an optional string of "always" or "never" (when omitted, it defaults to "always"), and an optional options object with one key, `includeCommonJSModuleExports`, and a boolean value. This option defaults to `false`, which means that `module.exports` and `module["exports"]` are ignored by this rule. If `includeCommonJSModuleExports` is set to true, `module.exports` and `module["exports"]` will be checked by this rule. - Examples of **incorrect** code for this rule: ```js @@ -21,14 +17,6 @@ var obj = {foo: function bar() {}}; ({['foo']: function bar() {}}); ``` -```js -/*eslint func-name-matching: ["error", { "includeCommonJSModuleExports": true }]*/ -/*eslint func-name-matching: ["error", "always", { "includeCommonJSModuleExports": true }]*/ // these are equivalent - -module.exports = function foo(name) {}; -module['exports'] = function foo(name) {}; -``` - ```js /*eslint func-name-matching: ["error", "never"] */ @@ -97,6 +85,52 @@ module.exports = function foo(name) {}; module['exports'] = function foo(name) {}; ``` +## Options + +This rule takes an optional string of "always" or "never" (when omitted, it defaults to "always"), and an optional options object with two properties `considerPropertyDescriptor` and `includeCommonJSModuleExports`. + +### considerPropertyDescriptor + +A boolean value that defaults to `false`. If `considerPropertyDescriptor` is set to true, the check will take into account the use `Object.create`, `Object.defineProperty`, `Object.defineProperties`, and `Reflect.defineProperty`. + +Examples of **correct** code for the `{ considerPropertyDescriptor: true }` option: + +```js +/*eslint func-name-matching: ["error", { "considerPropertyDescriptor": true }]*/ +/*eslint func-name-matching: ["error", "always", { "considerPropertyDescriptor": true }]*/ // these are equivalent +var obj = {}; +Object.create(obj, {foo:{value: function foo() {}}}); +Object.defineProperty(obj, 'bar', {value: function bar() {}}); +Object.defineProperties(obj, {baz:{value: function baz() {} }}); +Reflect.defineProperty(obj, 'foo', {value: function foo() {}}); +``` + +Examples of **incorrect** code for the `{ considerPropertyDescriptor: true }` option: + +```js +/*eslint func-name-matching: ["error", { "considerPropertyDescriptor": true }]*/ +/*eslint func-name-matching: ["error", "always", { "considerPropertyDescriptor": true }]*/ // these are equivalent +var obj = {}; +Object.create(obj, {foo:{value: function bar() {}}}); +Object.defineProperty(obj, 'bar', {value: function baz() {}}); +Object.defineProperties(obj, {baz:{value: function foo() {} }}); +Reflect.defineProperty(obj, 'foo', {value: function value() {}}); +``` + +### includeCommonJSModuleExports + +A boolean value that defaults to `false`. If `includeCommonJSModuleExports` is set to true, `module.exports` and `module["exports"]` will be checked by this rule. + +Examples of **incorrect** code for the `{ includeCommonJSModuleExports: true }` option: + +```js +/*eslint func-name-matching: ["error", { "includeCommonJSModuleExports": true }]*/ +/*eslint func-name-matching: ["error", "always", { "includeCommonJSModuleExports": true }]*/ // these are equivalent + +module.exports = function foo(name) {}; +module['exports'] = function foo(name) {}; +``` + ## When Not To Use It Do not use this rule if you want to allow named functions to have different names from the variable or property to which they are assigned. diff --git a/lib/rules/func-name-matching.js b/lib/rules/func-name-matching.js index 85fd7d4f1ee..b717995967f 100644 --- a/lib/rules/func-name-matching.js +++ b/lib/rules/func-name-matching.js @@ -58,6 +58,9 @@ const alwaysOrNever = { enum: ["always", "never"] }; const optionsObject = { type: "object", properties: { + considerPropertyDescriptor: { + type: "boolean" + }, includeCommonJSModuleExports: { type: "boolean" } @@ -90,9 +93,26 @@ module.exports = { create(context) { const options = (typeof context.options[0] === "object" ? context.options[0] : context.options[1]) || {}; const nameMatches = typeof context.options[0] === "string" ? context.options[0] : "always"; + const considerPropertyDescriptor = options.considerPropertyDescriptor; const includeModuleExports = options.includeCommonJSModuleExports; const ecmaVersion = context.parserOptions && context.parserOptions.ecmaVersion ? context.parserOptions.ecmaVersion : 5; + /** + * Check whether node is a certain CallExpression. + * @param {string} objName object name + * @param {string} funcName function name + * @param {ASTNode} node The node to check + * @returns {boolean} `true` if node matches CallExpression + */ + function isPropertyCall(objName, funcName, node) { + if (!node) { + return false; + } + return node.type === "CallExpression" && + node.callee.object.name === objName && + node.callee.property.name === funcName; + } + /** * Compares identifiers based on the nameMatches option * @param {string} x the first identifier @@ -147,7 +167,6 @@ module.exports = { //-------------------------------------------------------------------------- return { - VariableDeclarator(node) { if (!node.init || node.init.type !== "FunctionExpression" || node.id.type !== "Identifier") { return; @@ -179,9 +198,38 @@ module.exports = { if (node.value.type !== "FunctionExpression" || !node.value.id || node.computed && !isStringLiteral(node.key)) { return; } - if (node.key.type === "Identifier" && shouldWarn(node.key.name, node.value.id.name)) { - report(node, node.key.name, node.value.id.name, true); - } else if ( + + if (node.key.type === "Identifier") { + const functionName = node.value.id.name; + let propertyName = node.key.name; + + if (considerPropertyDescriptor && propertyName === "value") { + if (isPropertyCall("Object", "defineProperty", node.parent.parent) || isPropertyCall("Reflect", "defineProperty", node.parent.parent)) { + const property = node.parent.parent.arguments[1]; + + if (isStringLiteral(property) && shouldWarn(property.value, functionName)) { + report(node, property.value, functionName, true); + } + } else if (isPropertyCall("Object", "defineProperties", node.parent.parent.parent.parent)) { + propertyName = node.parent.parent.key.name; + if (!node.parent.parent.computed && shouldWarn(propertyName, functionName)) { + report(node, propertyName, functionName, true); + } + } else if (isPropertyCall("Object", "create", node.parent.parent.parent.parent)) { + propertyName = node.parent.parent.key.name; + if (!node.parent.parent.computed && shouldWarn(propertyName, functionName)) { + report(node, propertyName, functionName, true); + } + } else if (shouldWarn(propertyName, functionName)) { + report(node, propertyName, functionName, true); + } + } else if (shouldWarn(propertyName, functionName)) { + report(node, propertyName, functionName, true); + } + return; + } + + if ( isStringLiteral(node.key) && isIdentifier(node.key.value, ecmaVersion) && shouldWarn(node.key.value, node.value.id.name) diff --git a/tests/lib/rules/func-name-matching.js b/tests/lib/rules/func-name-matching.js index 6f33fbe9a6b..1459dc1e441 100644 --- a/tests/lib/rules/func-name-matching.js +++ b/tests/lib/rules/func-name-matching.js @@ -177,6 +177,84 @@ ruleTester.run("func-name-matching", rule, { { code: "var {a} = function foo() {}", parserOptions: { ecmaVersion: 6 } + }, + { + code: "({ value: function value() {} })", + options: [{ considerPropertyDescriptor: true }] + }, + { + code: "obj.foo = function foo() {};", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "obj.bar.foo = function foo() {};", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "var obj = {foo: function foo() {}};", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "var obj = {foo: function() {}};", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "var obj = { value: function value() {} }", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Object.defineProperty(foo, 'bar', { value: function bar() {} })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Object.defineProperties(foo, { bar: { value: function bar() {} } })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Object.create(proto, { bar: { value: function bar() {} } })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Object.defineProperty(foo, 'b' + 'ar', { value: function bar() {} })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Object.defineProperties(foo, { ['bar']: { value: function bar() {} } })", + options: ["always", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "Object.create(proto, { ['bar']: { value: function bar() {} } })", + options: ["always", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "Object.defineProperty(foo, 'bar', { value() {} })", + options: ["never", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "Object.defineProperties(foo, { bar: { value() {} } })", + options: ["never", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "Object.create(proto, { bar: { value() {} } })", + options: ["never", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } + }, + { + code: "Reflect.defineProperty(foo, 'bar', { value: function bar() {} })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Reflect.defineProperty(foo, 'b' + 'ar', { value: function baz() {} })", + options: ["always", { considerPropertyDescriptor: true }] + }, + { + code: "Reflect.defineProperty(foo, 'bar', { value() {} })", + options: ["never", { considerPropertyDescriptor: true }], + parserOptions: { ecmaVersion: 6 } } ], invalid: [ @@ -305,6 +383,69 @@ ruleTester.run("func-name-matching", rule, { errors: [ { message: "Function name `foo` should not match property name `foo`" } ] + }, + { + code: "Object.defineProperty(foo, 'bar', { value: function baz() {} })", + options: ["always", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `baz` should match property name `bar`" } + ] + }, + { + code: "Object.defineProperties(foo, { bar: { value: function baz() {} } })", + options: ["always", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `baz` should match property name `bar`" } + ] + }, + { + code: "Object.create(proto, { bar: { value: function baz() {} } })", + options: ["always", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `baz` should match property name `bar`" } + ] + }, + { + code: "var obj = { value: function foo(name) {} }", + options: ["always", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `foo` should match property name `value`" } + ] + }, + { + code: "Object.defineProperty(foo, 'bar', { value: function bar() {} })", + options: ["never", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `bar` should not match property name `bar`" } + ] + }, + { + code: "Object.defineProperties(foo, { bar: { value: function bar() {} } })", + options: ["never", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `bar` should not match property name `bar`" } + ] + }, + { + code: "Object.create(proto, { bar: { value: function bar() {} } })", + options: ["never", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `bar` should not match property name `bar`" } + ] + }, + { + code: "Reflect.defineProperty(foo, 'bar', { value: function baz() {} })", + options: ["always", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `baz` should match property name `bar`" } + ] + }, + { + code: "Reflect.defineProperty(foo, 'bar', { value: function bar() {} })", + options: ["never", { considerPropertyDescriptor: true }], + errors: [ + { message: "Function name `bar` should not match property name `bar`" } + ] } ] });