Refactor ParseProblem

* Remove the `pos` field, which was always being assigned Position::default()
* Remove one use of this `pos`, by removing the never-used SyntaxError::ConditionFailed variant
* Adjust the other use to do what was probably intended - which is to say, pointing to the beginning of the def with the error
* Rename to FileError, reuse `SourceError` as an inner field, to avoid duplicating the `bytes`

compiler/load/src/file.rs

@@ -27,7 +27,7 @@ use roc_parse::ast::{self, ExtractSpaces, Spaced, StrLiteral, TypeAnnotation};
 use roc_parse::header::PackageName;
 use roc_parse::header::{ExposedName, ImportsEntry, PackageEntry, PlatformHeader, To, TypedIdent};
 use roc_parse::module::module_defs;
-use roc_parse::parser::{ParseProblem, Parser, SyntaxError};
+use roc_parse::parser::{FileError, Parser, SyntaxError};
 use roc_region::all::{LineInfo, Loc, Region};
 use roc_solve::module::SolvedModule;
 use roc_solve::solve;
@@ -843,7 +843,7 @@ enum Msg<'a> {
         exposed_to_host: MutMap<Symbol, Variable>,
     },
 
-    FailedToParse(ParseProblem<'a, SyntaxError<'a>>),
+    FailedToParse(FileError<'a, SyntaxError<'a>>),
     FailedToReadFile {
         filename: PathBuf,
         error: io::ErrorKind,
@@ -1035,7 +1035,7 @@ pub enum LoadingProblem<'a> {
         filename: PathBuf,
         error: io::ErrorKind,
     },
-    ParsingFailed(ParseProblem<'a, SyntaxError<'a>>),
+    ParsingFailed(FileError<'a, SyntaxError<'a>>),
     UnexpectedHeader(String),
 
     MsgChannelDied,
@@ -2412,7 +2412,7 @@ fn load_pkg_config<'a>(
                 }
                 Err(fail) => Err(LoadingProblem::ParsingFailed(
                     fail.map_problem(SyntaxError::Header)
-                        .into_parse_problem(filename),
+                        .into_file_error(filename),
                 )),
             }
         }
@@ -2656,7 +2656,7 @@ fn parse_header<'a>(
         )),
         Err(fail) => Err(LoadingProblem::ParsingFailed(
             fail.map_problem(SyntaxError::Header)
-                .into_parse_problem(filename),
+                .into_file_error(filename),
         )),
     }
 }
@@ -3709,7 +3709,7 @@ fn parse<'a>(arena: &'a Bump, header: ModuleHeader<'a>) -> Result<Msg<'a>, Loadi
         Ok((_, success, _state)) => success,
         Err((_, fail, state)) => {
             return Err(LoadingProblem::ParsingFailed(
-                fail.into_parse_problem(header.module_path, &state),
+                fail.into_file_error(header.module_path, &state),
             ));
         }
     };
@@ -4309,14 +4309,14 @@ fn to_file_problem_report(filename: &Path, error: io::ErrorKind) -> String {
 }
 
 fn to_parse_problem_report<'a>(
