Only add parenthesis on ternaries inside of arrow functions if doesn't break (#1450)

This was added in order to follow some eslint rule but it's only confusing when it doesn't break, when it breaks the indentation makes it clear what is happening and you don't need parenthesis.

Fixes #1379
master
Christopher Chedeau 2017-05-01 14:32:52 -07:00 committed by GitHub
parent 93cad97b14
commit e39209386c
6 changed files with 113 additions and 58 deletions

View File

@ -6,6 +6,7 @@ var util = require("./util");
var n = types.namedTypes;
var isArray = types.builtInTypes.array;
var isNumber = types.builtInTypes.number;
var startsWithNoLookaheadToken = util.startsWithNoLookaheadToken;
function FastPath(value) {
assert.ok(this instanceof FastPath);
@ -464,7 +465,6 @@ FPp.needsParens = function() {
case "ExportDefaultDeclaration":
case "AwaitExpression":
case "JSXSpreadAttribute":
case "ArrowFunctionExpression":
return true;
case "NewExpression":
@ -554,58 +554,4 @@ function containsCallExpression(node) {
return false;
}
// Tests if an expression starts with `{`, or (if forbidFunctionAndClass holds) `function` or `class`.
// Will be overzealous if there's already necessary grouping parentheses.
function startsWithNoLookaheadToken(node, forbidFunctionAndClass) {
node = getLeftMost(node);
switch (node.type) {
case "FunctionExpression":
case "ClassExpression":
return forbidFunctionAndClass;
case "ObjectExpression":
return true;
case "MemberExpression":
return startsWithNoLookaheadToken(node.object, forbidFunctionAndClass);
case "TaggedTemplateExpression":
if (node.tag.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(node.tag, forbidFunctionAndClass);
case "CallExpression":
if (node.callee.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(node.callee, forbidFunctionAndClass);
case "ConditionalExpression":
return startsWithNoLookaheadToken(node.test, forbidFunctionAndClass);
case "UpdateExpression":
return (
!node.prefix &&
startsWithNoLookaheadToken(node.argument, forbidFunctionAndClass)
);
case "BindExpression":
return (
node.object &&
startsWithNoLookaheadToken(node.object, forbidFunctionAndClass)
);
case "SequenceExpression":
return startsWithNoLookaheadToken(
node.expressions[0],
forbidFunctionAndClass
);
default:
return false;
}
}
function getLeftMost(node) {
if (node.left) {
return getLeftMost(node.left);
} else {
return node;
}
}
module.exports = FastPath;

View File

@ -386,12 +386,29 @@ function genericPrintNoParens(path, options, print, args) {
// with the opening (.
const shouldAddSoftLine = args && args.expandLastArg;
// In order to avoid confusion between
// a => a ? a : a
// a <= a ? a : a
const shouldAddParens =
n.body.type === "ConditionalExpression" &&
!util.startsWithNoLookaheadToken(
n.body,
/* forbidFunctionAndClass */ false
);
return group(
concat([
concat(parts),
group(
concat([
indent(concat([line, body])),
indent(
concat([
line,
shouldAddParens ? ifBreak("", "(") : "",
body,
shouldAddParens ? ifBreak("", ")") : ""
])
),
shouldAddSoftLine
? concat([
ifBreak(shouldPrintComma(options, "all") ? "," : ""),

View File

@ -267,6 +267,61 @@ function getPrecedence(op) {
return PRECEDENCE[op];
}
// Tests if an expression starts with `{`, or (if forbidFunctionAndClass holds) `function` or `class`.
// Will be overzealous if there's already necessary grouping parentheses.
function startsWithNoLookaheadToken(node, forbidFunctionAndClass) {
node = getLeftMost(node);
switch (node.type) {
case "FunctionExpression":
case "ClassExpression":
return forbidFunctionAndClass;
case "ObjectExpression":
return true;
case "MemberExpression":
return startsWithNoLookaheadToken(node.object, forbidFunctionAndClass);
case "TaggedTemplateExpression":
if (node.tag.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(node.tag, forbidFunctionAndClass);
case "CallExpression":
if (node.callee.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(node.callee, forbidFunctionAndClass);
case "ConditionalExpression":
return startsWithNoLookaheadToken(node.test, forbidFunctionAndClass);
case "UpdateExpression":
return (
!node.prefix &&
startsWithNoLookaheadToken(node.argument, forbidFunctionAndClass)
);
case "BindExpression":
return (
node.object &&
startsWithNoLookaheadToken(node.object, forbidFunctionAndClass)
);
case "SequenceExpression":
return startsWithNoLookaheadToken(
node.expressions[0],
forbidFunctionAndClass
);
default:
return false;
}
}
function getLeftMost(node) {
if (node.left) {
return getLeftMost(node.left);
} else {
return node;
}
}
module.exports = {
getPrecedence,
isExportDeclaration,
@ -286,5 +341,6 @@ module.exports = {
locEnd,
setLocStart,
setLocEnd,
htmlEscapeInsideAngleBracket
htmlEscapeInsideAngleBracket,
startsWithNoLookaheadToken
};

View File

@ -104,7 +104,7 @@ true
}),
require("postcss-url")({
url: url =>
(url.startsWith("/") || /^[a-z]+:/.test(url) ? url : \`/static/\${url}\`)
url.startsWith("/") || /^[a-z]+:/.test(url) ? url : \`/static/\${url}\`
})
]
});

View File

@ -3,23 +3,55 @@
exports[`parenthesis.js 1`] = `
debug ? this.state.isVisible ? "partially visible" : "hidden" : null;
debug ? this.state.isVisible && somethingComplex ? "partially visible" : "hidden" : null;
a => a ? () => {a} : () => {a}
a => a ? a : a
a => a ? aasdasdasdasdasdasdaaasdasdasdasdasdasdasdasdasdasdasdasdasdaaaaaa : a
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
debug ? (this.state.isVisible ? "partially visible" : "hidden") : null;
debug
? this.state.isVisible && somethingComplex ? "partially visible" : "hidden"
: null;
a =>
a
? () => {
a;
}
: () => {
a;
};
a => (a ? a : a);
a =>
a ? aasdasdasdasdasdasdaaasdasdasdasdasdasdasdasdasdasdasdasdasdaaaaaa : a;
`;
exports[`parenthesis.js 2`] = `
debug ? this.state.isVisible ? "partially visible" : "hidden" : null;
debug ? this.state.isVisible && somethingComplex ? "partially visible" : "hidden" : null;
a => a ? () => {a} : () => {a}
a => a ? a : a
a => a ? aasdasdasdasdasdasdaaasdasdasdasdasdasdasdasdasdasdasdasdasdaaaaaa : a
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
debug ? (this.state.isVisible ? "partially visible" : "hidden") : null;
debug
? this.state.isVisible && somethingComplex ? "partially visible" : "hidden"
: null;
a =>
a
? () => {
a;
}
: () => {
a;
};
a => (a ? a : a);
a =>
a ? aasdasdasdasdasdasdaaasdasdasdasdasdasdasdasdasdasdasdasdasdaaaaaa : a;
`;
exports[`test.js 1`] = `

View File

@ -1,2 +1,6 @@
debug ? this.state.isVisible ? "partially visible" : "hidden" : null;
debug ? this.state.isVisible && somethingComplex ? "partially visible" : "hidden" : null;
a => a ? () => {a} : () => {a}
a => a ? a : a
a => a ? aasdasdasdasdasdasdaaasdasdasdasdasdasdasdasdasdasdasdasdasdaaaaaa : a