From 4a320a93322c364dbd4958ed8d841b19d7334d90 Mon Sep 17 00:00:00 2001
From: Lucas Azzola <derflatulator@gmail.com>
Date: Sat, 8 Jul 2017 17:31:28 +1000
Subject: [PATCH] Add supervisory parens for bitwise operations (#2429)

* Add supervisory parens for bitwise operations, fixes #2424

* Flatten x | y | z
---
 src/fast-path.js                              |  6 +--
 src/util.js                                   | 36 +++++++++++++++-
 .../__snapshots__/jsfmt.spec.js.snap          | 41 +++++++++++++++++++
 tests/binary-expressions/bitwise-flags.js     |  5 +++
 tests/binary-expressions/math.js              | 13 ++++++
 .../__snapshots__/jsfmt.spec.js.snap          |  2 +-
 6 files changed, 97 insertions(+), 6 deletions(-)
 create mode 100644 tests/binary-expressions/bitwise-flags.js

diff --git a/src/fast-path.js b/src/fast-path.js
index d122bb87e..6491eca0b 100644
--- a/src/fast-path.js
+++ b/src/fast-path.js
@@ -314,11 +314,11 @@ FastPath.prototype.needsParens = function(options) {
           const no = node.operator;
           const np = util.getPrecedence(no);
 
-          if (po === "||" && no === "&&") {
+          if (pp > np) {
             return true;
           }
 
-          if (pp > np) {
+          if (po === "||" && no === "&&") {
             return true;
           }
 
@@ -333,7 +333,7 @@ FastPath.prototype.needsParens = function(options) {
 
           // Add parenthesis when working with binary operators
           // It's not stricly needed but helps with code understanding
-          if (["|", "^", "&", ">>", "<<", ">>>"].indexOf(po) !== -1) {
+          if (util.isBitwiseOperator(po)) {
             return true;
           }
 
diff --git a/src/util.js b/src/util.js
index c6d053c5a..520520193 100644
--- a/src/util.js
+++ b/src/util.js
@@ -315,8 +315,25 @@ function getPrecedence(op) {
   return PRECEDENCE[op];
 }
 
-const equalityOperators = { "==": true, "!=": true, "===": true, "!==": true };
-const multiplicativeOperators = { "*": true, "/": true, "%": true };
+const equalityOperators = {
+  "==": true,
+  "!=": true,
+  "===": true,
+  "!==": true
+};
+const multiplicativeOperators = {
+  "*": true,
+  "/": true,
+  "%": true
+};
+const bitwiseOperators = {
+  ">>": true,
+  ">>>": true,
+  "<<": true,
+  "|": true,
+  "^": true,
+  "&": true
+};
 
 function shouldFlatten(parentOp, nodeOp) {
   if (getPrecedence(nodeOp) !== getPrecedence(parentOp)) {
@@ -342,9 +359,23 @@ function shouldFlatten(parentOp, nodeOp) {
     return false;
   }
 
+  // x << y << z --> (x << y) << z
+  if (
+    bitwiseOperators[parentOp] &&
+    bitwiseOperators[nodeOp] &&
+    // Flatten x | y | z
+    (nodeOp !== "|" || parentOp !== "|")
+  ) {
+    return false;
+  }
+
   return true;
 }
 
+function isBitwiseOperator(operator) {
+  return !!bitwiseOperators[operator];
+}
+
 // 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) {
@@ -436,6 +467,7 @@ function getAlignmentSize(value, tabWidth, startIndex) {
 module.exports = {
   getPrecedence,
   shouldFlatten,
+  isBitwiseOperator,
   isExportDeclaration,
   getParentExportDeclaration,
   getPenultimate,
diff --git a/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap b/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
index f8ab6629e..616f296f9 100644
--- a/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
@@ -19,6 +19,21 @@ function f() {
 
 `;
 
+exports[`bitwise-flags.js 1`] = `
+const FLAG_A = 1 << 0;
+const FLAG_B = 1 << 1;
+const FLAG_C = 1 << 2;
+
+const all = FLAG_A | FLAG_B | FLAG_C;
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const FLAG_A = 1 << 0;
+const FLAG_B = 1 << 1;
+const FLAG_C = 1 << 2;
+
+const all = FLAG_A | FLAG_B | FLAG_C;
+
+`;
+
 exports[`comment.js 1`] = `
 a = (
   // Commment 1
@@ -184,6 +199,19 @@ x % y * z;
 x % y / z;
 
 x % y % z;
+
+x << y >> z;
+x >>> y << z;
+x >>> y >>> z;
+x + y >> z;
+
+x | y & z;
+x & y | z;
+x ^ y ^ z;
+x & y & z;
+x | y | z;
+x & y >> z;
+x << y | z;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 x + y / z;
 x / y + z;
@@ -195,6 +223,19 @@ x / y + z;
 
 (x % y) % z;
 
+(x << y) >> z;
+(x >>> y) << z;
+(x >>> y) >>> z;
+(x + y) >> z;
+
+x | (y & z);
+(x & y) | z;
+(x ^ y) ^ z;
+(x & y) & z;
+x | y | z;
+x & (y >> z);
+(x << y) | z;
+
 `;
 
 exports[`short-right.js 1`] = `
diff --git a/tests/binary-expressions/bitwise-flags.js b/tests/binary-expressions/bitwise-flags.js
new file mode 100644
index 000000000..f9999f918
--- /dev/null
+++ b/tests/binary-expressions/bitwise-flags.js
@@ -0,0 +1,5 @@
+const FLAG_A = 1 << 0;
+const FLAG_B = 1 << 1;
+const FLAG_C = 1 << 2;
+
+const all = FLAG_A | FLAG_B | FLAG_C;
diff --git a/tests/binary-expressions/math.js b/tests/binary-expressions/math.js
index 353862fad..920fa7393 100644
--- a/tests/binary-expressions/math.js
+++ b/tests/binary-expressions/math.js
@@ -7,3 +7,16 @@ x % y * z;
 x % y / z;
 
 x % y % z;
+
+x << y >> z;
+x >>> y << z;
+x >>> y >>> z;
+x + y >> z;
+
+x | y & z;
+x & y | z;
+x ^ y ^ z;
+x & y & z;
+x | y | z;
+x & y >> z;
+x << y | z;
diff --git a/tests/binary_math/__snapshots__/jsfmt.spec.js.snap b/tests/binary_math/__snapshots__/jsfmt.spec.js.snap
index 12922bd77..084baae79 100644
--- a/tests/binary_math/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/binary_math/__snapshots__/jsfmt.spec.js.snap
@@ -17,7 +17,7 @@ var from = offset > left ? 0 : (left - offset) >> level;
 var to = ((right - offset) >> level) + 1;
 if (rawIndex < 1 << (list._level + SHIFT)) {
 }
-var res = size < SIZE ? 0 : (size - 1) >>> SHIFT << SHIFT;
+var res = size < SIZE ? 0 : ((size - 1) >>> SHIFT) << SHIFT;
 sign = 1 - 2 * (b[3] >> 7);
 exponent = (((b[3] << 1) & 0xff) | (b[2] >> 7)) - 127;
 mantissa = ((b[2] & 0x7f) << 16) | (b[1] << 8) | b[0];