Detect outdents too far

compiler/parse/src/expr.rs

@@ -190,6 +190,7 @@ fn record_field_access<'a>() -> impl Parser<'a, &'a str, EExpr<'a>> {
 /// pattern later
 fn parse_loc_term_or_underscore<'a>(
     min_indent: u32,
+    outdent_col: u32,
     options: ExprParseOptions,
     arena: &'a Bump,
     state: State<'a>,
@@ -201,8 +202,11 @@ fn parse_loc_term_or_underscore<'a>(
         loc!(specialize(EExpr::Number, positive_number_literal_help())),
         loc!(specialize(EExpr::Lambda, closure_help(min_indent, options))),
         loc!(underscore_expression()),
-        loc!(record_literal_help(min_indent)),
+        loc!(record_literal_help(min_indent, outdent_col)),
-        loc!(specialize(EExpr::List, list_literal_help(min_indent))),
+        loc!(specialize(
+            EExpr::List,
+            list_literal_help(min_indent, outdent_col)
+        )),
         loc!(map_with_arena!(
             assign_or_destructure_identifier(),
             ident_to_expr
@@ -213,6 +217,7 @@ fn parse_loc_term_or_underscore<'a>(
 
 fn parse_loc_term<'a>(
     min_indent: u32,
+    outdent_col: u32,
     options: ExprParseOptions,
     arena: &'a Bump,
     state: State<'a>,
@@ -223,8 +228,11 @@ fn parse_loc_term<'a>(
         loc!(specialize(EExpr::SingleQuote, single_quote_literal_help())),
         loc!(specialize(EExpr::Number, positive_number_literal_help())),
         loc!(specialize(EExpr::Lambda, closure_help(min_indent, options))),
-        loc!(record_literal_help(min_indent)),
+        loc!(record_literal_help(min_indent, outdent_col)),
-        loc!(specialize(EExpr::List, list_literal_help(min_indent))),
+        loc!(specialize(
+            EExpr::List,
+            list_literal_help(min_indent, outdent_col)
+        )),
         loc!(map_with_arena!(
             assign_or_destructure_identifier(),
             ident_to_expr
@@ -252,6 +260,7 @@ fn underscore_expression<'a>() -> impl Parser<'a, Expr<'a>, EExpr<'a>> {
 
 fn loc_possibly_negative_or_negated_term<'a>(
     min_indent: u32,
+    outdent_col: u32,
     options: ExprParseOptions,
 ) -> impl Parser<'a, Loc<Expr<'a>>, EExpr<'a>> {
     one_of![
@@ -259,7 +268,11 @@ fn loc_possibly_negative_or_negated_term<'a>(
             let initial = state.clone();
 
             let (_, (loc_op, loc_expr), state) = and!(loc!(unary_negate()), |a, s| parse_loc_term(
-                min_indent, options, a, s
+                min_indent,
+                outdent_col,
+                options,
+                a,
+                s
             ))
             .parse(arena, state)?;
 
@@ -271,13 +284,15 @@ fn loc_possibly_negative_or_negated_term<'a>(
         loc!(specialize(EExpr::Number, number_literal_help())),
         loc!(map_with_arena!(
             and!(loc!(word1(b'!', EExpr::Start)), |a, s| {
-                parse_loc_term(min_indent, options, a, s)
+                parse_loc_term(min_indent, outdent_col, options, a, s)
             }),
             |arena: &'a Bump, (loc_op, loc_expr): (Loc<_>, _)| {
                 Expr::UnaryOp(arena.alloc(loc_expr), Loc::at(loc_op.region, UnaryOp::Not))
             }
         )),
-        |arena, state| { parse_loc_term_or_underscore(min_indent, options, arena, state) }
+        |arena, state| {
+            parse_loc_term_or_underscore(min_indent, outdent_col, options, arena, state)
+        }
     ]
 }
 
@@ -336,8 +351,8 @@ fn parse_expr_operator_chain<'a>(
     arena: &'a Bump,
     state: State<'a>,
 ) -> ParseResult<'a, Expr<'a>, EExpr<'a>> {
-    let (_, expr, state) =
+    let (_, expr, state) = loc_possibly_negative_or_negated_term(min_indent, start_column, options)
-        loc_possibly_negative_or_negated_term(min_indent, options).parse(arena, state)?;
+        .parse(arena, state)?;
 
     let initial = state.clone();
     let end = state.pos();
@@ -1333,7 +1348,8 @@ fn parse_expr_operator<'a>(
         BinOp::Minus if expr_state.end != op_start && op_end == new_start => {
             // negative terms
 
-            let (_, negated_expr, state) = parse_loc_term(min_indent, options, arena, state)?;
+            let (_, negated_expr, state) =
+                parse_loc_term(min_indent, start_column, options, arena, state)?;
             let new_end = state.pos();
 
             let arg = numeric_negate_expression(
@@ -1467,7 +1483,9 @@ fn parse_expr_operator<'a>(
                 _ => unreachable!(),
             },
         ),
-        _ => match loc_possibly_negative_or_negated_term(min_indent, options).parse(arena, state) {
+        _ => match loc_possibly_negative_or_negated_term(min_indent, start_column, options)
+            .parse(arena, state)
+        {
             Err((MadeProgress, f, s)) => Err((MadeProgress, f, s)),
             Ok((_, mut new_expr, state)) => {
                 let new_end = state.pos();
@@ -1526,7 +1544,7 @@ fn parse_expr_end<'a>(
 ) -> ParseResult<'a, Expr<'a>, EExpr<'a>> {
     let parser = skip_first!(
         crate::blankspace::check_indent(min_indent, EExpr::IndentEnd),
-        move |a, s| parse_loc_term(min_indent, options, a, s)
+        move |a, s| parse_loc_term(min_indent, start_column, options, a, s)
     );
 
     match parser.parse(arena, state.clone()) {
@@ -2467,7 +2485,10 @@ fn ident_to_expr<'a>(arena: &'a Bump, src: Ident<'a>) -> Expr<'a> {
     }
 }
 
-fn list_literal_help<'a>(min_indent: u32) -> impl Parser<'a, Expr<'a>, EList<'a>> {
+fn list_literal_help<'a>(
+    min_indent: u32,
+    outdent_col: u32,
+) -> impl Parser<'a, Expr<'a>, EList<'a>> {
     move |arena, state| {
         let (_, elements, state) = collection_trailing_sep_e!(
             word1(b'[', EList::Open),
@@ -2478,8 +2499,10 @@ fn list_literal_help<'a>(min_indent: u32) -> impl Parser<'a, Expr<'a>, EList<'a>
             word1(b',', EList::End),
             word1(b']', EList::End),
             min_indent,
+            outdent_col,
             EList::Open,
             EList::IndentEnd,
+            EList::OutdentEnd,
             Expr::SpaceBefore
         )
         .parse(arena, state)?;
@@ -2555,6 +2578,7 @@ fn record_updateable_identifier<'a>() -> impl Parser<'a, Expr<'a>, ERecord<'a>>
 
 fn record_help<'a>(
     min_indent: u32,
+    outdent_col: u32,
 ) -> impl Parser<
     'a,
     (
@@ -2589,8 +2613,8 @@ fn record_help<'a>(
                 // `{  }` can be successfully parsed as an empty record, and then
                 // changed by the formatter back into `{}`.
                 zero_or_more!(word1(b' ', ERecord::End)),
-                skip_second!(
+                |arena, state| {
-                    and!(
+                    let (_, (parsed_elems, comments), state) = and!(
                         trailing_sep_by0(
                             word1(b',', ERecord::End),
                             space0_before_optional_after(
@@ -2600,19 +2624,35 @@ fn record_help<'a>(
                                 ERecord::IndentEnd
                             ),
                         ),
-                        // Allow outdented closing braces
+                        space0_e(0, ERecord::OutdentEnd)
-                        space0_e(0, ERecord::IndentEnd)
+                    )
-                    ),
+                    .parse(arena, state)?;
-                    word1(b'}', ERecord::End)
+
-                )
+                    let closing_brace_col = state.column();
+                    let closing_brace_pos = state.pos();
+
+                    let (_, _, state) = word1(b'}', ERecord::End).parse(arena, state)?;
+
+                    if closing_brace_col < outdent_col {
+                        return Err((MadeProgress, ERecord::OutdentEnd(closing_brace_pos), state));
+                    }
+
+                    Ok((MadeProgress, (parsed_elems, comments), state))
+                }
             ))
         )
     )
 }
 
