Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Add considerPropertyDescriptor option to func-name-matching #9078

Merged
merged 2 commits into from
Jun 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 46 additions & 12 deletions docs/rules/func-name-matching.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"] */

Expand Down Expand Up @@ -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.
Expand Down
56 changes: 52 additions & 4 deletions lib/rules/func-name-matching.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ const alwaysOrNever = { enum: ["always", "never"] };
const optionsObject = {
type: "object",
properties: {
considerPropertyDescriptor: {
type: "boolean"
},
includeCommonJSModuleExports: {
type: "boolean"
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -147,7 +167,6 @@ module.exports = {
//--------------------------------------------------------------------------

return {

VariableDeclarator(node) {
if (!node.init || node.init.type !== "FunctionExpression" || node.id.type !== "Identifier") {
return;
Expand Down Expand Up @@ -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)
Expand Down
141 changes: 141 additions & 0 deletions tests/lib/rules/func-name-matching.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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`" }
]
}
]
});