Update from PR comments

crates/cli/tests/cli/combine-tasks.roc

@@ -0,0 +1,22 @@
+app [main] { pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.10.0/vNe6s9hWzoTZtFmNkvEICPErI9ptji_ySjicO6CkucY.tar.br" }
+
+import pf.Stdout
+import pf.Task exposing [Task]
+
+main =
+    multipleIn =
+        { sequential <-
+            a: Task.ok 123,
+            b: Task.ok "abc",
+            c: Task.ok [123],
+            d: Task.ok ["abc"],
+            e: Task.ok (Dict.single "a" "b"),
+        }!
+
+    Stdout.line! "For multiple tasks: $(Inspect.toStr multipleIn)"
+
+sequential : Task a err, Task b err, (a, b -> c) -> Task c err
+sequential = \firstTask, secondTask, mapper ->
+    first = firstTask!
+    second = secondTask!
+    Task.ok (mapper first second)

crates/cli/tests/cli/parse-args.roc

@@ -0,0 +1,87 @@
+app [main] {
+    pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.10.0/vNe6s9hWzoTZtFmNkvEICPErI9ptji_ySjicO6CkucY.tar.br",
+}
+
+import pf.Arg
+import pf.Stdout
+import pf.Task exposing [Task]
+
+main =
+    file = strParam { name: "file" }
+    argParser =
+        { cliBuild <-
+            file,
+            count: numParam { name: "count" },
+            doubled: numParam { name: "doubled" }
+                |> cliMap \d -> d * 2,
+        }
+
+    args = ["parse-args", "file.txt", "5", "7"]
+    when argParser |> parseArgs args is
+        Ok data -> Stdout.line "Success: $(Inspect.toStr data)"
+        Err (FailedToParse message) -> Stdout.line "Failed: $(message)"
+
+ArgParseErr : [NoMoreArgs, InvalidParam ParamConfig]
+
+ParamConfig : {
+    name : Str,
+    type : [Num, Str],
+}
+
+ArgParser out : {
+    params : List ParamConfig,
+    parser : List Str -> Result (out, List Str) ArgParseErr,
+}
+
+strParam : { name : Str } -> ArgParser Str
+strParam = \{ name } ->
+    parser = \args ->
+        when args is
+            [] -> Err NoMoreArgs
+            [first, .. as rest] -> Ok (first, rest)
+
+    { params: [{ name, type: Str }], parser }
+
+numParam : { name : Str } -> ArgParser U64
+numParam = \{ name } ->
+    param = { name, type: Num }
+    parser = \args ->
+        when args is
+            [] -> Err NoMoreArgs
+            [first, .. as rest] ->
+                when Str.toU64 first is
+                    Ok num -> Ok (num, rest)
+                    Err InvalidNumStr -> Err (InvalidParam param)
+
+    { params: [param], parser }
+
+cliMap : ArgParser a, (a -> b) -> ArgParser b
+cliMap = \{ params, parser }, mapper -> {
+    params,
+    parser: \args ->
+        (data, afterData) <- parser args
+            |> Result.try
+
+        Ok (mapper data, afterData),
+}
+
+cliBuild : ArgParser a, ArgParser b, (a, b -> c) -> ArgParser c
+cliBuild = \firstWeaver, secondWeaver, combine ->
+    allParams = List.concat firstWeaver.params secondWeaver.params
+    combinedParser = \args ->
+        (firstValue, afterFirst) <- firstWeaver.parser args
+            |> Result.try
+        (secondValue, afterSecond) <- secondWeaver.parser afterFirst
+            |> Result.try
+
+        Ok (combine firstValue secondValue, afterSecond)
+
+    { params: allParams, parser: combinedParser }
+
+parseArgs : ArgParser a, List Str -> Result a [FailedToParse Str]
+parseArgs = \{ params: _, parser }, args ->
+    when parser (List.dropFirst args 1) is
+        Ok (data, []) -> Ok data
+        Ok (_data, extraArgs) -> Err (FailedToParse "Got $(List.len extraArgs |> Inspect.toStr) extra args")
+        Err NoMoreArgs -> Err (FailedToParse "I needed more args")
+        Err (InvalidParam param) -> Err (FailedToParse "Parameter '$(param.name)' needed a $(Inspect.toStr param.type)")

crates/cli/tests/cli_run.rs

@@ -988,6 +988,38 @@ mod cli_run {
         )
     }
 
+    #[test]
+    #[serial(cli_platform)]
+    #[cfg_attr(windows, ignore)]
+    fn combine_tasks_with_record_builder() {
+        test_roc_app(
+            "crates/cli/tests/cli",
+            "combine-tasks.roc",
+            &[],
+            &[],
+            &[],
+            "For multiple tasks: {a: 123, b: \"abc\", c: [123], d: [\"abc\"], e: {\"a\": \"b\"}}\n",
+            UseValgrind::No,
+            TestCliCommands::Run,
+        )
+    }
+
+    #[test]
+    #[serial(cli_platform)]
+    #[cfg_attr(windows, ignore)]
+    fn parse_args_with_record_builder() {
+        test_roc_app(
+            "crates/cli/tests/cli",
+            "parse-args.roc",
+            &[],
+            &[],
+            &[],
+            "Success: {count: 5, doubled: 14, file: \"file.txt\"}\n",
+            UseValgrind::No,
+            TestCliCommands::Run,
+        )
+    }
+
     #[test]
     #[serial(cli_platform)]
     #[cfg_attr(windows, ignore)]

crates/compiler/can/src/desugar.rs

@@ -302,9 +302,9 @@ pub fn desugar_expr<'a>(
         | PrecedenceConflict { .. }
         | MultipleOldRecordBuilders(_)
         | UnappliedOldRecordBuilder(_)
