From 36eed493530f47e4562d4eae2f9db346211e5037 Mon Sep 17 00:00:00 2001 From: Annie Zhang Date: Fri, 2 Sep 2016 16:55:38 -0400 Subject: [PATCH] New: `func-name-matching` rule (fixes #6065) --- conf/eslint.json | 1 + docs/rules/func-name-matching.md | 64 +++++++++++ lib/rules/func-name-matching.js | 149 ++++++++++++++++++++++++++ tests/lib/rules/func-name-matching.js | 120 +++++++++++++++++++++ 4 files changed, 334 insertions(+) create mode 100644 docs/rules/func-name-matching.md create mode 100644 lib/rules/func-name-matching.js create mode 100644 tests/lib/rules/func-name-matching.js diff --git a/conf/eslint.json b/conf/eslint.json index 4850d3dc465d..4d54e3ab6dac 100755 --- a/conf/eslint.json +++ b/conf/eslint.json @@ -158,6 +158,7 @@ "eqeqeq": "off", "func-call-spacing": "off", "func-names": "off", + "func-name-matching": "off", "func-style": "off", "generator-star-spacing": "off", "global-require": "off", diff --git a/docs/rules/func-name-matching.md b/docs/rules/func-name-matching.md new file mode 100644 index 000000000000..c6a2dda59444 --- /dev/null +++ b/docs/rules/func-name-matching.md @@ -0,0 +1,64 @@ +# require function names to match the name of the variable or property to which they are assigned (func-name-matching) + +## Rule Details + +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 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 +/*eslint func-name-matching: "error"*/ + +var foo = function bar() {}; +foo = function bar() {}; +obj.foo = function bar() {}; +obj['foo'] = function bar() {}; +var obj = {foo: function bar() {}}; +``` + +```js +/*eslint func-name-matching: ["error", { "includeCommonJSModuleExports": true }]*/ + +module.exports = function foo(name) {}; +module['exports'] = function foo(name) {}; +``` + +Examples of **correct** code for this rule: + +```js +/*eslint func-name-matching: "error"*/ +/*eslint-env es6*/ + +var foo = function foo() {}; +var foo = function() {}; +var foo = () => {}; +foo = function foo() {}; + +obj.foo = function foo() {}; +obj['foo'] = function foo() {}; +obj['foo//bar'] = function foo() {}; +obj[foo] = function bar() {}; + +var obj = {foo: function foo() {}}; +var obj = {[foo]: function bar() {}}; +var obj = {'foo//bar': function foo() {}}; +var obj = {foo: function() {}}; + +obj['x' + 2] = function bar(){}; +var [ bar ] = [ function bar(){} ]; + +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. + +## Compatibility + +* **JSCS**: [requireMatchingFunctionName](http://jscs.info/rule/requireMatchingFunctionName) diff --git a/lib/rules/func-name-matching.js b/lib/rules/func-name-matching.js new file mode 100644 index 000000000000..2ea7a87ecdd1 --- /dev/null +++ b/lib/rules/func-name-matching.js @@ -0,0 +1,149 @@ +/** + * @fileoverview Rule to require function names to match the name of the variable or property to which they are assigned. + * @author Annie Zhang, Pavel Strashkin + */ + +"use strict"; + +//-------------------------------------------------------------------------- +// Requirements +//-------------------------------------------------------------------------- + +const astUtils = require("../ast-utils"); +const esutils = require("esutils"); + +//-------------------------------------------------------------------------- +// Helpers +//-------------------------------------------------------------------------- + +/** + * Determines if a pattern is `module.exports` or `module["exports"]` + * @param {ASTNode} pattern The left side of the AssignmentExpression + * @returns {boolean} True if the pattern is `module.exports` or `module["exports"]` + */ +function isModuleExports(pattern) { + if (pattern.type === "MemberExpression" && pattern.object.type === "Identifier" && pattern.object.name === "module") { + + // module.exports + if (pattern.property.type === "Identifier" && pattern.property.name === "exports") { + return true; + } + + // module["exports"] + if (pattern.property.type === "Literal" && pattern.property.value === "exports") { + return true; + } + } + return false; +} + +/** + * Determines if a string name is a valid identifier + * @param {string} name The string to be checked + * @param {int} ecmaVersion The ECMAScript version if specified in the parserOptions config + * @returns {boolean} True if the string is a valid identifier + */ +function isIdentifier(name, ecmaVersion) { + if (ecmaVersion >= 6) { + return esutils.keyword.isIdentifierES6(name); + } + return esutils.keyword.isIdentifierES5(name); +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "require function names to match the name of the variable or property to which they are assigned", + category: "Stylistic Issues", + recommended: false + }, + + schema: [ + { + type: "object", + properties: { + includeCommonJSModuleExports: { + type: "boolean" + } + }, + additionalProperties: false + } + ] + }, + + create(context) { + + const includeModuleExports = context.options[0] && context.options[0].includeCommonJSModuleExports; + const ecmaVersion = context.parserOptions && context.parserOptions.ecmaVersion ? context.parserOptions.ecmaVersion : 5; + + /** + * Reports + * @param {ASTNode} node The node to report + * @param {string} name The variable or property name + * @param {string} funcName The function name + * @param {boolean} isProp True if the reported node is a property assignment + * @returns {void} + */ + function report(node, name, funcName, isProp) { + context.report({ + node, + message: isProp ? "Function name `{{funcName}}` should match property name `{{name}}`" + : "Function name `{{funcName}}` should match variable name `{{name}}`", + data: { + name, + funcName + } + }); + } + + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + return { + + VariableDeclarator(node) { + if (node.init.type !== "FunctionExpression") { + return; + } + if (node.init.id && node.id.name !== node.init.id.name) { + report(node, node.id.name, node.init.id.name, false); + } + }, + + AssignmentExpression(node) { + if (node.right.type !== "FunctionExpression" || + (node.left.computed && node.left.property.type !== "Literal") || + (!includeModuleExports && isModuleExports(node.left)) + ) { + return; + } + + const isProp = node.left.type === "MemberExpression" ? true : false; + const name = isProp ? astUtils.getStaticPropertyName(node.left) : node.left.name; + + if (node.right.id && isIdentifier(name) && name !== node.right.id.name) { + report(node, name, node.right.id.name, isProp); + } + }, + + Property(node) { + if (node.computed || node.value.type !== "FunctionExpression" || !node.value.id) { + return; + } + if (node.key.type === "Identifier" && node.key.name !== node.value.id.name) { + report(node, node.key.name, node.value.id.name, true); + } else if (node.key.type === "Literal" && + isIdentifier(node.key.value, ecmaVersion) && + node.key.value !== node.value.id.name + ) { + report(node, node.key.value, node.value.id.name, true); + } + } + }; + } +}; diff --git a/tests/lib/rules/func-name-matching.js b/tests/lib/rules/func-name-matching.js new file mode 100644 index 000000000000..115159405fa9 --- /dev/null +++ b/tests/lib/rules/func-name-matching.js @@ -0,0 +1,120 @@ +/** + * @fileoverview Tests for func-name-matching rule. + * @author Annie Zhang + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/func-name-matching"), + RuleTester = require("../../../lib/testers/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(); + +ruleTester.run("func-name-matching", rule, { + valid: [ + { code: "var foo = function foo() {};" }, + { code: "var foo = function() {}"}, + { code: "var foo = () => {}", parserOptions: { ecmaVersion: 6} }, + { code: "foo = function foo() {};" }, + { code: "obj.foo = function foo() {};" }, + { code: "obj.foo = function() {};" }, + { code: "obj.bar.foo = function foo() {};" }, + { code: "obj['foo'] = function foo() {};" }, + { code: "obj['foo//bar'] = function foo() {};"}, + { code: "obj[foo] = function bar() {};" }, + { code: "var obj = {foo: function foo() {}};" }, + { code: "var obj = {'foo': function foo() {}};" }, + { code: "var obj = {'foo//bar': function foo() {}};" }, + { code: "var obj = {foo: function() {}};" }, + { code: "var obj = {[foo]: function bar() {}} ", parserOptions: { ecmaVersion: 6} }, + { code: "var obj = {['x' + 2]: function bar(){}};", parserOptions: { ecmaVersion: 6} }, + { code: "obj['x' + 2] = function bar(){};" }, + { code: "var [ bar ] = [ function bar(){} ];", parserOptions: { ecmaVersion: 6} }, + { code: "module.exports = function foo(name) {};" }, + { code: "module['exports'] = function foo(name) {};" }, + { + code: "module.exports = function foo(name) {};", + options: [{ includeCommonJSModuleExports: false }], + parserOptions: { ecmaVersion: 6} + }, + { + code: "module['exports'] = function foo(name) {};", + options: [{ includeCommonJSModuleExports: false }], + parserOptions: { ecmaVersion: 6} } + ], + invalid: [ + { + code: "let foo = function bar() {};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match variable name `foo`" } + ] + }, + { + code: "foo = function bar() {};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match variable name `foo`" } + ] + }, + { + code: "obj.foo = function bar() {};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match property name `foo`" } + ] + }, + { + code: "obj.bar.foo = function bar() {};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match property name `foo`" } + ] + }, + { + code: "obj['foo'] = function bar() {};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match property name `foo`" } + ] + }, + { + code: "let obj = {foo: function bar() {}};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match property name `foo`" } + ] + }, + { + code: "let obj = {'foo': function bar() {}};", + parserOptions: { ecmaVersion: 6}, + errors: [ + { message: "Function name `bar` should match property name `foo`" } + ] + }, + { + code: "module.exports = function foo(name) {};", + parserOptions: { ecmaVersion: 6}, + options: [{ includeCommonJSModuleExports: true }], + errors: [ + { message: "Function name `foo` should match property name `exports`" } + ] + }, + { + code: "module['exports'] = function foo(name) {};", + parserOptions: { ecmaVersion: 6}, + options: [{ includeCommonJSModuleExports: true }], + errors: [ + { message: "Function name `foo` should match property name `exports`" } + ] + } + ] +});