Skip to content

Commit

Permalink
Update: requireCurlyBraces: create exceptions for if and else state…
Browse files Browse the repository at this point in the history
…ments

Make an exception for return, break, continue statements

Fixes jscs-dev#244
Closes jscs-devgh-1938
  • Loading branch information
hzoo committed Oct 28, 2015
1 parent 9469688 commit e4acb27
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 13 deletions.
66 changes: 53 additions & 13 deletions 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)
*
Expand Down Expand Up @@ -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;
}
},

Expand All @@ -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';
Expand All @@ -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)));
}
});
Expand Down
26 changes: 26 additions & 0 deletions test/specs/rules/require-curly-braces.js
Expand Up @@ -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();
});
});
});

0 comments on commit e4acb27

Please sign in to comment.