From 30ecd378a07e63ffa52a0a5efe47e54f446a7057 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 14:31:48 +0100 Subject: [PATCH 1/7] refactor parse AST to allow multiple if branches --- compiler/can/src/expr.rs | 39 ++++++--- compiler/can/src/operator.rs | 21 +++-- compiler/fmt/src/expr.rs | 164 +++++++++++++++++++---------------- compiler/parse/src/ast.rs | 2 +- compiler/parse/src/expr.rs | 5 +- editor/src/lang/expr.rs | 23 +++-- 6 files changed, 145 insertions(+), 109 deletions(-) diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index a2248b7391..d1718703e6 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -674,32 +674,43 @@ pub fn canonicalize_expr<'a>( Output::default(), ) } - ast::Expr::If(cond, then_branch, else_branch) => { - let (loc_cond, mut output) = - canonicalize_expr(env, var_store, scope, cond.region, &cond.value); - let (loc_then, then_output) = canonicalize_expr( - env, - var_store, - scope, - then_branch.region, - &then_branch.value, - ); + ast::Expr::If(if_thens, final_else_branch) => { + let mut branches = Vec::with_capacity(1); + let mut output = Output::default(); + + for (condition, then_branch) in if_thens.iter() { + let (loc_cond, cond_output) = + canonicalize_expr(env, var_store, scope, condition.region, &condition.value); + + let (loc_then, then_output) = canonicalize_expr( + env, + var_store, + scope, + then_branch.region, + &then_branch.value, + ); + + branches.push((loc_cond, loc_then)); + + output.references = output.references.union(cond_output.references); + output.references = output.references.union(then_output.references); + } + let (loc_else, else_output) = canonicalize_expr( env, var_store, scope, - else_branch.region, - &else_branch.value, + final_else_branch.region, + &final_else_branch.value, ); - output.references = output.references.union(then_output.references); output.references = output.references.union(else_output.references); ( If { cond_var: var_store.fresh(), branch_var: var_store.fresh(), - branches: vec![(loc_cond, loc_then)], + branches, final_else: Box::new(loc_else), }, output, diff --git a/compiler/can/src/operator.rs b/compiler/can/src/operator.rs index 5f9d73bb70..048c2b9d83 100644 --- a/compiler/can/src/operator.rs +++ b/compiler/can/src/operator.rs @@ -290,16 +290,21 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Located>) -> &'a }), ) } - If(condition, then_branch, else_branch) - | Nested(If(condition, then_branch, else_branch)) => { - // If does not get desugared yet so we can give more targetted error messages during - // type checking. - let desugared_cond = &*arena.alloc(desugar_expr(arena, &condition)); - let desugared_then = &*arena.alloc(desugar_expr(arena, &then_branch)); - let desugared_else = &*arena.alloc(desugar_expr(arena, &else_branch)); + If(if_thens, final_else_branch) | Nested(If(if_thens, final_else_branch)) => { + // If does not get desugared into `when` so we can give more targetted error messages during type checking. + let desugared_final_else = &*arena.alloc(desugar_expr(arena, &final_else_branch)); + + let mut desugared_if_thens = Vec::with_capacity_in(if_thens.len(), arena); + + for (condition, then_branch) in if_thens.iter() { + desugared_if_thens.push(( + desugar_expr(arena, condition).clone(), + desugar_expr(arena, then_branch).clone(), + )); + } arena.alloc(Located { - value: If(desugared_cond, desugared_then, desugared_else), + value: If(desugared_if_thens.into_bump_slice(), desugared_final_else), region: loc_expr.region, }) } diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index 1be56ee514..80056cce7f 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -58,8 +58,11 @@ impl<'a> Formattable<'a> for Expr<'a> { loc_expr.is_multiline() || args.iter().any(|loc_arg| loc_arg.is_multiline()) } - If(loc_cond, loc_if_true, loc_if_false) => { - loc_cond.is_multiline() || loc_if_true.is_multiline() || loc_if_false.is_multiline() + If(branches, final_else) => { + final_else.is_multiline() + || branches + .iter() + .any(|(c, t)| c.is_multiline() || t.is_multiline()) } BinOp((loc_left, _, loc_right)) => { @@ -257,8 +260,8 @@ impl<'a> Formattable<'a> for Expr<'a> { // still print the return value. ret.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); } - If(loc_condition, loc_then, loc_else) => { - fmt_if(buf, loc_condition, loc_then, loc_else, indent); + If(branches, final_else) => { + fmt_if(buf, branches, final_else, self.is_multiline(), indent); } When(loc_condition, branches) => fmt_when(buf, loc_condition, branches, indent), List { @@ -629,15 +632,15 @@ fn fmt_when<'a>( fn fmt_if<'a>( buf: &mut String<'a>, - loc_condition: &'a Located>, - loc_then: &'a Located>, - loc_else: &'a Located>, + branches: &'a [(Located>, Located>)], + final_else: &'a Located>, + is_multiline: bool, indent: u16, ) { - let is_multiline_then = loc_then.is_multiline(); - let is_multiline_else = loc_else.is_multiline(); - let is_multiline_condition = loc_condition.is_multiline(); - let is_multiline = is_multiline_then || is_multiline_else || is_multiline_condition; + // let is_multiline_then = loc_then.is_multiline(); + // let is_multiline_else = final_else.is_multiline(); + // let is_multiline_condition = loc_condition.is_multiline(); + // let is_multiline = is_multiline_then || is_multiline_else || is_multiline_condition; let return_indent = if is_multiline { indent + INDENT @@ -645,80 +648,89 @@ fn fmt_if<'a>( indent }; - buf.push_str("if"); + for (loc_condition, loc_then) in branches.iter() { + let is_multiline_condition = loc_condition.is_multiline(); - if is_multiline_condition { - match &loc_condition.value { - Expr::SpaceBefore(expr_below, spaces_above_expr) => { - fmt_comments_only(buf, spaces_above_expr.iter(), NewlineAt::Top, return_indent); - newline(buf, return_indent); + buf.push_str("if"); - match &expr_below { - Expr::SpaceAfter(expr_above, spaces_below_expr) => { - expr_above.format(buf, return_indent); - fmt_comments_only( - buf, - spaces_below_expr.iter(), - NewlineAt::Top, - return_indent, - ); - newline(buf, indent); - } + if is_multiline_condition { + match &loc_condition.value { + Expr::SpaceBefore(expr_below, spaces_above_expr) => { + fmt_comments_only(buf, spaces_above_expr.iter(), NewlineAt::Top, return_indent); + newline(buf, return_indent); - _ => { - expr_below.format(buf, return_indent); + match &expr_below { + Expr::SpaceAfter(expr_above, spaces_below_expr) => { + expr_above.format(buf, return_indent); + fmt_comments_only( + buf, + spaces_below_expr.iter(), + NewlineAt::Top, + return_indent, + ); + newline(buf, indent); + } + + _ => { + expr_below.format(buf, return_indent); + } } } - } - Expr::SpaceAfter(expr_above, spaces_below_expr) => { - newline(buf, return_indent); - expr_above.format(buf, return_indent); - fmt_comments_only(buf, spaces_below_expr.iter(), NewlineAt::Top, return_indent); - newline(buf, indent); - } + Expr::SpaceAfter(expr_above, spaces_below_expr) => { + newline(buf, return_indent); + expr_above.format(buf, return_indent); + fmt_comments_only(buf, spaces_below_expr.iter(), NewlineAt::Top, return_indent); + newline(buf, indent); + } - _ => { - newline(buf, return_indent); - loc_condition.format(buf, return_indent); - newline(buf, indent); - } - } - } else { - buf.push(' '); - loc_condition.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); - buf.push(' '); - } - - buf.push_str("then"); - - if is_multiline { - match &loc_then.value { - Expr::SpaceBefore(expr_below, spaces_below) => { - // we want exactly one newline, user-inserted extra newlines are ignored. - newline(buf, return_indent); - fmt_comments_only(buf, spaces_below.iter(), NewlineAt::Bottom, return_indent); - - match &expr_below { - Expr::SpaceAfter(expr_above, spaces_above) => { - expr_above.format(buf, return_indent); - - fmt_comments_only(buf, spaces_above.iter(), NewlineAt::Top, return_indent); - newline(buf, indent); - } - - _ => { - expr_below.format(buf, return_indent); - } + _ => { + newline(buf, return_indent); + loc_condition.format(buf, return_indent); + newline(buf, indent); } } - _ => { - loc_condition.format(buf, return_indent); - } + } else { + buf.push(' '); + loc_condition.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); + buf.push(' '); + } + + buf.push_str("then"); + + if is_multiline { + match &loc_then.value { + Expr::SpaceBefore(expr_below, spaces_below) => { + // we want exactly one newline, user-inserted extra newlines are ignored. + newline(buf, return_indent); + fmt_comments_only(buf, spaces_below.iter(), NewlineAt::Bottom, return_indent); + + match &expr_below { + Expr::SpaceAfter(expr_above, spaces_above) => { + expr_above.format(buf, return_indent); + + fmt_comments_only( + buf, + spaces_above.iter(), + NewlineAt::Top, + return_indent, + ); + newline(buf, indent); + } + + _ => { + expr_below.format(buf, return_indent); + } + } + } + _ => { + loc_condition.format(buf, return_indent); + } + } + } else { + buf.push_str(" "); + loc_then.format(buf, return_indent); } - } else { - buf.push_str(" "); - loc_then.format(buf, return_indent); } if is_multiline { @@ -728,7 +740,7 @@ fn fmt_if<'a>( buf.push_str(" else "); } - loc_else.format(buf, return_indent); + final_else.format(buf, return_indent); } pub fn fmt_closure<'a>( diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 70964e246e..f33ba8149b 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -127,7 +127,7 @@ pub enum Expr<'a> { UnaryOp(&'a Loc>, Loc), // Conditionals - If(&'a Loc>, &'a Loc>, &'a Loc>), + If(&'a [(Loc>, Loc>)], &'a Loc>), When( /// The condition &'a Loc>, diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 8d187f10fb..5f05d89b17 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -324,7 +324,7 @@ pub fn expr_to_pattern<'a>( | Expr::Closure(_, _) | Expr::BinOp(_) | Expr::Defs(_, _) - | Expr::If(_, _, _) + | Expr::If(_, _) | Expr::When(_, _) | Expr::MalformedClosure | Expr::PrecedenceConflict(_, _, _, _) @@ -1264,8 +1264,7 @@ pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a> ), |arena: &'a Bump, (condition, (then_branch, else_branch))| { Expr::If( - &*arena.alloc(condition), - &*arena.alloc(then_branch), + arena.alloc([(condition, then_branch)]), &*arena.alloc(else_branch), ) } diff --git a/editor/src/lang/expr.rs b/editor/src/lang/expr.rs index 83791facb1..3cc1a4f912 100644 --- a/editor/src/lang/expr.rs +++ b/editor/src/lang/expr.rs @@ -508,22 +508,31 @@ pub fn to_expr2<'a>( Output::default(), ), - If(cond, then_branch, else_branch) => { - let (cond, mut output) = to_expr2(env, scope, &cond.value, cond.region); + If(branches, final_else) => { + let mut new_branches = Vec::with_capacity(branches.len()); + let mut output = Output::default(); - let (then_expr, then_output) = - to_expr2(env, scope, &then_branch.value, then_branch.region); + for (condition, then_branch) in branches.iter() { + let (cond, cond_output) = to_expr2(env, scope, &condition.value, condition.region); + + let (then_expr, then_output) = + to_expr2(env, scope, &then_branch.value, then_branch.region); + + output.references.union_mut(cond_output.references); + output.references.union_mut(then_output.references); + + new_branches.push((cond, then_expr)); + } let (else_expr, else_output) = - to_expr2(env, scope, &else_branch.value, else_branch.region); + to_expr2(env, scope, &final_else.value, final_else.region); - output.references.union_mut(then_output.references); output.references.union_mut(else_output.references); let expr = Expr2::If { cond_var: env.var_store.fresh(), expr_var: env.var_store.fresh(), - branches: PoolVec::new(vec![(cond, then_expr)].into_iter(), env.pool), + branches: PoolVec::new(new_branches.into_iter(), env.pool), final_else: env.pool.add(else_expr), }; From 5d8944fc6a4a9ca6910219bf66d6e46369fe334b Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 15:05:25 +0100 Subject: [PATCH 2/7] use new parser for If --- compiler/parse/src/expr.rs | 90 ++++++++++++++++++++++-------------- compiler/parse/src/parser.rs | 20 ++++++++ 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 5f05d89b17..7fe80a1b02 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -11,7 +11,7 @@ use crate::number_literal::number_literal; use crate::parser::{ self, allocated, and_then_with_indent_level, ascii_char, ascii_string, attempt, backtrackable, fail, map, newline_char, not, not_followed_by, optional, sep_by1, specialize, specialize_ref, - then, unexpected, unexpected_eof, word1, word2, EExpr, Either, ParseResult, Parser, State, + then, unexpected, unexpected_eof, word1, word2, EExpr, Either, If, ParseResult, Parser, State, SyntaxError, When, }; use crate::pattern::loc_closure_param; @@ -1234,40 +1234,62 @@ mod when { } } -pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - map_with_arena!( - and!( - skip_first!( - parser::keyword(keyword::IF, min_indent), - space1_around( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) +pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { + move |arena: &'a Bump, state| { + let (_, _, state) = parser::keyword_e(keyword::IF, If::If).parse(arena, state)?; + + let mut branches = Vec::with_capacity_in(1, arena); + + let (_, cond, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), ), - and!( - skip_first!( - parser::keyword(keyword::THEN, min_indent), - space1_around( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) - ), - skip_first!( - parser::keyword(keyword::ELSE, min_indent), - // NOTE changed this from space1_around to space1_before - space1_before( - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - min_indent, - ) - ) - ) - ), - |arena: &'a Bump, (condition, (then_branch, else_branch))| { - Expr::If( - arena.alloc([(condition, then_branch)]), - &*arena.alloc(else_branch), - ) - } + min_indent, + If::Space, + If::IndentCondition, + ) + .parse(arena, state)?; + + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then).parse(arena, state)?; + + let (_, then_branch, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThen, + ) + .parse(arena, state)?; + + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else).parse(arena, state)?; + + branches.push((cond, then_branch)); + + let (_, else_branch, state) = space0_before_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentElse, + ) + .parse(arena, state)?; + + // parse the final else + let expr = Expr::If(branches.into_bump_slice(), arena.alloc(else_branch)); + + Ok((MadeProgress, expr, state)) + } +} + +pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { + specialize( + |e, r, c| SyntaxError::Expr(EExpr::If(e, r, c)), + if_expr_help(min_indent), ) } diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 71d2259bc1..a9da276780 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -378,6 +378,7 @@ pub enum EExpr<'a> { Space(BadInputError, Row, Col), When(When<'a>, Row, Col), + If(If<'a>, Row, Col), // EInParens(PInParens<'a>, Row, Col), IndentStart(Row, Col), @@ -408,6 +409,25 @@ pub enum When<'a> { PatternAlignment(u16, Row, Col), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum If<'a> { + Space(BadInputError, Row, Col), + If(Row, Col), + Then(Row, Col), + Else(Row, Col), + // TODO make EEXpr + Condition(&'a EExpr<'a>, Row, Col), + ThenBranch(&'a EExpr<'a>, Row, Col), + ElseBranch(&'a EExpr<'a>, Row, Col), + Syntax(&'a SyntaxError<'a>, Row, Col), + + IndentCondition(Row, Col), + IndentThen(Row, Col), + IndentElse(Row, Col), + + PatternAlignment(u16, Row, Col), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum EPattern<'a> { Record(PRecord<'a>, Row, Col), From 3907680536dd608505395a3b82a3d842932cf3ad Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 15:21:19 +0100 Subject: [PATCH 3/7] parse multiple if-then-else pairs into one AST node --- compiler/parse/src/expr.rs | 82 ++++++++++++++-------- compiler/parse/src/parser.rs | 1 + compiler/reporting/tests/test_reporting.rs | 59 ++++++++-------- 3 files changed, 85 insertions(+), 57 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 7fe80a1b02..84964e164b 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -3,7 +3,7 @@ use crate::ast::{ }; use crate::blankspace::{ line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_e, - space0_before, space0_before_e, space0_e, space1, space1_around, space1_before, spaces_exactly, + space0_before, space0_before_e, space0_e, space1, space1_before, spaces_exactly, }; use crate::ident::{global_tag_or_ident, ident, lowercase_ident, Ident}; use crate::keyword; @@ -1240,33 +1240,59 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { let mut branches = Vec::with_capacity_in(1, arena); - let (_, cond, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentCondition, - ) - .parse(arena, state)?; + let mut loop_state = state; - let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then).parse(arena, state)?; + let state_final_else = loop { + let state = loop_state; + let (_, cond, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentCondition, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - let (_, then_branch, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentThen, - ) - .parse(arena, state)?; + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else).parse(arena, state)?; + let (_, then_branch, state) = space0_around_e( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThen, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - branches.push((cond, then_branch)); + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + branches.push((cond, then_branch)); + + // try to parse another `if` + // NOTE this drops spaces between the `else` and the `if` + let optional_if = and!( + backtrackable(space0_e(min_indent, If::Space, If::IndentIf)), + parser::keyword_e(keyword::IF, If::If) + ); + + match optional_if.parse(arena, state) { + Err((_, _, state)) => break state, + Ok((_, _, state)) => { + loop_state = state; + continue; + } + } + }; let (_, else_branch, state) = space0_before_e( specialize_ref( @@ -1277,9 +1303,9 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { If::Space, If::IndentElse, ) - .parse(arena, state)?; + .parse(arena, state_final_else) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; - // parse the final else let expr = Expr::If(branches.into_bump_slice(), arena.alloc(else_branch)); Ok((MadeProgress, expr, state)) @@ -1287,10 +1313,10 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { } pub fn if_expr<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - specialize( + debug!(specialize( |e, r, c| SyntaxError::Expr(EExpr::If(e, r, c)), if_expr_help(min_indent), - ) + )) } /// This is a helper function for parsing function args. diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index a9da276780..9f6eba16bd 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -422,6 +422,7 @@ pub enum If<'a> { Syntax(&'a SyntaxError<'a>, Row, Col), IndentCondition(Row, Col), + IndentIf(Row, Col), IndentThen(Row, Col), IndentElse(Row, Col), diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 435f092b1d..66be20f5ca 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -801,35 +801,36 @@ mod test_reporting { ) } - // #[test] - // fn if_3_branch_mismatch() { - // report_problem_as( - // indoc!( - // r#" - // if True then 2 else if False then 2 else "foo" - // "# - // ), - // indoc!( - // r#" - // ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - - // The 2nd branch of this `if` does not match all the previous branches: - - // 1│ if True then 2 else "foo" - // ^^^^^ - - // The 2nd branch is a string of type - - // Str - - // But all the previous branches have the type - - // Num a - - // "# - // ), - // ) - // } + #[test] + fn if_3_branch_mismatch() { + report_problem_as( + indoc!( + r#" + if True then 2 else if False then 2 else "foo" + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + The 3rd branch of this `if` does not match all the previous branches: + + 1│ if True then 2 else if False then 2 else "foo" + ^^^^^ + + The 3rd branch is a string of type: + + Str + + But all the previous branches have type: + + Num a + + I need all branches in an `if` to have the same type! + "# + ), + ) + } #[test] fn when_branch_mismatch() { From 6eab8abe9e4b75e2fd5289ca64bbdc4546ede774 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 18:34:08 +0100 Subject: [PATCH 4/7] improve message for outdented then --- compiler/parse/src/blankspace.rs | 12 +- compiler/parse/src/expr.rs | 88 +++++++------ compiler/parse/src/parser.rs | 11 +- compiler/parse/src/pattern.rs | 5 +- compiler/parse/src/type_annotation.rs | 10 +- compiler/reporting/src/error/parse.rs | 141 ++++++++++++++++++++- compiler/reporting/tests/test_reporting.rs | 60 ++++++++- 7 files changed, 270 insertions(+), 57 deletions(-) diff --git a/compiler/parse/src/blankspace.rs b/compiler/parse/src/blankspace.rs index 10b256b9b8..f2234ebed0 100644 --- a/compiler/parse/src/blankspace.rs +++ b/compiler/parse/src/blankspace.rs @@ -60,11 +60,12 @@ where ) } -pub fn space0_around_e<'a, P, S, E>( +pub fn space0_around_ee<'a, P, S, E>( parser: P, min_indent: u16, space_problem: fn(BadInputError, Row, Col) -> E, - indent_problem: fn(Row, Col) -> E, + indent_before_problem: fn(Row, Col) -> E, + indent_after_problem: fn(Row, Col) -> E, ) -> impl Parser<'a, Located, E> where S: Spaceable<'a>, @@ -75,8 +76,11 @@ where { parser::map_with_arena( and( - space0_e(min_indent, space_problem, indent_problem), - and(parser, space0_e(min_indent, space_problem, indent_problem)), + space0_e(min_indent, space_problem, indent_before_problem), + and( + parser, + space0_e(min_indent, space_problem, indent_after_problem), + ), ), move |arena: &'a Bump, tuples: ( diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 84964e164b..1b01a08edb 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -2,7 +2,7 @@ use crate::ast::{ AssignedField, Attempting, CommentOrNewline, Def, Expr, Pattern, Spaceable, TypeAnnotation, }; use crate::blankspace::{ - line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_e, + line_comment, space0, space0_after, space0_after_e, space0_around, space0_around_ee, space0_before, space0_before_e, space0_e, space1, space1_before, spaces_exactly, }; use crate::ident::{global_tag_or_ident, ident, lowercase_ident, Ident}; @@ -1029,14 +1029,15 @@ mod when { and!( when_with_indent(), skip_second!( - space0_around_e( + space0_around_ee( loc!(specialize_ref( When::Syntax, move |arena, state| parse_expr(min_indent, arena, state) )), min_indent, When::Space, - When::IndentCondition + When::IndentCondition, + When::IndentIs, ), parser::keyword_e(keyword::IS, When::Is) ) @@ -1182,13 +1183,14 @@ mod when { skip_first!( parser::keyword_e(keyword::IF, When::IfToken), // TODO we should require space before the expression but not after - space0_around_e( + space0_around_ee( loc!(specialize_ref(When::IfGuard, move |arena, state| { parse_expr(min_indent, arena, state) })), min_indent, When::Space, When::IndentIfGuard, + When::IndentArrow, ) ), Some @@ -1234,6 +1236,49 @@ mod when { } } +fn if_branch<'a>( + min_indent: u16, +) -> impl Parser<'a, (Located>, Located>), If<'a>> { + move |arena, state| { + // NOTE: only parse spaces before the expression + let (_, cond, state) = space0_around_ee( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentCondition, + If::IndentThenToken, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, then_branch, state) = space0_around_ee( + specialize_ref( + If::Syntax, + loc!(move |arena, state| parse_expr(min_indent, arena, state)), + ), + min_indent, + If::Space, + If::IndentThenBranch, + If::IndentElseToken, + ) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) + .parse(arena, state) + .map_err(|(_, f, s)| (MadeProgress, f, s))?; + + Ok((MadeProgress, (cond, then_branch), state)) + } +} + pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { move |arena: &'a Bump, state| { let (_, _, state) = parser::keyword_e(keyword::IF, If::If).parse(arena, state)?; @@ -1243,38 +1288,7 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { let mut loop_state = state; let state_final_else = loop { - let state = loop_state; - let (_, cond, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentCondition, - ) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, _, state) = parser::keyword_e(keyword::THEN, If::Then) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, then_branch, state) = space0_around_e( - specialize_ref( - If::Syntax, - loc!(move |arena, state| parse_expr(min_indent, arena, state)), - ), - min_indent, - If::Space, - If::IndentThen, - ) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; - - let (_, _, state) = parser::keyword_e(keyword::ELSE, If::Else) - .parse(arena, state) - .map_err(|(_, f, s)| (MadeProgress, f, s))?; + let (_, (cond, then_branch), state) = if_branch(min_indent).parse(arena, loop_state)?; branches.push((cond, then_branch)); @@ -1301,7 +1315,7 @@ pub fn if_expr_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, If<'a>> { ), min_indent, If::Space, - If::IndentElse, + If::IndentElseBranch, ) .parse(arena, state_final_else) .map_err(|(_, f, s)| (MadeProgress, f, s))?; diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 9f6eba16bd..b1d3d3545a 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -423,10 +423,10 @@ pub enum If<'a> { IndentCondition(Row, Col), IndentIf(Row, Col), - IndentThen(Row, Col), - IndentElse(Row, Col), - - PatternAlignment(u16, Row, Col), + IndentThenToken(Row, Col), + IndentElseToken(Row, Col), + IndentThenBranch(Row, Col), + IndentElseBranch(Row, Col), } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1452,10 +1452,11 @@ macro_rules! collection_trailing_sep_e { and!( $crate::parser::trailing_sep_by0( $delimiter, - $crate::blankspace::space0_around_e( + $crate::blankspace::space0_around_ee( $elem, $min_indent, $space_problem, + $indent_problem, $indent_problem ) ), diff --git a/compiler/parse/src/pattern.rs b/compiler/parse/src/pattern.rs index b9b3f576e6..02daa8c5a9 100644 --- a/compiler/parse/src/pattern.rs +++ b/compiler/parse/src/pattern.rs @@ -1,5 +1,5 @@ use crate::ast::Pattern; -use crate::blankspace::{space0_around_e, space0_before_e, space0_e}; +use crate::blankspace::{space0_around_ee, space0_before_e, space0_e}; use crate::ident::{ident, lowercase_ident, Ident}; use crate::number_literal::number_literal; use crate::parser::Progress::{self, *}; @@ -133,11 +133,12 @@ fn loc_pattern_in_parens_help<'a>( ) -> impl Parser<'a, Located>, PInParens<'a>> { between!( word1(b'(', PInParens::Open), - space0_around_e( + space0_around_ee( move |arena, state| specialize_ref(PInParens::Syntax, loc_pattern(min_indent)) .parse(arena, state), min_indent, PInParens::Space, + PInParens::IndentOpen, PInParens::IndentEnd, ), word1(b')', PInParens::End) diff --git a/compiler/parse/src/type_annotation.rs b/compiler/parse/src/type_annotation.rs index 974ec7f94b..88181f0908 100644 --- a/compiler/parse/src/type_annotation.rs +++ b/compiler/parse/src/type_annotation.rs @@ -1,5 +1,5 @@ use crate::ast::{AssignedField, Tag, TypeAnnotation}; -use crate::blankspace::{space0_around_e, space0_before_e, space0_e}; +use crate::blankspace::{space0_around_ee, space0_before_e, space0_e}; use crate::ident::join_module_parts; use crate::keyword; use crate::parser::{ @@ -146,11 +146,12 @@ fn loc_type_in_parens<'a>( ) -> impl Parser<'a, Located>, TInParens<'a>> { between!( word1(b'(', TInParens::Open), - space0_around_e( + space0_around_ee( move |arena, state| specialize_ref(TInParens::Type, expression(min_indent)) .parse(arena, state), min_indent, TInParens::Space, + TInParens::IndentOpen, TInParens::IndentEnd, ), word1(b')', TInParens::End) @@ -436,11 +437,12 @@ fn expression<'a>(min_indent: u16) -> impl Parser<'a, Located let (p2, rest, state) = zero_or_more!(skip_first!( word1(b',', Type::TFunctionArgument), one_of![ - space0_around_e( + space0_around_ee( term(min_indent), min_indent, Type::TSpace, - Type::TIndentStart + Type::TIndentStart, + Type::TIndentEnd ), |_, state: State<'a>| Err(( NoProgress, diff --git a/compiler/reporting/src/error/parse.rs b/compiler/reporting/src/error/parse.rs index 3431c87379..33f09653c1 100644 --- a/compiler/reporting/src/error/parse.rs +++ b/compiler/reporting/src/error/parse.rs @@ -158,7 +158,9 @@ enum Context { enum Node { WhenCondition, WhenBranch, - // WhenIfGuard, + IfCondition, + IfThenBranch, + IfElseBranch, } fn to_expr_report<'a>( @@ -173,10 +175,130 @@ fn to_expr_report<'a>( match parse_problem { EExpr::When(when, row, col) => to_when_report(alloc, filename, context, &when, *row, *col), + EExpr::If(when, row, col) => to_if_report(alloc, filename, context, &when, *row, *col), _ => todo!("unhandled parse error: {:?}", parse_problem), } } +fn to_if_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + context: Context, + parse_problem: &roc_parse::parser::If<'a>, + start_row: Row, + start_col: Col, +) -> Report<'a> { + use roc_parse::parser::If; + + match *parse_problem { + If::Syntax(syntax, row, col) => to_syntax_report(alloc, filename, syntax, row, col), + If::Space(error, row, col) => to_space_report(alloc, filename, &error, row, col), + + If::Condition(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfCondition, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::ThenBranch(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfThenBranch, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::ElseBranch(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::IfElseBranch, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + If::If(_row, _col) => unreachable!("another branch would be taken"), + If::IndentIf(_row, _col) => unreachable!("another branch would be taken"), + + If::Then(row, col) | If::IndentThenBranch(row, col) | If::IndentThenToken(row, col) => { + to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see the "), + alloc.keyword("then"), + alloc.reflow(r" keyword next."), + ]), + ) + } + + If::Else(row, col) | If::IndentElseBranch(row, col) | If::IndentElseToken(row, col) => { + to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see the "), + alloc.keyword("else"), + alloc.reflow(r" keyword next."), + ]), + ) + } + + If::IndentCondition(row, col) => to_unfinished_if_report( + alloc, + filename, + row, + col, + start_row, + start_col, + alloc.concat(vec![ + alloc.reflow(r"I was expecting to see a expression next") + ]), + ), + } +} + +fn to_unfinished_if_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + row: Row, + col: Col, + start_row: Row, + start_col: Col, + message: RocDocBuilder<'a>, +) -> Report<'a> { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow(r"I was partway through parsing an "), + alloc.keyword("if"), + alloc.reflow(r" expression, but I got stuck here:"), + ]), + alloc.region_with_subregion(surroundings, region), + message, + ]); + + Report { + filename, + doc, + title: "UNFINISHED IF".to_string(), + } +} + fn to_when_report<'a>( alloc: &'a RocDocAllocator<'a>, filename: PathBuf, @@ -792,6 +914,23 @@ fn to_type_report<'a>( } } + Type::TIndentEnd(row, col) => { + let surroundings = Region::from_rows_cols(start_row, start_col, *row, *col); + let region = Region::from_row_col(*row, *col); + + let doc = alloc.stack(vec![ + alloc.reflow(r"I am partway through parsing a type, but I got stuck here:"), + alloc.region_with_subregion(surroundings, region), + alloc.note("I may be confused by indentation"), + ]); + + Report { + filename, + doc, + title: "UNFINISHED TYPE".to_string(), + } + } + Type::TAsIndentStart(row, col) => { let surroundings = Region::from_rows_cols(start_row, start_col, *row, *col); let region = Region::from_row_col(*row, *col); diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 66be20f5ca..aa54cccaae 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -4636,12 +4636,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TYPE ───────────────────────────────────────────────────────────── - - I just started parsing a type, but I got stuck here: - + + I am partway through parsing a type, but I got stuck here: + 1│ f : I64, I64 ^ - + Note: I may be confused by indentation "# ), @@ -4950,4 +4950,56 @@ mod test_reporting { ), ) } + + #[test] + fn if_outdented_then() { + // TODO I think we can do better here + report_problem_as( + indoc!( + r#" + x = + if 5 == 5 + then 2 else 3 + + x + "# + ), + indoc!( + r#" + ── UNFINISHED IF ─────────────────────────────────────────────────────────────── + + I was partway through parsing an `if` expression, but I got stuck here: + + 2│ if 5 == 5 + ^ + + I was expecting to see the `then` keyword next. + "# + ), + ) + } + + #[test] + fn if_missing_else() { + // this should get better with time + report_problem_as( + indoc!( + r#" + if 5 == 5 then 2 + "# + ), + indoc!( + r#" + ── UNFINISHED IF ─────────────────────────────────────────────────────────────── + + I was partway through parsing an `if` expression, but I got stuck here: + + 1│ if 5 == 5 then 2 + ^ + + I was expecting to see the `else` keyword next. + "# + ), + ) + } } From f3234e002ab8ee6c74be55c148dda5aca711ef28 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 20:05:58 +0100 Subject: [PATCH 5/7] change list over --- compiler/parse/src/expr.rs | 65 +++++++++++++++++++----------------- compiler/parse/src/parser.rs | 14 ++++++++ 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 1b01a08edb..aebe5044b7 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -11,8 +11,8 @@ use crate::number_literal::number_literal; use crate::parser::{ self, allocated, and_then_with_indent_level, ascii_char, ascii_string, attempt, backtrackable, fail, map, newline_char, not, not_followed_by, optional, sep_by1, specialize, specialize_ref, - then, unexpected, unexpected_eof, word1, word2, EExpr, Either, If, ParseResult, Parser, State, - SyntaxError, When, + then, unexpected, unexpected_eof, word1, word2, BadInputError, EExpr, Either, If, List, + ParseResult, Parser, State, SyntaxError, When, }; use crate::pattern::loc_closure_param; use crate::type_annotation; @@ -1693,37 +1693,42 @@ fn binop<'a>() -> impl Parser<'a, BinOp, SyntaxError<'a>> { map!(ascii_char(b'%'), |_| BinOp::Percent) ) } - -pub fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { - let elems = collection_trailing_sep!( - ascii_char(b'['), - loc!(expr(min_indent)), - ascii_char(b','), - ascii_char(b']'), - min_indent - ); - - parser::attempt( - Attempting::List, - map_with_arena!(elems, |arena, - (parsed_elems, final_comments): ( - Vec<'a, Located>>, - &'a [CommentOrNewline<'a>] - )| { - let mut allocated = Vec::with_capacity_in(parsed_elems.len(), arena); - - for parsed_elem in parsed_elems { - allocated.push(&*arena.alloc(parsed_elem)); - } - - Expr::List { - items: allocated.into_bump_slice(), - final_comments, - } - }), +fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { + specialize( + |e, r, c| SyntaxError::Expr(EExpr::List(e, r, c)), + list_literal_help(min_indent), ) } +fn list_literal_help<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, List<'a>> { + move |arena, state| { + let (_, (parsed_elems, final_comments), state) = collection_trailing_sep_e!( + word1(b'[', List::Open), + specialize_ref(List::Syntax, loc!(expr(min_indent))), + word1(b',', List::End), + word1(b']', List::End), + min_indent, + List::Open, + List::Space, + List::IndentEnd + ) + .parse(arena, state)?; + + let mut allocated = Vec::with_capacity_in(parsed_elems.len(), arena); + + for parsed_elem in parsed_elems { + allocated.push(&*arena.alloc(parsed_elem)); + } + + let expr = Expr::List { + items: allocated.into_bump_slice(), + final_comments, + }; + + Ok((MadeProgress, expr, state)) + } +} + // Parser<'a, Vec<'a, Located>>> fn record_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>, SyntaxError<'a>> { then( diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index b1d3d3545a..1b2ea71cce 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -380,11 +380,25 @@ pub enum EExpr<'a> { When(When<'a>, Row, Col), If(If<'a>, Row, Col), + List(List<'a>, Row, Col), + // EInParens(PInParens<'a>, Row, Col), IndentStart(Row, Col), IndentEnd(Row, Col), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum List<'a> { + Open(Row, Col), + End(Row, Col), + Space(BadInputError, Row, Col), + + Syntax(&'a SyntaxError<'a>, Row, Col), + + IndentStart(Row, Col), + IndentEnd(Row, Col), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum When<'a> { Space(BadInputError, Row, Col), From 80b64b42ff348ec22d9aec5950a65a76dce5e4aa Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 23 Feb 2021 23:57:17 +0100 Subject: [PATCH 6/7] tests and list error messages --- compiler/parse/src/parser.rs | 3 +- compiler/reporting/src/error/parse.rs | 114 ++++++++++++++++++++- compiler/reporting/tests/test_reporting.rs | 82 +++++++++++++++ 3 files changed, 197 insertions(+), 2 deletions(-) diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 1b2ea71cce..ab980a5b38 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -394,8 +394,9 @@ pub enum List<'a> { Space(BadInputError, Row, Col), Syntax(&'a SyntaxError<'a>, Row, Col), + Expr(&'a EExpr<'a>, Row, Col), - IndentStart(Row, Col), + IndentOpen(Row, Col), IndentEnd(Row, Col), } diff --git a/compiler/reporting/src/error/parse.rs b/compiler/reporting/src/error/parse.rs index 33f09653c1..37c155e2cd 100644 --- a/compiler/reporting/src/error/parse.rs +++ b/compiler/reporting/src/error/parse.rs @@ -161,6 +161,7 @@ enum Node { IfCondition, IfThenBranch, IfElseBranch, + ListElement, } fn to_expr_report<'a>( @@ -175,11 +176,121 @@ fn to_expr_report<'a>( match parse_problem { EExpr::When(when, row, col) => to_when_report(alloc, filename, context, &when, *row, *col), - EExpr::If(when, row, col) => to_if_report(alloc, filename, context, &when, *row, *col), + EExpr::If(if_, row, col) => to_if_report(alloc, filename, context, &if_, *row, *col), + EExpr::List(list, row, col) => to_list_report(alloc, filename, context, &list, *row, *col), _ => todo!("unhandled parse error: {:?}", parse_problem), } } +fn to_list_report<'a>( + alloc: &'a RocDocAllocator<'a>, + filename: PathBuf, + context: Context, + parse_problem: &roc_parse::parser::List<'a>, + start_row: Row, + start_col: Col, +) -> Report<'a> { + use roc_parse::parser::List; + + match *parse_problem { + List::Syntax(syntax, row, col) => to_syntax_report(alloc, filename, syntax, row, col), + List::Space(error, row, col) => to_space_report(alloc, filename, &error, row, col), + + List::Expr(expr, row, col) => to_expr_report( + alloc, + filename, + Context::InNode(Node::ListElement, start_row, start_col, Box::new(context)), + expr, + row, + col, + ), + + List::Open(row, col) | List::End(row, col) => { + match dbg!(what_is_next(alloc.src_lines, row, col)) { + Next::Other(Some(',')) => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow( + r"I am partway through started parsing a list, but I got stuck here:", + ), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc + .reflow(r"I was expecting to see a list entry before this comma, "), + alloc.reflow(r"so try adding a list entry"), + alloc.reflow(r" and see if that helps?"), + ]), + ]); + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + _ => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow( + r"I am partway through started parsing a list, but I got stuck here:", + ), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc.reflow( + r"I was expecting to see a closing square bracket before this, ", + ), + alloc.reflow(r"so try adding a "), + alloc.parser_suggestion("]"), + alloc.reflow(r" and see if that helps?"), + ]), + alloc.concat(vec![ + alloc.note("When "), + alloc.reflow(r"I get stuck like this, "), + alloc.reflow(r"it usually means that there is a missing parenthesis "), + alloc.reflow(r"or bracket somewhere earlier. "), + alloc.reflow(r"It could also be a stray keyword or operator."), + ]), + ]); + + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + } + } + + List::IndentOpen(row, col) | List::IndentEnd(row, col) => { + let surroundings = Region::from_rows_cols(start_row, start_col, row, col); + let region = Region::from_row_col(row, col); + + let doc = alloc.stack(vec![ + alloc.reflow(r"I cannot find the end of this list:"), + alloc.region_with_subregion(surroundings, region), + alloc.concat(vec![ + alloc.reflow(r"You could change it to something like "), + alloc.parser_suggestion("[ 1, 2, 3 ]"), + alloc.reflow(" or even just "), + alloc.parser_suggestion("[]"), + alloc.reflow(". Anything where there is an open and a close square bracket, "), + alloc.reflow("and where the elements of the list are separated by commas."), + ]), + note_for_tag_union_type_indent(alloc), + ]); + + Report { + filename, + doc, + title: "UNFINISHED LIST".to_string(), + } + } + } +} + fn to_if_report<'a>( alloc: &'a RocDocAllocator<'a>, filename: PathBuf, @@ -1745,6 +1856,7 @@ fn to_space_report<'a>( } } +#[derive(Debug)] enum Next<'a> { Keyword(&'a str), // Operator(&'a str), diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index aa54cccaae..7ab6eefa14 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -5002,4 +5002,86 @@ mod test_reporting { ), ) } + + #[test] + fn list_double_comma() { + report_problem_as( + indoc!( + r#" + [ 1, 2, , 3 ] + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I am partway through started parsing a list, but I got stuck here: + + 1│ [ 1, 2, , 3 ] + ^ + + I was expecting to see a list entry before this comma, so try adding a + list entry and see if that helps? + "# + ), + ) + } + + #[test] + fn list_without_end() { + report_problem_as( + indoc!( + r#" + [ 1, 2, + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I am partway through started parsing a list, but I got stuck here: + + 1│ [ 1, 2, + ^ + + I was expecting to see a closing square bracket before this, so try + adding a ] and see if that helps? + + Note: When I get stuck like this, it usually means that there is a + missing parenthesis or bracket somewhere earlier. It could also be a + stray keyword or operator. + "# + ), + ) + } + + #[test] + fn list_bad_indent() { + report_problem_as( + indoc!( + r#" + x = [ 1, 2, + ] + + x + "# + ), + indoc!( + r#" + ── UNFINISHED LIST ───────────────────────────────────────────────────────────── + + I cannot find the end of this list: + + 1│ x = [ 1, 2, + ^ + + You could change it to something like [ 1, 2, 3 ] or even just []. + Anything where there is an open and a close square bracket, and where + the elements of the list are separated by commas. + + Note: I may be confused by indentation + "# + ), + ) + } } From 1c98bca071b71bc7db4985ccd1974f526dbb3ee3 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 24 Feb 2021 00:56:27 +0100 Subject: [PATCH 7/7] astar test does not use stdin --- cli/tests/cli_run.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/tests/cli_run.rs b/cli/tests/cli_run.rs index 6ad19aed58..49388b27b0 100644 --- a/cli/tests/cli_run.rs +++ b/cli/tests/cli_run.rs @@ -230,9 +230,8 @@ mod cli_run { #[test] #[serial(astar)] fn run_astar_optimized_1() { - check_output_with_stdin( + check_output( &example_file("benchmarks", "AStarTests.roc"), - "1", "astar-tests", &[], "True\n",