-        | EmptyNewRecordBuilder(_)
+        | EmptyRecordBuilder(_)
-        | SingleFieldNewRecordBuilder(_)
+        | SingleFieldRecordBuilder(_)
-        | OptionalFieldInNewRecordBuilder { .. }
+        | OptionalFieldInRecordBuilder { .. }
         | Tag(_)
         | OpaqueRef(_)
         | Crash => loc_expr,
@@ -492,25 +492,25 @@ pub fn desugar_expr<'a>(
             value: UnappliedOldRecordBuilder(loc_expr),
             region: loc_expr.region,
         }),
-        NewRecordBuilder { mapper, fields } => {
+        RecordBuilder { mapper, fields } => {
             // NOTE the `mapper` is always a `Var { .. }`, we only desugar it to get rid of
             // any spaces before/after
             let new_mapper = desugar_expr(arena, mapper, src, line_info, module_path);
 
             if fields.is_empty() {
                 return arena.alloc(Loc {
-                    value: EmptyNewRecordBuilder(loc_expr),
+                    value: EmptyRecordBuilder(loc_expr),
                     region: loc_expr.region,
                 });
             } else if fields.len() == 1 {
                 return arena.alloc(Loc {
-                    value: SingleFieldNewRecordBuilder(loc_expr),
+                    value: SingleFieldRecordBuilder(loc_expr),
                     region: loc_expr.region,
                 });
             }
 
-            let mut field_names = vec![];
+            let mut field_names = Vec::with_capacity_in(fields.len(), arena);
-            let mut field_vals = vec![];
+            let mut field_vals = Vec::with_capacity_in(fields.len(), arena);
 
             for field in fields.items {
                 match desugar_field(arena, &field.value, src, line_info, module_path) {
@@ -531,7 +531,7 @@ pub fn desugar_expr<'a>(
                     AssignedField::OptionalValue(loc_name, _, loc_val) => {
                         return arena.alloc(Loc {
                             region: loc_expr.region,
-                            value: OptionalFieldInNewRecordBuilder(arena.alloc(loc_name), loc_val),
+                            value: OptionalFieldInRecordBuilder(arena.alloc(loc_name), loc_val),
                         });
                     }
                     AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => {
@@ -548,35 +548,54 @@ pub fn desugar_expr<'a>(
                 },
             };
 
-            let combiner_closure = arena.alloc(Loc::at_zero(Closure(
+            let combiner_closure_in_region = |region| {
-                arena.alloc_slice_copy(&[
+                let closure_body = Tuple(Collection::with_items(
-                    Loc::at_zero(Pattern::Identifier {
-                        ident: "#record_builder_closure_arg_a",
-                    }),
-                    Loc::at_zero(Pattern::Identifier {
-                        ident: "#record_builder_closure_arg_b",
-                    }),
-                ]),
-                arena.alloc(Loc::at_zero(Tuple(Collection::with_items(
                     Vec::from_iter_in(
                         [
-                            &*arena.alloc(Loc::at_zero(Expr::Var {
+                            &*arena.alloc(Loc::at(
-                                module_name: "",
+                                region,
-                                ident: "#record_builder_closure_arg_a",
+                                Expr::Var {
-                            })),
+                                    module_name: "",
-                            &*arena.alloc(Loc::at_zero(Expr::Var {
+                                    ident: "#record_builder_closure_arg_a",
-                                module_name: "",
+                                },
-                                ident: "#record_builder_closure_arg_b",
+                            )),
-                            })),
+                            &*arena.alloc(Loc::at(
+                                region,
+                                Expr::Var {
+                                    module_name: "",
+                                    ident: "#record_builder_closure_arg_b",
+                                },
+                            )),
                         ],
                         arena,
                     )
                     .into_bump_slice(),
-                )))),
+                ));
-            )));
+
+                arena.alloc(Loc::at(
+                    region,
+                    Closure(
+                        arena.alloc_slice_copy(&[
+                            Loc::at(
+                                region,
+                                Pattern::Identifier {
+                                    ident: "#record_builder_closure_arg_a",
+                                },
+                            ),
+                            Loc::at(
+                                region,
+                                Pattern::Identifier {
+                                    ident: "#record_builder_closure_arg_b",
+                                },
+                            ),
+                        ]),
+                        arena.alloc(Loc::at(region, closure_body)),
+                    ),
+                ))
+            };
 
             let closure_args = {
-                if field_names.len() <= 2 {
+                if field_names.len() == 2 {
                     arena.alloc_slice_copy(&[
                         closure_arg_from_field(field_names[0]),
                         closure_arg_from_field(field_names[1]),
@@ -589,17 +608,22 @@ pub fn desugar_expr<'a>(
                     let mut second_arg = Pattern::Tuple(Collection::with_items(
                         arena.alloc_slice_copy(&[second_to_last_arg, last_arg]),
                     ));
+                    let mut second_arg_region =
+                        Region::span_across(&second_to_last_arg.region, &last_arg.region);
+
                     for index in (1..(field_names.len() - 2)).rev() {
                         second_arg =
                             Pattern::Tuple(Collection::with_items(arena.alloc_slice_copy(&[
                                 closure_arg_from_field(field_names[index]),
-                                Loc::at_zero(second_arg),
+                                Loc::at(second_arg_region, second_arg),
                             ])));
+                        second_arg_region =
+                            Region::span_across(&field_names[index].region, &second_arg_region);
                     }
 
                     arena.alloc_slice_copy(&[
                         closure_arg_from_field(field_names[0]),
-                        Loc::at_zero(second_arg),
+                        Loc::at(second_arg_region, second_arg),
                     ])
                 }
             };
@@ -612,10 +636,13 @@ pub fn desugar_expr<'a>(
                             AssignedField::RequiredValue(
                                 Loc::at(field_name.region, field_name.value),
                                 &[],
-                                arena.alloc(Loc::at_zero(Expr::Var {
+                                arena.alloc(Loc::at(
-                                    module_name: "",
+                                    field_name.region,
-                                    ident: arena.alloc_str(&format!("#{}", field_name.value)),
+                                    Expr::Var {
-                                })),
+                                        module_name: "",
+                                        ident: arena.alloc_str(&format!("#{}", field_name.value)),
+                                    },
+                                )),
                             ),
                         )
                     }),
@@ -626,7 +653,10 @@ pub fn desugar_expr<'a>(
 
             let record_combiner_closure = arena.alloc(Loc {
                 region: loc_expr.region,
-                value: Closure(closure_args, arena.alloc(Loc::at_zero(record_val))),
+                value: Closure(
+                    closure_args,
+                    arena.alloc(Loc::at(loc_expr.region, record_val)),
+                ),
             });
 
             if field_names.len() == 2 {
@@ -639,30 +669,40 @@ pub fn desugar_expr<'a>(
                             field_vals[1],
                             record_combiner_closure,
                         ]),
-                        CalledVia::NewRecordBuilder,
+                        CalledVia::RecordBuilder,
                     ),
                 });
             }
 
             let mut inner_combined = arena.alloc(Loc {
-                region: loc_expr.region,
+                region: Region::span_across(
+                    &field_vals[field_names.len() - 2].region,
+                    &field_vals[field_names.len() - 1].region,
+                ),
                 value: Apply(
                     new_mapper,
                     arena.alloc_slice_copy(&[
                         field_vals[field_names.len() - 2],
                         field_vals[field_names.len() - 1],
-                        combiner_closure,
+                        combiner_closure_in_region(loc_expr.region),
                     ]),
-                    CalledVia::NewRecordBuilder,
+                    CalledVia::RecordBuilder,
                 ),
             });
 
             for index in (1..(field_names.len() - 2)).rev() {
-                inner_combined = arena.alloc(Loc::at_zero(Apply(
+                inner_combined = arena.alloc(Loc {
-                    new_mapper,
+                    region: Region::span_across(&field_vals[index].region, &inner_combined.region),
-                    arena.alloc_slice_copy(&[field_vals[index], inner_combined, combiner_closure]),
+                    value: Apply(
-                    CalledVia::NewRecordBuilder,
+                        new_mapper,
-                )));
+                        arena.alloc_slice_copy(&[
+                            field_vals[index],
+                            inner_combined,
+                            combiner_closure_in_region(loc_expr.region),
+                        ]),
+                        CalledVia::RecordBuilder,
+                    ),
+                });
             }
 
             arena.alloc(Loc {
@@ -674,7 +714,7 @@ pub fn desugar_expr<'a>(
                         inner_combined,
                         record_combiner_closure,
                     ]),