-    problem: ParseProblem<'a, SyntaxError<'a>>,
+    problem: FileError<'a, SyntaxError<'a>>,
     mut module_ids: ModuleIds,
     all_ident_ids: MutMap<ModuleId, IdentIds>,
 ) -> String {
     use roc_reporting::report::{parse_problem, RocDocAllocator, DEFAULT_PALETTE};
 
     // TODO this is not in fact safe
-    let src = unsafe { from_utf8_unchecked(problem.bytes) };
+    let src = unsafe { from_utf8_unchecked(problem.problem.bytes) };
     let src_lines = src.lines().collect::<Vec<_>>();
     // let mut src_lines: Vec<&str> = problem.prefix.lines().collect();
     // src_lines.extend(src.lines().skip(1));

compiler/parse/src/module.rs

@@ -7,8 +7,8 @@ use crate::header::{
 use crate::ident::{lowercase_ident, unqualified_ident, uppercase_ident};
 use crate::parser::Progress::{self, *};
 use crate::parser::{
-    backtrackable, specialize, word1, word2, EEffects, EExposes, EHeader, EImports, EPackages,
+    backtrackable, specialize, specialize_region, word1, word2, EEffects, EExposes, EHeader,
-    EProvides, ERequires, ETypedIdent, Parser, SourceError, SyntaxError,
+    EImports, EPackages, EProvides, ERequires, ETypedIdent, Parser, SourceError, SyntaxError,
 };
 use crate::state::State;
 use crate::string_literal;
@@ -31,7 +31,10 @@ pub fn module_defs<'a>() -> impl Parser<'a, Vec<'a, Loc<Def<'a>>>, SyntaxError<'
     // force that we parse until the end of the input
     let min_indent = 0;
     skip_second!(
-        specialize(|e, _| SyntaxError::Expr(e), crate::expr::defs(min_indent),),
+        specialize_region(
+            |e, r| SyntaxError::Expr(e, r.start()),
+            crate::expr::defs(min_indent),
+        ),
         end_of_file()
     )
 }

compiler/parse/src/parser.rs

@@ -48,7 +48,6 @@ impl Progress {
 pub enum SyntaxError<'a> {
     Unexpected(Region),
     OutdentedTooFar,
-    ConditionFailed,
     TooManyLines,
     Eof(Region),
     InvalidPattern,
@@ -59,7 +58,7 @@ pub enum SyntaxError<'a> {
     Todo,
     Type(EType<'a>),
     Pattern(EPattern<'a>),
-    Expr(EExpr<'a>),
+    Expr(EExpr<'a>, Position),
     Header(EHeader<'a>),
     Space(BadInputError),
     NotEndOfFile(Position),
@@ -237,12 +236,10 @@ impl<'a, T> SourceError<'a, T> {
         }
     }
 
-    pub fn into_parse_problem(self, filename: std::path::PathBuf) -> ParseProblem<'a, T> {
+    pub fn into_file_error(self, filename: std::path::PathBuf) -> FileError<'a, T> {
-        ParseProblem {
+        FileError {
-            pos: Position::default(),
+            problem: self,
-            problem: self.problem,
             filename,
-            bytes: self.bytes,
         }
     }
 }
@@ -255,17 +252,12 @@ impl<'a> SyntaxError<'a> {
         }
     }
 
-    pub fn into_parse_problem(
+    pub fn into_file_error(
         self,
         filename: std::path::PathBuf,
         state: &State<'a>,
-    ) -> ParseProblem<'a, SyntaxError<'a>> {
+    ) -> FileError<'a, SyntaxError<'a>> {
-        ParseProblem {
+        self.into_source_error(state).into_file_error(filename)
-            pos: Position::default(),
-            problem: self,
-            filename,
-            bytes: state.original_bytes(),
-        }
     }
 }
 
@@ -594,11 +586,9 @@ pub struct SourceError<'a, T> {
 }
 
 #[derive(Debug)]
-pub struct ParseProblem<'a, T> {
+pub struct FileError<'a, T> {
-    pub pos: Position,
+    pub problem: SourceError<'a, T>,
-    pub problem: T,
     pub filename: std::path::PathBuf,
-    pub bytes: &'a [u8],
 }
 
 pub trait Parser<'a, Output, Error> {
@@ -1295,6 +1285,22 @@ where
     }
 }
 
+/// Like `specialize`, except the error function receives a Region representing the begin/end of the error
+pub fn specialize_region<'a, F, P, T, X, Y>(map_error: F, parser: P) -> impl Parser<'a, T, Y>
+where
+    F: Fn(X, Region) -> Y,
+    P: Parser<'a, T, X>,
+    Y: 'a,
+{
+    move |a, s: State<'a>| {
+        let start = s.pos();
+        match parser.parse(a, s) {
+            Ok(t) => Ok(t),
+            Err((p, error, s)) => Err((p, map_error(error, Region::new(start, s.pos())), s)),
+        }
+    }
+}
+
 pub fn specialize_ref<'a, F, P, T, X, Y>(map_error: F, parser: P) -> impl Parser<'a, T, Y>
 where
     F: Fn(&'a X, Position) -> Y,

compiler/parse/src/test_helpers.rs

@@ -8,6 +8,7 @@ use crate::state::State;
 use bumpalo::collections::Vec as BumpVec;
 use bumpalo::Bump;
 use roc_region::all::Loc;
+use roc_region::all::Position;
 
 pub fn parse_expr_with<'a>(
     arena: &'a Bump,
@@ -27,7 +28,7 @@ pub fn parse_loc_with<'a>(
 
     match crate::expr::test_parse_expr(0, arena, state.clone()) {
         Ok(loc_expr) => Ok(loc_expr),
-        Err(fail) => Err(SyntaxError::Expr(fail).into_source_error(&state)),
+        Err(fail) => Err(SyntaxError::Expr(fail, Position::default()).into_source_error(&state)),
     }
 }
 

compiler/parse/tests/snapshots/fail/type_argument_no_arrow.expr.result-ast

@@ -1 +1 @@
-Expr(Type(TIndentEnd(@12), @4))
+Expr(Type(TIndentEnd(@12), @4), @0)

compiler/parse/tests/snapshots/fail/type_double_comma.expr.result-ast

@@ -1 +1 @@
-Expr(Type(TFunctionArgument(@8), @4))
+Expr(Type(TFunctionArgument(@8), @4), @0)

