Skip to content

Commit

Permalink
Break if () if conditional inside breaks (#1344)
Browse files Browse the repository at this point in the history
Fixes #868
  • Loading branch information
vjeux committed Apr 19, 2017
1 parent 042e603 commit aafcf5f
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 11 deletions.
46 changes: 35 additions & 11 deletions src/printer.js
Expand Up @@ -196,21 +196,43 @@ function genericPrintNoParens(path, options, print, args) {
);
case "BinaryExpression":
case "LogicalExpression": {
const parts = printBinaryishExpressions(path, print, options);
const parent = path.getParentNode();
const isInsideParenthesis =
n !== parent.body &&
(parent.type === "IfStatement" ||
parent.type === "WhileStatement" ||
parent.type === "DoStatement");

// Avoid indenting sub-expressions in if/etc statements.
const parts = printBinaryishExpressions(
path,
print,
options,
/* isNested */ false,
isInsideParenthesis
);

// if (
// this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft
// ) {
//
// looks super weird, we want to break the children if the parent breaks
//
// if (
// this.hasPlugin("dynamicImports") &&
// this.lookahead().type === tt.parenLeft
// ) {
if (isInsideParenthesis) {
return concat(parts);
}

// Avoid indenting sub-expressions in assignment/return/etc statements.
if (
parent.type === "AssignmentExpression" ||
parent.type === "VariableDeclarator" ||
shouldInlineLogicalExpression(n) ||
parent.type === "ReturnStatement" ||
(n !== parent.body &&
(parent.type === "IfStatement" ||
parent.type === "WhileStatement" ||
parent.type === "DoStatement" ||
parent.type === "ForStatement")) ||
(n === parent.body && parent.type === "ArrowFunctionExpression")
(n === parent.body && parent.type === "ArrowFunctionExpression") ||
(n !== parent.body && parent.type === "ForStatement")
) {
return group(concat(parts));
}
Expand Down Expand Up @@ -1490,7 +1512,7 @@ function genericPrintNoParens(path, options, print, args) {
!shouldTypeScriptTypeAvoidColon(path) &&
// TypeScript should not have a colon before type parameter constraints
!(path.getParentNode().type === "TypeParameter" &&
path.getParentNode().constraint) &&
path.getParentNode().constraint) &&
// TypeScript should not have a colon in TSFirstTypeNode nodes
// `a is number`
!(path.getParentNode().type === "TypeAnnotation" &&
Expand Down Expand Up @@ -3136,7 +3158,7 @@ function shouldInlineLogicalExpression(node) {
// precedence level and the AST is structured based on precedence
// level, things are naturally broken up correctly, i.e. `&&` is
// broken before `+`.
function printBinaryishExpressions(path, print, options, isNested) {
function printBinaryishExpressions(path, print, options, isNested, isInsideParenthesis) {
let parts = [];
let node = path.getValue();

Expand All @@ -3163,7 +3185,8 @@ function printBinaryishExpressions(path, print, options, isNested) {
left,
print,
options,
/* isNested */ true
/* isNested */ true,
isInsideParenthesis
),
"left"
)
Expand All @@ -3182,6 +3205,7 @@ function printBinaryishExpressions(path, print, options, isNested) {
// in order to avoid having a small right part like -1 be on its own line.
const parent = path.getParentNode();
const shouldGroup =
!isInsideParenthesis &&
parent.type !== node.type &&
node.left.type !== node.type &&
node.right.type !== node.type;
Expand Down
24 changes: 24 additions & 0 deletions tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -55,6 +55,30 @@ a ** (b * c);
`;

exports[`if.js 1`] = `
if (this.hasPlugin("dynamicImports") && this.lookahead().type) {}
if (this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft) {}
if (this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft.right) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
if (this.hasPlugin("dynamicImports") && this.lookahead().type) {
}
if (
this.hasPlugin("dynamicImports") &&
this.lookahead().type === tt.parenLeft
) {
}
if (
this.hasPlugin("dynamicImports") &&
this.lookahead().type === tt.parenLeft.right
) {
}
`;

exports[`inline-object-array.js 1`] = `
prevState = prevState || {
catalogs: [],
Expand Down
5 changes: 5 additions & 0 deletions tests/binary-expressions/if.js
@@ -0,0 +1,5 @@
if (this.hasPlugin("dynamicImports") && this.lookahead().type) {}

if (this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft) {}

if (this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft.right) {}

0 comments on commit aafcf5f

Please sign in to comment.