Skip to content

Commit

Permalink
Make ListStart branch less brittle; Add ListEnd branch
Browse files Browse the repository at this point in the history
  • Loading branch information
@jaredramirez
jaredramirez committed Dec 31, 2024
1 parent 69c36af commit c28d6b9
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 59 deletions.
89 changes: 66 additions & 23 deletions crates/compiler/load/tests/test_reporting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6433,59 +6433,102 @@ All branches in an `if` must have the same type!
}

#[test]
fn invalid_app_name() {
fn exposes_start() {
report_header_problem_as(
indoc!(
r"
app foobar
exposes [main, @Foo]
imports [pf.Task, Base64]
module foobar []
"
),
indoc!(
r#"
── WEIRD APP NAME in /code/proj/Main.roc ───────────────────────────────────────
── WEIRD EXPOSES in /code/proj/Main.roc ────────────────────────────────────────
I am partway through parsing a header, but got stuck here:
I am partway through parsing a header, but I got stuck here:
1│ app foobar
^
1│ module foobar []
^
I am expecting an application name next, like app "main" or
app "editor". App names are surrounded by quotation marks.
I was expecting an `exposes` list like
[Animal, default, tame]
"#
),
)
}

#[test]
fn unneeded_module_name() {
fn exposes_missing_comma() {
report_header_problem_as(
indoc!(
r"
module foobar []
module [value func]
"
),
indoc!(
r#"
── WEIRD MODULE NAME in /code/proj/Main.roc ────────────────────────────────────
── WEIRD EXPOSES in /code/proj/Main.roc ────────────────────────────────────────
I am partway through parsing a header, but I got stuck here:
I am partway through parsing an `exposes` list, but I got stuck here:
1│ module foobar []
^
1│ module [value func]
^
I am expecting a list of exports like
I was expecting a type name, value name or function name next, like
module [func, value]
[Animal, default, tame]
"#
),
)
}

or module params like
#[test]
fn exposes_end() {
report_header_problem_as(
indoc!(
r"
module [value
"
),
indoc!(
r#"
── WEIRD EXPOSES in /code/proj/Main.roc ────────────────────────────────────────
I am partway through parsing an `exposes` list, but I got stuck here:
1│ module [value
2│
^
I was expecting a type name, value name or function name next, like
module { echo } -> [func, value]
[Animal, default, tame]
"#
),
)
}

If you're trying to specify a module name, recall that unlike
application names, module names are not specified in the header.
Instead, they are derived from the name of the module's filename.
#[test]
fn invalid_app_name() {
report_header_problem_as(
indoc!(
r"
app foobar
exposes [main, @Foo]
imports [pf.Task, Base64]
"
),
indoc!(
r#"
── WEIRD APP NAME in /code/proj/Main.roc ───────────────────────────────────────
I am partway through parsing a header, but got stuck here:
1│ app foobar
^
I am expecting an application name next, like app "main" or
app "editor". App names are surrounded by quotation marks.
"#
),
)
Expand Down
80 changes: 44 additions & 36 deletions crates/reporting/src/error/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4049,20 +4049,24 @@ fn to_exposes_report<'a>(
let severity = Severity::RuntimeError;

match *parse_problem {
EExposes::ListEnd(pos) | // TODO: give this its own error message
EExposes::Identifier(pos) => {
// TODO: It would be nice to give ListEnd it's own error message.
// However with how parsing error are currently reported, ListEnd and
// Identifier are tightly coupled and Identifier is basically never
// "thrown". Splitting these up to make more specific error messages
// would require tweaking how parsing works.
EExposes::ListEnd(pos) | EExposes::Identifier(pos) => {
let surroundings = Region::new(start, pos);
let region = LineColumnRegion::from_pos(lines.convert_pos(pos));

let doc = alloc.stack([
alloc.reflow(r"I am partway through parsing an `exposes` list, but I got stuck here:"),
alloc.concat([
alloc.reflow("I am partway through parsing an "),
alloc.keyword("exposes"),
alloc.reflow(" list, but I got stuck here:"),
]),
alloc.region_with_subregion(lines.convert_region(surroundings), region, severity),
alloc.concat([alloc.reflow(
"I was expecting a type name, value name or function name next, like",
)]),
alloc
.parser_suggestion("[Animal, default, tame]")
.indent(4),
alloc.reflow("I was expecting a type name, value name or function name next, like"),
alloc.parser_suggestion("[Animal, default, tame]").indent(4),
]);

Report {
Expand All @@ -4085,9 +4089,7 @@ fn to_exposes_report<'a>(
alloc.keyword("exposes"),
alloc.reflow(" keyword next, like"),
]),
alloc
.parser_suggestion("[Animal, default, tame]")
.indent(4),
alloc.parser_suggestion("[Animal, default, tame]").indent(4),
]);

Report {
Expand All @@ -4100,48 +4102,54 @@ fn to_exposes_report<'a>(

EExposes::Space(error, pos) => to_space_report(alloc, lines, filename, &error, pos),

EExposes::ListStart(pos@Position { offset: 7 }) => {
// TODO: Depending on the type of header we're parsing, it would be
// great to add more context to this error message. For example, if
// we're parsing a `module` then saying something like:
//
// I was expecting an exposes list like
//
// ...
//
// or module params like
// ...
//
// would be great. But currently we don't capture the type of header in
// the parse problem data type, so this isn't possible
EExposes::ListStart(pos) => {
let surroundings = Region::new(start, pos);
let region = LineColumnRegion::from_pos(lines.convert_pos(pos));

let doc = alloc.stack([
alloc.reflow(r"I am partway through parsing a header, but I got stuck here:"),
alloc.region_with_subregion(lines.convert_region(surroundings), region, severity),
alloc.concat([
alloc.reflow("I am expecting a list of exports like"),
]),
alloc
.parser_suggestion("module [func, value]")
.indent(4),
alloc.concat([
alloc.reflow("or module params like"),
]),
alloc
.parser_suggestion("module { echo } -> [func, value]")
.indent(4),
alloc.concat([
alloc.reflow("If you're trying to specify a module name, recall that unlike "),
alloc.reflow("application names, module names are not specified in the header. "),
alloc.reflow("Instead, they are derived from the name of the module's filename."),
alloc.reflow("I was expecting an "),
alloc.keyword("exposes"),
alloc.reflow(" list like"),
]),
alloc.parser_suggestion("[Animal, default, tame]").indent(4),
]);


Report {
filename,
doc,
title: "WEIRD MODULE NAME".to_string(),
title: "WEIRD EXPOSES".to_string(),
severity,
}
},
}

// If you're adding or changing syntax, please handle the case with a
// good error message above instead of adding more unhandled cases below.
EExposes::Open(pos) |
EExposes::IndentExposes(pos) |
EExposes::IndentListStart(pos) |
EExposes::ListStart(pos) =>
to_unhandled_parse_error_report(alloc, lines, filename, format!("{:?}", parse_problem), pos, start)
EExposes::Open(pos) | EExposes::IndentExposes(pos) | EExposes::IndentListStart(pos) => {
to_unhandled_parse_error_report(
alloc,
lines,
filename,
format!("{:?}", parse_problem),
pos,
start,
)
}
}
}

Expand Down

0 comments on commit c28d6b9

Please sign in to comment.