From b636959aa1463226d5fabef67686ea1910c8f1db Mon Sep 17 00:00:00 2001 From: Pavel Strashkin Date: Sun, 21 Dec 2014 16:26:44 -0800 Subject: [PATCH] requireMatchingFunctionName: requires member and property names to match function names Example of member name mismatch: ```js function Test() {}; Test.prototype.foo = function bar() {}; ``` Example of property name mismatch: ```js var test = {foo: function bar() {}}; ``` Fixes #846 Closes #850 --- README.md | 50 ++++++++++++++++ lib/config/configuration.js | 2 + lib/rules/require-function-name-match.js | 72 +++++++++++++++++++++++ test/rules/require-function-name-match.js | 64 ++++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 lib/rules/require-function-name-match.js create mode 100644 test/rules/require-function-name-match.js diff --git a/README.md b/README.md index 0a788717a..9723782b1 100644 --- a/README.md +++ b/README.md @@ -3697,6 +3697,56 @@ var a = 1; [b].forEach(c); ``` +### requireMatchingFunctionName + +Requires member and property names to match function names, including anonymous. + +Type: `Boolean` + +Value: `true` + +#### Example + +```js +"requireMatchingFunctionName": true +``` + +##### Valid + +```js +function Test() {}; +Test.prototype.foo = function foo() {}; +``` + +```js +var test = {foo: function foo() {}}; +``` + +```js +var test = {}; +test['foo'] = function foo() {}; +``` + +##### Invalid + +```js +function Test() {}; +Test.prototype.foo = function bar() {}; +``` + +```js +var test = {foo: function bar() {}}; +``` + +```js +var test = {foo: function() {}}; +``` + +```js +var test = {}; +test['foo'] = function bar() {}; +``` + ### ~~validateJSDoc~~ Please use the [JSCS-JSDoc](https://github.com/jscs-dev/jscs-jsdoc) plugin instead. diff --git a/lib/config/configuration.js b/lib/config/configuration.js index 369252187..fa2b8c293 100644 --- a/lib/config/configuration.js +++ b/lib/config/configuration.js @@ -595,6 +595,8 @@ Configuration.prototype.registerDefaultRules = function() { this.registerRule(require('../rules/require-line-break-after-variable-assignment')); this.registerRule(require('../rules/disallow-semicolons')); + + this.registerRule(require('../rules/require-function-name-match')); }; /** diff --git a/lib/rules/require-function-name-match.js b/lib/rules/require-function-name-match.js new file mode 100644 index 000000000..460533b74 --- /dev/null +++ b/lib/rules/require-function-name-match.js @@ -0,0 +1,72 @@ +var assert = require('assert'); + +module.exports = function() {}; + +module.exports.prototype = { + configure: function(requireMatchingFunctionName) { + assert( + requireMatchingFunctionName === true, + 'requireMatchingFunctionName option requires true value or should be removed' + ); + }, + + getOptionName: function() { + return 'requireMatchingFunctionName'; + }, + + check: function(file, errors) { + file.iterateNodesByType(['FunctionExpression'], function(node) { + var checker; + switch (node.parentNode.type) { + // object.foo = function bar() {} + // object['foo'] = function bar() {} + case 'AssignmentExpression': + checker = checkForMember; + break; + + // object = {foo: function bar() {}} + case 'Property': + checker = checkForProperty; + break; + } + + if (checker) { + checker(node.parentNode, errors); + } + }); + } +}; + +function checkForMember(assignment, errors) { + // We don't care about anonymous functions as + // those should be enforced by separate rule + if (isAnonymousFunction(assignment.right)) { + return; + } + + if (assignment.left.property.name !== assignment.right.id.name) { + errors.add( + 'Function name does not match member name', + assignment.loc.start + ); + } +} + +function checkForProperty(property, errors) { + // We don't care about anonymous functions as + // those should be enforced by separate rule + if (isAnonymousFunction(property.value)) { + return; + } + + if (property.key.name !== property.value.id.name) { + errors.add( + 'Function name does not match property name', + property.loc.start + ); + } +} + +function isAnonymousFunction(node) { + return !node.id; +} diff --git a/test/rules/require-function-name-match.js b/test/rules/require-function-name-match.js new file mode 100644 index 000000000..fd176b496 --- /dev/null +++ b/test/rules/require-function-name-match.js @@ -0,0 +1,64 @@ +var Checker = require('../../lib/checker'); +var assert = require('assert'); + +describe('rules/require-function-name-match', function() { + var checker; + + beforeEach(function() { + checker = new Checker(); + checker.registerDefaultRules(); + }); + + describe('option value true', function() { + beforeEach(function() { + checker.configure({ requireMatchingFunctionName: true }); + }); + + // TODO: + // - assigning function to variable (both names should match) + // - assigning function declaration to member + // - assigning function declaration to property + // For both cases we should track identifier back, check whether + // it's a function and if so, compare the names + + it('should report function name mismatch when assigning to member', function() { + assertErrorForMemberNameMismatch('function Test() {}; Test.prototype.foo = function bar() {};'); + }); + + it('should report function name mismatch when assigning to member via ["..."]', function() { + assertErrorForMemberNameMismatch('var test = {}; test["foo"] = function bar() {};'); + }); + + it('should NOT report function name mismatch when assign anonymous to member', function() { + assertNoError('function Test() {}; Test.prototype.foo = function() {};'); + }); + + it('should report function name mismatch when assigning to property', function() { + assertErrorForPropertyNameMismatch('var test = {foo: function bar() {}};'); + }); + + it('should NOT report function name mismatch when assign anonymous to property', function() { + assertNoError('var test = {foo: function() {}};'); + }); + + function assertErrorForMemberNameMismatch(js) { + assertError(js, 'Function name does not match member name'); + } + + function assertErrorForPropertyNameMismatch(js) { + assertError(js, 'Function name does not match property name'); + } + + function assertError(js, message) { + var errors = checker.checkString(js).getErrorList(); + assert(errors.length); + assert.equal(errors[0].rule, 'requireMatchingFunctionName'); + assert.equal(errors[0].message, message); + } + + function assertNoError(js) { + var errors = checker.checkString(js).getErrorList(); + assert.equal(errors.length, 0); + } + }); +});