improve message for outdented then

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<S>, E>
 where
     S: Spaceable<'a>,
@@ -75,8 +76,11 @@ where
 {
     parser::map_with_arena(
         and(
-            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_problem)),
+            and(
+                parser,
+                space0_e(min_indent, space_problem, indent_after_problem),
+            ),
         ),
         move |arena: &'a Bump,
               tuples: (

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<Expr<'a>>, Located<Expr<'a>>), 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, then_branch), state) = if_branch(min_indent).parse(arena, 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))?;
 
             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))?;

compiler/parse/src/parser.rs

@@ -423,10 +423,10 @@ pub enum If<'a> {
 
     IndentCondition(Row, Col),
     IndentIf(Row, Col),
-    IndentThen(Row, Col),
+    IndentThenToken(Row, Col),
-    IndentElse(Row, Col),
+    IndentElseToken(Row, Col),
-
+    IndentThenBranch(Row, Col),
-    PatternAlignment(u16, 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
                                             )
                                         ),

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<Pattern<'a>>, 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)

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<TypeAnnotation<'a>>, 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<TypeAnnotation<'a>
         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,

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);

compiler/reporting/tests/test_reporting.rs

@@ -4637,7 +4637,7 @@ mod test_reporting {
                 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
                                 ^
@@ -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.
+            "#
+            ),
+        )
+    }
 }