From e4acb27d6ce30dddb1c100a8a969430d2bc1070c Mon Sep 17 00:00:00 2001 From: Henry Zhu Date: Wed, 28 Oct 2015 11:12:43 -0400 Subject: [PATCH] Update: `requireCurlyBraces`: create exceptions for if and else statements Make an exception for return, break, continue statements Fixes #244 Closes gh-1938 --- lib/rules/require-curly-braces.js | 66 +++++++++++++++++++----- test/specs/rules/require-curly-braces.js | 26 ++++++++++ 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/lib/rules/require-curly-braces.js b/lib/rules/require-curly-braces.js index dc9bdbec6..0a75f7a37 100644 --- a/lib/rules/require-curly-braces.js +++ b/lib/rules/require-curly-braces.js @@ -1,9 +1,17 @@ /** * Requires curly braces after statements. * - * Types: `Array` or `Boolean` + * Types: `Array` or `Boolean` or `Object` * - * Values: Array of quoted keywords or `true` to require curly braces after the following keywords: + * Values: + * - Array of quoted keywords + * - `true` to require curly braces after the following keywords + * - `Object` + * - `'keywords'` + * - Array of quoted keywords + * - `'allExcept'` + * - Array of keywords inside of the block that would allow curly braces + * - Ex: ["return" , "continue", "break"] * * JSHint: [`curly`](http://jshint.com/docs/options/#curly) * @@ -45,19 +53,47 @@ module.exports = function() {}; module.exports.prototype = { - configure: function(statementTypes) { + configure: function(options) { assert( - Array.isArray(statementTypes) || statementTypes === true, - this.getOptionName() + ' option requires array or true value' + Array.isArray(options) || options === true || typeof options === 'object', + this.getOptionName() + ' option requires array, true value, or object' ); - if (statementTypes === true) { - statementTypes = defaultKeywords; + var keywordMap = { + 'return': 'ReturnStatement', + 'break': 'BreakStatement', + 'continue': 'ContinueStatement' + }; + + if (options === true) { + options = defaultKeywords; + } + + if (!Array.isArray(options)) { + assert( + Array.isArray(options.allExcept), + this.getOptionName() + '.allExcept ' + + 'property requires an array value' + ); + assert( + Array.isArray(options.keywords) || options.keywords === true, + this.getOptionName() + '.keywords ' + + 'property requires an array value or a value of true' + ); + + if (options.keywords === true) { + options.keywords = defaultKeywords; + } + + this._exceptions = options.allExcept.map(function(statementType) { + return keywordMap[statementType]; + }); + options = options.keywords; } this._typeIndex = {}; - for (var i = 0, l = statementTypes.length; i < l; i++) { - this._typeIndex[statementTypes[i]] = true; + for (var i = 0, l = options.length; i < l; i++) { + this._typeIndex[options[i]] = true; } }, @@ -66,6 +102,8 @@ module.exports.prototype = { }, check: function(file, errors) { + var typeIndex = this._typeIndex; + var exceptions = this._exceptions; function isNotABlockStatement(node) { return node && node.type !== 'BlockStatement'; @@ -87,15 +125,17 @@ module.exports.prototype = { }); } - var typeIndex = this._typeIndex; - if (typeIndex.if || typeIndex.else) { file.iterateNodesByType('IfStatement', function(node) { - if (typeIndex.if && isNotABlockStatement(node.consequent)) { + if (typeIndex.if && isNotABlockStatement(node.consequent) && + // check exceptions for if and else + !(exceptions && exceptions.indexOf(node.consequent.type) !== -1)) { addError('If', node); } if (typeIndex.else && isNotABlockStatement(node.alternate) && - node.alternate.type !== 'IfStatement') { + node.alternate.type !== 'IfStatement' && + // check exceptions for if and else + !(exceptions && exceptions.indexOf(node.consequent.type) !== -1)) { addError('Else', file.getPrevToken(file.getFirstNodeToken(node.alternate))); } }); diff --git a/test/specs/rules/require-curly-braces.js b/test/specs/rules/require-curly-braces.js index 51677c813..e927b99d8 100644 --- a/test/specs/rules/require-curly-braces.js +++ b/test/specs/rules/require-curly-braces.js @@ -170,4 +170,30 @@ describe('rules/require-curly-braces', function() { checker.configure({ requireCurlyBraces: ['else'] }); expect(checker.checkString('if (x) { x++; } else if (x) { x++; }')).to.have.no.errors(); }); + + describe('option with exceptions', function() { + it('should report on if and else', function() { + checker.configure({ + requireCurlyBraces: { + allExcept: ['return', 'break', 'continue'], + keywords: ['if', 'else'] + } + }); + + expect(checker.checkString('if (x) x++;')).to.have.errors(); + expect(checker.checkString('if (x) {x++} else x--;')).to.have.errors(); + }); + + it('should not report on if and else with exceptions', function() { + checker.configure({ + requireCurlyBraces: { + allExcept: ['return', 'break', 'continue'], + keywords: ['if', 'else'] + } + }); + + expect(checker.checkString('if (x) return;')).to.have.no.errors(); + expect(checker.checkString('if (x) return; else return;')).to.have.no.errors(); + }); + }); });