-fn record_literal_help<'a>(min_indent: u32) -> impl Parser<'a, Expr<'a>, EExpr<'a>> {
+fn record_literal_help<'a>(
+    min_indent: u32,
+    outdent_col: u32,
+) -> impl Parser<'a, Expr<'a>, EExpr<'a>> {
     then(
-        loc!(specialize(EExpr::Record, record_help(min_indent))),
+        loc!(specialize(
+            EExpr::Record,
+            record_help(min_indent, outdent_col)
+        )),
         move |arena, state, _, loc_record| {
             let (opt_update, loc_assigned_fields_with_comments) = loc_record.value;
 

compiler/parse/src/module.rs

@@ -416,6 +416,7 @@ fn provides_without_to<'a>() -> impl Parser<
     EProvides<'a>,
 > {
     let min_indent = 1;
+    let outdent_col = 0;
     and!(
         spaces_around_keyword(
             min_indent,
@@ -431,8 +432,10 @@ fn provides_without_to<'a>() -> impl Parser<
                 word1(b',', EProvides::ListEnd),
                 word1(b']', EProvides::ListEnd),
                 min_indent,
+                outdent_col,
                 EProvides::Open,
                 EProvides::IndentListEnd,
+                EProvides::IndentListEnd,
                 Spaced::SpaceBefore
             ),
             // Optionally
@@ -445,6 +448,7 @@ fn provides_without_to<'a>() -> impl Parser<
 fn provides_types<'a>(
 ) -> impl Parser<'a, Collection<'a, Loc<Spaced<'a, UppercaseIdent<'a>>>>, EProvides<'a>> {
     let min_indent = 1;
+    let outdent_col = 0;
 
     skip_first!(
         // We only support spaces here, not newlines, because this is not intended
@@ -464,8 +468,10 @@ fn provides_types<'a>(
             word1(b',', EProvides::ListEnd),
             word1(b'}', EProvides::ListEnd),
             min_indent,
+            outdent_col,
             EProvides::Open,
             EProvides::IndentListEnd,
+            EProvides::IndentListEnd,
             Spaced::SpaceBefore
         )
     )
@@ -545,8 +551,10 @@ fn requires_rigids<'a>(
         word1(b',', ERequires::ListEnd),
         word1(b'}', ERequires::ListEnd),
         min_indent,
+        0,
         ERequires::Open,
         ERequires::IndentListEnd,
+        ERequires::IndentListEnd,
         Spaced::SpaceBefore
     )
 }
