Skip to content

Commit

Permalink
Merge pull request #4033 from BenoitZugmeyer/issue2390
Browse files Browse the repository at this point in the history 
New: add "consistent" option to the "curly" rule (refs #2390)
  • Loading branch information
@nzakas
nzakas committed Oct 6, 2015
2 parents ae6ad3f + 5a24e79 commit 45389f9
Show file tree
Hide file tree
Showing 3 changed files with 250 additions and 27 deletions.
71 changes: 71 additions & 0 deletions docs/rules/curly.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,77 @@ for (var i = 0; foo; i++)
doSomething();
```

#### consistent

When using any of the `multi*` option, you can add an option to enforce all bodies of a `if`,
`else if` and `else` chain to be with or without braces.

```json
curly: [2, "multi", "consistent"]
```

With this configuration, the rule will warn for those patterns:

```js
/*eslint curly: [2, "multi", "consistent"]*/

if (foo) {
bar();
baz();
} else /*error Expected { after 'else'.*/
buz();

if (foo) /*error Expected { after 'if' condition.*/
bar();
else if (faa) /*error Expected { after 'if' condition.*/
bor();
else {
other();
things();
}

if (true)
foo();
else { /*error Unnecessary { after 'else'.*/
baz();
}

if (foo) { /*error Unnecessary { after 'if' condition.*/
foo++;
}
```

It will not warn for these patterns:

```js
/*eslint curly: [2, "multi", "consistent"]*/

if (foo) {
bar();
baz();
} else {
buz();
}

if (foo) {
bar();
} else if (faa) {
bor();
} else {
other();
things();
}

if (true)
foo();
else
baz();

if (foo)
foo++;

```

The default configuration is:

```json
Expand Down
136 changes: 109 additions & 27 deletions lib/rules/curly.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module.exports = function(context) {
var multiOnly = (context.options[0] === "multi");
var multiLine = (context.options[0] === "multi-line");
var multiOrNest = (context.options[0] === "multi-or-nest");
var always = !(multiOnly || multiLine || multiOrNest);
var consistent = (context.options[1] === "consistent");

//--------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -114,36 +114,91 @@ module.exports = function(context) {
}

/**
* Checks the body of a node to see if it's a block statement. Depending on
* the rule options, reports the appropriate problems.
* Prepares to check the body of a node to see if it's a block statement.
* @param {ASTNode} node The node to report if there's a problem.
* @param {ASTNode} body The body node to check for blocks.
* @param {string} name The name to report if there's a problem.
* @param {string} suffix Additional string to add to the end of a report.
* @returns {void}
* @returns {object} a prepared check object, with "actual", "expected", "check" properties.
* "actual" will be `true` or `false` whether the body is already a block statement.
* "expected" will be `true` or `false` if the body should be a block statement or not, or
* `null` if it doesn't matter, depending on the rule options. It can be modified to change
* the final behavior of "check".
* "check" will be a function reporting appropriate problems depending on the other
* properties.
*/
function checkBody(node, body, name, suffix) {
function prepareCheck(node, body, name, suffix) {
var hasBlock = (body.type === "BlockStatement");
var expected = null;

if (multiOnly) {
if (node.type === "IfStatement" && node.consequent === body && requiresBraceOfConsequent(node)) {
expected = true;
} else if (multiOnly) {
if (hasBlock && body.body.length === 1) {
reportUnnecessaryBraceError(node, name, suffix);
expected = false;
}
} else if (multiLine) {
if (!hasBlock && !isCollapsedOneLiner(body)) {
reportExpectedBraceError(node, name, suffix);
if (!isCollapsedOneLiner(body)) {
expected = true;
}
} else if (multiOrNest) {
if (hasBlock && body.body.length === 1 && isOneLiner(body.body[0])) {
reportUnnecessaryBraceError(node, name, suffix);
} else if (!hasBlock && !isOneLiner(body)) {
reportExpectedBraceError(node, name, suffix);
expected = false;
} else if (!isOneLiner(body)) {
expected = true;
}
} else {
if (!hasBlock) {
reportExpectedBraceError(node, name, suffix);
expected = true;
}

return {
actual: hasBlock,
expected: expected,
check: function() {
if (this.expected !== null && this.expected !== this.actual) {
if (this.expected) {
reportExpectedBraceError(node, name, suffix);
} else {
reportUnnecessaryBraceError(node, name, suffix);
}
}
}
};
}

/**
* Prepares to check the bodies of a "if", "else if" and "else" chain.
* @param {ASTNode} node The first IfStatement node of the chain.
* @returns {object[]} prepared checks for each body of the chain. See `prepareCheck` for more
* information.
*/
function prepareIfChecks(node) {
var preparedChecks = [];
do {
preparedChecks.push(prepareCheck(node, node.consequent, "if", "condition"));
if (node.alternate && node.alternate.type !== "IfStatement") {
preparedChecks.push(prepareCheck(node, node.alternate, "else"));
break;
}
node = node.alternate;
} while (node);

if (consistent) {
// If any node should have or already have braces, make sure they all have braces.
// If all nodes shouldn't have braces, make sure they don't.
var expected = preparedChecks.some(function(preparedCheck) {
if (preparedCheck.expected !== null) {
return preparedCheck.expected;
}
return preparedCheck.actual;
});

preparedChecks.forEach(function(preparedCheck) {
preparedCheck.expected = expected;
});
}

return preparedChecks;
}

//--------------------------------------------------------------------------
Expand All @@ -152,31 +207,58 @@ module.exports = function(context) {

return {
"IfStatement": function(node) {
if (always || !requiresBraceOfConsequent(node)) {
checkBody(node, node.consequent, "if", "condition");
}
if (node.alternate && node.alternate.type !== "IfStatement") {
checkBody(node, node.alternate, "else");
if (node.parent.type !== "IfStatement") {
prepareIfChecks(node).forEach(function(preparedCheck) {
preparedCheck.check();
});
}
},

"WhileStatement": function(node) {
checkBody(node, node.body, "while", "condition");
prepareCheck(node, node.body, "while", "condition").check();
},

"DoWhileStatement": function(node) {
checkBody(node, node.body, "do");
prepareCheck(node, node.body, "do").check();
},

"ForStatement": function(node) {
checkBody(node, node.body, "for", "condition");
prepareCheck(node, node.body, "for", "condition").check();
}
};

};

module.exports.schema = [
{
"enum": ["all", "multi", "multi-line", "multi-or-nest"]
}
];
module.exports.schema = {
"anyOf": [
{
"type": "array",
"items": [
{
"enum": [0, 1, 2]
},
{
"enum": ["all"]
}
],
"minItems": 1,
"maxItems": 2
},
{
"type": "array",
"items": [
{
"enum": [0, 1, 2]
},
{
"enum": ["multi", "multi-line", "multi-or-nest"]
},
{
"enum": ["consistent"]
}
],
"minItems": 1,
"maxItems": 3
}
]
};
70 changes: 70 additions & 0 deletions tests/lib/rules/curly.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,23 @@ ruleTester.run("curly", rule, {
{
code: "if (true) { while(a) if(b) while(c) if (d); else; } else;",
options: ["multi"]
},
{
code: "if (true) foo(); else { bar(); baz(); }",
options: ["multi"]
},

{
code: "if (true) { foo(); } else { bar(); baz(); }",
options: ["multi", "consistent"]
},
{
code: "if (true) { foo(); } else if (true) { faa(); } else { bar(); baz(); }",
options: ["multi", "consistent"]
},
{
code: "if (true) { foo(); faa(); } else { bar(); }",
options: ["multi", "consistent"]
}
],
invalid: [
Expand All @@ -142,6 +159,15 @@ ruleTester.run("curly", rule, {
}
]
},
{
code: "if (foo) { bar() } else if (faa) baz()",
errors: [
{
message: "Expected { after 'if' condition.",
type: "IfStatement"
}
]
},
{
code: "while (foo) bar()",
errors: [
Expand Down Expand Up @@ -338,6 +364,50 @@ ruleTester.run("curly", rule, {
type: "ForStatement"
}
]
},
{
code: "if (true) foo(); \n else { \n bar(); \n baz(); \n }",
options: ["multi", "consistent"],
errors: [
{
message: "Expected { after 'if' condition.",
type: "IfStatement"
}
]
},
{
code: "if (true) { foo(); faa(); }\n else bar();",
options: ["multi", "consistent"],
errors: [
{
message: "Expected { after 'else'.",
type: "IfStatement"
}
]
},
{
code: "if (true) foo(); else { baz(); }",
options: ["multi", "consistent"],
errors: [
{
message: "Unnecessary { after 'else'.",
type: "IfStatement"
}
]
},
{
code: "if (true) foo(); else if (true) faa(); else { bar(); baz(); }",
options: ["multi", "consistent"],
errors: [
{
message: "Expected { after 'if' condition.",
type: "IfStatement"
},
{
message: "Expected { after 'if' condition.",
type: "IfStatement"
}
]
}
]
});

0 comments on commit 45389f9

Please sign in to comment.