Place parentheses in expressions when necessary

src/ast/expr.rs

@@ -4,18 +4,30 @@ use crate::span::{HasSpan, Span};
 
 use super::{Call, Field, FuncDef, Lit, Space, TableConstr, TableDestr, Var};
 
+// Warning: If you change these precedences and associativities, you need to
+// update the parser and pretty-printer as well.
+
+// Warning: Operators at the same precedence must also have the same
+// associativity.
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Assoc {
+    Left,
+    Right,
+}
+
 #[derive(Debug, Clone, Copy)]
 pub enum BinOp {
-    /// `+`
-    Add,
-    /// `-`
-    Sub,
     /// `*`
     Mul,
     /// `/`
     Div,
     /// `%`
     Mod,
+    /// `+`
+    Add,
+    /// `-`
+    Sub,
     /// `==`
     Eq,
     /// `!=`
@@ -34,6 +46,36 @@ pub enum BinOp {
     Or,
 }
 
+impl BinOp {
+    /// The higher the precedence, the more strongly the operator binds.
+    pub fn precedence(self) -> u8 {
+        match self {
+            Self::Mul | Self::Div | Self::Mod => 4,
+            Self::Add | Self::Sub => 3,
+            Self::Eq | Self::Neq | Self::Gt | Self::Ge | Self::Lt | Self::Le => 2,
+            Self::And => 1,
+            Self::Or => 0,
+        }
+    }
+
+    pub fn assoc(self) -> Assoc {
+        match self {
+            Self::Mul
+            | Self::Div
+            | Self::Mod
+            | Self::Add
+            | Self::Sub
+            | Self::Eq
+            | Self::Neq
+            | Self::Gt
+            | Self::Ge
+            | Self::Lt
+            | Self::Le => Assoc::Left,
+            Self::And | Self::Or => Assoc::Right,
+        }
+    }
+}
+
 #[derive(Clone)]
 pub enum Expr {
     Lit(Lit),

src/pretty/expr.rs

@@ -1,6 +1,26 @@
 use pretty::{DocAllocator, DocBuilder, Pretty};
 
-use crate::ast::Expr;
+use crate::ast::{Assoc, BinOp, Expr, Field, Var};
+
+impl<'a, D: DocAllocator<'a>> Pretty<'a, D> for BinOp {
+    fn pretty(self, allocator: &'a D) -> DocBuilder<'a, D> {
+        allocator.text(match self {
+            Self::Mul => "*",
+            Self::Div => "/",
+            Self::Mod => "%",
+            Self::Add => "+",
+            Self::Sub => "-",
+            Self::Eq => "==",
+            Self::Neq => "!=",
+            Self::Gt => ">",
+            Self::Ge => ">=",
+            Self::Lt => "<",
+            Self::Le => "<=",
+            Self::And => "and",
+            Self::Or => "or",
+        })
+    }
+}
 
 impl<'a, D: DocAllocator<'a>> Pretty<'a, D> for Expr {
     fn pretty(self, allocator: &'a D) -> DocBuilder<'a, D> {
@@ -19,22 +39,33 @@ impl<'a, D: DocAllocator<'a>> Pretty<'a, D> for Expr {
                 span: _,
             } => inner.pretty(allocator).parens(),
 
-            // TODO Check whether parentheses are necessary
             Self::Neg {
                 minus: _,
                 s0,
                 expr,
                 span: _,
-            } => allocator.text("-").append(expr.pretty(allocator)),
+            } => {
+                let parenthesize = matches!(*expr, Self::BinOp { .. });
+                let inner = expr.pretty(allocator);
+                allocator
+                    .text("-")
+                    .append(if parenthesize { inner.parens() } else { inner })
+            }
 
-            // TODO Check whether parentheses are necessary
             Self::Not {
                 not: _,
                 s0,
                 expr,
                 span: _,
-            } => allocator.text("not ").append(expr.pretty(allocator)),
+            } => {
+                let parenthesize = matches!(*expr, Self::BinOp { .. });
+                let inner = expr.pretty(allocator);
+                allocator
+                    .text("not ")
+                    .append(if parenthesize { inner.parens() } else { inner })
+            }
 
+            // TODO Add newlines and group properly
             Self::BinOp {
                 left,
                 s0,
@@ -42,7 +73,55 @@ impl<'a, D: DocAllocator<'a>> Pretty<'a, D> for Expr {
                 s1,
                 right,
                 span: _,
-            } => allocator.text("<binop>"),
+            } => {
+                // If we're left-associative, then the left subexpression can be
+                // at the same precedence and the right subexpression must be at
+                // a higher precedence.
+
+                // If we're right-associative, then the left subexpression must
+                // be at a higher precedence and the right subexpression can be
+                // at the same precedence.
+
+                // Minimum precedence that the left subexpression can be at
+                // without requiring parentheses.
+                let min_left_prec = match op.assoc() {
+                    Assoc::Left => op.precedence(),
+                    Assoc::Right => op.precedence() + 1,
+                };
+
+                // Minimum precedence that the right subexpression can be at
+                // without requiring parentheses.
+                let min_right_prec = match op.assoc() {
+                    Assoc::Left => op.precedence() + 1,
+                    Assoc::Right => op.precedence(),
+                };
+
+                let left_paren = match *left {
+                    // These end with an arbitrary expression on the right. If
+                    // we don't add parentheses, we'll be assimilated into that
+                    // expression.
+                    Self::Field(Field::Assign { .. } | Field::AssignIdent { .. }) => true,
+                    Self::Var(Var::Assign { .. } | Var::AssignIdent { .. }) => true,
+
+                    Self::BinOp { op, .. } if op.precedence() < min_left_prec => true,
+
+                    _ => false,
+                };
+
+                let right_paren =
+                    matches!(*right, Self::BinOp { op, .. } if op.precedence() < min_right_prec);
+
+                let left = left.pretty(allocator);
+                let left = if left_paren { left.parens() } else { left };
+
+                let right = right.pretty(allocator);
+                let right = if right_paren { right.parens() } else { right };
+
+                left.append(allocator.space())
+                    .append(op.pretty(allocator))
+                    .append(allocator.space())
+                    .append(right)
+            }
         }
     }
 }