Skip to content

Commit

Permalink
Update: Option for object literals in arrow-body-style (fixes #5936)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alberto
alberto committed May 18, 2016
1 parent 04bd586 commit faf46db
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 9 deletions.
28 changes: 26 additions & 2 deletions docs/rules/arrow-body-style.md
View file
Expand Up @@ -8,12 +8,12 @@ This rule can enforce the use of braces around arrow function body.

## Options

The rule takes one option, a string, which can be:
The rule takes one or two options. The first is a string, which can be:

* `"always"` enforces braces around the function body
* `"as-needed"` enforces no braces where they can be omitted (default)

### "always"
The second one is an object for more fine-grained configuration when the first option is `"as-needed"`. Currently, the only available parameter is `allowObjectLiteralBody`, which indicates if object literals should be declared using braces, thus requiring an explicit return.

```json
"arrow-body-style": ["error", "always"]
Expand Down Expand Up @@ -50,6 +50,8 @@ When the rule is set to `"as-needed"` the following patterns are considered prob
let foo = () => {
return 0;
};

let foo = () => { return { bar: 0 }; }
```

The following patterns are not considered problems:
Expand All @@ -69,4 +71,26 @@ let foo = () => { /* do nothing */ };
let foo = () => {
// do nothing.
};
let foo = () => ({ bar: 0 });
```

#### allowObjectLiteralBody

When the rule is set to `"as-needed", { allowObjectLiteralBody: true }` the following patterns are considered problems:

This comment has been minimized.

Sign in to view

Copy link
@BYK

BYK May 19, 2016

Member

Okay, I made a mistake :D

Looking at this code and my reasoning, I expected let foo = () => ({ bar: 0 }); to not be a problem along with let foo = () => { return { bar: 0 }; }; but let foo = () => { return { bar: 0 }; }; to be a problem when this option was false.

Looking back at the ticket, it seems like what you implemented is what was asked and then this option name makes it a bit confusing.

Do you agree? If so I'd go with enforceObjectLiteralBody as the option name (although it is an option making this stricter).

This comment has been minimized.

Sign in to view

Copy link
@alberto

alberto May 20, 2016

Author Member

Heh, yes, that's what I was trying to explain in the issue. :)

This comment has been minimized.

Sign in to view

Copy link
@BYK

BYK May 20, 2016

Member

Alright, sorry for the extra effort there. Are you okay with enforceObjectLiteralBody?

```js
/*eslint arrow-body-style: ["error", "as-needed", { allowObjectLiteralBody: true }]*/
/*eslint-env es6*/
let foo = () => ({});
let foo = () => ({ bar: 0 });
```

The following patterns are not considered problems:

```js
/*eslint arrow-body-style: ["error", "as-needed", { allowObjectLiteralBody: true }]*/
/*eslint-env es6*/

let foo = () => {};
let foo = () => { return { bar: 0 }; };
```
44 changes: 38 additions & 6 deletions lib/rules/arrow-body-style.js
View file
Expand Up @@ -16,16 +16,43 @@ module.exports = {
recommended: false
},

schema: [
{
enum: ["always", "as-needed"]
}
]
schema: {
anyOf: [
{
type: "array",
items: [
{
enum: ["always"]
}
],
minItems: 0,
maxItems: 1
},
{
type: "array",
items: [
{
enum: ["as-needed"]
},
{
type: "object",
properties: {
allowObjectLiteralBody: {type: "boolean"}
},
additionalProperties: false
}
],
minItems: 0,
maxItems: 2
}
]
}
},

create: function(context) {
var always = context.options[0] === "always";
var asNeeded = !context.options[0] || context.options[0] === "as-needed";
var allowObjectLiteralBody = context.options[1] && context.options[1].allowObjectLiteralBody === true;

This comment has been minimized.

Sign in to view

Copy link
@BYK

BYK May 19, 2016

Member

Did you mean context.options[1].allowObjectLiteralBody !== false since the default should be true?

This comment has been minimized.

Sign in to view

Copy link
@alberto

alberto May 20, 2016

Author Member

I thought the option meant "allow braces for object literal".

So, enforceObjectLiteralBody would also be true by default and you would set it to false to allow explicitly returning object literals, right?

This comment has been minimized.

Sign in to view

Copy link
@alberto

alberto May 20, 2016 •
edited

Author Member

I'm not sure why this comments don't show up in the PR. :/ Maybe you want to repost them from there.

This comment has been minimized.

Sign in to view

Copy link
@BYK

BYK May 20, 2016

Member

Oh, right. Moving there.

/**
* Determines whether a arrow function body needs braces
Expand All @@ -42,6 +69,11 @@ module.exports = {
return;
}

if (asNeeded && allowObjectLiteralBody && blockBody[0].type === "ReturnStatement" &&
blockBody[0].argument.type === "ObjectExpression") {
return;
}

if (asNeeded && blockBody[0].type === "ReturnStatement") {
context.report({
node: node,
Expand All @@ -50,7 +82,7 @@ module.exports = {
});
}
} else {
if (always) {
if (always || (asNeeded && allowObjectLiteralBody && arrowBody.type === "ObjectExpression")) {
context.report({
node: node,
loc: arrowBody.loc.start,
Expand Down
53 changes: 52 additions & 1 deletion tests/lib/rules/arrow-body-style.js
View file
Expand Up @@ -26,11 +26,22 @@ ruleTester.run("arrow-body-style", rule, {
{ code: "var foo = () => {\n /* do nothing */ \n};", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = (retv, name) => {\nretv[name] = true;\nreturn retv;\n};", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => ({});", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => bar();", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => { bar(); };", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => { b = a };", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => { bar: 1 };", parserOptions: { ecmaVersion: 6 } },
{ code: "var foo = () => { return 0; };", parserOptions: { ecmaVersion: 6 }, options: ["always"] },
{ code: "var foo = () => { return bar(); };", parserOptions: { ecmaVersion: 6 }, options: ["always"] }
{ code: "var foo = () => { return bar(); };", parserOptions: { ecmaVersion: 6 }, options: ["always"] },
{ code: "var foo = () => {};", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => 0;", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var addToB = (a) => { b = b + a };", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => { /* do nothing */ };", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => {\n /* do nothing */ \n};", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = (retv, name) => {\nretv[name] = true;\nreturn retv;\n};", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => bar();", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => { bar(); };", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var addToB = (a) => { b = b + a };", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] },
{ code: "var foo = () => { return { bar: 0 }; };", parserOptions: { ecmaVersion: 6 }, options: ["as-needed", {allowObjectLiteralBody: true }] }
],
invalid: [
{
Expand Down Expand Up @@ -64,6 +75,46 @@ ruleTester.run("arrow-body-style", rule, {
errors: [
{ line: 1, column: 17, type: "ArrowFunctionExpression", message: "Unexpected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => { return { bar: 0 }; };",
parserOptions: { ecmaVersion: 6 },
options: ["as-needed"],
errors: [
{ line: 1, column: 17, type: "ArrowFunctionExpression", message: "Unexpected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => { return 0; };",
parserOptions: { ecmaVersion: 6 },
options: ["as-needed", {allowObjectLiteralBody: true }],
errors: [
{ line: 1, column: 17, type: "ArrowFunctionExpression", message: "Unexpected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => { return bar(); };",
parserOptions: { ecmaVersion: 6 },
options: ["as-needed", {allowObjectLiteralBody: true }],
errors: [
{ line: 1, column: 17, type: "ArrowFunctionExpression", message: "Unexpected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => ({});",
parserOptions: { ecmaVersion: 6 },
options: ["as-needed", {allowObjectLiteralBody: true }],
errors: [
{ line: 1, column: 18, type: "ArrowFunctionExpression", message: "Expected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => ({ bar: 0 });",
parserOptions: { ecmaVersion: 6 },
options: ["as-needed", {allowObjectLiteralBody: true }],
errors: [
{ line: 1, column: 18, type: "ArrowFunctionExpression", message: "Expected block statement surrounding arrow body." }
]
}
]
});

0 comments on commit faf46db

Please sign in to comment.