-                    CalledVia::NewRecordBuilder,
+                    CalledVia::RecordBuilder,
                 ),
             })
         }
@@ -710,7 +750,7 @@ pub fn desugar_expr<'a>(
                                 });
                             }
 
-                            let builder_arg = record_builder_arg(arena, loc_arg.region, fields);
+                            let builder_arg = old_record_builder_arg(arena, loc_arg.region, fields);
                             builder_apply_exprs = Some(builder_arg.apply_exprs);
 
                             break builder_arg.closure;
@@ -1196,16 +1236,16 @@ fn desugar_pattern<'a>(
     }
 }
 
-struct RecordBuilderArg<'a> {
+struct OldRecordBuilderArg<'a> {
     closure: &'a Loc<Expr<'a>>,
     apply_exprs: Vec<'a, &'a Loc<Expr<'a>>>,
 }
 
-fn record_builder_arg<'a>(
+fn old_record_builder_arg<'a>(
     arena: &'a Bump,
     region: Region,
     fields: Collection<'a, Loc<OldRecordBuilderField<'a>>>,
-) -> RecordBuilderArg<'a> {
+) -> OldRecordBuilderArg<'a> {
     let mut record_fields = Vec::with_capacity_in(fields.len(), arena);
     let mut apply_exprs = Vec::with_capacity_in(fields.len(), arena);
     let mut apply_field_names = Vec::with_capacity_in(fields.len(), arena);
@@ -1281,7 +1321,7 @@ fn record_builder_arg<'a>(
         });
     }
 
