Merge pull request #5481 from wontem/fix_pizza

Fix desugar step for pizza operator

crates/ast/src/lang/core/expr/expr_to_expr2.rs

@@ -665,14 +665,10 @@ pub fn expr_to_expr2<'a>(
             ident,
         } => canonicalize_lookup(env, scope, module_name, ident, region),
 
+        ParensAround(sub_expr) => expr_to_expr2(env, scope, sub_expr, region),
+
         // Below this point, we shouln't see any of these nodes anymore because
         // operator desugaring should have removed them!
-        bad_expr @ ParensAround(_) => {
-            panic!(
-                "A ParensAround did not get removed during operator desugaring somehow: {:#?}",
-                bad_expr
-            );
-        }
         bad_expr @ SpaceBefore(_, _) => {
             panic!(
                 "A SpaceBefore did not get removed during operator desugaring somehow: {:#?}",

crates/compiler/can/src/def.rs

@@ -2272,12 +2272,18 @@ fn canonicalize_pending_body<'a>(
 
     opt_loc_annotation: Option<Loc<crate::annotation::Annotation>>,
 ) -> DefOutput {
+    let mut loc_value = &loc_expr.value;
+
+    while let ast::Expr::ParensAround(value) = loc_value {
+        loc_value = value;
+    }
+
     // We treat closure definitions `foo = \a, b -> ...` differently from other body expressions,
     // because they need more bookkeeping (for tail calls, closure captures, etc.)
     //
     // Only defs of the form `foo = ...` can be closure declarations or self tail calls.
     let (loc_can_expr, def_references) = {
-        match (&loc_can_pattern.value, &loc_expr.value) {
+        match (&loc_can_pattern.value, &loc_value) {
             (
                 Pattern::Identifier(defined_symbol)
                 | Pattern::AbilityMemberSpecialization {

crates/compiler/can/src/expr.rs

@@ -1403,14 +1403,13 @@ pub fn canonicalize_expr<'a>(
 
             (answer, Output::default())
         }
+        &ast::Expr::ParensAround(sub_expr) => {
+            let (loc_expr, output) = canonicalize_expr(env, var_store, scope, region, sub_expr);
+
+            (loc_expr.value, output)
+        }
         // Below this point, we shouln't see any of these nodes anymore because
         // operator desugaring should have removed them!
-        bad_expr @ ast::Expr::ParensAround(_) => {
-            internal_error!(
-                "A ParensAround did not get removed during operator desugaring somehow: {:#?}",
-                bad_expr
-            );
-        }
         bad_expr @ ast::Expr::SpaceBefore(_, _) => {
             internal_error!(
                 "A SpaceBefore did not get removed during operator desugaring somehow: {:#?}",

crates/compiler/can/src/operator.rs

@@ -288,7 +288,7 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc<Expr<'a>>) -> &'a Loc
 
                             break builder_arg.closure;
                         }
-                        SpaceBefore(expr, _) | SpaceAfter(expr, _) | ParensAround(expr) => {
+                        SpaceBefore(expr, _) | SpaceAfter(expr, _) => {
                             current = *expr;
                         }
                         _ => break loc_arg,
@@ -382,7 +382,7 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc<Expr<'a>>) -> &'a Loc
                 region: loc_expr.region,
             })
         }
-        SpaceBefore(expr, _) | SpaceAfter(expr, _) | ParensAround(expr) => {
+        SpaceBefore(expr, _) | SpaceAfter(expr, _) => {
             // Since we've already begun canonicalization, spaces and parens
             // are no longer needed and should be dropped.
             desugar_expr(
@@ -393,6 +393,20 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc<Expr<'a>>) -> &'a Loc
                 }),
             )
         }
+        ParensAround(expr) => {
+            let desugared = desugar_expr(
+                arena,
+                arena.alloc(Loc {
+                    value: **expr,
+                    region: loc_expr.region,
+                }),
+            );
+
+            arena.alloc(Loc {
+                value: ParensAround(&desugared.value),
+                region: loc_expr.region,
+            })
+        }
         If(if_thens, final_else_branch) => {
             // If does not get desugared into `when` so we can give more targeted error messages during type checking.
             let desugared_final_else = &*arena.alloc(desugar_expr(arena, final_else_branch));

crates/compiler/test_gen/src/gen_definitions.rs

@@ -0,0 +1,47 @@
+#[cfg(feature = "gen-llvm")]
+use crate::helpers::llvm::assert_evals_to;
+
+#[cfg(feature = "gen-dev")]
+use crate::helpers::dev::assert_evals_to;
+
+#[cfg(feature = "gen-wasm")]
+use crate::helpers::wasm::assert_evals_to;
+
+// use crate::helpers::with_larger_debug_stack;
+//use crate::assert_wasm_evals_to as assert_evals_to;
+#[allow(unused_imports)]
+use indoc::indoc;
+#[allow(unused_imports)]
+use roc_std::{RocList, RocResult, RocStr};
+
+#[test]
+#[cfg(any(feature = "gen-llvm", feature = "gen-dev", feature = "gen-wasm"))]
+fn def_closure_in_parens() {
+    assert_evals_to!(
+        indoc!(
+            r#"
+            id = (\x -> x)
+
+            id 42u32
+            "#
+        ),
+        42,
+        u32
+    );
+}
+
+#[test]
+#[cfg(any(feature = "gen-llvm", feature = "gen-dev", feature = "gen-wasm"))]
+fn def_closure_in_multiple_parens() {
+    assert_evals_to!(
+        indoc!(
+            r#"
+            id = (((\x -> x)))
+
+            id 42u32
+            "#
+        ),
+        42,
+        u32
+    );
+}

crates/compiler/test_gen/src/tests.rs

@@ -6,6 +6,7 @@
 
 pub mod gen_abilities;
 pub mod gen_compare;
+pub mod gen_definitions;
 pub mod gen_dict;
 pub mod gen_list;
 pub mod gen_num;

crates/reporting/tests/test_reporting.rs

@@ -13875,4 +13875,44 @@ In roc, functions are always written as a lambda, like{}
     Tip: It looks like it takes too many arguments. I'm seeing 1 extra.
     "###
     );
+
+    test_report!(
+        pizza_parens_right,
+        indoc!(
+            r#"
+            2 |> (Num.sub 3)
+            "#
+        ),
+        @r###"
+    ── TOO FEW ARGS ────────────────────────────────────────── /code/proj/Main.roc ─
+
+    The `sub` function expects 2 arguments, but it got only 1:
+    
+    4│      2 |> (Num.sub 3)
+                  ^^^^^^^
+    
+    Roc does not allow functions to be partially applied. Use a closure to
+    make partial application explicit.
+    "###
+    );
+
+    test_report!(
+        pizza_parens_middle,
+        indoc!(
+            r#"
+            2 |> (Num.sub 3) |> Num.sub 3
+            "#
+        ),
+        @r###"
+    ── TOO FEW ARGS ────────────────────────────────────────── /code/proj/Main.roc ─
+
+    The `sub` function expects 2 arguments, but it got only 1:
+    
+    4│      2 |> (Num.sub 3) |> Num.sub 3
+                  ^^^^^^^
+    
+    Roc does not allow functions to be partially applied. Use a closure to
+    make partial application explicit.
+    "###
+    );
 }