From f415017c9000510e7b2043ed4dceae0893701e33 Mon Sep 17 00:00:00 2001 From: Sam Mohr Date: Sun, 7 Jul 2024 03:22:05 -0700 Subject: [PATCH 1/3] Implement new builder syntax alongside old one --- crates/compiler/can/src/desugar.rs | 219 ++++++++++++++++-- crates/compiler/can/src/expr.rs | 100 +++++--- crates/compiler/can/tests/test_can.rs | 199 +++++++++++++++- crates/compiler/fmt/src/annotation.rs | 12 +- crates/compiler/fmt/src/expr.rs | 65 ++++-- crates/compiler/fmt/src/spaces.rs | 35 +-- crates/compiler/load/tests/test_reporting.rs | 102 ++++++-- crates/compiler/module/src/called_via.rs | 17 +- crates/compiler/parse/src/ast.rs | 113 ++++++--- crates/compiler/parse/src/expr.rs | 122 +++++++--- crates/compiler/parse/src/parser.rs | 7 +- crates/compiler/problem/src/can.rs | 31 ++- crates/compiler/test_syntax/tests/test_fmt.rs | 94 +++++++- crates/language_server/src/analysis/tokens.rs | 29 ++- crates/reporting/src/error/canonicalize.rs | 48 +++- crates/reporting/src/error/parse.rs | 29 ++- crates/reporting/src/error/type.rs | 12 +- 17 files changed, 1023 insertions(+), 211 deletions(-) diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index f72c91cabd5..5387af40dbe 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -9,7 +9,7 @@ use roc_module::called_via::{BinOp, CalledVia}; use roc_module::ident::ModuleName; use roc_parse::ast::Expr::{self, *}; use roc_parse::ast::{ - AssignedField, Collection, ModuleImportParams, Pattern, RecordBuilderField, StrLiteral, + AssignedField, Collection, ModuleImportParams, OldRecordBuilderField, Pattern, StrLiteral, StrSegment, ValueDef, WhenBranch, }; use roc_region::all::{LineInfo, Loc, Region}; @@ -300,8 +300,11 @@ pub fn desugar_expr<'a>( | MalformedClosure | MalformedSuffixed(..) | PrecedenceConflict { .. } - | MultipleRecordBuilders { .. } - | UnappliedRecordBuilder { .. } + | MultipleOldRecordBuilders(_) + | UnappliedOldRecordBuilder(_) + | EmptyNewRecordBuilder(_) + | SingleFieldNewRecordBuilder(_) + | OptionalFieldInNewRecordBuilder { .. } | Tag(_) | OpaqueRef(_) | Crash => loc_expr, @@ -485,10 +488,196 @@ pub fn desugar_expr<'a>( } } } - RecordBuilder(_) => arena.alloc(Loc { - value: UnappliedRecordBuilder(loc_expr), + OldRecordBuilder(_) => arena.alloc(Loc { + value: UnappliedOldRecordBuilder(loc_expr), region: loc_expr.region, }), + NewRecordBuilder { 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), + region: loc_expr.region, + }); + } else if fields.len() == 1 { + return arena.alloc(Loc { + value: SingleFieldNewRecordBuilder(loc_expr), + region: loc_expr.region, + }); + } + + let mut field_names = vec![]; + let mut field_vals = vec![]; + + for field in fields.items { + match desugar_field(arena, &field.value, src, line_info, module_path) { + AssignedField::RequiredValue(loc_name, _, loc_val) => { + field_names.push(loc_name); + field_vals.push(loc_val); + } + AssignedField::LabelOnly(loc_name) => { + field_names.push(loc_name); + field_vals.push(arena.alloc(Loc { + region: loc_name.region, + value: Expr::Var { + module_name: "", + ident: loc_name.value, + }, + })); + } + AssignedField::OptionalValue(loc_name, _, loc_val) => { + return arena.alloc(Loc { + region: loc_expr.region, + value: OptionalFieldInNewRecordBuilder(arena.alloc(loc_name), loc_val), + }); + } + AssignedField::SpaceBefore(_, _) | AssignedField::SpaceAfter(_, _) => { + unreachable!("Should have been desugared in `desugar_field`") + } + AssignedField::Malformed(_name) => {} + } + } + + let closure_arg_from_field = |field: Loc<&'a str>| Loc { + region: field.region, + value: Pattern::Identifier { + ident: arena.alloc_str(&format!("#{}", field.value)), + }, + }; + + let combiner_closure = arena.alloc(Loc::at_zero(Closure( + arena.alloc_slice_copy(&[ + 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 { + module_name: "", + ident: "#record_builder_closure_arg_a", + })), + &*arena.alloc(Loc::at_zero(Expr::Var { + module_name: "", + ident: "#record_builder_closure_arg_b", + })), + ], + arena, + ) + .into_bump_slice(), + )))), + ))); + + let closure_args = { + if field_names.len() <= 2 { + arena.alloc_slice_copy(&[ + closure_arg_from_field(field_names[0]), + closure_arg_from_field(field_names[1]), + ]) + } else { + let second_to_last_arg = + closure_arg_from_field(field_names[field_names.len() - 2]); + let last_arg = closure_arg_from_field(field_names[field_names.len() - 1]); + + let mut second_arg = Pattern::Tuple(Collection::with_items( + arena.alloc_slice_copy(&[second_to_last_arg, last_arg]), + )); + 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), + ]))); + } + + arena.alloc_slice_copy(&[ + closure_arg_from_field(field_names[0]), + Loc::at_zero(second_arg), + ]) + } + }; + + let record_val = Record(Collection::with_items( + Vec::from_iter_in( + field_names.iter().map(|field_name| { + Loc::at( + field_name.region, + AssignedField::RequiredValue( + Loc::at(field_name.region, field_name.value), + &[], + arena.alloc(Loc::at_zero(Expr::Var { + module_name: "", + ident: arena.alloc_str(&format!("#{}", field_name.value)), + })), + ), + ) + }), + arena, + ) + .into_bump_slice(), + )); + + let record_combiner_closure = arena.alloc(Loc { + region: loc_expr.region, + value: Closure(closure_args, arena.alloc(Loc::at_zero(record_val))), + }); + + if field_names.len() == 2 { + return arena.alloc(Loc { + region: loc_expr.region, + value: Apply( + new_mapper, + arena.alloc_slice_copy(&[ + field_vals[0], + field_vals[1], + record_combiner_closure, + ]), + CalledVia::NewRecordBuilder, + ), + }); + } + + let mut inner_combined = arena.alloc(Loc { + region: loc_expr.region, + value: Apply( + new_mapper, + arena.alloc_slice_copy(&[ + field_vals[field_names.len() - 2], + field_vals[field_names.len() - 1], + combiner_closure, + ]), + CalledVia::NewRecordBuilder, + ), + }); + + for index in (1..(field_names.len() - 2)).rev() { + inner_combined = arena.alloc(Loc::at_zero(Apply( + new_mapper, + arena.alloc_slice_copy(&[field_vals[index], inner_combined, combiner_closure]), + CalledVia::NewRecordBuilder, + ))); + } + + arena.alloc(Loc { + region: loc_expr.region, + value: Apply( + new_mapper, + arena.alloc_slice_copy(&[ + field_vals[0], + inner_combined, + record_combiner_closure, + ]), + CalledVia::NewRecordBuilder, + ), + }) + } BinOps(lefts, right) => desugar_bin_ops( arena, loc_expr.region, @@ -513,10 +702,10 @@ pub fn desugar_expr<'a>( let mut current = loc_arg.value; let arg = loop { match current { - RecordBuilder(fields) => { + OldRecordBuilder(fields) => { if builder_apply_exprs.is_some() { return arena.alloc(Loc { - value: MultipleRecordBuilders(loc_expr), + value: MultipleOldRecordBuilders(loc_expr), region: loc_expr.region, }); } @@ -557,7 +746,7 @@ pub fn desugar_expr<'a>( let args = std::slice::from_ref(arena.alloc(apply)); apply = arena.alloc(Loc { - value: Apply(desugared_expr, args, CalledVia::RecordBuilder), + value: Apply(desugared_expr, args, CalledVia::OldRecordBuilder), region: loc_expr.region, }); } @@ -1015,7 +1204,7 @@ struct RecordBuilderArg<'a> { fn record_builder_arg<'a>( arena: &'a Bump, region: Region, - fields: Collection<'a, Loc>>, + fields: Collection<'a, Loc>>, ) -> RecordBuilderArg<'a> { let mut record_fields = Vec::with_capacity_in(fields.len(), arena); let mut apply_exprs = Vec::with_capacity_in(fields.len(), arena); @@ -1028,10 +1217,10 @@ fn record_builder_arg<'a>( let new_field = loop { match current { - RecordBuilderField::Value(label, spaces, expr) => { + OldRecordBuilderField::Value(label, spaces, expr) => { break AssignedField::RequiredValue(label, spaces, expr) } - RecordBuilderField::ApplyValue(label, _, _, expr) => { + OldRecordBuilderField::ApplyValue(label, _, _, expr) => { apply_field_names.push(label); apply_exprs.push(expr); @@ -1045,14 +1234,14 @@ fn record_builder_arg<'a>( break AssignedField::RequiredValue(label, &[], var); } - RecordBuilderField::LabelOnly(label) => break AssignedField::LabelOnly(label), - RecordBuilderField::SpaceBefore(sub_field, _) => { + OldRecordBuilderField::LabelOnly(label) => break AssignedField::LabelOnly(label), + OldRecordBuilderField::SpaceBefore(sub_field, _) => { current = *sub_field; } - RecordBuilderField::SpaceAfter(sub_field, _) => { + OldRecordBuilderField::SpaceAfter(sub_field, _) => { current = *sub_field; } - RecordBuilderField::Malformed(malformed) => { + OldRecordBuilderField::Malformed(malformed) => { break AssignedField::Malformed(malformed) } } diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 70e8f4dec31..44a970ff4c0 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -1021,8 +1021,11 @@ pub fn canonicalize_expr<'a>( can_defs_with_return(env, var_store, inner_scope, env.arena.alloc(defs), loc_ret) }) } - ast::Expr::RecordBuilder(_) => { - internal_error!("RecordBuilder should have been desugared by now") + ast::Expr::OldRecordBuilder(_) => { + internal_error!("Old record builder should have been desugared by now") + } + ast::Expr::NewRecordBuilder { .. } => { + internal_error!("New record builder should have been desugared by now") } ast::Expr::Backpassing(_, _, _) => { internal_error!("Backpassing should have been desugared by now") @@ -1340,18 +1343,43 @@ pub fn canonicalize_expr<'a>( use roc_problem::can::RuntimeError::*; (RuntimeError(MalformedSuffixed(region)), Output::default()) } - ast::Expr::MultipleRecordBuilders(sub_expr) => { + ast::Expr::MultipleOldRecordBuilders(sub_expr) => { + use roc_problem::can::RuntimeError::*; + + let problem = MultipleOldRecordBuilders(sub_expr.region); + env.problem(Problem::RuntimeError(problem.clone())); + + (RuntimeError(problem), Output::default()) + } + ast::Expr::UnappliedOldRecordBuilder(sub_expr) => { + use roc_problem::can::RuntimeError::*; + + let problem = UnappliedOldRecordBuilder(sub_expr.region); + env.problem(Problem::RuntimeError(problem.clone())); + + (RuntimeError(problem), Output::default()) + } + ast::Expr::EmptyNewRecordBuilder(sub_expr) => { + use roc_problem::can::RuntimeError::*; + + let problem = EmptyNewRecordBuilder(sub_expr.region); + env.problem(Problem::RuntimeError(problem.clone())); + + (RuntimeError(problem), Output::default()) + } + ast::Expr::SingleFieldNewRecordBuilder(sub_expr) => { use roc_problem::can::RuntimeError::*; - let problem = MultipleRecordBuilders(sub_expr.region); + let problem = SingleFieldNewRecordBuilder(sub_expr.region); env.problem(Problem::RuntimeError(problem.clone())); (RuntimeError(problem), Output::default()) } - ast::Expr::UnappliedRecordBuilder(sub_expr) => { + ast::Expr::OptionalFieldInNewRecordBuilder(loc_name, loc_value) => { use roc_problem::can::RuntimeError::*; - let problem = UnappliedRecordBuilder(sub_expr.region); + let sub_region = Region::span_across(&loc_name.region, &loc_value.region); + let problem = OptionalFieldInNewRecordBuilder {record: region, field: sub_region }; env.problem(Problem::RuntimeError(problem.clone())); (RuntimeError(problem), Output::default()) @@ -2414,9 +2442,12 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool { ast::Expr::Tuple(fields) => fields .iter() .all(|loc_field| is_valid_interpolation(&loc_field.value)), - ast::Expr::MultipleRecordBuilders(loc_expr) - | ast::Expr::MalformedSuffixed(loc_expr) - | ast::Expr::UnappliedRecordBuilder(loc_expr) + ast::Expr::MalformedSuffixed(loc_expr) + | ast::Expr::MultipleOldRecordBuilders(loc_expr) + | ast::Expr::UnappliedOldRecordBuilder(loc_expr) + | ast::Expr::EmptyNewRecordBuilder(loc_expr) + | ast::Expr::SingleFieldNewRecordBuilder(loc_expr) + | ast::Expr::OptionalFieldInNewRecordBuilder(_, 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), @@ -2458,24 +2489,39 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool { | ast::AssignedField::SpaceAfter(_, _) => false, }) } - ast::Expr::RecordBuilder(fields) => fields.iter().all(|loc_field| match loc_field.value { - ast::RecordBuilderField::Value(_label, comments, loc_expr) => { - comments.is_empty() && is_valid_interpolation(&loc_expr.value) - } - ast::RecordBuilderField::ApplyValue( - _label, - comments_before, - comments_after, - loc_expr, - ) => { - comments_before.is_empty() - && comments_after.is_empty() - && is_valid_interpolation(&loc_expr.value) - } - ast::RecordBuilderField::Malformed(_) | ast::RecordBuilderField::LabelOnly(_) => true, - ast::RecordBuilderField::SpaceBefore(_, _) - | ast::RecordBuilderField::SpaceAfter(_, _) => false, - }), + ast::Expr::OldRecordBuilder(fields) => { + fields.iter().all(|loc_field| match loc_field.value { + ast::OldRecordBuilderField::Value(_label, comments, loc_expr) => { + comments.is_empty() && is_valid_interpolation(&loc_expr.value) + } + ast::OldRecordBuilderField::ApplyValue( + _label, + comments_before, + comments_after, + loc_expr, + ) => { + comments_before.is_empty() + && comments_after.is_empty() + && is_valid_interpolation(&loc_expr.value) + } + ast::OldRecordBuilderField::Malformed(_) + | ast::OldRecordBuilderField::LabelOnly(_) => true, + ast::OldRecordBuilderField::SpaceBefore(_, _) + | ast::OldRecordBuilderField::SpaceAfter(_, _) => false, + }) + } + ast::Expr::NewRecordBuilder { mapper, fields } => { + is_valid_interpolation(&mapper.value) + && fields.iter().all(|loc_field| match loc_field.value { + ast::AssignedField::RequiredValue(_label, loc_comments, loc_val) + | ast::AssignedField::OptionalValue(_label, loc_comments, loc_val) => { + loc_comments.is_empty() && is_valid_interpolation(&loc_val.value) + } + ast::AssignedField::Malformed(_) | ast::AssignedField::LabelOnly(_) => true, + ast::AssignedField::SpaceBefore(_, _) + | ast::AssignedField::SpaceAfter(_, _) => false, + }) + } } } diff --git a/crates/compiler/can/tests/test_can.rs b/crates/compiler/can/tests/test_can.rs index 29d9ad73ba4..4313fafb4da 100644 --- a/crates/compiler/can/tests/test_can.rs +++ b/crates/compiler/can/tests/test_can.rs @@ -17,9 +17,12 @@ mod test_can { use core::panic; use roc_can::expr::Expr::{self, *}; use roc_can::expr::{ClosureData, IntValue, Recursive}; + use roc_can::pattern::Pattern; + use roc_module::called_via::CalledVia; use roc_module::symbol::Symbol; use roc_problem::can::{CycleEntry, FloatErrorKind, IntErrorKind, Problem, RuntimeError}; - use roc_region::all::{Position, Region}; + use roc_region::all::{Loc, Position, Region}; + use roc_types::subs::Variable; use std::{f64, i64}; fn assert_can_runtime_error(input: &str, expected: RuntimeError) { @@ -651,7 +654,7 @@ mod test_can { // RECORD BUILDERS #[test] - fn record_builder_desugar() { + fn old_record_builder_desugar() { let src = indoc!( r#" succeed = \_ -> crash "succeed" @@ -766,7 +769,7 @@ mod test_can { } #[test] - fn record_builder_field_names_do_not_shadow() { + fn old_record_builder_field_names_do_not_shadow() { let src = indoc!( r#" succeed = \_ -> crash "succeed" @@ -806,7 +809,7 @@ mod test_can { } #[test] - fn multiple_record_builders_error() { + fn multiple_old_record_builders_error() { let src = indoc!( r#" succeed @@ -822,17 +825,17 @@ mod test_can { assert_eq!(problems.len(), 1); assert!(problems.iter().all(|problem| matches!( problem, - Problem::RuntimeError(roc_problem::can::RuntimeError::MultipleRecordBuilders { .. }) + Problem::RuntimeError(roc_problem::can::RuntimeError::MultipleOldRecordBuilders { .. }) ))); assert!(matches!( loc_expr.value, - Expr::RuntimeError(roc_problem::can::RuntimeError::MultipleRecordBuilders { .. }) + Expr::RuntimeError(roc_problem::can::RuntimeError::MultipleOldRecordBuilders { .. }) )); } #[test] - fn hanging_record_builder() { + fn hanging_old_record_builder() { let src = indoc!( r#" { a: <- apply "a" } @@ -846,15 +849,193 @@ mod test_can { assert_eq!(problems.len(), 1); assert!(problems.iter().all(|problem| matches!( problem, - Problem::RuntimeError(roc_problem::can::RuntimeError::UnappliedRecordBuilder { .. }) + Problem::RuntimeError(roc_problem::can::RuntimeError::UnappliedOldRecordBuilder { .. }) ))); assert!(matches!( loc_expr.value, - Expr::RuntimeError(roc_problem::can::RuntimeError::UnappliedRecordBuilder { .. }) + Expr::RuntimeError(roc_problem::can::RuntimeError::UnappliedOldRecordBuilder { .. }) )); } + #[test] + fn new_record_builder_desugar() { + let src = indoc!( + r#" + map2 = \a, b, combine -> combine a b + double = \n -> n * 2 + + c = 3 + + { map2 <- + a: 1, + b: double 2, + c + } + "# + ); + let arena = Bump::new(); + let out = can_expr_with(&arena, test_home(), src); + + assert_eq!(out.problems.len(), 0); + + // Assert that we desugar to: + // + // map2 + // (1) + // (map2 + // (double 2) + // (c) + // (\#a, #b -> (#a, #b)) + // ) + // (\a, (b, c) -> { a: #a, b: #b, c: #c }) + + let first_map2_args = assert_func_call( + &out.loc_expr.value, + "map2", + CalledVia::NewRecordBuilder, + &out.interns, + ); + let (first_arg, second_arg, third_arg) = match &first_map2_args[..] { + [first, second, third] => (&first.1.value, &second.1.value, &third.1.value), + _ => panic!("map2 didn't receive three arguments"), + }; + + assert_num_value(first_arg, 1); + + let inner_map2_args = assert_func_call( + second_arg, + "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"), + }; + + let double_args = + assert_func_call(first_inner_arg, "double", CalledVia::Space, &out.interns); + assert_eq!(double_args.len(), 1); + assert_num_value(&double_args[0].1.value, 2); + + assert_var_usage(second_inner_arg, "c", &out.interns); + + match third_inner_arg { + Expr::Closure(ClosureData { + arguments, + loc_body, + .. + }) => { + assert_eq!(arguments.len(), 2); + assert_pattern_name( + &arguments[0].2.value, + "#record_builder_closure_arg_a", + &out.interns, + ); + assert_pattern_name( + &arguments[1].2.value, + "#record_builder_closure_arg_b", + &out.interns, + ); + + match &loc_body.value { + Expr::Tuple { elems, .. } => { + assert_eq!(elems.len(), 2); + assert_var_usage( + &elems[0].1.value, + "#record_builder_closure_arg_a", + &out.interns, + ); + assert_var_usage( + &elems[1].1.value, + "#record_builder_closure_arg_b", + &out.interns, + ); + } + _ => panic!("Closure body was not a tuple"), + } + } + _ => panic!("inner map2's combiner was not a closure"), + } + + match third_arg { + Expr::Closure(ClosureData { + arguments, + loc_body, + .. + }) => { + assert_eq!(arguments.len(), 2); + assert_pattern_name(&arguments[0].2.value, "#a", &out.interns); + match &arguments[1].2.value { + Pattern::TupleDestructure { destructs, .. } => { + assert_eq!(destructs.len(), 2); + assert_pattern_name(&destructs[0].value.typ.1.value, "#b", &out.interns); + assert_pattern_name(&destructs[1].value.typ.1.value, "#c", &out.interns); + } + _ => panic!("Second arg to builder func was not a tuple destructure"), + } + + match &loc_body.value { + Expr::Record { fields, .. } => { + assert_eq!(fields.len(), 3); + + assert_eq!(get_field_var_sym(fields, "a").as_str(&out.interns), "#a"); + assert_eq!(get_field_var_sym(fields, "b").as_str(&out.interns), "#b"); + assert_eq!(get_field_var_sym(fields, "c").as_str(&out.interns), "#c"); + } + _ => panic!("Closure body was not a tuple"), + } + } + _ => panic!("inner map2's combiner was not a closure"), + } + } + + fn assert_num_value(expr: &Expr, num: usize) { + match expr { + Expr::Num(_, num_str, _, _) => { + assert_eq!(&**num_str, &num.to_string()) + } + _ => panic!("Expr wasn't a Num with value {num}: {:?}", expr), + } + } + + fn assert_var_usage(expr: &Expr, name: &str, interns: &roc_module::symbol::Interns) { + match expr { + Expr::Var(sym, _) => assert_eq!(sym.as_str(interns), name), + _ => panic!("Expr was not a variable usage: {:?}", expr), + } + } + + fn assert_func_call( + expr: &Expr, + name: &str, + called_via: CalledVia, + interns: &roc_module::symbol::Interns, + ) -> Vec<(Variable, Loc)> { + match expr { + Expr::LetNonRec(_, loc_expr) => { + assert_func_call(&loc_expr.value, name, called_via, interns) + } + Expr::Call(fun, args, called) if called == &called_via => { + match &fun.1.value { + Expr::Var(sym, _) => assert_eq!(sym.as_str(interns), name), + _ => panic!("Builder didn't desugar with mapper at front"), + }; + + args.clone() + } + _ => panic!("Expr was not a NewRecordBuilder Call: {:?}", expr), + } + } + + fn assert_pattern_name(pattern: &Pattern, name: &str, interns: &roc_module::symbol::Interns) { + match pattern { + Pattern::Identifier(sym) => assert_eq!(sym.as_str(interns), name), + _ => panic!("Pattern was not an identifier: {:?}", pattern), + } + } + // TAIL CALLS fn get_closure(expr: &Expr, i: usize) -> roc_can::expr::Recursive { match expr { diff --git a/crates/compiler/fmt/src/annotation.rs b/crates/compiler/fmt/src/annotation.rs index 56762d9c319..25076b59406 100644 --- a/crates/compiler/fmt/src/annotation.rs +++ b/crates/compiler/fmt/src/annotation.rs @@ -5,7 +5,7 @@ use crate::{ }; use roc_parse::ast::{ AbilityImpls, AssignedField, Collection, Expr, ExtractSpaces, ImplementsAbilities, - ImplementsAbility, ImplementsClause, RecordBuilderField, Tag, TypeAnnotation, TypeHeader, + ImplementsAbility, ImplementsClause, OldRecordBuilderField, Tag, TypeAnnotation, TypeHeader, }; use roc_parse::ident::UppercaseIdent; use roc_region::all::Loc; @@ -505,7 +505,7 @@ fn format_assigned_field_help( } } -impl<'a> Formattable for RecordBuilderField<'a> { +impl<'a> Formattable for OldRecordBuilderField<'a> { fn is_multiline(&self) -> bool { is_multiline_record_builder_field_help(self) } @@ -516,8 +516,8 @@ impl<'a> Formattable for RecordBuilderField<'a> { } } -fn is_multiline_record_builder_field_help(afield: &RecordBuilderField<'_>) -> bool { - use self::RecordBuilderField::*; +fn is_multiline_record_builder_field_help(afield: &OldRecordBuilderField<'_>) -> bool { + use self::OldRecordBuilderField::*; match afield { Value(_, spaces, ann) => !spaces.is_empty() || ann.value.is_multiline(), @@ -531,12 +531,12 @@ fn is_multiline_record_builder_field_help(afield: &RecordBuilderField<'_>) -> bo } fn format_record_builder_field_help( - zelf: &RecordBuilderField, + zelf: &OldRecordBuilderField, buf: &mut Buf, indent: u16, is_multiline: bool, ) { - use self::RecordBuilderField::*; + use self::OldRecordBuilderField::*; match zelf { Value(name, spaces, ann) => { diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index c8d746c68fb..77f942c8118 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -10,7 +10,7 @@ use crate::Buf; use roc_module::called_via::{self, BinOp}; use roc_parse::ast::{ is_expr_suffixed, AssignedField, Base, Collection, CommentOrNewline, Expr, ExtractSpaces, - Pattern, RecordBuilderField, WhenBranch, + OldRecordBuilderField, Pattern, WhenBranch, }; use roc_parse::ast::{StrLiteral, StrSegment}; use roc_parse::ident::Accessor; @@ -85,8 +85,11 @@ impl<'a> Formattable for Expr<'a> { | PrecedenceConflict(roc_parse::ast::PrecedenceConflict { expr: loc_subexpr, .. }) - | MultipleRecordBuilders(loc_subexpr) - | UnappliedRecordBuilder(loc_subexpr) => loc_subexpr.is_multiline(), + | MultipleOldRecordBuilders(loc_subexpr) + | UnappliedOldRecordBuilder(loc_subexpr) + | EmptyNewRecordBuilder(loc_subexpr) + | SingleFieldNewRecordBuilder(loc_subexpr) + | OptionalFieldInNewRecordBuilder(_, loc_subexpr) => loc_subexpr.is_multiline(), ParensAround(subexpr) => subexpr.is_multiline(), @@ -109,7 +112,8 @@ impl<'a> Formattable for Expr<'a> { Record(fields) => is_collection_multiline(fields), Tuple(fields) => is_collection_multiline(fields), RecordUpdate { fields, .. } => is_collection_multiline(fields), - RecordBuilder(fields) => is_collection_multiline(fields), + OldRecordBuilder(fields) => is_collection_multiline(fields), + NewRecordBuilder { fields, .. } => is_collection_multiline(fields), } } @@ -237,7 +241,7 @@ impl<'a> Formattable for Expr<'a> { Expr::Tuple(_) | Expr::List(_) | Expr::Record(_) - | Expr::RecordBuilder(_) + | Expr::OldRecordBuilder(_) ) && a.extract_spaces().before == [CommentOrNewline::Newline] }) @@ -365,14 +369,24 @@ impl<'a> Formattable for Expr<'a> { RecordUpdate { update, fields } => { fmt_record_like( buf, - Some(*update), + Some(RecordPrefix::Update(update)), *fields, indent, format_assigned_field_multiline, assigned_field_to_space_before, ); } - RecordBuilder(fields) => { + NewRecordBuilder { mapper, fields } => { + fmt_record_like( + buf, + Some(RecordPrefix::Mapper(mapper)), + *fields, + indent, + format_assigned_field_multiline, + assigned_field_to_space_before, + ); + } + OldRecordBuilder(fields) => { fmt_record_like( buf, None, @@ -520,8 +534,11 @@ impl<'a> Formattable for Expr<'a> { } MalformedClosure => {} PrecedenceConflict { .. } => {} - MultipleRecordBuilders { .. } => {} - UnappliedRecordBuilder { .. } => {} + MultipleOldRecordBuilders { .. } => {} + UnappliedOldRecordBuilder { .. } => {} + EmptyNewRecordBuilder { .. } => {} + SingleFieldNewRecordBuilder { .. } => {} + OptionalFieldInNewRecordBuilder(_, _) => {} } } } @@ -582,7 +599,7 @@ fn is_outdentable(expr: &Expr) -> bool { Expr::Tuple(_) | Expr::List(_) | Expr::Record(_) - | Expr::RecordBuilder(_) + | Expr::OldRecordBuilder(_) | Expr::Closure(..) ) } @@ -1366,9 +1383,14 @@ fn pattern_needs_parens_when_backpassing(pat: &Pattern) -> bool { } } +enum RecordPrefix<'a> { + Update(&'a Loc>), + Mapper(&'a Loc>), +} + fn fmt_record_like<'a, Field, Format, ToSpaceBefore>( buf: &mut Buf, - update: Option<&'a Loc>>, + prefix: Option>, fields: Collection<'a, Loc>, indent: u16, format_field_multiline: Format, @@ -1381,22 +1403,27 @@ fn fmt_record_like<'a, Field, Format, ToSpaceBefore>( let loc_fields = fields.items; let final_comments = fields.final_comments(); buf.indent(indent); - if loc_fields.is_empty() && final_comments.iter().all(|c| c.is_newline()) && update.is_none() { + if loc_fields.is_empty() && final_comments.iter().all(|c| c.is_newline()) && prefix.is_none() { buf.push_str("{}"); } else { buf.push('{'); - match update { + match prefix { None => {} // We are presuming this to be a Var() // If it wasnt a Var() we would not have made // it this far. For example "{ 4 & hello = 9 }" // doesnt make sense. - Some(record_var) => { + Some(RecordPrefix::Update(record_var)) => { buf.spaces(1); record_var.format(buf, indent); buf.push_str(" &"); } + Some(RecordPrefix::Mapper(mapper_var)) => { + buf.spaces(1); + mapper_var.format(buf, indent); + buf.push_str(" <-"); + } } let is_multiline = loc_fields.iter().any(|loc_field| loc_field.is_multiline()) @@ -1554,11 +1581,11 @@ fn assigned_field_to_space_before<'a, T>( fn format_record_builder_field_multiline( buf: &mut Buf, - field: &RecordBuilderField, + field: &OldRecordBuilderField, indent: u16, separator_prefix: &str, ) { - use self::RecordBuilderField::*; + use self::OldRecordBuilderField::*; match field { Value(name, spaces, ann) => { buf.newline(); @@ -1651,10 +1678,10 @@ fn format_record_builder_field_multiline( } fn record_builder_field_to_space_before<'a>( - field: &'a RecordBuilderField<'a>, -) -> Option<(&RecordBuilderField<'a>, &'a [CommentOrNewline<'a>])> { + field: &'a OldRecordBuilderField<'a>, +) -> Option<(&OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>])> { match field { - RecordBuilderField::SpaceBefore(sub_field, spaces) => Some((sub_field, spaces)), + OldRecordBuilderField::SpaceBefore(sub_field, spaces) => Some((sub_field, spaces)), _ => None, } } diff --git a/crates/compiler/fmt/src/spaces.rs b/crates/compiler/fmt/src/spaces.rs index 774d06fa5c3..a70e94d4788 100644 --- a/crates/compiler/fmt/src/spaces.rs +++ b/crates/compiler/fmt/src/spaces.rs @@ -6,8 +6,8 @@ use roc_parse::{ AbilityImpls, AbilityMember, AssignedField, Collection, CommentOrNewline, Defs, Expr, Header, Implements, ImplementsAbilities, ImplementsAbility, ImplementsClause, ImportAlias, ImportAsKeyword, ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, - IngestedFileImport, Module, ModuleImport, ModuleImportParams, Pattern, PatternAs, - RecordBuilderField, Spaced, Spaces, StrLiteral, StrSegment, Tag, TypeAnnotation, TypeDef, + IngestedFileImport, Module, ModuleImport, ModuleImportParams, OldRecordBuilderField, + Pattern, PatternAs, Spaced, Spaces, StrLiteral, StrSegment, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, WhenBranch, }, header::{ @@ -717,26 +717,26 @@ impl<'a, T: RemoveSpaces<'a> + Copy + std::fmt::Debug> RemoveSpaces<'a> for Assi } } -impl<'a> RemoveSpaces<'a> for RecordBuilderField<'a> { +impl<'a> RemoveSpaces<'a> for OldRecordBuilderField<'a> { fn remove_spaces(&self, arena: &'a Bump) -> Self { match *self { - RecordBuilderField::Value(a, _, c) => RecordBuilderField::Value( + OldRecordBuilderField::Value(a, _, c) => OldRecordBuilderField::Value( a.remove_spaces(arena), &[], arena.alloc(c.remove_spaces(arena)), ), - RecordBuilderField::ApplyValue(a, _, _, c) => RecordBuilderField::ApplyValue( + OldRecordBuilderField::ApplyValue(a, _, _, c) => OldRecordBuilderField::ApplyValue( a.remove_spaces(arena), &[], &[], arena.alloc(c.remove_spaces(arena)), ), - RecordBuilderField::LabelOnly(a) => { - RecordBuilderField::LabelOnly(a.remove_spaces(arena)) + OldRecordBuilderField::LabelOnly(a) => { + OldRecordBuilderField::LabelOnly(a.remove_spaces(arena)) } - RecordBuilderField::Malformed(a) => RecordBuilderField::Malformed(a), - RecordBuilderField::SpaceBefore(a, _) => a.remove_spaces(arena), - RecordBuilderField::SpaceAfter(a, _) => a.remove_spaces(arena), + OldRecordBuilderField::Malformed(a) => OldRecordBuilderField::Malformed(a), + OldRecordBuilderField::SpaceBefore(a, _) => a.remove_spaces(arena), + OldRecordBuilderField::SpaceAfter(a, _) => a.remove_spaces(arena), } } } @@ -790,7 +790,11 @@ impl<'a> RemoveSpaces<'a> for Expr<'a> { fields: fields.remove_spaces(arena), }, Expr::Record(a) => Expr::Record(a.remove_spaces(arena)), - Expr::RecordBuilder(a) => Expr::RecordBuilder(a.remove_spaces(arena)), + Expr::OldRecordBuilder(a) => Expr::OldRecordBuilder(a.remove_spaces(arena)), + Expr::NewRecordBuilder { mapper, fields } => Expr::NewRecordBuilder { + mapper: arena.alloc(mapper.remove_spaces(arena)), + fields: fields.remove_spaces(arena), + }, Expr::Tuple(a) => Expr::Tuple(a.remove_spaces(arena)), Expr::Var { module_name, ident } => Expr::Var { module_name, ident }, Expr::Underscore(a) => Expr::Underscore(a), @@ -857,8 +861,13 @@ impl<'a> RemoveSpaces<'a> for Expr<'a> { Expr::MalformedClosure => Expr::MalformedClosure, Expr::MalformedSuffixed(a) => Expr::MalformedSuffixed(a), Expr::PrecedenceConflict(a) => Expr::PrecedenceConflict(a), - Expr::MultipleRecordBuilders(a) => Expr::MultipleRecordBuilders(a), - Expr::UnappliedRecordBuilder(a) => Expr::UnappliedRecordBuilder(a), + Expr::MultipleOldRecordBuilders(a) => Expr::MultipleOldRecordBuilders(a), + Expr::UnappliedOldRecordBuilder(a) => Expr::UnappliedOldRecordBuilder(a), + Expr::EmptyNewRecordBuilder(a) => Expr::EmptyNewRecordBuilder(a), + Expr::SingleFieldNewRecordBuilder(a) => Expr::SingleFieldNewRecordBuilder(a), + Expr::OptionalFieldInNewRecordBuilder(name, a) => { + Expr::OptionalFieldInNewRecordBuilder(name, a) + } Expr::SpaceBefore(a, _) => a.remove_spaces(arena), Expr::SpaceAfter(a, _) => a.remove_spaces(arena), Expr::SingleQuote(a) => Expr::Num(a), diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index d73f10d3999..a295bc9707c 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -4972,7 +4972,7 @@ mod test_reporting { } " ),@r###" - ── RECORD BUILDER IN MODULE PARAMS in ...ord_builder_in_module_params/Test.roc ─ + ── OLD-STYLE RECORD BUILDER IN MODULE PARAMS in ...r_in_module_params/Test.roc ─ I was partway through parsing module params, but I got stuck here: @@ -4981,8 +4981,8 @@ mod test_reporting { 6│ name: <- applyName ^^^^^^^^^^^^^^^^^^ - This looks like a record builder field, but those are not allowed in - module params. + This looks like an old-style record builder field, but those are not + allowed in module params. "### ); @@ -10699,7 +10699,7 @@ In roc, functions are always written as a lambda, like{} // Record Builders test_report!( - optional_field_in_record_builder, + optional_field_in_old_record_builder, indoc!( r#" { @@ -10710,7 +10710,7 @@ In roc, functions are always written as a lambda, like{} "# ), @r#" - ── BAD RECORD BUILDER in tmp/optional_field_in_record_builder/Test.roc ───────── + ── BAD OLD-STYLE RECORD BUILDER in ...nal_field_in_old_record_builder/Test.roc ─ I am partway through parsing a record builder, and I found an optional field: @@ -10729,7 +10729,7 @@ In roc, functions are always written as a lambda, like{} ); test_report!( - record_update_builder, + record_update_old_builder, indoc!( r#" { rec & @@ -10739,10 +10739,10 @@ In roc, functions are always written as a lambda, like{} "# ), @r#" - ── BAD RECORD UPDATE in tmp/record_update_builder/Test.roc ───────────────────── + ── BAD RECORD UPDATE in tmp/record_update_old_builder/Test.roc ───────────────── - I am partway through parsing a record update, and I found a record - builder field: + I am partway through parsing a record update, and I found an old-style + record builder field: 1│ app "test" provides [main] to "./platform" 2│ @@ -10751,12 +10751,12 @@ In roc, functions are always written as a lambda, like{} 5│ a: <- apply "a", ^^^^^^^^^^^^^^^ - Record builders cannot be updated like records. + Old-style record builders cannot be updated like records. "# ); test_report!( - multiple_record_builders, + multiple_old_record_builders, indoc!( r#" succeed @@ -10765,32 +10765,31 @@ In roc, functions are always written as a lambda, like{} "# ), @r#" - ── MULTIPLE RECORD BUILDERS in /code/proj/Main.roc ───────────────────────────── + ── MULTIPLE OLD-STYLE RECORD BUILDERS in /code/proj/Main.roc ─────────────────── - This function is applied to multiple record builders: + This function is applied to multiple old-style record builders: 4│> succeed 5│> { a: <- apply "a" } 6│> { b: <- apply "b" } - Note: Functions can only take at most one record builder! + Note: Functions can only take at most one old-style record builder! Tip: You can combine them or apply them separately. - "# ); test_report!( - unapplied_record_builder, + unapplied_old_record_builder, indoc!( r#" { a: <- apply "a" } "# ), @r#" - ── UNAPPLIED RECORD BUILDER in /code/proj/Main.roc ───────────────────────────── + ── UNAPPLIED OLD-STYLE RECORD BUILDER in /code/proj/Main.roc ─────────────────── - This record builder was not applied to a function: + This old-style record builder was not applied to a function: 4│ { a: <- apply "a" } ^^^^^^^^^^^^^^^^^^^ @@ -10802,7 +10801,7 @@ In roc, functions are always written as a lambda, like{} ); test_report!( - record_builder_apply_non_function, + old_record_builder_apply_non_function, indoc!( r#" succeed = \_ -> crash "" @@ -10856,6 +10855,71 @@ In roc, functions are always written as a lambda, like{} // "# // ); + test_report!( + empty_new_record_builder, + indoc!( + r#" + { a <- } + "# + ), + @r#" + ── EMPTY RECORD BUILDER in /code/proj/Main.roc ───────────────────────────────── + + This record builder has no fields: + + 4│ { a <- } + ^^^^^^^^ + + Note: We need at least two fields to combine their values into a record. + "# + ); + + test_report!( + single_field_new_record_builder, + indoc!( + r#" + { a <- + b: 123 + } + "# + ), + @r#" + ── NOT ENOUGH FIELDS IN RECORD BUILDER in /code/proj/Main.roc ────────────────── + + This record builder only has one field: + + 4│> { a <- + 5│> b: 123 + 6│> } + + Note: We need at least two fields to combine their values into a record. + "# + ); + + test_report!( + optional_field_in_new_record_builder, + indoc!( + r#" + { a <- + 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│ } + + Note: Record builders can only have required values for their fields. + "# + ); + test_report!( destructure_assignment_introduces_no_variables_nested, indoc!( diff --git a/crates/compiler/module/src/called_via.rs b/crates/compiler/module/src/called_via.rs index ca6ff45e5b1..16428c593d2 100644 --- a/crates/compiler/module/src/called_via.rs +++ b/crates/compiler/module/src/called_via.rs @@ -89,9 +89,22 @@ pub enum CalledVia { /// e.g. "$(first) $(last)" is transformed into Str.concat (Str.concat first " ") last. StringInterpolation, - /// This call is the result of desugaring a Record Builder field. + /// This call is the result of desugaring an old style Record Builder field. /// e.g. succeed { a <- get "a" } is transformed into (get "a") (succeed \a -> { a }) - RecordBuilder, + OldRecordBuilder, + + /// This call is the result of desugaring a map2-based Record Builder field. e.g. + /// ```roc + /// { Task.parallel <- + /// foo: get "a", + /// bar: get "b", + /// } + /// ``` + /// is transformed into + /// ```roc + /// Task.parallel (get "a") (get "b") \foo, bar -> { foo, bar } + /// ``` + NewRecordBuilder, /// This call is the result of desugaring a Task.await from `!` syntax /// e.g. Stdout.line! "Hello" becomes Task.await (Stdout.line "Hello") \{} -> ... diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index a3371688b0a..6e5fa5e9aeb 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -438,7 +438,21 @@ pub enum Expr<'a> { Tuple(Collection<'a, &'a Loc>>), // Record Builders - RecordBuilder(Collection<'a, Loc>>), + /// Applicative record builders, e.g. + /// build { + /// foo: <- getData Foo, + /// bar: <- getData Bar, + /// } + OldRecordBuilder(Collection<'a, Loc>>), + /// Mapper-based record builders, e.g. + /// { Task.parallel <- + /// foo: Task.getData Foo, + /// bar: Task.getData Bar, + /// } + NewRecordBuilder { + mapper: &'a Loc>, + fields: Collection<'a, Loc>>>, + }, // Lookups Var { @@ -501,8 +515,11 @@ pub enum Expr<'a> { // Both operators were non-associative, e.g. (True == False == False). // We should tell the author to disambiguate by grouping them with parens. PrecedenceConflict(&'a PrecedenceConflict<'a>), - MultipleRecordBuilders(&'a Loc>), - UnappliedRecordBuilder(&'a Loc>), + MultipleOldRecordBuilders(&'a Loc>), + UnappliedOldRecordBuilder(&'a Loc>), + EmptyNewRecordBuilder(&'a Loc>), + SingleFieldNewRecordBuilder(&'a Loc>), + OptionalFieldInNewRecordBuilder(&'a Loc<&'a str>, &'a Loc>), } impl Expr<'_> { @@ -615,9 +632,12 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool { .iter() .any(|field| is_assigned_value_suffixed(&field.value)), Expr::Tuple(items) => items.iter().any(|x| is_expr_suffixed(&x.value)), - Expr::RecordBuilder(items) => items + Expr::OldRecordBuilder(items) => items .iter() .any(|rbf| is_record_builder_field_suffixed(&rbf.value)), + Expr::NewRecordBuilder { mapper: _, fields } => fields + .iter() + .any(|field| is_assigned_value_suffixed(&field.value)), Expr::Underscore(_) => false, Expr::Crash => false, Expr::Tag(_) => false, @@ -637,8 +657,11 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool { Expr::MalformedClosure => false, Expr::MalformedSuffixed(_) => false, Expr::PrecedenceConflict(_) => false, - Expr::MultipleRecordBuilders(_) => false, - Expr::UnappliedRecordBuilder(_) => false, + Expr::MultipleOldRecordBuilders(_) => false, + Expr::UnappliedOldRecordBuilder(_) => false, + Expr::EmptyNewRecordBuilder(_) => false, + Expr::SingleFieldNewRecordBuilder(_) => false, + Expr::OptionalFieldInNewRecordBuilder(_, _) => false, } } @@ -663,14 +686,14 @@ fn is_assigned_value_suffixed<'a>(value: &AssignedField<'a, Expr<'a>>) -> bool { } } -fn is_record_builder_field_suffixed(field: &RecordBuilderField<'_>) -> bool { +fn is_record_builder_field_suffixed(field: &OldRecordBuilderField<'_>) -> bool { match field { - RecordBuilderField::Value(_, _, a) => is_expr_suffixed(&a.value), - RecordBuilderField::ApplyValue(_, _, _, a) => is_expr_suffixed(&a.value), - RecordBuilderField::LabelOnly(_) => false, - RecordBuilderField::SpaceBefore(a, _) => is_record_builder_field_suffixed(a), - RecordBuilderField::SpaceAfter(a, _) => is_record_builder_field_suffixed(a), - RecordBuilderField::Malformed(_) => false, + OldRecordBuilderField::Value(_, _, a) => is_expr_suffixed(&a.value), + OldRecordBuilderField::ApplyValue(_, _, _, a) => is_expr_suffixed(&a.value), + OldRecordBuilderField::LabelOnly(_) => false, + OldRecordBuilderField::SpaceBefore(a, _) => is_record_builder_field_suffixed(a), + OldRecordBuilderField::SpaceAfter(a, _) => is_record_builder_field_suffixed(a), + OldRecordBuilderField::Malformed(_) => false, } } @@ -840,10 +863,10 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { use AssignedField::*; match current { - RequiredValue(_, _, loc_val) => break expr_stack.push(&loc_val.value), - OptionalValue(_, _, loc_val) => break expr_stack.push(&loc_val.value), - SpaceBefore(next, _) => current = *next, - SpaceAfter(next, _) => current = *next, + RequiredValue(_, _, loc_val) | OptionalValue(_, _, loc_val) => { + break expr_stack.push(&loc_val.value) + } + SpaceBefore(next, _) | SpaceAfter(next, _) => current = *next, LabelOnly(_) | Malformed(_) => break, } } @@ -881,13 +904,13 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { expr_stack.push(&loc_expr.value); } } - RecordBuilder(fields) => { + OldRecordBuilder(fields) => { expr_stack.reserve(fields.len()); for loc_record_builder_field in fields.items { let mut current_field = loc_record_builder_field.value; loop { - use RecordBuilderField::*; + use OldRecordBuilderField::*; match current_field { Value(_, _, loc_val) => break expr_stack.push(&loc_val.value), @@ -901,6 +924,14 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { } } } + NewRecordBuilder { + mapper: map2, + fields, + } => { + expr_stack.reserve(fields.len() + 1); + expr_stack.push(&map2.value); + push_stack_from_record_fields!(fields); + } Closure(_, body) => expr_stack.push(&body.value), Backpassing(_, a, b) => { expr_stack.reserve(2); @@ -965,9 +996,11 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { | SpaceAfter(expr, _) | ParensAround(expr) => expr_stack.push(expr), - MultipleRecordBuilders(loc_expr) | UnappliedRecordBuilder(loc_expr) => { - expr_stack.push(&loc_expr.value) - } + MultipleOldRecordBuilders(loc_expr) + | UnappliedOldRecordBuilder(loc_expr) + | EmptyNewRecordBuilder(loc_expr) + | SingleFieldNewRecordBuilder(loc_expr) + | OptionalFieldInNewRecordBuilder(_, loc_expr) => expr_stack.push(&loc_expr.value), Float(_) | Num(_) @@ -1568,7 +1601,7 @@ impl<'a, Val> AssignedField<'a, Val> { } #[derive(Debug, Clone, Copy, PartialEq)] -pub enum RecordBuilderField<'a> { +pub enum OldRecordBuilderField<'a> { // A field with a value, e.g. `{ name: "blah" }` Value(Loc<&'a str>, &'a [CommentOrNewline<'a>], &'a Loc>), @@ -1584,8 +1617,8 @@ pub enum RecordBuilderField<'a> { LabelOnly(Loc<&'a str>), // We preserve this for the formatter; canonicalization ignores it. - SpaceBefore(&'a RecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), - SpaceAfter(&'a RecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), + SpaceBefore(&'a OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), + SpaceAfter(&'a OldRecordBuilderField<'a>, &'a [CommentOrNewline<'a>]), /// A malformed assigned field, which will code gen to a runtime error Malformed(&'a str), @@ -2099,12 +2132,12 @@ impl<'a, Val> Spaceable<'a> for AssignedField<'a, Val> { } } -impl<'a> Spaceable<'a> for RecordBuilderField<'a> { +impl<'a> Spaceable<'a> for OldRecordBuilderField<'a> { fn before(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self { - RecordBuilderField::SpaceBefore(self, spaces) + OldRecordBuilderField::SpaceBefore(self, spaces) } fn after(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self { - RecordBuilderField::SpaceAfter(self, spaces) + OldRecordBuilderField::SpaceAfter(self, spaces) } } @@ -2403,7 +2436,8 @@ impl<'a> Malformed for Expr<'a> { Record(items) => items.is_malformed(), Tuple(items) => items.is_malformed(), - RecordBuilder(items) => items.is_malformed(), + OldRecordBuilder(items) => items.is_malformed(), + NewRecordBuilder { 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(), @@ -2425,8 +2459,11 @@ impl<'a> Malformed for Expr<'a> { MalformedClosure | MalformedSuffixed(..) | PrecedenceConflict(_) | - MultipleRecordBuilders(_) | - UnappliedRecordBuilder(_) => true, + MultipleOldRecordBuilders(_) | + UnappliedOldRecordBuilder(_) | + EmptyNewRecordBuilder(_) | + SingleFieldNewRecordBuilder(_) | + OptionalFieldInNewRecordBuilder(_, _) => true, } } } @@ -2503,15 +2540,15 @@ impl<'a, T: Malformed> Malformed for AssignedField<'a, T> { } } -impl<'a> Malformed for RecordBuilderField<'a> { +impl<'a> Malformed for OldRecordBuilderField<'a> { fn is_malformed(&self) -> bool { match self { - RecordBuilderField::Value(_, _, expr) - | RecordBuilderField::ApplyValue(_, _, _, expr) => expr.is_malformed(), - RecordBuilderField::LabelOnly(_) => false, - RecordBuilderField::SpaceBefore(field, _) - | RecordBuilderField::SpaceAfter(field, _) => field.is_malformed(), - RecordBuilderField::Malformed(_) => true, + OldRecordBuilderField::Value(_, _, expr) + | OldRecordBuilderField::ApplyValue(_, _, _, expr) => expr.is_malformed(), + OldRecordBuilderField::LabelOnly(_) => false, + OldRecordBuilderField::SpaceBefore(field, _) + | OldRecordBuilderField::SpaceAfter(field, _) => field.is_malformed(), + OldRecordBuilderField::Malformed(_) => true, } } } diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index f02dfe79beb..36e378ccdf5 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -2,7 +2,7 @@ use crate::ast::{ is_expr_suffixed, is_top_level_suffixed, AssignedField, Collection, CommentOrNewline, Defs, Expr, ExtractSpaces, Implements, ImplementsAbilities, ImportAlias, ImportAsKeyword, ImportExposingKeyword, ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, - ModuleImport, ModuleImportParams, Pattern, RecordBuilderField, Spaceable, Spaced, Spaces, + ModuleImport, ModuleImportParams, OldRecordBuilderField, Pattern, Spaceable, Spaced, Spaces, TypeAnnotation, TypeDef, TypeHeader, ValueDef, }; use crate::blankspace::{ @@ -996,11 +996,21 @@ fn import_params<'a>() -> impl Parser<'a, ModuleImportParams<'a>, EImportParams< specialize_err(EImportParams::Record, record_help()), ), |arena, state, _, (before, record): (_, RecordHelp<'a>)| { - if let Some(update) = record.update { - return Err(( - MadeProgress, - EImportParams::RecordUpdateFound(update.region), - )); + if let Some(prefix) = record.prefix { + match prefix { + RecordHelpPrefix::Update(update) => { + return Err(( + MadeProgress, + EImportParams::RecordUpdateFound(update.region), + )) + } + RecordHelpPrefix::Mapper(mapper) => { + return Err(( + MadeProgress, + EImportParams::RecordBuilderFound(mapper.region), + )) + } + } } let params = record.fields.map_items_result(arena, |loc_field| { @@ -2370,7 +2380,8 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result unreachable!(), + | Expr::OldRecordBuilder(..) + | Expr::NewRecordBuilder { .. } => unreachable!(), Expr::Record(fields) => { let patterns = fields.map_items_result(arena, |loc_assigned_field| { @@ -2417,8 +2428,11 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result RecordField<'a> { fn to_builder_field( self, arena: &'a Bump, - ) -> Result, FoundOptionalValue> { - use RecordBuilderField::*; + ) -> Result, FoundOptionalValue> { + use OldRecordBuilderField::*; match self { RecordField::RequiredValue(loc_label, spaces, loc_expr) => { @@ -3234,8 +3248,20 @@ fn record_updateable_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>> ) } +fn record_builder_mapper_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>> { + specialize_err( + |_, pos| ERecord::BuilderMapper(pos), + map_with_arena(parse_ident, ident_to_expr), + ) +} + +enum RecordHelpPrefix<'a> { + Update(Loc>), + Mapper(Loc>), +} + struct RecordHelp<'a> { - update: Option>>, + prefix: Option>, fields: Collection<'a, Loc>>, } @@ -3245,16 +3271,28 @@ 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" }. - update: optional(backtrackable(skip_second( - spaces_around( - // We wrap the ident in an Expr here, - // so that we have a Spaceable value to work with, - // and then in canonicalization verify that it's an Expr::Var - // (and not e.g. an `Expr::Access`) and extract its string. - loc(record_updateable_identifier()), + prefix: optional(map_with_arena( + either( + backtrackable(skip_second( + spaces_around( + // We wrap the ident in an Expr here, + // so that we have a Spaceable value to work with, + // and then in canonicalization verify that it's an Expr::Var + // (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), + )), ), - byte(b'&', ERecord::Ampersand) - ))), + |_arena, output| match output { + Either::First(update) => RecordHelpPrefix::Update(update), + Either::Second(mapper) => RecordHelpPrefix::Mapper(mapper), + } + )), fields: collection_inner( loc(record_field()), byte(b',', ERecord::End), @@ -3273,16 +3311,21 @@ fn record_literal_help<'a>() -> impl Parser<'a, Expr<'a>, EExpr<'a>> { record_field_access_chain(), ), move |arena, state, _, (record, accessors)| { - let expr_result = match record.update { - Some(update) => record_update_help(arena, update, record.fields), + let expr_result = match record.prefix { + Some(RecordHelpPrefix::Update(update)) => { + record_update_help(arena, update, record.fields) + } + Some(RecordHelpPrefix::Mapper(mapper)) => { + new_record_builder_help(arena, mapper, record.fields) + } None => { - let is_record_builder = record + let is_old_record_builder = record .fields .iter() .any(|field| field.value.is_apply_value()); - if is_record_builder { - record_builder_help(arena, record.fields) + if is_old_record_builder { + old_record_builder_help(arena, record.fields) } else { let fields = record.fields.map_items(arena, |loc_field| { loc_field.map(|field| field.to_assigned_field(arena).unwrap()) @@ -3316,7 +3359,7 @@ fn record_update_help<'a>( region: loc_field.region, value: builder_field, }), - Err(FoundApplyValue) => Err(EExpr::RecordUpdateBuilder(loc_field.region)), + Err(FoundApplyValue) => Err(EExpr::RecordUpdateAccumulator(loc_field.region)), } }); @@ -3326,7 +3369,28 @@ fn record_update_help<'a>( }) } -fn record_builder_help<'a>( +fn new_record_builder_help<'a>( + arena: &'a Bump, + mapper: Loc>, + fields: Collection<'a, Loc>>, +) -> Result, EExpr<'a>> { + let result = fields.map_items_result(arena, |loc_field| { + match loc_field.value.to_assigned_field(arena) { + Ok(builder_field) => Ok(Loc { + region: loc_field.region, + value: builder_field, + }), + Err(FoundApplyValue) => Err(EExpr::RecordBuilderAccumulator(loc_field.region)), + } + }); + + result.map(|fields| Expr::NewRecordBuilder { + mapper: &*arena.alloc(mapper), + fields, + }) +} + +fn old_record_builder_help<'a>( arena: &'a Bump, fields: Collection<'a, Loc>>, ) -> Result, EExpr<'a>> { @@ -3340,7 +3404,7 @@ fn record_builder_help<'a>( } }); - result.map(Expr::RecordBuilder) + result.map(Expr::OldRecordBuilder) } fn apply_expr_access_chain<'a>( diff --git a/crates/compiler/parse/src/parser.rs b/crates/compiler/parse/src/parser.rs index 93a6b1554ed..059440b5188 100644 --- a/crates/compiler/parse/src/parser.rs +++ b/crates/compiler/parse/src/parser.rs @@ -370,7 +370,8 @@ pub enum EExpr<'a> { InParens(EInParens<'a>, Position), Record(ERecord<'a>, Position), OptionalValueInRecordBuilder(Region), - RecordUpdateBuilder(Region), + RecordUpdateAccumulator(Region), + RecordBuilderAccumulator(Region), // SingleQuote errors are folded into the EString Str(EString<'a>, Position), @@ -422,6 +423,7 @@ pub enum ERecord<'a> { Open(Position), Updateable(Position), + BuilderMapper(Position), Field(Position), Colon(Position), QuestionMark(Position), @@ -570,6 +572,7 @@ pub enum EImportParams<'a> { Indent(Position), Record(ERecord<'a>, Position), RecordUpdateFound(Region), + RecordBuilderFound(Region), RecordApplyFound(Region), Space(BadInputError, Position), } @@ -737,6 +740,7 @@ pub enum ETypeAbilityImpl<'a> { Space(BadInputError, Position), Updateable(Position), + BuilderMapper(Position), QuestionMark(Position), Ampersand(Position), Expr(&'a EExpr<'a>, Position), @@ -754,6 +758,7 @@ impl<'a> From> for ETypeAbilityImpl<'a> { ERecord::Arrow(p) => ETypeAbilityImpl::Arrow(p), ERecord::Space(s, p) => ETypeAbilityImpl::Space(s, p), ERecord::Updateable(p) => ETypeAbilityImpl::Updateable(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), diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index c935d230496..6ee1eca2f0d 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -412,8 +412,14 @@ impl Problem { | Problem::RuntimeError(RuntimeError::EmptySingleQuote(region)) | Problem::RuntimeError(RuntimeError::MultipleCharsInSingleQuote(region)) | Problem::RuntimeError(RuntimeError::DegenerateBranch(region)) - | Problem::RuntimeError(RuntimeError::MultipleRecordBuilders(region)) - | Problem::RuntimeError(RuntimeError::UnappliedRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::MultipleOldRecordBuilders(region)) + | Problem::RuntimeError(RuntimeError::UnappliedOldRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::EmptyNewRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::SingleFieldNewRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::OptionalFieldInNewRecordBuilder { + record: _, + field: region, + }) | Problem::RuntimeError(RuntimeError::ReadIngestedFileError { region, .. }) | Problem::InvalidAliasRigid { region, .. } | Problem::InvalidInterpolation(region) @@ -663,8 +669,15 @@ pub enum RuntimeError { DegenerateBranch(Region), - MultipleRecordBuilders(Region), - UnappliedRecordBuilder(Region), + MultipleOldRecordBuilders(Region), + UnappliedOldRecordBuilder(Region), + + EmptyNewRecordBuilder(Region), + SingleFieldNewRecordBuilder(Region), + OptionalFieldInNewRecordBuilder { + record: Region, + field: Region, + }, MalformedSuffixed(Region), } @@ -709,8 +722,14 @@ impl RuntimeError { | RuntimeError::DegenerateBranch(region) | RuntimeError::InvalidInterpolation(region) | RuntimeError::InvalidHexadecimal(region) - | RuntimeError::MultipleRecordBuilders(region) - | RuntimeError::UnappliedRecordBuilder(region) + | RuntimeError::MultipleOldRecordBuilders(region) + | RuntimeError::UnappliedOldRecordBuilder(region) + | RuntimeError::EmptyNewRecordBuilder(region) + | RuntimeError::SingleFieldNewRecordBuilder(region) + | RuntimeError::OptionalFieldInNewRecordBuilder { + record: _, + field: region, + } | RuntimeError::ReadIngestedFileError { region, .. } => *region, RuntimeError::InvalidUnicodeCodePt(region) => *region, RuntimeError::UnresolvedTypeVar | RuntimeError::ErroneousType => Region::zero(), diff --git a/crates/compiler/test_syntax/tests/test_fmt.rs b/crates/compiler/test_syntax/tests/test_fmt.rs index 23e8d26f690..b57277a70ac 100644 --- a/crates/compiler/test_syntax/tests/test_fmt.rs +++ b/crates/compiler/test_syntax/tests/test_fmt.rs @@ -1979,7 +1979,99 @@ mod test_fmt { } #[test] - fn record_builder() { + fn new_record_builder() { + expr_formats_same(indoc!( + r" + { shoes <- leftShoe: nothing } + " + )); + + expr_formats_to( + indoc!( + r" + { shoes <- rightShoe : nothing } + " + ), + indoc!( + r" + { shoes <- rightShoe: nothing } + " + ), + ); + + expr_formats_to( + indoc!( + r" + { shoes <- rightShoe : nothing } + " + ), + indoc!( + r" + { shoes <- rightShoe: nothing } + " + ), + ); + + expr_formats_same(indoc!( + r" + { shoes <- + rightShoe, + leftShoe: newLeftShoe, + } + " + )); + + expr_formats_same(indoc!( + r" + { shoes <- + # some comment + rightShoe, + # some other comment + leftShoe: newLeftShoe, + } + " + )); + + expr_formats_to( + indoc!( + r" + { shoes + <- rightShoe: bareFoot + , leftShoe: bareFoot } + " + ), + indoc!( + r" + { shoes <- + rightShoe: bareFoot, + leftShoe: bareFoot, + } + " + ), + ); + + expr_formats_to( + indoc!( + r" + { shoes + <- rightShoe: bareFoot, # some comment + leftShoe: bareFoot } + " + ), + indoc!( + r" + { shoes <- + rightShoe: bareFoot, + # some comment + leftShoe: bareFoot, + } + " + ), + ); + } + + #[test] + fn old_record_builder() { expr_formats_same(indoc!( r#" { a: 1, b: <- get "b" |> batch, c: <- get "c" |> batch, d } diff --git a/crates/language_server/src/analysis/tokens.rs b/crates/language_server/src/analysis/tokens.rs index 869817080bf..797b786c350 100644 --- a/crates/language_server/src/analysis/tokens.rs +++ b/crates/language_server/src/analysis/tokens.rs @@ -6,8 +6,8 @@ use roc_module::called_via::{BinOp, UnaryOp}; use roc_parse::{ ast::{ AbilityImpls, AbilityMember, AssignedField, Collection, Defs, Expr, Header, Implements, - ImplementsAbilities, ImplementsAbility, ImplementsClause, Module, Pattern, PatternAs, - RecordBuilderField, Spaced, StrLiteral, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, + ImplementsAbilities, ImplementsAbility, ImplementsClause, Module, OldRecordBuilderField, + Pattern, PatternAs, Spaced, StrLiteral, Tag, TypeAnnotation, TypeDef, TypeHeader, ValueDef, WhenBranch, }, header::{ @@ -683,7 +683,10 @@ impl IterTokens for Loc> { .collect_in(arena), Expr::Record(rcd) => rcd.iter_tokens(arena), Expr::Tuple(tup) => tup.iter_tokens(arena), - Expr::RecordBuilder(rb) => rb.iter_tokens(arena), + Expr::OldRecordBuilder(rb) => rb.iter_tokens(arena), + Expr::NewRecordBuilder { 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), Expr::Underscore(_) => onetoken(Token::Variable, region, arena), Expr::Crash => onetoken(Token::Keyword, region, arena), @@ -727,8 +730,11 @@ impl IterTokens for Loc> { Loc::at(region, *e).iter_tokens(arena) } Expr::ParensAround(e) => Loc::at(region, *e).iter_tokens(arena), - Expr::MultipleRecordBuilders(e) => e.iter_tokens(arena), - Expr::UnappliedRecordBuilder(e) => 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::SingleFieldNewRecordBuilder(e) => e.iter_tokens(arena), + Expr::OptionalFieldInNewRecordBuilder(_name, e) => e.iter_tokens(arena), Expr::MalformedIdent(_, _) | Expr::MalformedClosure | Expr::PrecedenceConflict(_) @@ -748,19 +754,20 @@ impl IterTokens for Loc> { } } -impl IterTokens for Loc> { +impl IterTokens for Loc> { fn iter_tokens<'a>(&self, arena: &'a Bump) -> BumpVec<'a, Loc> { match self.value { - RecordBuilderField::Value(field, _, e) - | RecordBuilderField::ApplyValue(field, _, _, e) => field_token(field.region, arena) + OldRecordBuilderField::Value(field, _, e) + | OldRecordBuilderField::ApplyValue(field, _, _, e) => field_token(field.region, arena) .into_iter() .chain(e.iter_tokens(arena)) .collect_in(arena), - RecordBuilderField::LabelOnly(field) => field_token(field.region, arena), - RecordBuilderField::SpaceBefore(rbf, _) | RecordBuilderField::SpaceAfter(rbf, _) => { + OldRecordBuilderField::LabelOnly(field) => field_token(field.region, arena), + OldRecordBuilderField::SpaceBefore(rbf, _) + | OldRecordBuilderField::SpaceAfter(rbf, _) => { Loc::at(self.region, *rbf).iter_tokens(arena) } - RecordBuilderField::Malformed(_) => bumpvec![in arena;], + OldRecordBuilderField::Malformed(_) => bumpvec![in arena;], } } } diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 0033cc7d0de..3f28ae03aab 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -2421,23 +2421,23 @@ fn pretty_runtime_error<'b>( title = "DEGENERATE BRANCH"; } - RuntimeError::MultipleRecordBuilders(region) => { + RuntimeError::MultipleOldRecordBuilders(region) => { let tip = alloc .tip() .append(alloc.reflow("You can combine them or apply them separately.")); doc = alloc.stack([ - alloc.reflow("This function is applied to multiple record builders:"), + alloc.reflow("This function is applied to multiple old-style record builders:"), alloc.region(lines.convert_region(region), severity), - alloc.note("Functions can only take at most one record builder!"), + alloc.note("Functions can only take at most one old-style record builder!"), tip, ]); - title = "MULTIPLE RECORD BUILDERS"; + title = "MULTIPLE OLD-STYLE RECORD BUILDERS"; } - RuntimeError::UnappliedRecordBuilder(region) => { + RuntimeError::UnappliedOldRecordBuilder(region) => { doc = alloc.stack([ - alloc.reflow("This record builder was not applied to a function:"), + alloc.reflow("This old-style record builder was not applied to a function:"), alloc.region(lines.convert_region(region), severity), alloc.reflow("However, we need a function to construct the record."), alloc.note( @@ -2445,7 +2445,41 @@ fn pretty_runtime_error<'b>( ), ]); - title = "UNAPPLIED RECORD BUILDER"; + title = "UNAPPLIED OLD-STYLE RECORD BUILDER"; + } + RuntimeError::EmptyNewRecordBuilder(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."), + ]); + + title = "EMPTY RECORD BUILDER"; + } + RuntimeError::SingleFieldNewRecordBuilder(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."), + ]); + + title = "NOT ENOUGH FIELDS IN RECORD BUILDER"; + } + RuntimeError::OptionalFieldInNewRecordBuilder { + record: record_region, + field: field_region, + } => { + doc = alloc.stack([ + alloc.reflow("Optional fields are not allowed to be used in record builders."), + alloc.region_with_subregion( + lines.convert_region(record_region), + lines.convert_region(field_region), + severity, + ), + alloc.note("Record builders can only have required values for their fields."), + ]); + + title = "OPTIONAL FIELD IN RECORD BUILDER"; } } diff --git a/crates/reporting/src/error/parse.rs b/crates/reporting/src/error/parse.rs index f4d943e70f1..111c523ccc1 100644 --- a/crates/reporting/src/error/parse.rs +++ b/crates/reporting/src/error/parse.rs @@ -560,21 +560,21 @@ fn to_expr_report<'a>( Report { filename, doc, - title: "BAD RECORD BUILDER".to_string(), + title: "BAD OLD-STYLE RECORD BUILDER".to_string(), severity, } } - EExpr::RecordUpdateBuilder(region) => { + EExpr::RecordUpdateAccumulator(region) => { let surroundings = Region::new(start, region.end()); let region = lines.convert_region(*region); let doc = alloc.stack([ alloc.reflow( - r"I am partway through parsing a record update, and I found a record builder field:", + r"I am partway through parsing a record update, and I found an old-style record builder field:", ), alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), - alloc.reflow("Record builders cannot be updated like records."), + alloc.reflow("Old-style record builders cannot be updated like records."), ]); Report { @@ -1547,13 +1547,13 @@ fn to_import_report<'a>( let doc = alloc.stack([ alloc.reflow("I was partway through parsing module params, but I got stuck here:"), alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), - alloc.reflow("This looks like a record builder field, but those are not allowed in module params."), + alloc.reflow("This looks like an old-style record builder field, but those are not allowed in module params."), ]); Report { filename, doc, - title: "RECORD BUILDER IN MODULE PARAMS".to_string(), + title: "OLD-STYLE RECORD BUILDER IN MODULE PARAMS".to_string(), severity, } } @@ -1574,6 +1574,23 @@ fn to_import_report<'a>( severity, } } + Params(EImportParams::RecordBuilderFound(region), _) => { + let surroundings = Region::new(start, region.end()); + let region = lines.convert_region(*region); + + let doc = alloc.stack([ + alloc.reflow("I was partway through parsing module params, but I got stuck here:"), + alloc.region_with_subregion(lines.convert_region(surroundings), region, severity), + alloc.reflow("It looks like you're trying to use a record builder, but module params require a standalone record literal."), + ]); + + Report { + filename, + doc, + title: "RECORD BUILDER IN MODULE PARAMS".to_string(), + severity, + } + } IndentAlias(pos) | Alias(pos) => to_unfinished_import_report( alloc, lines, diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 1b3d9e4062e..582b9b3d243 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -1145,7 +1145,7 @@ fn to_expr_report<'b>( ]), alloc.region(lines.convert_region(expr_region), severity), match called_via { - CalledVia::RecordBuilder => { + CalledVia::OldRecordBuilder => { alloc.hint("Did you mean to apply it to a function first?") }, _ => { @@ -1167,7 +1167,7 @@ fn to_expr_report<'b>( ]), alloc.region(lines.convert_region(expr_region), severity), match called_via { - CalledVia::RecordBuilder => { + CalledVia::OldRecordBuilder => { alloc.concat([ alloc.tip(), alloc.reflow("Remove "), @@ -1175,6 +1175,14 @@ fn to_expr_report<'b>( alloc.reflow(" to assign the field directly.") ]) } + CalledVia::NewRecordBuilder => { + 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?") } From fe1b6d71fc7b7c4b74b6e4a0d87d975cd05aa1f5 Mon Sep 17 00:00:00 2001 From: Sam Mohr Date: Sun, 7 Jul 2024 18:33:20 -0700 Subject: [PATCH 2/3] Update from PR comments --- crates/cli/tests/cli/combine-tasks.roc | 22 +++ crates/cli/tests/cli/parse-args.roc | 87 +++++++++++ crates/cli/tests/cli_run.rs | 32 ++++ crates/compiler/can/src/desugar.rs | 144 +++++++++++------- crates/compiler/can/src/expr.rs | 22 +-- crates/compiler/can/tests/test_can.rs | 12 +- crates/compiler/fmt/src/expr.rs | 16 +- crates/compiler/fmt/src/spaces.rs | 10 +- crates/compiler/load/tests/test_reporting.rs | 49 +++++- crates/compiler/module/src/called_via.rs | 2 +- crates/compiler/parse/src/ast.rs | 32 ++-- crates/compiler/parse/src/expr.rs | 76 ++++----- crates/compiler/parse/src/parser.rs | 9 +- crates/compiler/problem/src/can.rs | 18 +-- crates/language_server/src/analysis/tokens.rs | 8 +- crates/reporting/src/error/canonicalize.rs | 12 +- crates/reporting/src/error/type.rs | 2 +- 17 files changed, 377 insertions(+), 176 deletions(-) create mode 100644 crates/cli/tests/cli/combine-tasks.roc create mode 100644 crates/cli/tests/cli/parse-args.roc diff --git a/crates/cli/tests/cli/combine-tasks.roc b/crates/cli/tests/cli/combine-tasks.roc new file mode 100644 index 00000000000..aacb0539eeb --- /dev/null +++ b/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) diff --git a/crates/cli/tests/cli/parse-args.roc b/crates/cli/tests/cli/parse-args.roc new file mode 100644 index 00000000000..d8595ddc407 --- /dev/null +++ b/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)") diff --git a/crates/cli/tests/cli_run.rs b/crates/cli/tests/cli_run.rs index beddb89d990..6c42c562a57 100644 --- a/crates/cli/tests/cli_run.rs +++ b/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)] diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index 5387af40dbe..051569943ec 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -302,9 +302,9 @@ pub fn desugar_expr<'a>( | PrecedenceConflict { .. } | MultipleOldRecordBuilders(_) | UnappliedOldRecordBuilder(_) - | EmptyNewRecordBuilder(_) - | SingleFieldNewRecordBuilder(_) - | OptionalFieldInNewRecordBuilder { .. } + | EmptyRecordBuilder(_) + | SingleFieldRecordBuilder(_) + | 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_vals = vec![]; + let mut field_names = Vec::with_capacity_in(fields.len(), arena); + 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( - arena.alloc_slice_copy(&[ - 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( + let combiner_closure_in_region = |region| { + let closure_body = Tuple(Collection::with_items( Vec::from_iter_in( [ - &*arena.alloc(Loc::at_zero(Expr::Var { - module_name: "", - ident: "#record_builder_closure_arg_a", - })), - &*arena.alloc(Loc::at_zero(Expr::Var { - module_name: "", - ident: "#record_builder_closure_arg_b", - })), + &*arena.alloc(Loc::at( + region, + Expr::Var { + module_name: "", + ident: "#record_builder_closure_arg_a", + }, + )), + &*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 { - module_name: "", - ident: arena.alloc_str(&format!("#{}", field_name.value)), - })), + arena.alloc(Loc::at( + field_name.region, + 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( - new_mapper, - arena.alloc_slice_copy(&[field_vals[index], inner_combined, combiner_closure]), - CalledVia::NewRecordBuilder, - ))); + inner_combined = arena.alloc(Loc { + region: Region::span_across(&field_vals[index].region, &inner_combined.region), + value: Apply( + 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>, apply_exprs: Vec<'a, &'a Loc>>, } -fn record_builder_arg<'a>( +fn old_record_builder_arg<'a>( arena: &'a Bump, region: Region, fields: Collection<'a, Loc>>, -) -> 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, } diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 44a970ff4c0..5d3f3e18eb8 100644 --- a/crates/compiler/can/src/expr.rs +++ b/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::SingleFieldNewRecordBuilder(loc_expr) - | ast::Expr::OptionalFieldInNewRecordBuilder(_, loc_expr) + | ast::Expr::EmptyRecordBuilder(loc_expr) + | ast::Expr::SingleFieldRecordBuilder(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) diff --git a/crates/compiler/can/tests/test_can.rs b/crates/compiler/can/tests/test_can.rs index 4313fafb4da..429924b43b2 100644 --- a/crates/compiler/can/tests/test_can.rs +++ b/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( - second_arg, - "map2", - CalledVia::NewRecordBuilder, - &out.interns, - ); + let inner_map2_args = + assert_func_call(second_arg, "map2", CalledVia::RecordBuilder, &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), } } diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index 77f942c8118..2f42225f00c 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/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) - | SingleFieldNewRecordBuilder(loc_subexpr) - | OptionalFieldInNewRecordBuilder(_, loc_subexpr) => loc_subexpr.is_multiline(), + | EmptyRecordBuilder(loc_subexpr) + | SingleFieldRecordBuilder(loc_subexpr) + | 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 { .. } => {} - SingleFieldNewRecordBuilder { .. } => {} - OptionalFieldInNewRecordBuilder(_, _) => {} + EmptyRecordBuilder { .. } => {} + SingleFieldRecordBuilder { .. } => {} + OptionalFieldInRecordBuilder(_, _) => {} } } } diff --git a/crates/compiler/fmt/src/spaces.rs b/crates/compiler/fmt/src/spaces.rs index a70e94d4788..7bb057bedc4 100644 --- a/crates/compiler/fmt/src/spaces.rs +++ b/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::SingleFieldNewRecordBuilder(a) => Expr::SingleFieldNewRecordBuilder(a), - Expr::OptionalFieldInNewRecordBuilder(name, a) => { - Expr::OptionalFieldInNewRecordBuilder(name, a) + Expr::EmptyRecordBuilder(a) => Expr::EmptyRecordBuilder(a), + Expr::SingleFieldRecordBuilder(a) => Expr::SingleFieldRecordBuilder(a), + Expr::OptionalFieldInRecordBuilder(name, a) => { + Expr::OptionalFieldInRecordBuilder(name, a) } Expr::SpaceBefore(a, _) => a.remove_spaces(arena), Expr::SpaceAfter(a, _) => a.remove_spaces(arena), diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index a295bc9707c..8d8af2bb7f9 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/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. "# ); diff --git a/crates/compiler/module/src/called_via.rs b/crates/compiler/module/src/called_via.rs index 16428c593d2..72fc2c97810 100644 --- a/crates/compiler/module/src/called_via.rs +++ b/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") \{} -> ... diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index 6e5fa5e9aeb..22ae5d9bb8d 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/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>, fields: Collection<'a, Loc>>>, }, @@ -517,9 +517,9 @@ pub enum Expr<'a> { PrecedenceConflict(&'a PrecedenceConflict<'a>), MultipleOldRecordBuilders(&'a Loc>), UnappliedOldRecordBuilder(&'a Loc>), - EmptyNewRecordBuilder(&'a Loc>), - SingleFieldNewRecordBuilder(&'a Loc>), - OptionalFieldInNewRecordBuilder(&'a Loc<&'a str>, &'a Loc>), + EmptyRecordBuilder(&'a Loc>), + SingleFieldRecordBuilder(&'a Loc>), + OptionalFieldInRecordBuilder(&'a Loc<&'a str>, &'a Loc>), } 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::SingleFieldNewRecordBuilder(_) => false, - Expr::OptionalFieldInNewRecordBuilder(_, _) => false, + Expr::EmptyRecordBuilder(_) => false, + Expr::SingleFieldRecordBuilder(_) => 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) - | SingleFieldNewRecordBuilder(loc_expr) - | OptionalFieldInNewRecordBuilder(_, loc_expr) => expr_stack.push(&loc_expr.value), + | EmptyRecordBuilder(loc_expr) + | SingleFieldRecordBuilder(loc_expr) + | 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(_) | - SingleFieldNewRecordBuilder(_) | - OptionalFieldInNewRecordBuilder(_, _) => true, + EmptyRecordBuilder(_) | + SingleFieldRecordBuilder(_) | + OptionalFieldInRecordBuilder(_, _) => true, } } } diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 36e378ccdf5..abba33fd35c 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/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 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() -> 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>> { - specialize_err( - |_, pos| ERecord::BuilderMapper(pos), - map_with_arena(parse_ident, ident_to_expr), - ) -} - -enum RecordHelpPrefix<'a> { - Update(Loc>), - Mapper(Loc>), +enum RecordHelpPrefix { + Update, + Mapper, } struct RecordHelp<'a> { - prefix: Option>, + prefix: Option<(Loc>, RecordHelpPrefix)>, fields: Collection<'a, Loc>>, } @@ -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( - either( - backtrackable(skip_second( - spaces_around( - // We wrap the ident in an Expr here, - // so that we have a Spaceable value to work with, - // and then in canonicalization verify that it's an Expr::Var - // (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), - )), + prefix: optional(backtrackable(and( + spaces_around( + // We wrap the ident in an Expr here, + // so that we have a Spaceable value to work with, + // and then in canonicalization verify that it's an Expr::Var + // (and not e.g. an `Expr::Access`) and extract its string. + loc(record_prefix_identifier()), ), - |_arena, output| match output { - Either::First(update) => RecordHelpPrefix::Update(update), - Either::Second(mapper) => RecordHelpPrefix::Mapper(mapper), - } - )), + map_with_arena( + either( + 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, }) diff --git a/crates/compiler/parse/src/parser.rs b/crates/compiler/parse/src/parser.rs index 059440b5188..57ba95dbd28 100644 --- a/crates/compiler/parse/src/parser.rs +++ b/crates/compiler/parse/src/parser.rs @@ -422,8 +422,7 @@ pub enum ERecord<'a> { End(Position), Open(Position), - Updateable(Position), - BuilderMapper(Position), + Prefix(Position), Field(Position), Colon(Position), QuestionMark(Position), @@ -739,8 +738,7 @@ pub enum ETypeAbilityImpl<'a> { Space(BadInputError, Position), - Updateable(Position), - BuilderMapper(Position), + Prefix(Position), QuestionMark(Position), Ampersand(Position), Expr(&'a EExpr<'a>, Position), @@ -757,8 +755,7 @@ impl<'a> From> 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::BuilderMapper(p) => ETypeAbilityImpl::BuilderMapper(p), + ERecord::Prefix(p) => ETypeAbilityImpl::Prefix(p), ERecord::QuestionMark(p) => ETypeAbilityImpl::QuestionMark(p), ERecord::Ampersand(p) => ETypeAbilityImpl::Ampersand(p), ERecord::Expr(e, p) => ETypeAbilityImpl::Expr(e, p), diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 6ee1eca2f0d..7666e9fcdea 100644 --- a/crates/compiler/problem/src/can.rs +++ b/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::SingleFieldNewRecordBuilder(region)) - | Problem::RuntimeError(RuntimeError::OptionalFieldInNewRecordBuilder { + | Problem::RuntimeError(RuntimeError::EmptyRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::SingleFieldRecordBuilder(region)) + | Problem::RuntimeError(RuntimeError::OptionalFieldInRecordBuilder { record: _, field: region, }) @@ -672,9 +672,9 @@ pub enum RuntimeError { MultipleOldRecordBuilders(Region), UnappliedOldRecordBuilder(Region), - EmptyNewRecordBuilder(Region), - SingleFieldNewRecordBuilder(Region), - OptionalFieldInNewRecordBuilder { + EmptyRecordBuilder(Region), + SingleFieldRecordBuilder(Region), + OptionalFieldInRecordBuilder { record: Region, field: Region, }, @@ -724,9 +724,9 @@ impl RuntimeError { | RuntimeError::InvalidHexadecimal(region) | RuntimeError::MultipleOldRecordBuilders(region) | RuntimeError::UnappliedOldRecordBuilder(region) - | RuntimeError::EmptyNewRecordBuilder(region) - | RuntimeError::SingleFieldNewRecordBuilder(region) - | RuntimeError::OptionalFieldInNewRecordBuilder { + | RuntimeError::EmptyRecordBuilder(region) + | RuntimeError::SingleFieldRecordBuilder(region) + | RuntimeError::OptionalFieldInRecordBuilder { record: _, field: region, } diff --git a/crates/language_server/src/analysis/tokens.rs b/crates/language_server/src/analysis/tokens.rs index 797b786c350..22816f51422 100644 --- a/crates/language_server/src/analysis/tokens.rs +++ b/crates/language_server/src/analysis/tokens.rs @@ -684,7 +684,7 @@ impl IterTokens for Loc> { 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::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::SingleFieldNewRecordBuilder(e) => e.iter_tokens(arena), - Expr::OptionalFieldInNewRecordBuilder(_name, e) => e.iter_tokens(arena), + Expr::EmptyRecordBuilder(e) => e.iter_tokens(arena), + Expr::SingleFieldRecordBuilder(e) => e.iter_tokens(arena), + Expr::OptionalFieldInRecordBuilder(_name, e) => e.iter_tokens(arena), Expr::MalformedIdent(_, _) | Expr::MalformedClosure | Expr::PrecedenceConflict(_) diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 3f28ae03aab..cfb9e4308f3 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/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"; diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 582b9b3d243..b13c24781cc 100644 --- a/crates/reporting/src/error/type.rs +++ b/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 "), From 74f05eca40ad17a3572acebdf1754ca368d5ff36 Mon Sep 17 00:00:00 2001 From: Sam Mohr Date: Sun, 7 Jul 2024 19:02:31 -0700 Subject: [PATCH 3/3] Fix broken test_reporting test --- crates/compiler/load/tests/test_reporting.rs | 23 ++++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index 8d8af2bb7f9..71418e4288c 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -10935,25 +10935,24 @@ In roc, functions are always written as a lambda, like{} record_builder_with_non_function_mapper, indoc!( r#" - x = "abc" + xyz = "abc" - { x <- + { xyz <- 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. + ── TOO MANY ARGS in /code/proj/Main.roc ──────────────────────────────────────── + + The `xyz` value is not a function, but it was given 3 arguments: + + 6│ { xyz <- + ^^^ + + Note: Record builders need a mapper function before the `<-` to combine + fields together with. "# );