diff --git a/src/fast-path.js b/src/fast-path.js index de8b4572..19eb2be2 100644 --- a/src/fast-path.js +++ b/src/fast-path.js @@ -1,5 +1,6 @@ var assert = require("assert"); var types = require("ast-types"); +var util = require("./util"); var n = types.namedTypes; var Node = n.Node; var isArray = types.builtInTypes.array; @@ -262,9 +263,9 @@ FPp.needsParens = function(assumeExpressionContext) { case "BinaryExpression": case "LogicalExpression": var po = parent.operator; - var pp = PRECEDENCE[po]; + var pp = util.getPrecedence(po); var no = node.operator; - var np = PRECEDENCE[no]; + var np = util.getPrecedence(no); if (pp > np) { return true; @@ -424,24 +425,6 @@ function isUnaryLike(node) { ) || n.SpreadElement && n.SpreadElement.check(node) || n.SpreadProperty && n.SpreadProperty.check(node); } -var PRECEDENCE = {}; -[ - [ "||" ], - [ "&&" ], - [ "|" ], - [ "^" ], - [ "&" ], - [ "==", "===", "!=", "!==" ], - [ "<", ">", "<=", ">=", "in", "instanceof" ], - [ ">>", "<<", ">>>" ], - [ "+", "-" ], - [ "*", "/", "%", "**" ] -].forEach(function(tier, i) { - tier.forEach(function(op) { - PRECEDENCE[op] = i; - }); -}); - function containsCallExpression(node) { if (n.CallExpression.check(node)) { return true; diff --git a/src/printer.js b/src/printer.js index 7735f109..3e2fd50f 100644 --- a/src/printer.js +++ b/src/printer.js @@ -179,15 +179,18 @@ function genericPrintNoParens(path, options, print) { ]) ); case "BinaryExpression": - case "LogicalExpression": - return group( - concat([ - path.call(print, "left"), - " ", - n.operator, - indent(options.tabWidth, concat([ line, path.call(print, "right") ])) - ]) - ); + case "LogicalExpression": { + const parts = []; + printBinaryishExpressions(path, parts, print, options); + + return group(concat([ + // Don't include the initial expression in the indentation + // level. The first item is guaranteed to be the first + // left-most expression. + parts.length > 0 ? parts[0] : "", + indent(options.tabWidth, concat(parts.slice(1))) + ])); + } case "AssignmentPattern": return concat([ path.call(print, "left"), @@ -2254,6 +2257,55 @@ function maybeWrapJSXElementInParens(path, elem, options) { ); } +function isBinaryish(node) { + return node.type === "BinaryExpression" || node.type === "LogicalExpression"; +} + +// For binary expressions to be consistent, we need to group +// subsequent operators with the same precedence level under a single +// group. Otherwise they will be nested such that some of them break +// onto new lines but not all. Operators with the same precedence +// level should either all break or not. Because we group them by +// 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, parts, print) { + let node = path.getValue(); + + // We treat BinaryExpression and LogicalExpression nodes the same. + if(isBinaryish(node)) { + // Put all operators with the same precedence level in the same + // group. The reason we only need to do this with the `left` + // expression is because given an expression like `1 + 2 - 3`, it + // is always parsed like `((1 + 2) - 3)`, meaning the `left` side + // is where the rest of the expression will exist. Binary + // expressions on the right side mean they have a difference + // precedence level and should be treated as a separate group, so + // print them normally. + if(util.getPrecedence(node.left.operator) === util.getPrecedence(node.operator)) { + // Flatten them out by recursively calling this function. The + // printed values will all be appended to `parts`. + path.call(left => printBinaryishExpressions(left, parts, print), "left"); + } + else { + parts.push(path.call(print, "left")); + } + + parts.push( + " ", + node.operator, + line, + path.call(print, "right") + ); + } + else { + // Our stopping case. Simply print the node normally. + parts.push(path.call(print)); + } + + return parts; +} + function adjustClause(clause, options, forceSpace) { if (clause === "") { return ";"; diff --git a/src/util.js b/src/util.js index 91c6e99e..9f128958 100644 --- a/src/util.js +++ b/src/util.js @@ -222,3 +222,25 @@ function htmlEscapeInsideDoubleQuote(str) { // .replace(/>/g, '>'); } util.htmlEscapeInsideDoubleQuote = htmlEscapeInsideDoubleQuote; + +var PRECEDENCE = {}; +[ + [ "||" ], + [ "&&" ], + [ "|" ], + [ "^" ], + [ "&" ], + [ "==", "===", "!=", "!==" ], + [ "<", ">", "<=", ">=", "in", "instanceof" ], + [ ">>", "<<", ">>>" ], + [ "+", "-" ], + [ "*", "/", "%", "**" ] +].forEach(function(tier, i) { + tier.forEach(function(op) { + PRECEDENCE[op] = i; + }); +}); + +util.getPrecedence = function(op) { + return PRECEDENCE[op]; +} diff --git a/tests/intersection/__snapshots__/jsfmt.spec.js.snap b/tests/intersection/__snapshots__/jsfmt.spec.js.snap index efee6757..b745dc7a 100644 --- a/tests/intersection/__snapshots__/jsfmt.spec.js.snap +++ b/tests/intersection/__snapshots__/jsfmt.spec.js.snap @@ -87,12 +87,14 @@ type DuplexStreamOptions = }; function hasObjectMode_bad(options: DuplexStreamOptions): boolean { - return options.objectMode || options.readableObjectMode || + return options.objectMode || + options.readableObjectMode || options.writableObjectMode; // error, undefined ~> boolean } function hasObjectMode_ok(options: DuplexStreamOptions): boolean { - return !!(options.objectMode || options.readableObjectMode || + return !!(options.objectMode || + options.readableObjectMode || options.writableObjectMode); } " diff --git a/tests/prettier/__snapshots__/jsfmt.spec.js.snap b/tests/prettier/__snapshots__/jsfmt.spec.js.snap index a7d0701c..f3a305f9 100644 --- a/tests/prettier/__snapshots__/jsfmt.spec.js.snap +++ b/tests/prettier/__snapshots__/jsfmt.spec.js.snap @@ -1,3 +1,65 @@ +exports[`test binary-expressions.js 1`] = ` +"// It should always break the highest precedence operators first, and +// break them all at the same time. + +const x = longVariable + longVariable + longVariable; +const x = longVariable + longVariable + longVariable + longVariable - longVariable + longVariable; +const x = longVariable + longVariable * longVariable + longVariable - longVariable + longVariable; +const x = longVariable + longVariable * longVariable * longVariable / longVariable + longVariable; + +const x = longVariable && longVariable && longVariable && longVariable && longVariable && longVariable; +const x = longVariable && longVariable || longVariable && longVariable || longVariable && longVariable; + +const x = longVariable * longint && longVariable >> 0 && longVariable + longVariable; + +const x = longVariable > longint && longVariable === 0 + longVariable * longVariable; + +foo(obj.property * new Class() && obj instanceof Class && longVariable ? number + 5 : false); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// It should always break the highest precedence operators first, and +// break them all at the same time. + +const x = longVariable + longVariable + longVariable; +const x = longVariable + + longVariable + + longVariable + + longVariable - + longVariable + + longVariable; +const x = longVariable + + longVariable * longVariable + + longVariable - + longVariable + + longVariable; +const x = longVariable + + longVariable * longVariable * longVariable / longVariable + + longVariable; + +const x = longVariable && + longVariable && + longVariable && + longVariable && + longVariable && + longVariable; +const x = longVariable && longVariable || + longVariable && longVariable || + longVariable && longVariable; + +const x = longVariable * longint && + longVariable >> 0 && + longVariable + longVariable; + +const x = longVariable > longint && + longVariable === 0 + longVariable * longVariable; + +foo( + obj.property * new Class() && obj instanceof Class && longVariable + ? number + 5 + : false +); +" +`; + exports[`test directives.js 1`] = ` "\"use strict\"; diff --git a/tests/prettier/binary-expressions.js b/tests/prettier/binary-expressions.js new file mode 100644 index 00000000..8b0660f4 --- /dev/null +++ b/tests/prettier/binary-expressions.js @@ -0,0 +1,16 @@ +// It should always break the highest precedence operators first, and +// break them all at the same time. + +const x = longVariable + longVariable + longVariable; +const x = longVariable + longVariable + longVariable + longVariable - longVariable + longVariable; +const x = longVariable + longVariable * longVariable + longVariable - longVariable + longVariable; +const x = longVariable + longVariable * longVariable * longVariable / longVariable + longVariable; + +const x = longVariable && longVariable && longVariable && longVariable && longVariable && longVariable; +const x = longVariable && longVariable || longVariable && longVariable || longVariable && longVariable; + +const x = longVariable * longint && longVariable >> 0 && longVariable + longVariable; + +const x = longVariable > longint && longVariable === 0 + longVariable * longVariable; + +foo(obj.property * new Class() && obj instanceof Class && longVariable ? number + 5 : false);