From 6670fbb1abd37a52fa4042a9005f5c7010b0286b Mon Sep 17 00:00:00 2001 From: Agustin Zubiaga Date: Mon, 8 May 2023 19:26:47 -0300 Subject: [PATCH] Multiple record builder error --- crates/compiler/can/src/expr.rs | 8 ++++++ crates/compiler/can/src/operator.rs | 8 ++++-- crates/compiler/can/tests/test_can.rs | 26 ++++++++++++++++++ crates/compiler/fmt/src/expr.rs | 6 ++++- crates/compiler/fmt/src/spaces.rs | 3 +++ crates/compiler/parse/src/ast.rs | 4 ++- crates/compiler/parse/src/expr.rs | 1 + crates/compiler/problem/src/can.rs | 3 +++ crates/compiler/test_syntax/tests/test_fmt.rs | 19 +++++++++++++ crates/reporting/src/error/canonicalize.rs | 14 ++++++++++ crates/reporting/tests/test_reporting.rs | 27 +++++++++++++++++++ 11 files changed, 115 insertions(+), 4 deletions(-) diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 07b1b0c72a..5609266c41 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -1359,6 +1359,14 @@ pub fn canonicalize_expr<'a>( (RuntimeError(problem), Output::default()) } + ast::Expr::MultipleRecordBuilders(sub_expr) => { + use roc_problem::can::RuntimeError::*; + + let problem = MultipleRecordBuilders(sub_expr.region); + env.problem(Problem::RuntimeError(problem.clone())); + + (RuntimeError(problem), Output::default()) + } &ast::Expr::NonBase10Int { string, base, diff --git a/crates/compiler/can/src/operator.rs b/crates/compiler/can/src/operator.rs index 60bd6cd071..baba063d15 100644 --- a/crates/compiler/can/src/operator.rs +++ b/crates/compiler/can/src/operator.rs @@ -137,6 +137,7 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc>) -> &'a Loc | MalformedIdent(_, _) | MalformedClosure | PrecedenceConflict { .. } + | MultipleRecordBuilders { .. } | Tag(_) | OpaqueRef(_) | IngestedFile(_, _) @@ -253,7 +254,7 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc>) -> &'a Loc } } RecordBuilder(_) => { - todo!("Compiler error: Record builders must be applied to functions"); + todo!("Compiler error: Record builders must be applied to functions") } BinOps(lefts, right) => desugar_bin_ops(arena, loc_expr.region, lefts, right), Defs(defs, loc_ret) => { @@ -274,7 +275,10 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Loc>) -> &'a Loc match current { RecordBuilder(fields) => { if builder_apply_exprs.is_some() { - todo!("Compiler error: A function application can only be passed one record builder") + return arena.alloc(Loc { + value: MultipleRecordBuilders(loc_expr), + region: loc_expr.region, + }); } let builder_arg = record_builder_arg(arena, loc_arg.region, fields); diff --git a/crates/compiler/can/tests/test_can.rs b/crates/compiler/can/tests/test_can.rs index ad4c5f6a57..eb855da7ec 100644 --- a/crates/compiler/can/tests/test_can.rs +++ b/crates/compiler/can/tests/test_can.rs @@ -773,6 +773,32 @@ mod test_can { } } + #[test] + fn multiple_record_builders_error() { + let src = indoc!( + r#" + succeed + { a <- apply "a" } + { b <- apply "b" } + "# + ); + let arena = Bump::new(); + let CanExprOut { + problems, loc_expr, .. + } = can_expr_with(&arena, test_home(), src); + + assert_eq!(problems.len(), 1); + assert!(problems.iter().all(|problem| matches!( + problem, + Problem::RuntimeError(roc_problem::can::RuntimeError::MultipleRecordBuilders { .. }) + ))); + + assert!(matches!( + loc_expr.value, + Expr::RuntimeError(roc_problem::can::RuntimeError::MultipleRecordBuilders { .. }) + )); + } + // TAIL CALLS fn get_closure(expr: &Expr, i: usize) -> roc_can::expr::Recursive { match expr { diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index e6bad73f24..e9a201fd08 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -78,7 +78,8 @@ impl<'a> Formattable for Expr<'a> { UnaryOp(loc_subexpr, _) | PrecedenceConflict(roc_parse::ast::PrecedenceConflict { expr: loc_subexpr, .. - }) => loc_subexpr.is_multiline(), + }) + | MultipleRecordBuilders(loc_subexpr) => loc_subexpr.is_multiline(), ParensAround(subexpr) => subexpr.is_multiline(), @@ -498,6 +499,9 @@ impl<'a> Formattable for Expr<'a> { } MalformedClosure => {} PrecedenceConflict { .. } => {} + MultipleRecordBuilders(sub_expr) => { + sub_expr.format_with_options(buf, parens, newlines, indent) + } IngestedFile(_, _) => {} } } diff --git a/crates/compiler/fmt/src/spaces.rs b/crates/compiler/fmt/src/spaces.rs index aa8fff3df9..b8eb353338 100644 --- a/crates/compiler/fmt/src/spaces.rs +++ b/crates/compiler/fmt/src/spaces.rs @@ -747,6 +747,9 @@ impl<'a> RemoveSpaces<'a> for Expr<'a> { Expr::MalformedIdent(a, b) => Expr::MalformedIdent(a, remove_spaces_bad_ident(b)), Expr::MalformedClosure => Expr::MalformedClosure, Expr::PrecedenceConflict(a) => Expr::PrecedenceConflict(a), + Expr::MultipleRecordBuilders(a) => { + Expr::MultipleRecordBuilders(arena.alloc(a.remove_spaces(arena))) + } 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/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index f8e2961984..f303179100 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -327,6 +327,7 @@ 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>), } #[derive(Clone, Copy, Debug, PartialEq)] @@ -1533,7 +1534,8 @@ impl<'a> Malformed for Expr<'a> { MalformedIdent(_, _) | MalformedClosure | - PrecedenceConflict(_) => true, + PrecedenceConflict(_) | + MultipleRecordBuilders(_) => true, } } } diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index 17046be802..14e50094db 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -1928,6 +1928,7 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result return Err(()), diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 11914256c1..94f3cba5c0 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -361,6 +361,7 @@ impl Problem { | Problem::RuntimeError(RuntimeError::EmptySingleQuote(region)) | Problem::RuntimeError(RuntimeError::MultipleCharsInSingleQuote(region)) | Problem::RuntimeError(RuntimeError::DegenerateBranch(region)) + | Problem::RuntimeError(RuntimeError::MultipleRecordBuilders(region)) | Problem::InvalidAliasRigid { region, .. } | Problem::InvalidInterpolation(region) | Problem::InvalidHexadecimal(region) @@ -588,6 +589,8 @@ pub enum RuntimeError { MultipleCharsInSingleQuote(Region), DegenerateBranch(Region), + + MultipleRecordBuilders(Region), } impl RuntimeError { diff --git a/crates/compiler/test_syntax/tests/test_fmt.rs b/crates/compiler/test_syntax/tests/test_fmt.rs index c05d9ebb1c..b9d3d6e05f 100644 --- a/crates/compiler/test_syntax/tests/test_fmt.rs +++ b/crates/compiler/test_syntax/tests/test_fmt.rs @@ -1999,6 +1999,25 @@ mod test_fmt { ); } + #[test] + fn can_format_multiple_record_builders() { + expr_formats_to( + indoc!( + r#" + succeed { a <- get "a" } + { b <- get "b" } + "# + ), + indoc!( + r#" + succeed + { a <- get "a" } + { b <- get "b" } + "# + ), + ); + } + #[test] fn final_comments_in_records() { expr_formats_same(indoc!( diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index ead5643ea3..4c71a13830 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -2133,6 +2133,20 @@ fn pretty_runtime_error<'b>( title = "DEGENERATE BRANCH"; } + RuntimeError::MultipleRecordBuilders(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.region(lines.convert_region(region)), + alloc.note("Functions can only take at most one record builder!"), + tip, + ]); + + title = "MULTIPLE RECORD BUILDERS"; + } } (doc, title) diff --git a/crates/reporting/tests/test_reporting.rs b/crates/reporting/tests/test_reporting.rs index c3409e268b..02f1b73cc4 100644 --- a/crates/reporting/tests/test_reporting.rs +++ b/crates/reporting/tests/test_reporting.rs @@ -10177,6 +10177,33 @@ I recommend using camelCase. It's the standard style in Roc code! ) ); + // Record Builders + + test_report!( + multiple_record_builders, + indoc!( + r#" + succeed + { a <- apply "a" } + { b <- apply "b" } + "# + ), + @r###" + ── MULTIPLE RECORD BUILDERS ────────────────────────────── /code/proj/Main.roc ─ + + This function is applied to multiple record builders: + + 4│> succeed + 5│> { a <- apply "a" } + 6│> { b <- apply "b" } + + Note: Functions can only take at most one record builder! + + Tip: You can combine them or apply them separately. + + "### + ); + test_report!( destructure_assignment_introduces_no_variables_nested, indoc!(