docs/src/lib.rs

@@ -16,7 +16,7 @@ use roc_module::symbol::{IdentIds, Interns, ModuleId, ModuleIds};
 use roc_parse::ident::{parse_ident, Ident};
 use roc_parse::parser::SyntaxError;
 use roc_parse::state::State;
-use roc_region::all::Region;
+use roc_region::all::{Position, Region};
 use std::fs;
 use std::path::{Path, PathBuf};
 
@@ -128,7 +128,7 @@ pub fn syntax_highlight_expr<'a>(
 
             Ok(buf.to_string())
         }
-        Err(fail) => Err(SyntaxError::Expr(fail)),
+        Err(fail) => Err(SyntaxError::Expr(fail, Position::default())),
     }
 }
 

reporting/src/error/parse.rs

@@ -1,4 +1,4 @@
-use roc_parse::parser::{ParseProblem, SyntaxError};
+use roc_parse::parser::{FileError, SyntaxError};
 use roc_region::all::{LineColumn, LineColumnRegion, LineInfo, Position, Region};
 use std::path::PathBuf;
 
@@ -10,15 +10,9 @@ pub fn parse_problem<'a>(
     lines: &LineInfo,
     filename: PathBuf,
     _starting_line: u32,
-    parse_problem: ParseProblem<SyntaxError<'a>>,
+    parse_problem: FileError<SyntaxError<'a>>,
 ) -> Report<'a> {
-    to_syntax_report(
+    to_syntax_report(alloc, lines, filename, &parse_problem.problem.problem)
-        alloc,
-        lines,
-        filename,
-        &parse_problem.problem,
-        parse_problem.pos,
-    )
 }
 
 fn note_for_record_type_indent<'a>(alloc: &'a RocDocAllocator<'a>) -> RocDocBuilder<'a> {
@@ -68,7 +62,6 @@ fn to_syntax_report<'a>(
     lines: &LineInfo,
     filename: PathBuf,
     parse_problem: &roc_parse::parser::SyntaxError<'a>,
-    start: Position,
 ) -> Report<'a> {
     use SyntaxError::*;
 
@@ -79,22 +72,7 @@ fn to_syntax_report<'a>(
         severity: Severity::RuntimeError,
     };
 
-    let region = Region::from_pos(start);
-
     match parse_problem {
-        SyntaxError::ConditionFailed => {
-            let doc = alloc.stack(vec![
-                alloc.reflow("A condition failed:"),
-                alloc.region(lines.convert_region(region)),
-            ]);
-
-            Report {
-                filename,
-                doc,
-                title: "PARSE PROBLEM".to_string(),
-                severity: Severity::RuntimeError,
-            }
-        }
         SyntaxError::ArgumentsBeforeEquals(region) => {
             let doc = alloc.stack(vec![
                 alloc.reflow("Unexpected tokens in front of the `=` symbol:"),
@@ -126,12 +104,11 @@ fn to_syntax_report<'a>(
             report(doc)
         }
         NotEndOfFile(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 expected to reach the end of the file, but got stuck here:"),
-                alloc.region_with_subregion(lines.convert_region(surroundings), region),
+                alloc.region(region),
                 alloc.concat(vec![alloc.reflow("no hints")]),
             ]);
 
@@ -167,11 +144,11 @@ fn to_syntax_report<'a>(
         }
         Type(typ) => to_type_report(alloc, lines, filename, typ, Position::default()),
         Pattern(pat) => to_pattern_report(alloc, lines, filename, pat, Position::default()),
-        Expr(expr) => to_expr_report(
+        Expr(expr, start) => to_expr_report(
             alloc,
             lines,
             filename,
-            Context::InDef(start),
+            Context::InDef(*start),
             expr,
             Position::default(),
         ),

reporting/tests/test_reporting.rs

@@ -142,7 +142,7 @@ mod test_reporting {
 
                 let alloc = RocDocAllocator::new(&src_lines, home, &interns);
 
-                let problem = fail.into_parse_problem(filename.clone());
+                let problem = fail.into_file_error(filename.clone());
                 let doc = parse_problem(&alloc, &lines, filename, 0, problem);
 
                 callback(doc.pretty(&alloc).append(alloc.line()), buf)
@@ -209,7 +209,7 @@ mod test_reporting {
                 use roc_parse::parser::SyntaxError;
                 let problem = fail
                     .map_problem(SyntaxError::Header)
-                    .into_parse_problem(filename.clone());
+                    .into_file_error(filename.clone());
                 let doc = parse_problem(&alloc, &lines, filename, 0, problem);
 
                 callback(doc.pretty(&alloc).append(alloc.line()), buf)