Fix logical operator precedence in parser.

Whoopsie... || and && were treated with the same precedence, causing very surprising behavior down the line.

  I noticed this because of the auto-formatter adding parenthesis where it really shouldn't. The problem came actually from the parser and how it constructed the AST.

crates/aiken-lang/src/parser.rs

@@ -1301,13 +1301,9 @@ pub fn expr_parser(
             })
             .boxed();
 
-        // Logical
+        // Conjunction
-        let op = choice((
+        let op = just(Token::AmperAmper).to(BinOp::And);
-            just(Token::AmperAmper).to(BinOp::And),
+        let conjunction = comparison
-            just(Token::VbarVbar).to(BinOp::Or),
-        ));
-
-        let logical = comparison
             .clone()
             .then(op.then(comparison).repeated())
             .foldl(|a, (op, b)| expr::UntypedExpr::BinOp {
@@ -1318,10 +1314,23 @@ pub fn expr_parser(
             })
             .boxed();
 
-        // Pipeline
+        // Disjunction
-        logical
+        let op = just(Token::VbarVbar).to(BinOp::Or);
+        let disjunction = conjunction
             .clone()
-            .then(just(Token::Pipe).ignore_then(logical).repeated())
+            .then(op.then(conjunction).repeated())
+            .foldl(|a, (op, b)| expr::UntypedExpr::BinOp {
+                location: a.location().union(b.location()),
+                name: op,
+                left: Box::new(a),
+                right: Box::new(b),
+            })
+            .boxed();
+
+        // Pipeline
+        disjunction
+            .clone()
+            .then(just(Token::Pipe).ignore_then(disjunction).repeated())
             .foldl(|l, r| {
                 let expressions = if let expr::UntypedExpr::PipeLine { mut expressions } = l {
                     expressions.push(r);
@@ -1421,30 +1430,33 @@ pub fn when_clause_guard_parser() -> impl Parser<Token, ast::ClauseGuard<(), ()>
             })
             .boxed();
 
-        let logical_op = choice((
+        let and_op = just(Token::AmperAmper);
-            just(Token::AmperAmper).to(BinOp::And),
+        let conjunction = comparison
-            just(Token::VbarVbar).to(BinOp::Or),
-        ));
-
-        comparison
             .clone()
-            .then(logical_op.then(comparison).repeated())
+            .then(and_op.then(comparison).repeated())
-            .foldl(|left, (op, right)| {
+            .foldl(|left, (_tok, right)| {
                 let location = left.location().union(right.location());
                 let left = Box::new(left);
                 let right = Box::new(right);
-                match op {
+                ast::ClauseGuard::And {
-                    BinOp::And => ast::ClauseGuard::And {
                     location,
                     left,
                     right,
-                    },
+                }
-                    BinOp::Or => ast::ClauseGuard::Or {
+            });
+
+        let or_op = just(Token::VbarVbar);
+        conjunction
+            .clone()
+            .then(or_op.then(conjunction).repeated())
+            .foldl(|left, (_tok, right)| {
+                let location = left.location().union(right.location());
+                let left = Box::new(left);
+                let right = Box::new(right);
+                ast::ClauseGuard::Or {
                     location,
                     left,
                     right,
-                    },
-                    _ => unreachable!(),
                 }
             })
     })

crates/aiken-lang/src/tests/format.rs

@@ -14,7 +14,7 @@ fn assert_fmt(src: &str, expected: &str) {
     let (module2, extra2) = parser::module(&out, ModuleKind::Lib).unwrap();
     let mut out2 = String::new();
     format::pretty(&mut out2, module2, extra2, &out);
-    assert!(out == out2, "formatting isn't idempotent");
+    assert_eq!(out, out2, "formatting isn't idempotent");
 }
 
 #[test]
@@ -233,7 +233,7 @@ fn test_negate() {
 }
 
 #[test]
-fn test_block_expr() {
+fn test_block_arithmetic_expr() {
     let src = indoc! {r#"
         fn foo() {
           ( 14 + 42 ) * 1337
@@ -257,6 +257,39 @@ fn test_block_expr() {
     assert_fmt(src, expected);
 }
 
+#[test]
+fn test_block_logical_expr() {
+    let src = indoc! {r#"
+        fn foo() {
+          !(a && b)
+        }
+
+        fn bar() {
+          (a || b) && (c || d)
+        }
+
+        fn baz() {
+          a || (b && c) || d
+        }
+    "#};
+
+    let expected = indoc! {r#"
+        fn foo() {
+          !(a && b)
+        }
+
+        fn bar() {
+          ( a || b ) && ( c || d )
+        }
+
+        fn baz() {
+          a || b && c || d
+        }
+    "#};
+
+    assert_fmt(src, expected);
+}
+
 #[test]
 fn test_format_bytearray_literals() {
     let src = indoc! {r#"

crates/aiken-lang/src/tests/parser.rs

@@ -2166,7 +2166,7 @@ fn clause_guards() {
               _ if bar -> Void
               _ if True -> Void
               _ if a || b && c -> Void
-              _ if a || (b && c) -> Void
+              _ if (a || b) && c -> Void
               _ if a <= 42 || b > 14 || "str" -> Void
               _ if a == 14 && !b -> Void
               _ if !!True -> Void
@@ -2241,27 +2241,27 @@ fn clause_guards() {
                             location: Span::new((), 92..93),
                         }],
                         alternative_patterns: vec![],
-                        guard: Some(ast::ClauseGuard::And {
+                        guard: Some(ast::ClauseGuard::Or {
                             location: Span::new((), 97..108),
-                            left: Box::new(ast::ClauseGuard::Or {
-                                location: Span::new((), 97..103),
                             left: Box::new(ast::ClauseGuard::Var {
                                 location: Span::new((), 97..98),
                                 name: "a".to_string(),
                                 tipo: (),
                             }),
-                                right: Box::new(ast::ClauseGuard::Var {
+                            right: Box::new(ast::ClauseGuard::And {
+                                location: Span::new((), 102..108),
+                                left: Box::new(ast::ClauseGuard::Var {
                                     location: Span::new((), 102..103),
                                     name: "b".to_string(),
                                     tipo: (),
                                 }),
-                            }),
                                 right: Box::new(ast::ClauseGuard::Var {
                                     location: Span::new((), 107..108),
                                     name: "c".to_string(),
                                     tipo: (),
                                 }),
                             }),
+                        }),
                         then: expr::UntypedExpr::Var {
                             location: Span::new((), 112..116),
                             name: "Void".to_string(),
@@ -2274,27 +2274,27 @@ fn clause_guards() {
                             location: Span::new((), 121..122),
                         }],
                         alternative_patterns: vec![],
-                        guard: Some(ast::ClauseGuard::Or {
+                        guard: Some(ast::ClauseGuard::And {
-                            location: Span::new((), 126..138),
+                            location: Span::new((), 127..139),
+                            left: Box::new(ast::ClauseGuard::Or {
+                                location: Span::new((), 127..133),
                                 left: Box::new(ast::ClauseGuard::Var {
-                                location: Span::new((), 126..127),
+                                    location: Span::new((), 127..128),
                                     name: "a".to_string(),
                                     tipo: (),
                                 }),
-                            right: Box::new(ast::ClauseGuard::And {
+                                right: Box::new(ast::ClauseGuard::Var {
-                                location: Span::new((), 132..138),
-                                left: Box::new(ast::ClauseGuard::Var {
                                     location: Span::new((), 132..133),
                                     name: "b".to_string(),
                                     tipo: (),
                                 }),
+                            }),
                             right: Box::new(ast::ClauseGuard::Var {
-                                    location: Span::new((), 137..138),
+                                location: Span::new((), 138..139),
                                 name: "c".to_string(),
                                 tipo: (),
                             }),
                         }),
-                        }),
                         then: expr::UntypedExpr::Var {
                             location: Span::new((), 143..147),
                             name: "Void".to_string(),
@@ -2420,3 +2420,98 @@ fn clause_guards() {
         })],
     );
 }
+
+#[test]
+fn scope_logical_expression() {
+    let code = indoc! {r#"
+          fn foo() {
+            let x = !(a && b)
+            let y = a || b && c || d
+            x
+          }
+        "#};
+    assert_definitions(
+        code,
+        vec![ast::Definition::Fn(Function {
+            arguments: vec![],
+            body: expr::UntypedExpr::Sequence {
+                location: Span::new((), 13..61),
+                expressions: vec![
+                    expr::UntypedExpr::Assignment {
+                        location: Span::new((), 13..30),
+                        value: Box::new(expr::UntypedExpr::UnOp {
+                            op: ast::UnOp::Not,
+                            location: Span::new((), 21..29),
+                            value: Box::new(expr::UntypedExpr::BinOp {
+                                location: Span::new((), 23..29),
+                                name: ast::BinOp::And,
+                                left: Box::new(expr::UntypedExpr::Var {
+                                    location: Span::new((), 23..24),
+                                    name: "a".to_string(),
+                                }),
+                                right: Box::new(expr::UntypedExpr::Var {
+                                    location: Span::new((), 28..29),
+                                    name: "b".to_string(),
+                                }),
+                            }),
+                        }),
+                        pattern: ast::Pattern::Var {
+                            location: Span::new((), 17..18),
+                            name: "x".to_string(),
+                        },
+                        kind: ast::AssignmentKind::Let,
+                        annotation: None,
+                    },
+                    expr::UntypedExpr::Assignment {
+                        location: Span::new((), 33..57),
+                        value: Box::new(expr::UntypedExpr::BinOp {
+                            location: Span::new((), 41..57),
+                            name: ast::BinOp::Or,
+                            left: Box::new(expr::UntypedExpr::BinOp {
+                                location: Span::new((), 41..52),
+                                name: ast::BinOp::Or,
+                                left: Box::new(expr::UntypedExpr::Var {
+                                    location: Span::new((), 41..42),
+                                    name: "a".to_string(),
+                                }),
+                                right: Box::new(expr::UntypedExpr::BinOp {
+                                    location: Span::new((), 46..52),
+                                    name: ast::BinOp::And,
+                                    left: Box::new(expr::UntypedExpr::Var {
+                                        location: Span::new((), 46..47),
+                                        name: "b".to_string(),
+                                    }),
+                                    right: Box::new(expr::UntypedExpr::Var {
+                                        location: Span::new((), 51..52),
+                                        name: "c".to_string(),
+                                    }),
+                                }),
+                            }),
+                            right: Box::new(expr::UntypedExpr::Var {
+                                location: Span::new((), 56..57),
+                                name: "d".to_string(),
+                            }),
+                        }),
+                        pattern: ast::Pattern::Var {
+                            location: Span::new((), 37..38),
+                            name: "y".to_string(),
+                        },
+                        kind: ast::AssignmentKind::Let,
+                        annotation: None,
+                    },
+                    expr::UntypedExpr::Var {
+                        location: Span::new((), 60..61),
+                        name: "x".to_string(),
+                    },
+                ],
+            },
+            doc: None,
+            location: Span::new((), 0..8),
+            name: "foo".to_string(),
+            public: false,
+            return_annotation: None,
+            return_type: (),
+            end_position: 62,
+        })],
+    )
+}