Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: arrow-parens no reporting for comments inside (fixes #12995) #13312

Merged
merged 8 commits into from May 22, 2020
2 changes: 2 additions & 0 deletions docs/rules/arrow-parens.md
Expand Up @@ -159,6 +159,7 @@ Examples of **incorrect** code for this rule with the `"as-needed"` option:
a.then((foo) => {});
a.then((foo) => a);
a((foo) => { if (true) {} });
const f = /** @type {number} */(a) => a + a;
```

Examples of **correct** code for this rule with the `"as-needed"` option:
Expand All @@ -177,6 +178,7 @@ a.then(foo => { if (true) {} });
(a = 10) => a;
([a, b]) => a;
({a, b}) => a;
const f = (/** @type {number} */a) => a + a;
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
```

### requireForBlockBody
Expand Down
22 changes: 19 additions & 3 deletions lib/rules/arrow-parens.js
Expand Up @@ -105,10 +105,27 @@ module.exports = {
], `${shouldAddSpaceForAsync ? " " : ""}${paramToken.value}`);
}

/**
* Checks whether there are comments inside the params or not.
* @returns {boolean} `true` if there are comments inside of parens, else `false`
*/
function hasCommentsInParens() {
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
if (firstTokenOfParam.type === "Punctuator" && firstTokenOfParam.value === "(") {
anikethsaha marked this conversation as resolved.
Show resolved Hide resolved
const closingParenToken = sourceCode.getTokenAfter(node.params[0], astUtils.isClosingParenToken);

return closingParenToken && sourceCode.commentsExistBetween(firstTokenOfParam, closingParenToken);
}
return false;

}

if (hasCommentsInParens()) {
return;
}

// "as-needed", { "requireForBlockBody": true }: x => x
if (
requireForBlockBody &&
node.params.length === 1 &&
node.params[0].type === "Identifier" &&
!node.params[0].typeAnnotation &&
node.body.type !== "BlockStatement" &&
Expand Down Expand Up @@ -144,7 +161,6 @@ module.exports = {

// "as-needed": x => x
if (asNeeded &&
node.params.length === 1 &&
node.params[0].type === "Identifier" &&
!node.params[0].typeAnnotation &&
!node.returnType
Expand Down Expand Up @@ -178,7 +194,7 @@ module.exports = {
}

return {
ArrowFunctionExpression: parens
"ArrowFunctionExpression[params.length=1]": parens
};
}
};
141 changes: 140 additions & 1 deletion tests/lib/rules/arrow-parens.js
Expand Up @@ -29,6 +29,12 @@ const valid = [
"(a) => {\n}",
"a.then((foo) => {});",
"a.then((foo) => { if (true) {}; });",
"const f = (/* */a) => a + a;",
"const f = (a/** */) => a + a;",
"const f = (a//\n) => a + a;",
"const f = (//\na) => a + a;",
"const f = (/*\n */a//\n) => a + a;",
"const f = (/** @type {number} */a/**hello*/) => a + a;",
{ code: "a.then(async (foo) => { if (true) {}; });", parserOptions: { ecmaVersion: 8 } },

// "always" (explicit)
Expand Down Expand Up @@ -68,7 +74,69 @@ const valid = [
{ code: "async a => ({})", options: ["as-needed", { requireForBlockBody: true }], parserOptions: { ecmaVersion: 8 } },
{ code: "async a => a", options: ["as-needed", { requireForBlockBody: true }], parserOptions: { ecmaVersion: 8 } },
{ code: "(a: T) => a", options: ["as-needed", { requireForBlockBody: true }], parser: parser("identifer-type") },
{ code: "(a): T => a", options: ["as-needed", { requireForBlockBody: true }], parser: parser("return-type") }
{ code: "(a): T => a", options: ["as-needed", { requireForBlockBody: true }], parser: parser("return-type") },
{
code: "const f = (/** @type {number} */a/**hello*/) => a + a;",
options: ["as-needed"]
},
{
code: "const f = (/* */a) => a + a;",
options: ["as-needed"]
},
{
code: "const f = (a/** */) => a + a;",
options: ["as-needed"]
},
{
code: "const f = (a//\n) => a + a;",
options: ["as-needed"]
},
{
code: "const f = (//\na) => a + a;",
options: ["as-needed"]
},
{
code: "const f = (/*\n */a//\n) => a + a;",
options: ["as-needed"]
},
{
code: "var foo = (a,/**/) => b;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "var foo = (a , /**/) => b;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "var foo = (a\n,\n/**/) => b;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "var foo = (a,//\n) => b;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "const i = (a/**/,) => a + a;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "const i = (a \n /**/,) => a + a;",
parserOptions: { ecmaVersion: 2017 },
options: ["as-needed"]
},
{
code: "var bar = ({/*comment here*/a}) => a",
options: ["as-needed"]
},
{
code: "var bar = (/*comment here*/{a}) => a",
options: ["as-needed"]
}
];

const type = "ArrowFunctionExpression";
Expand Down Expand Up @@ -271,7 +339,78 @@ const invalid = [
messageId: "unexpectedParensInline",
type
}]
},
{
code: "const f = /** @type {number} */(a)/**hello*/ => a + a;",
options: ["as-needed"],
output: "const f = /** @type {number} */a/**hello*/ => a + a;",
errors: [{
line: 1,
column: 33,
type,
messageId: "unexpectedParens",
endLine: 1,
endColumn: 34
}]
},
{
code: "const f = //\n(a) => a + a;",
output: "const f = //\na => a + a;",
options: ["as-needed"],
errors: [{
line: 2,
column: 2,
type,
messageId: "unexpectedParens",
endLine: 2,
endColumn: 3
}]
},
{
code: "var foo = /**/ a => b;",
output: "var foo = /**/ (a) => b;",
errors: [{
line: 1,
column: 16,
type: "ArrowFunctionExpression",
messageId: "expectedParens",
endLine: 1,
endColumn: 17
}]
},
{
code: "var bar = a /**/ => b;",
output: "var bar = (a) /**/ => b;",
errors: [{
line: 1,
column: 11,
type: "ArrowFunctionExpression",
messageId: "expectedParens",
endLine: 1,
endColumn: 12
}]
},
{
code: `const foo = a => {};

// comment between 'a' and an unrelated closing paren

bar();`,
output: `const foo = (a) => {};

// comment between 'a' and an unrelated closing paren

bar();`,
errors: [{
line: 1,
column: 13,
type: "ArrowFunctionExpression",
messageId: "expectedParens",
endLine: 1,
endColumn: 14
}]
}

];

ruleTester.run("arrow-parens", rule, {
Expand Down