Skip to content

Commit

Permalink
Fix: no-extra-parens false positive with exports and object literals (#…
Browse files Browse the repository at this point in the history 
…8359)

This commit fixes an issue where no-extra-parens would incorrectly report parenthesized functions and classes in `export default` declarations, and object literals at the beginning of expression statements.
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Mar 31, 2017
1 parent 91baed4 commit e09132f
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 114 deletions.
139 changes: 39 additions & 100 deletions lib/rules/no-extra-parens.js
View file
Expand Up @@ -60,6 +60,7 @@ module.exports = {
create(context) {
const sourceCode = context.getSourceCode();

const tokensToIgnore = new WeakSet();
const isParenthesised = astUtils.isParenthesised.bind(astUtils, sourceCode);
const precedence = astUtils.getPrecedence;
const ALL_NODES = context.options[0] !== "functions";
Expand Down Expand Up @@ -238,69 +239,6 @@ module.exports = {
return hasDoubleExcessParens(node);
}

/**
* Checks whether or not a given node is located at the head of ExpressionStatement.
* @param {ASTNode} node - A node to check.
* @returns {boolean} `true` if the node is located at the head of ExpressionStatement.
*/
function isHeadOfExpressionStatement(node) {
let parent = node.parent;

while (parent) {
switch (parent.type) {
case "SequenceExpression":
if (parent.expressions[0] !== node || isParenthesised(node)) {
return false;
}
break;

case "UnaryExpression":
case "UpdateExpression":
if (parent.prefix || isParenthesised(node)) {
return false;
}
break;

case "BinaryExpression":
case "LogicalExpression":
if (parent.left !== node || isParenthesised(node)) {
return false;
}
break;

case "ConditionalExpression":
if (parent.test !== node || isParenthesised(node)) {
return false;
}
break;

case "CallExpression":
if (parent.callee !== node || isParenthesised(node)) {
return false;
}
break;

case "MemberExpression":
if (parent.object !== node || isParenthesised(node)) {
return false;
}
break;

case "ExpressionStatement":
return true;

default:
return false;
}

node = parent;
parent = parent.parent;
}

/* istanbul ignore next */
throw new Error("unreachable");
}

/**
* Determines whether a node should be preceded by an additional space when removing parens
* @param {ASTNode} node node to evaluate; must be surrounded by parentheses
Expand Down Expand Up @@ -345,9 +283,8 @@ module.exports = {
function report(node) {
const leftParenToken = sourceCode.getTokenBefore(node);
const rightParenToken = sourceCode.getTokenAfter(node);
const tokenBeforeLeftParen = sourceCode.getTokenBefore(leftParenToken);

if (tokenBeforeLeftParen && astUtils.isArrowToken(tokenBeforeLeftParen) && astUtils.isOpeningBraceToken(sourceCode.getFirstToken(node))) {
if (tokensToIgnore.has(sourceCode.getFirstToken(node)) && !isParenthesisedTwice(node)) {
return;
}

Expand Down Expand Up @@ -471,6 +408,34 @@ module.exports = {
}
}

/**
* Checks the parentheses for an ExpressionStatement or ExportDefaultDeclaration
* @param {ASTNode} node The ExpressionStatement.expression or ExportDefaultDeclaration.declaration node
* @returns {void}
*/
function checkExpressionOrExportStatement(node) {
const firstToken = isParenthesised(node) ? sourceCode.getTokenBefore(node) : sourceCode.getFirstToken(node);
const secondToken = sourceCode.getTokenAfter(firstToken, astUtils.isNotOpeningParenToken);

if (
astUtils.isOpeningParenToken(firstToken) &&
(
astUtils.isOpeningBraceToken(secondToken) ||
secondToken.type === "Keyword" && (
secondToken.value === "function" ||
secondToken.value === "class" ||
secondToken.value === "let" && astUtils.isOpeningBracketToken(sourceCode.getTokenAfter(secondToken))
)
)
) {
tokensToIgnore.add(secondToken);
}

if (hasExcessParens(node)) {
report(node);
}
}

return {
ArrayExpression(node) {
[].forEach.call(node.elements, e => {
Expand All @@ -486,13 +451,13 @@ module.exports = {
}

if (node.body.type !== "BlockStatement") {
if (hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
report(node.body);
return;
}
const firstBodyToken = sourceCode.getFirstToken(node.body, astUtils.isNotOpeningParenToken);
const tokenBeforeFirst = sourceCode.getTokenBefore(firstBodyToken);

// Object literals *must* be parenthesised
if (node.body.type === "ObjectExpression" && hasDoubleExcessParens(node.body)) {
if (astUtils.isOpeningParenToken(tokenBeforeFirst) && astUtils.isOpeningBraceToken(firstBodyToken)) {
tokensToIgnore.add(firstBodyToken);
}
if (hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
report(node.body);
}
}
Expand Down Expand Up @@ -535,27 +500,8 @@ module.exports = {
}
},

ExpressionStatement(node) {
if (hasExcessParens(node.expression)) {
const firstTokens = sourceCode.getFirstTokens(node.expression, 2);
const firstToken = firstTokens[0];
const secondToken = firstTokens[1];

if (
!firstToken ||
firstToken.value !== "{" &&
firstToken.value !== "function" &&
firstToken.value !== "class" &&
(
firstToken.value !== "let" ||
!secondToken ||
secondToken.value !== "["
)
) {
report(node.expression);
}
}
},
ExportDefaultDeclaration: node => checkExpressionOrExportStatement(node.declaration),
ExpressionStatement: node => checkExpressionOrExportStatement(node.expression),

ForInStatement(node) {
if (hasExcessParens(node.right)) {
Expand Down Expand Up @@ -598,18 +544,11 @@ module.exports = {
(
node.computed ||
!(
(node.object.type === "Literal" &&
typeof node.object.value === "number" &&
astUtils.isDecimalInteger(node.object)) ||
astUtils.isDecimalInteger(node.object) ||

// RegExp literal is allowed to have parens (#1589)
(node.object.type === "Literal" && node.object.regex)
)
) &&
!(
(node.object.type === "FunctionExpression" || node.object.type === "ClassExpression") &&
isHeadOfExpressionStatement(node) &&
!hasDoubleExcessParens(node.object)
)
) {
report(node.object);
Expand Down
42 changes: 28 additions & 14 deletions tests/lib/rules/no-extra-parens.js
View file
Expand Up @@ -426,6 +426,27 @@ ruleTester.run("no-extra-parens", rule, {
{
code: "() => ({ foo: 1 }.foo().bar + baz)",
parserOptions: { ecmaVersion: 2015 }
},
{
code: "export default (function(){}).foo",
parserOptions: { ecmaVersion: 2015, sourceType: "module" }
},
{
code: "export default (class{}).foo",
parserOptions: { ecmaVersion: 2015, sourceType: "module" }
},
"({}).hasOwnProperty.call(foo, bar)",
"({}) ? foo() : bar()",
"({}) + foo",
"(function(){}) + foo",
"(let[foo]) = 1", // setting the 'foo' property of the 'let' variable to 1
{
code: "((function(){}).foo.bar)();",
options: ["functions"]
},
{
code: "((function(){}).foo)();",
options: ["functions"]
}
],

Expand Down Expand Up @@ -545,8 +566,6 @@ ruleTester.run("no-extra-parens", rule, {
invalid("bar((function(){}).foo(), 0);", "bar(function(){}.foo(), 0);", "FunctionExpression"),
invalid("bar[(function(){}).foo()];", "bar[function(){}.foo()];", "FunctionExpression"),
invalid("var bar = (function(){}).foo();", "var bar = function(){}.foo();", "FunctionExpression"),
invalid("((function(){}).foo.bar)();", "(function(){}.foo.bar)();", "FunctionExpression", null, { options: ["functions"] }),
invalid("((function(){}).foo)();", "(function(){}.foo)();", "FunctionExpression", null, { options: ["functions"] }),

invalid("((class{})).foo();", "(class{}).foo();", "ClassExpression", null, { parserOptions: { ecmaVersion: 6 } }),
invalid("((class{}).foo());", "(class{}).foo();", "CallExpression", null, { parserOptions: { ecmaVersion: 6 } }),
Expand Down Expand Up @@ -976,17 +995,12 @@ ruleTester.run("no-extra-parens", rule, {
1,
{ parserOptions: { ecmaVersion: 2015 } }
),
{
code: "() => (({ foo: 1 }).foo)",
output: "() => ({ foo: 1 }).foo",
parserOptions: { ecmaVersion: 2015 },
errors: [

// 2 errors are reported, but fixing one gets rid of the other
{ message: "Gratuitous parentheses around expression.", type: "MemberExpression" },
{ message: "Gratuitous parentheses around expression.", type: "ObjectExpression" }
]

}
invalid(
"() => (({ foo: 1 }).foo)",
"() => ({ foo: 1 }).foo",
"MemberExpression",
1,
{ parserOptions: { ecmaVersion: 2015 } }
)
]
});

0 comments on commit e09132f

Please sign in to comment.