@@ -577,6 +585,7 @@ fn exposes_values<'a>() -> impl Parser<
     EExposes,
 > {
     let min_indent = 1;
+    let outdent_col = 0;
 
     and!(
         spaces_around_keyword(
@@ -592,8 +601,10 @@ fn exposes_values<'a>() -> impl Parser<
             word1(b',', EExposes::ListEnd),
             word1(b']', EExposes::ListEnd),
             min_indent,
+            outdent_col,
             EExposes::Open,
             EExposes::IndentListEnd,
+            EExposes::IndentListEnd,
             Spaced::SpaceBefore
         )
     )
@@ -628,6 +639,7 @@ fn exposes_modules<'a>() -> impl Parser<
     EExposes,
 > {
     let min_indent = 1;
+    let outdent_col = 0;
 
     and!(
         spaces_around_keyword(
@@ -643,8 +655,10 @@ fn exposes_modules<'a>() -> impl Parser<
             word1(b',', EExposes::ListEnd),
             word1(b']', EExposes::ListEnd),
             min_indent,
+            outdent_col,
             EExposes::Open,
             EExposes::IndentListEnd,
+            EExposes::IndentListEnd,
             Spaced::SpaceBefore
         )
     )
@@ -674,6 +688,7 @@ struct Packages<'a> {
 #[inline(always)]
 fn packages<'a>() -> impl Parser<'a, Packages<'a>, EPackages<'a>> {
     let min_indent = 1;
+    let outdent_col = 0;
 
     map!(
         and!(
@@ -690,8 +705,10 @@ fn packages<'a>() -> impl Parser<'a, Packages<'a>, EPackages<'a>> {
                 word1(b',', EPackages::ListEnd),
                 word1(b'}', EPackages::ListEnd),
                 min_indent,
+                outdent_col,
                 EPackages::Open,
                 EPackages::IndentListEnd,
+                EPackages::IndentListEnd,
                 Spaced::SpaceBefore
             )
         ),
@@ -741,6 +758,7 @@ fn generates_with<'a>() -> impl Parser<
     EGeneratesWith,
 > {
     let min_indent = 1;
+    let outdent_col = 0;
 
     and!(
         spaces_around_keyword(
@@ -756,8 +774,10 @@ fn generates_with<'a>() -> impl Parser<
             word1(b',', EGeneratesWith::ListEnd),
             word1(b']', EGeneratesWith::ListEnd),
             min_indent,
+            outdent_col,
             EGeneratesWith::Open,
             EGeneratesWith::IndentListEnd,
+            EGeneratesWith::IndentListEnd,
             Spaced::SpaceBefore
         )
     )
@@ -773,6 +793,7 @@ fn imports<'a>() -> impl Parser<
     EImports,
 > {
     let min_indent = 1;
+    let outdent_col = 0;
 
     and!(
         spaces_around_keyword(
@@ -788,8 +809,10 @@ fn imports<'a>() -> impl Parser<
             word1(b',', EImports::ListEnd),
             word1(b']', EImports::ListEnd),
             min_indent,
+            outdent_col,
             EImports::Open,
             EImports::IndentListEnd,
+            EImports::IndentListEnd,
             Spaced::SpaceBefore
         )
     )
@@ -849,6 +872,7 @@ where
 #[inline(always)]
 fn imports_entry<'a>() -> impl Parser<'a, Spaced<'a, ImportsEntry<'a>>, EImports> {
     let min_indent = 1;
+    let outdent_col = 0;
 
     type Temp<'a> = (
         (Option<&'a str>, ModuleName<'a>),
@@ -875,8 +899,10 @@ fn imports_entry<'a>() -> impl Parser<'a, Spaced<'a, ImportsEntry<'a>>, EImports
                     word1(b',', EImports::SetEnd),
                     word1(b'}', EImports::SetEnd),
                     min_indent,
+                    outdent_col,
                     EImports::Open,
                     EImports::IndentSetEnd,
+                    EImports::IndentSetEnd,
                     Spaced::SpaceBefore
                 )
             ))

compiler/parse/src/parser.rs

@@ -407,6 +407,7 @@ pub enum ERecord<'a> {
     IndentBar(Position),
     IndentAmpersand(Position),
     IndentEnd(Position),
+    OutdentEnd(Position),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -448,6 +449,7 @@ pub enum EList<'a> {
 
     IndentOpen(Position),
     IndentEnd(Position),
+    OutdentEnd(Position),
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -1211,7 +1213,7 @@ macro_rules! collection {
 
 #[macro_export]
 macro_rules! collection_trailing_sep_e {
-    ($opening_brace:expr, $elem:expr, $delimiter:expr, $closing_brace:expr, $min_indent:expr, $open_problem:expr, $indent_problem:expr, $space_before:expr) => {
+    ($opening_brace:expr, $elem:expr, $delimiter:expr, $closing_brace:expr, $min_indent:expr, $outdent_col:expr, $open_problem:expr, $indent_problem:expr, $outdent_problem:expr, $space_before:expr) => {
         skip_first!(
             $opening_brace,
             |arena, state| {
@@ -1230,12 +1232,13 @@ macro_rules! collection_trailing_sep_e {
                                         )
                                     ),
                                     $crate::blankspace::space0_e(
-                                        // we use min_indent=0 because we want to parse incorrectly indented closing braces
+                                        0,
-                                        // and later fix these up in the formatter.
-                                        0 /* min_indent */,
                                         $indent_problem)
                                 ).parse(arena, state)?;
 
+                let closing_brace_col = state.column();
+                let closing_brace_pos = state.pos();
+
                 let (_,_, state) =
                         if parsed_elems.is_empty() {
                             one_of_with_error![$open_problem; $closing_brace].parse(arena, state)?
@@ -1243,6 +1246,13 @@ macro_rules! collection_trailing_sep_e {
                             $closing_brace.parse(arena, state)?
                         };
 
+                #[allow(unused_comparisons)] // sometimes $outdent_col is 0
+                if closing_brace_col < $outdent_col {
+                    // We successfully parsed the collection but the closing brace was outdented
+                    // further than expected.
+                    return Err((MadeProgress, $outdent_problem(closing_brace_pos), state));
+                }
+
                 if !spaces.is_empty() {
                     if let Some(first) = parsed_elems.first_mut() {
                         first.value = $space_before(arena.alloc(first.value), spaces)

compiler/parse/src/pattern.rs

@@ -365,8 +365,10 @@ fn record_pattern_help<'a>(min_indent: u32) -> impl Parser<'a, Pattern<'a>, PRec
             // word1_check_indent!(b'}', PRecord::End, min_indent, PRecord::IndentEnd),
             word1(b'}', PRecord::End),
             min_indent,
+            0,
             PRecord::Open,
             PRecord::IndentEnd,
+            PRecord::IndentEnd,
             Pattern::SpaceBefore
         )
         .parse(arena, state)?;

compiler/parse/src/type_annotation.rs

@@ -31,8 +31,10 @@ fn tag_union_type<'a>(min_indent: u32) -> impl Parser<'a, TypeAnnotation<'a>, ET
             word1(b',', ETypeTagUnion::End),
             word1(b']', ETypeTagUnion::End),
             min_indent,
+            0,
             ETypeTagUnion::Open,
             ETypeTagUnion::IndentEnd,
+            ETypeTagUnion::IndentEnd,
             Tag::SpaceBefore
         )
         .parse(arena, state)?;
@@ -323,8 +325,10 @@ fn record_type<'a>(min_indent: u32) -> impl Parser<'a, TypeAnnotation<'a>, EType
             // word1_check_indent!(b'}', TRecord::End, min_indent, TRecord::IndentEnd),
             word1(b'}', ETypeRecord::End),
             min_indent,
+            0,
             ETypeRecord::Open,
             ETypeRecord::IndentEnd,
+            ETypeRecord::IndentEnd,
             AssignedField::SpaceBefore
         )
         .parse(arena, state)?;

reporting/src/error/parse.rs

@@ -1,4 +1,4 @@
-use roc_parse::parser::{ENumber, FileError, SyntaxError};
+use roc_parse::parser::{ENumber, ERecord, FileError, SyntaxError};
 use roc_region::all::{LineColumn, LineColumnRegion, LineInfo, Position, Region};
 use std::path::PathBuf;
 
@@ -516,23 +516,42 @@ fn to_expr_report<'a>(
             }
         }
 
-        EExpr::Record(_erecord, pos) => {
+        EExpr::Record(erecord, pos) => match erecord {
-            let surroundings = Region::new(start, *pos);
+            &ERecord::OutdentEnd(pos) => {
-            let region = LineColumnRegion::from_pos(lines.convert_pos(*pos));
+                let surroundings = Region::new(start, pos);
+                let region = LineColumnRegion::from_pos(lines.convert_pos(pos));
 
-            let doc = alloc.stack(vec![
+                let doc = alloc.stack(vec![
-                alloc.reflow(r"I am partway through parsing an record, but I got stuck here:"),
+                    alloc.reflow(r"I found the end of this record, but it's outdented too far:"),
-                alloc.region_with_subregion(lines.convert_region(surroundings), region),
+                    alloc.region_with_subregion(lines.convert_region(surroundings), region),
-                alloc.concat(vec![alloc.reflow("TODO provide more context.")]),
+                    alloc.reflow(r"Did you mean to indent it further?"),
-            ]);
+                ]);
 
-            Report {
+                Report {
-                filename,
+                    filename,
-                doc,
+                    doc,
-                title: "RECORD PARSE PROBLEM".to_string(),
+                    title: "RECORD END OUDENTED TOO FAR".to_string(),
-                severity: Severity::RuntimeError,
+                    severity: Severity::RuntimeError,
+                }
             }
-        }
+            _ => {
+                let surroundings = Region::new(start, *pos);
+                let region = LineColumnRegion::from_pos(lines.convert_pos(*pos));
+
+                let doc = alloc.stack(vec![
+                    alloc.reflow(r"I am partway through parsing an record, but I got stuck here:"),
+                    alloc.region_with_subregion(lines.convert_region(surroundings), region),
+                    alloc.concat(vec![alloc.reflow("TODO provide more context.")]),
+                ]);
+
+                Report {
+                    filename,
+                    doc,
+                    title: "RECORD PARSE PROBLEM".to_string(),
+                    severity: Severity::RuntimeError,
+                }
+            }
+        },
 
         EExpr::Space(error, pos) => to_space_report(alloc, lines, filename, error, *pos),
 
@@ -542,6 +561,23 @@ fn to_expr_report<'a>(
 
         EExpr::Ability(err, pos) => to_ability_def_report(alloc, lines, filename, err, *pos),
 
+        EExpr::IndentEnd(pos) => {
+            let surroundings = Region::new(start, *pos);
+            let region = LineColumnRegion::from_pos(lines.convert_pos(*pos));
+
+            let doc = alloc.stack(vec![
+                alloc.reflow("Indentation unexpectedly ended here:"),
+                alloc.region_with_subregion(lines.convert_region(surroundings), region),
+            ]);
+
+            Report {
+                filename,
+                doc,
+                title: "INDENTATION ENDED EARLY".to_string(),
+                // In an ideal world, this is recoverable and we keep parsing.
+                severity: Severity::Warning,
+            }
+        }
         _ => todo!("unhandled parse error: {:?}", parse_problem),
     }
 }
@@ -1116,6 +1152,24 @@ fn to_list_report<'a>(
                 severity: Severity::RuntimeError,
             }
         }
+
+        EList::OutdentEnd(pos) => {
+            let surroundings = Region::new(start, pos);
+            let region = LineColumnRegion::from_pos(lines.convert_pos(pos));
+
+            let doc = alloc.stack(vec![
+                alloc.reflow(r"I found the end of this list, but it's outdented too far:"),
+                alloc.region_with_subregion(lines.convert_region(surroundings), region),
+                alloc.reflow(r"Did you mean to indent it further?"),
+            ]);
+
+            Report {
+                filename,
+                doc,
+                title: "LIST END OUDENTED TOO FAR".to_string(),
+                severity: Severity::RuntimeError,
+            }
+        }
     }
 }
 

reporting/tests/test_reporting.rs

@@ -9780,4 +9780,65 @@ I need all branches in an `if` to have the same type!
             ),
         )
     }
+
+    #[test]
+    fn list_outdented_too_far() {
+        report_problem_as(
+            indoc!(
+                r#"
+                w = 
+                    a = [
+                      1, 2, 3
+                  ]
+                    a
+                w
+                "#
+            ),
+            indoc!(
+                r#"
+                ── LIST END OUDENTED TOO FAR ───────────────────────────────────────────────────
+
+                I found the end of this list, but it's outdented too far:
+
+                2│      a = [
+                3│        1, 2, 3
+                4│    ]
+                      ^
+
+                Did you mean to indent it further?
+                "#
+            ),
+        )
+    }
+
+    #[test]
+    fn record_outdented_too_far() {
+        report_problem_as(
+            indoc!(
+                r#"
+                w =
+                    r = {
+                      sweet: "disposition"
+                  }
+                    r
+                w
+                "#
+            ),
+            indoc!(
+                r#"
+                ── RECORD END OUDENTED TOO FAR ─────────────────────────────────────────────────
+
+                I found the end of this record, but it's outdented too far:
+
+                1│  w =
+                2│      r = {
+                3│        sweet: "disposition"
+                4│    }
+                      ^
+
+                Did you mean to indent it further?
+                "#
+            ),
+        )
+    }
 }