-    RecordBuilderArg {
+    OldRecordBuilderArg {
         closure: body,
         apply_exprs,
     }

crates/compiler/can/src/expr.rs

@@ -1024,7 +1024,7 @@ pub fn canonicalize_expr<'a>(
         ast::Expr::OldRecordBuilder(_) => {
             internal_error!("Old record builder should have been desugared by now")
         }
-        ast::Expr::NewRecordBuilder { .. } => {
+        ast::Expr::RecordBuilder { .. } => {
             internal_error!("New record builder should have been desugared by now")
         }
         ast::Expr::Backpassing(_, _, _) => {
@@ -1359,27 +1359,27 @@ pub fn canonicalize_expr<'a>(
 
             (RuntimeError(problem), Output::default())
         }
-        ast::Expr::EmptyNewRecordBuilder(sub_expr) => {
+        ast::Expr::EmptyRecordBuilder(sub_expr) => {
             use roc_problem::can::RuntimeError::*;
 
-            let problem = EmptyNewRecordBuilder(sub_expr.region);
+            let problem = EmptyRecordBuilder(sub_expr.region);
             env.problem(Problem::RuntimeError(problem.clone()));
 
             (RuntimeError(problem), Output::default())
         }
-        ast::Expr::SingleFieldNewRecordBuilder(sub_expr) => {
+        ast::Expr::SingleFieldRecordBuilder(sub_expr) => {
             use roc_problem::can::RuntimeError::*;
 
-            let problem = SingleFieldNewRecordBuilder(sub_expr.region);
+            let problem = SingleFieldRecordBuilder(sub_expr.region);
             env.problem(Problem::RuntimeError(problem.clone()));
 
             (RuntimeError(problem), Output::default())
         }
-        ast::Expr::OptionalFieldInNewRecordBuilder(loc_name, loc_value) => {
+        ast::Expr::OptionalFieldInRecordBuilder(loc_name, loc_value) => {
             use roc_problem::can::RuntimeError::*;
 
             let sub_region = Region::span_across(&loc_name.region, &loc_value.region);
-            let problem = OptionalFieldInNewRecordBuilder {record: region, field: sub_region };
+            let problem = OptionalFieldInRecordBuilder {record: region, field: sub_region };
             env.problem(Problem::RuntimeError(problem.clone()));
 
             (RuntimeError(problem), Output::default())
@@ -2445,9 +2445,9 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
         ast::Expr::MalformedSuffixed(loc_expr)
         | ast::Expr::MultipleOldRecordBuilders(loc_expr)
         | ast::Expr::UnappliedOldRecordBuilder(loc_expr)
-        | ast::Expr::EmptyNewRecordBuilder(loc_expr)
+        | ast::Expr::EmptyRecordBuilder(loc_expr)
-        | ast::Expr::SingleFieldNewRecordBuilder(loc_expr)
+        | ast::Expr::SingleFieldRecordBuilder(loc_expr)
-        | ast::Expr::OptionalFieldInNewRecordBuilder(_, loc_expr)
+        | ast::Expr::OptionalFieldInRecordBuilder(_, loc_expr)
         | ast::Expr::PrecedenceConflict(PrecedenceConflict { expr: loc_expr, .. })
         | ast::Expr::UnaryOp(loc_expr, _)
         | ast::Expr::Closure(_, loc_expr) => is_valid_interpolation(&loc_expr.value),
@@ -2510,7 +2510,7 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool {
                 | ast::OldRecordBuilderField::SpaceAfter(_, _) => false,
             })
         }
-        ast::Expr::NewRecordBuilder { mapper, fields } => {
+        ast::Expr::RecordBuilder { mapper, fields } => {
             is_valid_interpolation(&mapper.value)
                 && fields.iter().all(|loc_field| match loc_field.value {
                     ast::AssignedField::RequiredValue(_label, loc_comments, loc_val)

crates/compiler/can/tests/test_can.rs

@@ -893,7 +893,7 @@ mod test_can {
         let first_map2_args = assert_func_call(
             &out.loc_expr.value,
             "map2",
-            CalledVia::NewRecordBuilder,
+            CalledVia::RecordBuilder,
             &out.interns,
         );
         let (first_arg, second_arg, third_arg) = match &first_map2_args[..] {
@@ -903,12 +903,8 @@ mod test_can {
 
         assert_num_value(first_arg, 1);
 
-        let inner_map2_args = assert_func_call(
+        let inner_map2_args =
-            second_arg,
+            assert_func_call(second_arg, "map2", CalledVia::RecordBuilder, &out.interns);
-            "map2",
-            CalledVia::NewRecordBuilder,
-            &out.interns,
-        );
         let (first_inner_arg, second_inner_arg, third_inner_arg) = match &inner_map2_args[..] {
             [first, second, third] => (&first.1.value, &second.1.value, &third.1.value),
             _ => panic!("inner map2 didn't receive three arguments"),
@@ -1025,7 +1021,7 @@ mod test_can {
 
                 args.clone()
             }
-            _ => panic!("Expr was not a NewRecordBuilder Call: {:?}", expr),
+            _ => panic!("Expr was not a RecordBuilder Call: {:?}", expr),
         }
     }
 

crates/compiler/fmt/src/expr.rs

@@ -87,9 +87,9 @@ impl<'a> Formattable for Expr<'a> {
             })
             | MultipleOldRecordBuilders(loc_subexpr)
             | UnappliedOldRecordBuilder(loc_subexpr)
-            | EmptyNewRecordBuilder(loc_subexpr)
+            | EmptyRecordBuilder(loc_subexpr)
-            | SingleFieldNewRecordBuilder(loc_subexpr)
+            | SingleFieldRecordBuilder(loc_subexpr)
-            | OptionalFieldInNewRecordBuilder(_, loc_subexpr) => loc_subexpr.is_multiline(),
+            | OptionalFieldInRecordBuilder(_, loc_subexpr) => loc_subexpr.is_multiline(),
 
             ParensAround(subexpr) => subexpr.is_multiline(),
 
@@ -113,7 +113,7 @@ impl<'a> Formattable for Expr<'a> {
             Tuple(fields) => is_collection_multiline(fields),
             RecordUpdate { fields, .. } => is_collection_multiline(fields),
             OldRecordBuilder(fields) => is_collection_multiline(fields),
-            NewRecordBuilder { fields, .. } => is_collection_multiline(fields),
+            RecordBuilder { fields, .. } => is_collection_multiline(fields),
         }
     }
 
@@ -376,7 +376,7 @@ impl<'a> Formattable for Expr<'a> {
                     assigned_field_to_space_before,
                 );
             }
-            NewRecordBuilder { mapper, fields } => {
+            RecordBuilder { mapper, fields } => {
                 fmt_record_like(
                     buf,
                     Some(RecordPrefix::Mapper(mapper)),
@@ -536,9 +536,9 @@ impl<'a> Formattable for Expr<'a> {
             PrecedenceConflict { .. } => {}
             MultipleOldRecordBuilders { .. } => {}
             UnappliedOldRecordBuilder { .. } => {}
-            EmptyNewRecordBuilder { .. } => {}
+            EmptyRecordBuilder { .. } => {}
-            SingleFieldNewRecordBuilder { .. } => {}
+            SingleFieldRecordBuilder { .. } => {}
-            OptionalFieldInNewRecordBuilder(_, _) => {}
+            OptionalFieldInRecordBuilder(_, _) => {}
         }
     }
 }

crates/compiler/fmt/src/spaces.rs

@@ -791,7 +791,7 @@ impl<'a> RemoveSpaces<'a> for Expr<'a> {
             },
             Expr::Record(a) => Expr::Record(a.remove_spaces(arena)),
             Expr::OldRecordBuilder(a) => Expr::OldRecordBuilder(a.remove_spaces(arena)),
-            Expr::NewRecordBuilder { mapper, fields } => Expr::NewRecordBuilder {
+            Expr::RecordBuilder { mapper, fields } => Expr::RecordBuilder {
                 mapper: arena.alloc(mapper.remove_spaces(arena)),
                 fields: fields.remove_spaces(arena),
             },
@@ -863,10 +863,10 @@ impl<'a> RemoveSpaces<'a> for Expr<'a> {
             Expr::PrecedenceConflict(a) => Expr::PrecedenceConflict(a),
             Expr::MultipleOldRecordBuilders(a) => Expr::MultipleOldRecordBuilders(a),
             Expr::UnappliedOldRecordBuilder(a) => Expr::UnappliedOldRecordBuilder(a),
-            Expr::EmptyNewRecordBuilder(a) => Expr::EmptyNewRecordBuilder(a),
+            Expr::EmptyRecordBuilder(a) => Expr::EmptyRecordBuilder(a),
-            Expr::SingleFieldNewRecordBuilder(a) => Expr::SingleFieldNewRecordBuilder(a),
+            Expr::SingleFieldRecordBuilder(a) => Expr::SingleFieldRecordBuilder(a),
-            Expr::OptionalFieldInNewRecordBuilder(name, a) => {
+            Expr::OptionalFieldInRecordBuilder(name, a) => {
-                Expr::OptionalFieldInNewRecordBuilder(name, a)
+                Expr::OptionalFieldInRecordBuilder(name, a)
             }
             Expr::SpaceBefore(a, _) => a.remove_spaces(arena),
             Expr::SpaceAfter(a, _) => a.remove_spaces(arena),

crates/compiler/load/tests/test_reporting.rs

@@ -10856,7 +10856,7 @@ In roc, functions are always written as a lambda, like{}
     // );
 
     test_report!(
-        empty_new_record_builder,
+        empty_record_builder,
         indoc!(
             r#"
             { a <- }
@@ -10870,12 +10870,12 @@ In roc, functions are always written as a lambda, like{}
     4│      { a <- }
             ^^^^^^^^
     
-    Note: We need at least two fields to combine their values into a record.
+    I need at least two fields to combine their values into a record.
     "#
     );
 
     test_report!(
-        single_field_new_record_builder,
+        single_field_record_builder,
         indoc!(
             r#"
             { a <-
@@ -10892,12 +10892,12 @@ In roc, functions are always written as a lambda, like{}
     5│>          b: 123
     6│>      }
     
-    Note: We need at least two fields to combine their values into a record.
+    I need at least two fields to combine their values into a record.
     "#
     );
 
     test_report!(
-        optional_field_in_new_record_builder,
+        optional_field_in_record_builder,
         indoc!(
             r#"
             { a <-
@@ -10916,7 +10916,44 @@ In roc, functions are always written as a lambda, like{}
     6│>          c? 456
     7│       }
     
-    Note: Record builders can only have required values for their fields.
+    Record builders can only have required values for their fields.
+    "#
+    );
+
+    // CalledVia::RecordBuilder => {
+    //     alloc.concat([
+    //         alloc.note(""),
+    //         alloc.reflow("Record builders need a mapper function before the "),
+    //         alloc.keyword("<-"),
+    //         alloc.reflow(" to combine fields together with.")
+    //     ])
+    // }
+    // _ => {
+    //     alloc.reflow("Are there any missing commas? Or missing parentheses?")
+
+    test_report!(
+        record_builder_with_non_function_mapper,
+        indoc!(
+            r#"
+            x = "abc"
+
+            { x <-
+                b: 123,
+                c: 456
+            }
+            "#
+        ),
+        @r#"
+    ── OPTIONAL FIELD IN RECORD BUILDER in /code/proj/Main.roc ─────────────────────
+    
+    Optional fields are not allowed to be used in record builders.
+    
+    4│       { a <-
+    5│           b: 123,
+    6│>          c? 456
+    7│       }
+    
+    Record builders can only have required values for their fields.
     "#
     );
 

crates/compiler/module/src/called_via.rs

@@ -104,7 +104,7 @@ pub enum CalledVia {
     /// ```roc
     /// Task.parallel (get "a") (get "b") \foo, bar -> { foo, bar }
     /// ```
-    NewRecordBuilder,
+    RecordBuilder,
 
     /// This call is the result of desugaring a Task.await from `!` syntax
     /// e.g. Stdout.line! "Hello" becomes Task.await (Stdout.line "Hello") \{} -> ...

crates/compiler/parse/src/ast.rs

@@ -449,7 +449,7 @@ pub enum Expr<'a> {
     ///     foo: Task.getData Foo,
     ///     bar: Task.getData Bar,
     /// }
-    NewRecordBuilder {
+    RecordBuilder {
         mapper: &'a Loc<Expr<'a>>,
         fields: Collection<'a, Loc<AssignedField<'a, Expr<'a>>>>,
     },
@@ -517,9 +517,9 @@ pub enum Expr<'a> {
     PrecedenceConflict(&'a PrecedenceConflict<'a>),
     MultipleOldRecordBuilders(&'a Loc<Expr<'a>>),
     UnappliedOldRecordBuilder(&'a Loc<Expr<'a>>),
-    EmptyNewRecordBuilder(&'a Loc<Expr<'a>>),
+    EmptyRecordBuilder(&'a Loc<Expr<'a>>),
-    SingleFieldNewRecordBuilder(&'a Loc<Expr<'a>>),
+    SingleFieldRecordBuilder(&'a Loc<Expr<'a>>),
-    OptionalFieldInNewRecordBuilder(&'a Loc<&'a str>, &'a Loc<Expr<'a>>),
+    OptionalFieldInRecordBuilder(&'a Loc<&'a str>, &'a Loc<Expr<'a>>),
 }
 
 impl Expr<'_> {
@@ -635,7 +635,7 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool {
         Expr::OldRecordBuilder(items) => items
             .iter()
             .any(|rbf| is_record_builder_field_suffixed(&rbf.value)),
-        Expr::NewRecordBuilder { mapper: _, fields } => fields
+        Expr::RecordBuilder { mapper: _, fields } => fields
             .iter()
             .any(|field| is_assigned_value_suffixed(&field.value)),
         Expr::Underscore(_) => false,
@@ -659,9 +659,9 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool {
         Expr::PrecedenceConflict(_) => false,
         Expr::MultipleOldRecordBuilders(_) => false,
         Expr::UnappliedOldRecordBuilder(_) => false,
-        Expr::EmptyNewRecordBuilder(_) => false,
+        Expr::EmptyRecordBuilder(_) => false,
-        Expr::SingleFieldNewRecordBuilder(_) => false,
+        Expr::SingleFieldRecordBuilder(_) => false,
-        Expr::OptionalFieldInNewRecordBuilder(_, _) => false,
+        Expr::OptionalFieldInRecordBuilder(_, _) => false,
     }
 }
 
@@ -924,7 +924,7 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> {
                         }
                     }
                 }
-                NewRecordBuilder {
+                RecordBuilder {
                     mapper: map2,
                     fields,
                 } => {
@@ -998,9 +998,9 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> {
 
                 MultipleOldRecordBuilders(loc_expr)
                 | UnappliedOldRecordBuilder(loc_expr)
-                | EmptyNewRecordBuilder(loc_expr)
+                | EmptyRecordBuilder(loc_expr)
-                | SingleFieldNewRecordBuilder(loc_expr)
+                | SingleFieldRecordBuilder(loc_expr)
-                | OptionalFieldInNewRecordBuilder(_, loc_expr) => expr_stack.push(&loc_expr.value),
+                | OptionalFieldInRecordBuilder(_, loc_expr) => expr_stack.push(&loc_expr.value),
 
                 Float(_)
                 | Num(_)
@@ -2437,7 +2437,7 @@ impl<'a> Malformed for Expr<'a> {
             Tuple(items) => items.is_malformed(),
 
             OldRecordBuilder(items) => items.is_malformed(),
-            NewRecordBuilder { mapper: map2, fields } => map2.is_malformed() || fields.is_malformed(),
+            RecordBuilder { mapper: map2, fields } => map2.is_malformed() || fields.is_malformed(),
 
             Closure(args, body) => args.iter().any(|arg| arg.is_malformed()) || body.is_malformed(),
             Defs(defs, body) => defs.is_malformed() || body.is_malformed(),
@@ -2461,9 +2461,9 @@ impl<'a> Malformed for Expr<'a> {
             PrecedenceConflict(_) |
             MultipleOldRecordBuilders(_) |
             UnappliedOldRecordBuilder(_) |
-            EmptyNewRecordBuilder(_) |
+            EmptyRecordBuilder(_) |
-            SingleFieldNewRecordBuilder(_) |
+            SingleFieldRecordBuilder(_) |
-            OptionalFieldInNewRecordBuilder(_, _) => true,
+            OptionalFieldInRecordBuilder(_, _) => true,
         }
     }
 }

crates/compiler/parse/src/expr.rs

@@ -998,13 +998,13 @@ fn import_params<'a>() -> impl Parser<'a, ModuleImportParams<'a>, EImportParams<
         |arena, state, _, (before, record): (_, RecordHelp<'a>)| {
             if let Some(prefix) = record.prefix {
                 match prefix {
-                    RecordHelpPrefix::Update(update) => {
+                    (update, RecordHelpPrefix::Update) => {
                         return Err((
                             MadeProgress,
                             EImportParams::RecordUpdateFound(update.region),
                         ))
                     }
-                    RecordHelpPrefix::Mapper(mapper) => {
+                    (mapper, RecordHelpPrefix::Mapper) => {
                         return Err((
                             MadeProgress,
                             EImportParams::RecordBuilderFound(mapper.region),
@@ -2381,7 +2381,7 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result<Pattern<
         | Expr::SpaceAfter(..)
         | Expr::ParensAround(..)
         | Expr::OldRecordBuilder(..)
-        | Expr::NewRecordBuilder { .. } => unreachable!(),
+        | Expr::RecordBuilder { .. } => unreachable!(),
 
         Expr::Record(fields) => {
             let patterns = fields.map_items_result(arena, |loc_assigned_field| {
@@ -2430,9 +2430,9 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result<Pattern<
         | Expr::PrecedenceConflict { .. }
         | Expr::MultipleOldRecordBuilders { .. }
         | Expr::UnappliedOldRecordBuilder { .. }
-        | Expr::EmptyNewRecordBuilder(_)
+        | Expr::EmptyRecordBuilder(_)
-        | Expr::SingleFieldNewRecordBuilder(_)
+        | Expr::SingleFieldRecordBuilder(_)
-        | Expr::OptionalFieldInNewRecordBuilder(_, _)
+        | Expr::OptionalFieldInRecordBuilder(_, _)
         | Expr::RecordUpdate { .. }
         | Expr::UnaryOp(_, _)
         | Expr::TaskAwaitBang(..)
@@ -3241,27 +3241,20 @@ fn record_field_expr<'a>() -> impl Parser<'a, RecordFieldExpr<'a>, ERecord<'a>>
     )
 }
 
-fn record_updateable_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>> {
+fn record_prefix_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>> {
     specialize_err(
-        |_, pos| ERecord::Updateable(pos),
+        |_, pos| ERecord::Prefix(pos),
         map_with_arena(parse_ident, ident_to_expr),
     )
 }
 
-fn record_builder_mapper_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>> {
+enum RecordHelpPrefix {
-    specialize_err(
+    Update,
-        |_, pos| ERecord::BuilderMapper(pos),
+    Mapper,
-        map_with_arena(parse_ident, ident_to_expr),
-    )
-}
-
-enum RecordHelpPrefix<'a> {
-    Update(Loc<Expr<'a>>),
-    Mapper(Loc<Expr<'a>>),
 }
 
 struct RecordHelp<'a> {
-    prefix: Option<RecordHelpPrefix<'a>>,
+    prefix: Option<(Loc<Expr<'a>>, RecordHelpPrefix)>,
     fields: Collection<'a, Loc<RecordField<'a>>>,
 }
 
@@ -3271,28 +3264,25 @@ fn record_help<'a>() -> impl Parser<'a, RecordHelp<'a>, ERecord<'a>> {
         reset_min_indent(record!(RecordHelp {
             // You can optionally have an identifier followed by an '&' to
             // make this a record update, e.g. { Foo.user & username: "blah" }.
-            prefix: optional(map_with_arena(
+            prefix: optional(backtrackable(and(
-                either(
+                spaces_around(
-                    backtrackable(skip_second(
+                    // We wrap the ident in an Expr here,
-                        spaces_around(
+                    // so that we have a Spaceable value to work with,
-                            // We wrap the ident in an Expr here,
+                    // and then in canonicalization verify that it's an Expr::Var
-                            // so that we have a Spaceable value to work with,
+                    // (and not e.g. an `Expr::Access`) and extract its string.
-                            // and then in canonicalization verify that it's an Expr::Var
+                    loc(record_prefix_identifier()),
-                            // (and not e.g. an `Expr::Access`) and extract its string.
-                            loc(record_updateable_identifier()),
-                        ),
-                        byte(b'&', ERecord::Ampersand)
-                    )),
-                    backtrackable(skip_second(
-                        spaces_around(loc(record_builder_mapper_identifier()),),
-                        two_bytes(b'<', b'-', ERecord::Arrow),
-                    )),
                 ),
-                |_arena, output| match output {
+                map_with_arena(
-                    Either::First(update) => RecordHelpPrefix::Update(update),
+                    either(
-                    Either::Second(mapper) => RecordHelpPrefix::Mapper(mapper),
+                        byte(b'&', ERecord::Ampersand),
-                }
+                        two_bytes(b'<', b'-', ERecord::Arrow),
-            )),
+                    ),
+                    |_arena, output| match output {
+                        Either::First(()) => RecordHelpPrefix::Update,
+                        Either::Second(()) => RecordHelpPrefix::Mapper,
+                    }
+                )
+            ))),
             fields: collection_inner(
                 loc(record_field()),
                 byte(b',', ERecord::End),
@@ -3312,10 +3302,10 @@ fn record_literal_help<'a>() -> impl Parser<'a, Expr<'a>, EExpr<'a>> {
         ),
         move |arena, state, _, (record, accessors)| {
             let expr_result = match record.prefix {
-                Some(RecordHelpPrefix::Update(update)) => {
+                Some((update, RecordHelpPrefix::Update)) => {
                     record_update_help(arena, update, record.fields)
                 }
-                Some(RecordHelpPrefix::Mapper(mapper)) => {
+                Some((mapper, RecordHelpPrefix::Mapper)) => {
                     new_record_builder_help(arena, mapper, record.fields)
                 }
                 None => {
@@ -3384,7 +3374,7 @@ fn new_record_builder_help<'a>(
         }
     });
 
-    result.map(|fields| Expr::NewRecordBuilder {
+    result.map(|fields| Expr::RecordBuilder {
         mapper: &*arena.alloc(mapper),
         fields,
     })

crates/compiler/parse/src/parser.rs

@@ -422,8 +422,7 @@ pub enum ERecord<'a> {
     End(Position),
     Open(Position),
 
-    Updateable(Position),
+    Prefix(Position),
-    BuilderMapper(Position),
     Field(Position),
     Colon(Position),
     QuestionMark(Position),
@@ -739,8 +738,7 @@ pub enum ETypeAbilityImpl<'a> {
 
     Space(BadInputError, Position),
 
-    Updateable(Position),
+    Prefix(Position),
-    BuilderMapper(Position),
     QuestionMark(Position),
     Ampersand(Position),
     Expr(&'a EExpr<'a>, Position),
@@ -757,8 +755,7 @@ impl<'a> From<ERecord<'a>> for ETypeAbilityImpl<'a> {
             ERecord::Colon(p) => ETypeAbilityImpl::Colon(p),
             ERecord::Arrow(p) => ETypeAbilityImpl::Arrow(p),
             ERecord::Space(s, p) => ETypeAbilityImpl::Space(s, p),
-            ERecord::Updateable(p) => ETypeAbilityImpl::Updateable(p),
+            ERecord::Prefix(p) => ETypeAbilityImpl::Prefix(p),
-            ERecord::BuilderMapper(p) => ETypeAbilityImpl::BuilderMapper(p),
             ERecord::QuestionMark(p) => ETypeAbilityImpl::QuestionMark(p),
             ERecord::Ampersand(p) => ETypeAbilityImpl::Ampersand(p),
             ERecord::Expr(e, p) => ETypeAbilityImpl::Expr(e, p),

crates/compiler/problem/src/can.rs

@@ -414,9 +414,9 @@ impl Problem {
             | Problem::RuntimeError(RuntimeError::DegenerateBranch(region))
             | Problem::RuntimeError(RuntimeError::MultipleOldRecordBuilders(region))
             | Problem::RuntimeError(RuntimeError::UnappliedOldRecordBuilder(region))
-            | Problem::RuntimeError(RuntimeError::EmptyNewRecordBuilder(region))
+            | Problem::RuntimeError(RuntimeError::EmptyRecordBuilder(region))
-            | Problem::RuntimeError(RuntimeError::SingleFieldNewRecordBuilder(region))
+            | Problem::RuntimeError(RuntimeError::SingleFieldRecordBuilder(region))
-            | Problem::RuntimeError(RuntimeError::OptionalFieldInNewRecordBuilder {
+            | Problem::RuntimeError(RuntimeError::OptionalFieldInRecordBuilder {
                 record: _,
                 field: region,
             })
@@ -672,9 +672,9 @@ pub enum RuntimeError {
     MultipleOldRecordBuilders(Region),
     UnappliedOldRecordBuilder(Region),
 
-    EmptyNewRecordBuilder(Region),
+    EmptyRecordBuilder(Region),
-    SingleFieldNewRecordBuilder(Region),
+    SingleFieldRecordBuilder(Region),
-    OptionalFieldInNewRecordBuilder {
+    OptionalFieldInRecordBuilder {
         record: Region,
         field: Region,
     },
@@ -724,9 +724,9 @@ impl RuntimeError {
             | RuntimeError::InvalidHexadecimal(region)
             | RuntimeError::MultipleOldRecordBuilders(region)
             | RuntimeError::UnappliedOldRecordBuilder(region)
-            | RuntimeError::EmptyNewRecordBuilder(region)
+            | RuntimeError::EmptyRecordBuilder(region)
-            | RuntimeError::SingleFieldNewRecordBuilder(region)
+            | RuntimeError::SingleFieldRecordBuilder(region)
-            | RuntimeError::OptionalFieldInNewRecordBuilder {
+            | RuntimeError::OptionalFieldInRecordBuilder {
                 record: _,
                 field: region,
             }

crates/language_server/src/analysis/tokens.rs

@@ -684,7 +684,7 @@ impl IterTokens for Loc<Expr<'_>> {
             Expr::Record(rcd) => rcd.iter_tokens(arena),
             Expr::Tuple(tup) => tup.iter_tokens(arena),
             Expr::OldRecordBuilder(rb) => rb.iter_tokens(arena),
-            Expr::NewRecordBuilder { mapper, fields } => (mapper.iter_tokens(arena).into_iter())
+            Expr::RecordBuilder { mapper, fields } => (mapper.iter_tokens(arena).into_iter())
                 .chain(fields.iter().flat_map(|f| f.iter_tokens(arena)))
                 .collect_in(arena),
             Expr::Var { .. } => onetoken(Token::Variable, region, arena),
@@ -732,9 +732,9 @@ impl IterTokens for Loc<Expr<'_>> {
             Expr::ParensAround(e) => Loc::at(region, *e).iter_tokens(arena),
             Expr::MultipleOldRecordBuilders(e) => e.iter_tokens(arena),
             Expr::UnappliedOldRecordBuilder(e) => e.iter_tokens(arena),
-            Expr::EmptyNewRecordBuilder(e) => e.iter_tokens(arena),
+            Expr::EmptyRecordBuilder(e) => e.iter_tokens(arena),
-            Expr::SingleFieldNewRecordBuilder(e) => e.iter_tokens(arena),
+            Expr::SingleFieldRecordBuilder(e) => e.iter_tokens(arena),
-            Expr::OptionalFieldInNewRecordBuilder(_name, e) => e.iter_tokens(arena),
+            Expr::OptionalFieldInRecordBuilder(_name, e) => e.iter_tokens(arena),
             Expr::MalformedIdent(_, _)
             | Expr::MalformedClosure
             | Expr::PrecedenceConflict(_)

crates/reporting/src/error/canonicalize.rs

@@ -2447,25 +2447,25 @@ fn pretty_runtime_error<'b>(
 
             title = "UNAPPLIED OLD-STYLE RECORD BUILDER";
         }
-        RuntimeError::EmptyNewRecordBuilder(region) => {
+        RuntimeError::EmptyRecordBuilder(region) => {
             doc = alloc.stack([
                 alloc.reflow("This record builder has no fields:"),
                 alloc.region(lines.convert_region(region), severity),
-                alloc.note("We need at least two fields to combine their values into a record."),
+                alloc.reflow("I need at least two fields to combine their values into a record."),
             ]);
 
             title = "EMPTY RECORD BUILDER";
         }
-        RuntimeError::SingleFieldNewRecordBuilder(region) => {
+        RuntimeError::SingleFieldRecordBuilder(region) => {
             doc = alloc.stack([
                 alloc.reflow("This record builder only has one field:"),
                 alloc.region(lines.convert_region(region), severity),
-                alloc.note("We need at least two fields to combine their values into a record."),
+                alloc.reflow("I need at least two fields to combine their values into a record."),
             ]);
 
             title = "NOT ENOUGH FIELDS IN RECORD BUILDER";
         }
-        RuntimeError::OptionalFieldInNewRecordBuilder {
+        RuntimeError::OptionalFieldInRecordBuilder {
             record: record_region,
             field: field_region,
         } => {
@@ -2476,7 +2476,7 @@ fn pretty_runtime_error<'b>(
                     lines.convert_region(field_region),
                     severity,
                 ),
-                alloc.note("Record builders can only have required values for their fields."),
+                alloc.reflow("Record builders can only have required values for their fields."),
             ]);
 
             title = "OPTIONAL FIELD IN RECORD BUILDER";

crates/reporting/src/error/type.rs

@@ -1175,7 +1175,7 @@ fn to_expr_report<'b>(
                                         alloc.reflow(" to assign the field directly.")
                                     ])
                                 }
-                                CalledVia::NewRecordBuilder => {
+                                CalledVia::RecordBuilder => {
                                     alloc.concat([
                                         alloc.note(""),
                                         alloc.reflow("Record builders need a mapper function before the "),