From 0929797b9ffc3ab3b59c2a43d1c7d7a583cd92e3 Mon Sep 17 00:00:00 2001 From: Jeff Gulbronson Date: Sat, 17 Oct 2015 20:39:34 -0700 Subject: [PATCH 1/2] requireAlignedMultilineParams: enforce indentation of parameters in multiline functions Closes gh-1896 Fixes #1894 --- lib/config/configuration.js | 2 + lib/rules/require-aligned-multiline-params.js | 117 ++++++++++++ .../rules/require-aligned-multiline-params.js | 177 ++++++++++++++++++ 3 files changed, 296 insertions(+) create mode 100644 lib/rules/require-aligned-multiline-params.js create mode 100644 test/specs/rules/require-aligned-multiline-params.js diff --git a/lib/config/configuration.js b/lib/config/configuration.js index 34bd93666..1e27de342 100644 --- a/lib/config/configuration.js +++ b/lib/config/configuration.js @@ -1073,6 +1073,8 @@ Configuration.prototype.registerDefaultRules = function() { this.registerRule(require('../rules/validate-order-in-object-keys')); this.registerRule(require('../rules/disallow-tabs')); + + this.registerRule(require('../rules/require-aligned-multiline-params')); }; /** diff --git a/lib/rules/require-aligned-multiline-params.js b/lib/rules/require-aligned-multiline-params.js new file mode 100644 index 000000000..4fc4bafb9 --- /dev/null +++ b/lib/rules/require-aligned-multiline-params.js @@ -0,0 +1,117 @@ +/** + * Enforces indentation of parameters in multiline functions + * + * Types: `Boolean`, `String`, `Number` + * + * Values: + * - `true` to require parameters are aligned with the body of the function + * - `'firstParam'` to require parameters to be aligned with the first parameter + * - `Number` for the number of columns the parameters should be indented past the function body + * + * #### Example + * + * ```js + * "disallowSpaceBeforeComma": true + * ``` + * + * ##### Valid for `true` + * + * ```js + * var test = function(one, two, + * three, four, five, + * six, seven, eight) { + * console.log(a); + * }; + * ``` + * + * ##### Valid for `2` + * + * ```js + * var test = function(one, two, + * three, four, five, + * six, seven, eight) { + * console.log(a); + * }; + * ``` + * + * ##### Valid for `'firstParam'` + * + * ```js + * var test = function(one, two, + * three, four, five, + * six, seven, eight) { + * console.log(a); + * }; + * ``` + * + */ + +var assert = require('assert'); + +module.exports = function() { +}; + +module.exports.prototype = { + + configure: function(option) { + if (typeof option === 'number') { + this._indentationLevel = option; + } else if (typeof option === 'string') { + assert( + option === 'firstParam', + this.getOptionName() + ' option requires string value to be "firstParam"' + ); + + this._alignWithFirstParam = true; + } else if (option === true) { + this._indentationLevel = 0; + } else { + assert( + false, + this.getOptionName() + ' option requires a valid option' + ); + } + }, + + getOptionName: function() { + return 'requireAlignedMultilineParams'; + }, + + check: function(file, errors) { + var _this = this; + file.iterateNodesByType(['FunctionDeclaration', 'FunctionExpression'], function(node) { + var params = node.params; + + // We can pass the check if there's no params + if (params.length === 0) { + return; + } + + var currentLine = params[0].loc.start.line; + var referenceColumn; + if (_this._alignWithFirstParam) { + referenceColumn = params[0].loc.start.column; + } else { + referenceColumn = node.body.body[0].loc.start.column + _this._indentationLevel; + } + + params.forEach(function(param) { + if (param.loc.start.line !== currentLine) { + if (param.loc.start.column !== referenceColumn) { + errors.assert.indentation({ + lineNumber: param.loc.start.line, + actual: param.loc.start.column, + expected: referenceColumn, + indentChar: ' ', + silent: false + }); + } + + currentLine = param.loc.start.line; + } + }); + + }); + } + +}; diff --git a/test/specs/rules/require-aligned-multiline-params.js b/test/specs/rules/require-aligned-multiline-params.js new file mode 100644 index 000000000..e2bf1fddc --- /dev/null +++ b/test/specs/rules/require-aligned-multiline-params.js @@ -0,0 +1,177 @@ +var Checker = require('../../../lib/checker'); +var expect = require('chai').expect; + +describe('rules/require-aligned-multiline-params', function() { + var checker; + var option; + function rules() { return { 'requireAlignedMultilineParams': option }; } + + describe('when we pass true as the config option', function() { + beforeEach(function() { + checker = new Checker(); + checker.registerDefaultRules(); + option = true; + checker.configure(rules()); + }); + + it('should validate a function with no params properly', function() { + var noParamFunction = 'var noParamFunction = function() { \n' + + ' return;\n' + + '};'; + + expect(checker.checkString(noParamFunction)).to.have.no.errors(); + }); + + it('should validate a function with a single line of params properly', function() { + var singleLineFunction = 'var singleLineFunction = function(a, b, c) { \n' + + ' console.log(a + b + c);\n' + + '};'; + + expect(checker.checkString(singleLineFunction)).to.have.no.errors(); + }); + + it('should validate a function with aligned params properly', function() { + var unalignedFunction = 'var alignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.no.errors(); + }); + + it('should validate a function with one unaligned param properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.one.validation.error.from( + 'requireAlignedMultilineParams'); + }); + + it('should validate a function with two unaligned params properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.error.count.equal(2); + }); + }); + + describe('when we pass a number as the config option', function() { + beforeEach(function() { + checker = new Checker(); + checker.registerDefaultRules(); + option = 2; + checker.configure(rules()); + }); + + it('should validate a function with no params properly', function() { + var noParamFunction = 'var noParamFunction = function() { \n' + + ' return;\n' + + '};'; + + expect(checker.checkString(noParamFunction)).to.have.no.errors(); + }); + + it('should validate a function with a single line of params properly', function() { + var singleLineFunction = 'var singleLineFunction = function(a, b, c) { \n' + + ' console.log(a + b + c);\n' + + '};'; + + expect(checker.checkString(singleLineFunction)).to.have.no.errors(); + }); + + it('should validate a function with aligned params properly', function() { + var unalignedFunction = 'var alignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.no.errors(); + }); + + it('should validate a function with one unaligned param properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.one.validation.error.from( + 'requireAlignedMultilineParams'); + }); + + it('should validate a function with two unaligned params properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.error.count.equal(2); + }); + }); + + describe('when we pass "firstParam" as the config option', function() { + beforeEach(function() { + checker = new Checker(); + checker.registerDefaultRules(); + option = 'firstParam'; + checker.configure(rules()); + }); + + it('should validate a function with no params properly', function() { + var noParamFunction = 'var noParamFunction = function() { \n' + + ' return;\n' + + '};'; + + expect(checker.checkString(noParamFunction)).to.have.no.errors(); + }); + + it('should validate a function with a single line of params properly', function() { + var singleLineFunction = 'var singleLineFunction = function(a, b, c) { \n' + + ' console.log(a + b + c);\n' + + '};'; + + expect(checker.checkString(singleLineFunction)).to.have.no.errors(); + }); + + it('should validate a function with aligned params properly', function() { + var unalignedFunction = 'var alignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.no.errors(); + }); + + it('should validate a function with one unaligned param properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.one.validation.error.from( + 'requireAlignedMultilineParams'); + }); + + it('should validate a function with two unaligned params properly', function() { + var unalignedFunction = 'var unalignedFunction = function(a,\n' + + ' b, c,\n' + + ' d, e) {\n' + + ' console.log(a + b + c + d + e);\n' + + '};'; + + expect(checker.checkString(unalignedFunction)).to.have.error.count.equal(2); + }); + }); +}); + From c842dfd6aa95c0b6f4cf5c1060a7072488a91558 Mon Sep 17 00:00:00 2001 From: Jeff Gulbronson Date: Thu, 22 Oct 2015 09:47:00 -0700 Subject: [PATCH 2/2] Increased code coverage and documentation --- lib/rules/require-aligned-multiline-params.js | 10 ++++++++ .../rules/require-aligned-multiline-params.js | 23 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/rules/require-aligned-multiline-params.js b/lib/rules/require-aligned-multiline-params.js index 4fc4bafb9..57f1cd9ef 100644 --- a/lib/rules/require-aligned-multiline-params.js +++ b/lib/rules/require-aligned-multiline-params.js @@ -44,6 +44,16 @@ * }; * ``` * + * ##### Invalid for `0` + * + * ```js + * var test = function(one, two, + * three, four, five, + * six, seven, eight) { + * console.log(a); + * }; + * ``` + * */ var assert = require('assert'); diff --git a/test/specs/rules/require-aligned-multiline-params.js b/test/specs/rules/require-aligned-multiline-params.js index e2bf1fddc..05f8488ad 100644 --- a/test/specs/rules/require-aligned-multiline-params.js +++ b/test/specs/rules/require-aligned-multiline-params.js @@ -4,7 +4,28 @@ var expect = require('chai').expect; describe('rules/require-aligned-multiline-params', function() { var checker; var option; - function rules() { return { 'requireAlignedMultilineParams': option }; } + function rules() { + return { 'requireAlignedMultilineParams': option }; + } + + describe('when we pass invalid config options', function() { + beforeEach(function() { + checker = new Checker(); + checker.registerDefaultRules(); + }); + + it('should error when an invalid string is passed as the config option', function() { + expect(function() { + checker.configure({ 'requireAlignedMultilineParams': 'invalid' }); + }).to.throw(); + }); + + it('should error when an object is passed as the config option', function() { + expect(function() { + checker.configure({ 'requireAlignedMultilineParams': {} }); + }).to.throw(); + }); + }); describe('when we pass true as the config option', function() { beforeEach(function() {