fix unstable formatting of logical expressions (#7026)

prettier · Nov 21, 2019 · bf29897 · bf29897
1 parent 35cc901
commit bf29897
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 34 deletions.
diff --git a/changelog_unreleased/javascript/pr-7026.md 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))
+
+<!-- prettier-ignore -->
+```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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,4 @@
+const radioSelectedAttr =
+  (isAnyValueSelected &&
+    node.getAttribute(radioAttr.toLowerCase()) === radioValue) ||
+  ((!isAnyValueSelected && values[a].default === true) || a === 0);