From 6ba760bf90654d2b25a0c0d51e627203000048db Mon Sep 17 00:00:00 2001
From: Louis Pilfold <louis@lpil.uk>
Date: Tue, 28 Apr 2020 20:04:34 +0100
Subject: [PATCH] Formatter surrounds operators with { } where required

Closes https://github.com/gleam-lang/gleam/issues/432
---
 src/ast.rs                                    | 37 ++++++++++++-
 src/ast/untyped.rs                            |  8 +++
 src/format.rs                                 | 35 +++++++++---
 src/format/tests.rs                           | 54 +++++++++++++++++++
 .../test/binary_operators_test.gleam          | 12 ++---
 5 files changed, 132 insertions(+), 14 deletions(-)

diff --git a/src/ast.rs b/src/ast.rs
index c6de6c71250..53dfda6d780 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -231,18 +231,25 @@ pub struct ExternalFnArg {
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum BinOp {
+    // Boolean logic
     And,
     Or,
+
+    // Equality
+    Eq,
+    NotEq,
+
+    // Order comparison
     LtInt,
     LtEqInt,
     LtFloat,
     LtEqFloat,
-    Eq,
-    NotEq,
     GtEqInt,
     GtInt,
     GtEqFloat,
     GtFloat,
+
+    // Maths
     AddInt,
     AddFloat,
     SubInt,
@@ -254,6 +261,32 @@ pub enum BinOp {
     ModuloInt,
 }
 
+impl BinOp {
+    pub fn precedence(&self) -> u8 {
+        match self {
+            Self::Or => 1,
+
+            Self::And => 2,
+
+            Self::Eq | Self::NotEq => 3,
+
+            Self::LtInt
+            | Self::LtEqInt
+            | Self::LtFloat
+            | Self::LtEqFloat
+            | Self::GtEqInt
+            | Self::GtInt
+            | Self::GtEqFloat
+            | Self::GtFloat => 4,
+
+            // Pipe is 5
+            Self::AddInt | Self::AddFloat | Self::SubInt | Self::SubFloat => 6,
+
+            Self::MultInt | Self::MultFloat | Self::DivInt | Self::DivFloat | Self::ModuloInt => 7,
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Clone)]
 pub struct CallArg<A> {
     pub label: Option<String>,
diff --git a/src/ast/untyped.rs b/src/ast/untyped.rs
index 2ce71bbd34c..2b92d74f5aa 100644
--- a/src/ast/untyped.rs
+++ b/src/ast/untyped.rs
@@ -130,4 +130,12 @@ impl UntypedExpr {
             _ => self.location().start,
         }
     }
+
+    pub fn binop_precedence(&self) -> u8 {
+        match self {
+            Self::BinOp { name, .. } => name.precedence(),
+            Self::Pipe { .. } => 5,
+            _ => std::u8::MAX,
+        }
+    }
 }
diff --git a/src/format.rs b/src/format.rs
index 059e356dd3c..ba85d9cb6d6 100644
--- a/src/format.rs
+++ b/src/format.rs
@@ -466,10 +466,7 @@ impl<'a> Formatter<'a> {
 
             UntypedExpr::BinOp {
                 name, left, right, ..
-            } => self
-                .expr(left)
-                .append(name)
-                .append(self.expr(right.as_ref())),
+            } => self.bin_op(name, left, right),
 
             UntypedExpr::Let {
                 value,
@@ -515,7 +512,33 @@ impl<'a> Formatter<'a> {
         commented(document, comments)
     }
 
+    pub fn bin_op(&mut self, name: &BinOp, left: &UntypedExpr, right: &UntypedExpr) -> Document {
+        let precedence = name.precedence();
+        let left_precedence = left.binop_precedence();
+        let right_precedence = right.binop_precedence();
+        let left = self.expr(left);
+        let right = self.expr(right);
+        self.operator_side(left, precedence, left_precedence)
+            .append(name)
+            .append(self.operator_side(right, precedence, right_precedence))
+    }
+
+    pub fn operator_side(&mut self, doc: Document, op: u8, side: u8) -> Document {
+        if op > side {
+            delim("{")
+                .append(doc)
+                .nest(INDENT)
+                .append(break_("", " "))
+                .append("}")
+                .group()
+        } else {
+            doc
+        }
+    }
+
     fn pipe(&mut self, left: &UntypedExpr, right: &UntypedExpr) -> Document {
+        let left_precedence = left.binop_precedence();
+        let right_precedence = right.binop_precedence();
         let left = self.expr(left);
         let right = match right {
             UntypedExpr::Fn {
@@ -527,10 +550,10 @@ impl<'a> Formatter<'a> {
             _ => self.expr(right),
         };
         force_break()
-            .append(left)
+            .append(self.operator_side(left, 4, left_precedence))
             .append(line())
             .append("|> ")
-            .append(right)
+            .append(self.operator_side(right, 4, right_precedence))
     }
 
     fn pipe_capture_right_hand_side(&mut self, fun: &UntypedExpr) -> Document {
diff --git a/src/format/tests.rs b/src/format/tests.rs
index 0911aefdb68..74b206a9fbd 100644
--- a/src/format/tests.rs
+++ b/src/format/tests.rs
@@ -2380,6 +2380,60 @@ type X {
   X
 }
 // Hello
+"
+    );
+
+    //
+    // Binary operator precedence
+    //
+
+    assert_format!(
+        "fn main() {
+  { 1 + 2 } * 3
+}
+"
+    );
+
+    assert_format!(
+        "fn main() {
+  3 * { 1 + 2 }
+}
+"
+    );
+
+    assert_format!(
+        "fn main() {
+  3 * {
+    1
+    |> inc
+  }
+}
+"
+    );
+
+    assert_format!(
+        "fn main() {
+  {
+    1
+    |> inc
+  } * 3
+}
+"
+    );
+
+    assert_format!(
+        "fn main() {
+  1
+  |> { a || b }
+}
+"
+    );
+
+    assert_format!(
+        "fn main() {
+  { a || b }
+  |> go
+}
 "
     );
 }
diff --git a/test/core_language/test/binary_operators_test.gleam b/test/core_language/test/binary_operators_test.gleam
index 6767aa6ae9f..d91681fc855 100644
--- a/test/core_language/test/binary_operators_test.gleam
+++ b/test/core_language/test/binary_operators_test.gleam
@@ -1,8 +1,8 @@
 import should
 
-// pub fn precedence_test() {
-//   should.equal(1 + 2 * 3, 7)
-//   should.equal(3 * 1 + 2, 5)
-//   should.equal({ 1 + 2 } * 3, 9)
-//   should.equal(3 * { 1 + 2 }, 9)
-// }
+pub fn precedence_test() {
+  should.equal(1 + 2 * 3, 7)
+  should.equal(3 * 1 + 2, 5)
+  should.equal({ 1 + 2 } * 3, 9)
+  should.equal(3 * { 1 + 2 }, 9)
+}