diff --git a/changelog_unreleased/javascript/pr-7026.md b/changelog_unreleased/javascript/pr-7026.md new file mode 100644 index 000000000..877722019 --- /dev/null +++ b/changelog_unreleased/javascript/pr-7026.md @@ -0,0 +1,26 @@ +#### Fix unstable formatting of logical expressions ([#7026](https://github.com/prettier/prettier/pull/7026) by [@thorn0](https://github.com/thorn0)) + + +```jsx +// Input +const averredBathersBoxroomBuggyNurl = + bifornCringerMoshedPerplexSawder === 1 || + (askTrovenaBeenaDependsRowans === 2 || glimseGlyphsHazardNoopsTieTie === 3); + +// Prettier stable (first output) +const averredBathersBoxroomBuggyNurl = + bifornCringerMoshedPerplexSawder === 1 || + askTrovenaBeenaDependsRowans === 2 || glimseGlyphsHazardNoopsTieTie === 3; + +// Prettier stable (second output) +const averredBathersBoxroomBuggyNurl = + bifornCringerMoshedPerplexSawder === 1 || + askTrovenaBeenaDependsRowans === 2 || + glimseGlyphsHazardNoopsTieTie === 3; + +// Prettier master (first output) +const averredBathersBoxroomBuggyNurl = + bifornCringerMoshedPerplexSawder === 1 || + askTrovenaBeenaDependsRowans === 2 || + glimseGlyphsHazardNoopsTieTie === 3; +``` diff --git a/src/language-js/clean.js b/src/language-js/clean.js index 77b4b9f1b..2801323f8 100644 --- a/src/language-js/clean.js +++ b/src/language-js/clean.js @@ -37,11 +37,6 @@ function clean(ast, newObj, parent) { return null; } - // We remove unneeded parens around same-operator LogicalExpressions - if (isUnbalancedLogicalTree(newObj)) { - return rebalanceLogicalTree(newObj); - } - // (TypeScript) Ignore `static` in `constructor(static p) {}` // and `export` in `constructor(export p) {}` if ( @@ -199,32 +194,4 @@ function clean(ast, newObj, parent) { } } -function isUnbalancedLogicalTree(newObj) { - return ( - newObj.type === "LogicalExpression" && - newObj.right.type === "LogicalExpression" && - newObj.operator === newObj.right.operator - ); -} - -function rebalanceLogicalTree(newObj) { - if (isUnbalancedLogicalTree(newObj)) { - return rebalanceLogicalTree({ - type: "LogicalExpression", - operator: newObj.operator, - left: rebalanceLogicalTree({ - type: "LogicalExpression", - operator: newObj.operator, - left: newObj.left, - right: newObj.right.left, - loc: {} - }), - right: newObj.right.right, - loc: {} - }); - } - - return newObj; -} - module.exports = clean; diff --git a/src/language-js/loc.js b/src/language-js/loc.js index e2d0e9586..214de144c 100644 --- a/src/language-js/loc.js +++ b/src/language-js/loc.js @@ -59,7 +59,24 @@ function locEnd(node) { return loc; } +function composeLoc(startNode, endNode = startNode) { + const loc = {}; + if (typeof startNode.start === "number") { + loc.start = startNode.start; + loc.end = endNode.end; + } + if (Array.isArray(startNode.range)) { + loc.range = [startNode.range[0], endNode.range[1]]; + } + loc.loc = { + start: startNode.loc.start, + end: endNode.loc.end + }; + return loc; +} + module.exports = { locStart, - locEnd + locEnd, + composeLoc }; diff --git a/src/language-js/postprocess.js b/src/language-js/postprocess.js index 888140e25..bf4a9d178 100644 --- a/src/language-js/postprocess.js +++ b/src/language-js/postprocess.js @@ -1,10 +1,18 @@ "use strict"; const { getLast } = require("../common/util"); +const { composeLoc } = require("./loc"); function postprocess(ast, options) { visitNode(ast, node => { switch (node.type) { + case "LogicalExpression": { + // We remove unneeded parens around same-operator LogicalExpressions + if (isUnbalancedLogicalTree(node)) { + return rebalanceLogicalTree(node); + } + break; + } // fix unexpected locEnd caused by --no-semi style case "VariableDeclaration": { const lastDeclaration = getLast(node.declarations); @@ -101,4 +109,40 @@ function visitNode(node, fn, parent, property) { } } +function isUnbalancedLogicalTree(node) { + return ( + node.type === "LogicalExpression" && + node.right.type === "LogicalExpression" && + node.operator === node.right.operator + ); +} + +function rebalanceLogicalTree(node) { + if (!isUnbalancedLogicalTree(node)) { + return node; + } + + return rebalanceLogicalTree( + Object.assign( + { + type: "LogicalExpression", + operator: node.operator, + left: rebalanceLogicalTree( + Object.assign( + { + type: "LogicalExpression", + operator: node.operator, + left: node.left, + right: node.right.left + }, + composeLoc(node.left, node.right.left) + ) + ), + right: node.right.right + }, + composeLoc(node) + ) + ); +} + module.exports = postprocess; diff --git a/tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap b/tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap index 4feb44940..714f2f726 100644 --- a/tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap +++ b/tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,26 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`issue-7024.js 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const radioSelectedAttr = + (isAnyValueSelected && + node.getAttribute(radioAttr.toLowerCase()) === radioValue) || + ((!isAnyValueSelected && values[a].default === true) || a === 0); + +=====================================output===================================== +const radioSelectedAttr = + (isAnyValueSelected && + node.getAttribute(radioAttr.toLowerCase()) === radioValue) || + (!isAnyValueSelected && values[a].default === true) || + a === 0; + +================================================================================ +`; + exports[`logical_expression_operators.js 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/logical_expressions/issue-7024.js b/tests/logical_expressions/issue-7024.js new file mode 100644 index 000000000..fa8e8ff1e --- /dev/null +++ b/tests/logical_expressions/issue-7024.js @@ -0,0 +1,4 @@ +const radioSelectedAttr = + (isAnyValueSelected && + node.getAttribute(radioAttr.toLowerCase()) === radioValue) || + ((!isAnyValueSelected && values[a].default === true) || a === 0);