improved malformed patterns

This commit is contained in:
Folkert 2021-03-01 16:12:37 +01:00
parent 2ae993d695
commit a87dfac7da
10 changed files with 316 additions and 126 deletions

View file

@ -379,6 +379,11 @@ pub fn canonicalize_pattern<'a>(
malformed_pattern(env, problem, region)
}
MalformedIdent(_str, problem) => {
let problem = MalformedPatternProblem::BadIdent(*problem);
malformed_pattern(env, problem, region)
}
QualifiedIdentifier { .. } => {
let problem = MalformedPatternProblem::QualifiedIdentifier;
malformed_pattern(env, problem, region)

View file

@ -39,6 +39,7 @@ impl<'a> Formattable<'a> for Pattern<'a> {
| Pattern::StrLiteral(_)
| Pattern::Underscore(_)
| Pattern::Malformed(_)
| Pattern::MalformedIdent(_, _)
| Pattern::QualifiedIdentifier { .. } => false,
}
}
@ -157,7 +158,7 @@ impl<'a> Formattable<'a> for Pattern<'a> {
}
// Malformed
Malformed(string) => buf.push_str(string),
Malformed(string) | MalformedIdent(string, _) => buf.push_str(string),
QualifiedIdentifier { module_name, ident } => {
if !module_name.is_empty() {
buf.push_str(module_name);

View file

@ -356,6 +356,7 @@ pub enum Pattern<'a> {
// Malformed
Malformed(&'a str),
MalformedIdent(&'a str, crate::ident::BadIdent),
QualifiedIdentifier {
module_name: &'a str,
ident: &'a str,

View file

@ -1028,6 +1028,9 @@ fn annotation_or_alias<'a>(
Malformed(_) => {
Def::NotYetImplemented("TODO translate a malformed pattern into a malformed annotation")
}
MalformedIdent(_, _) => {
Def::NotYetImplemented("TODO translate a malformed pattern into a malformed annotation")
}
Identifier(ident) => {
// This is a regular Annotation
Def::Annotation(

View file

@ -229,7 +229,7 @@ fn malformed_identifier<'a>(
Ok((ch, bytes_parsed)) => {
// We can't use ch.is_alphanumeric() here because that passes for
// things that are "numeric" but not ASCII digits, like `¾`
if ch == '.' || ch.is_alphabetic() || ch.is_ascii_digit() {
if ch == '.' || ch == '_' || ch.is_alphabetic() || ch.is_ascii_digit() {
state = state.advance_without_indenting_ee(bytes_parsed, |r, c| {
EExpr::Space(crate::parser::BadInputError::LineTooLong, r, c)
})?;
@ -255,6 +255,7 @@ fn malformed_identifier<'a>(
pub enum BadIdent {
Start(Row, Col),
Space(BadInputError, Row, Col),
Underscore(Row, Col),
QualifiedTag(Row, Col),
PrivateTagNotUppercase(Row, Col),
PartStartsWithNumber(Row, Col),
@ -393,6 +394,16 @@ pub fn parse_ident_help_help<'a>(
// Now that we've recorded the contents of the current buffer, reset it.
part_buf = String::new_in(arena);
} else if ch == '_' {
// we don't allow underscores in the middle of an identifier
// but still parse them (and generate a malformed identifier)
// to give good error messages for this case
state = advance_state!(state, bytes_parsed)?;
return Err((
MadeProgress,
BadIdent::Underscore(state.line, state.column),
state,
));
} else {
// This must be the end of the identifier. We're done!

View file

@ -267,12 +267,15 @@ fn loc_ident_pattern_help<'a>(
},
state,
)),
Ident::Malformed(malformed, _problem) => {
Ident::Malformed(malformed, problem) => {
debug_assert!(!malformed.is_empty());
Err((
Ok((
MadeProgress,
EPattern::Start(state.line, state.column),
Located {
region: loc_ident.region,
value: Pattern::MalformedIdent(malformed, problem),
},
state,
))
}

View file

@ -1495,15 +1495,26 @@ mod test_parse {
}
#[test]
#[ignore]
fn malformed_ident_due_to_underscore() {
// This is a regression test against a bug where if you included an
// underscore in an argument name, it would parse as three arguments
// (and would ignore the underscore as if it had been blank space).
let arena = Bump::new();
let pattern = Located::new(
0,
0,
1,
11,
Pattern::MalformedIdent(&"the_answer", roc_parse::ident::BadIdent::Underscore(0, 5)),
);
let patterns = &[pattern];
let expr = Located::new(0, 0, 15, 17, Expr::Num("42"));
let expected = Closure(patterns, &expr);
let actual = parse_expr_with(&arena, "\\the_answer -> 42");
assert_eq!(Ok(MalformedClosure), actual);
assert_eq!(Ok(expected), actual);
}
#[test]

View file

@ -167,4 +167,5 @@ pub enum MalformedPatternProblem {
MalformedBase(Base),
Unknown,
QualifiedIdentifier,
BadIdent(roc_parse::ident::BadIdent),
}

View file

@ -344,6 +344,253 @@ pub fn can_problem<'b>(
}
}
fn to_bad_ident_expr_report<'b>(
alloc: &'b RocDocAllocator<'b>,
bad_ident: roc_parse::ident::BadIdent,
surroundings: Region,
) -> RocDocBuilder<'b> {
use roc_parse::ident::BadIdent::*;
match bad_ident {
Start(_, _) | Space(_, _, _) => unreachable!("these are handled in the parser"),
WeirdDotAccess(row, col) | StrayDot(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow(r"I trying to parse a record field accessor here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("Something like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow(" that accesses a value from a record."),
]),
])
}
PartStartsWithNumber(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I trying to parse a record field access here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("So I expect to see a lowercase letter next, like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow("."),
]),
])
}
WeirdAccessor(_row, _col) => alloc.stack(vec![
alloc.reflow("I am very confused by this field access"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow("It looks like a field access on an accessor. I parse"),
alloc.parser_suggestion(".client.name"),
alloc.reflow(" as "),
alloc.parser_suggestion("(.client).name"),
alloc.reflow(". Maybe use an anonymous function like "),
alloc.parser_suggestion("(\\r -> r.client.name)"),
alloc.reflow(" instead"),
alloc.reflow("?"),
]),
]),
WeirdDotQualified(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("I was expecting to see an identifier next, like "),
alloc.parser_suggestion("height"),
alloc.reflow(". A complete qualified name looks something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text("."),
]),
])
}
QualifiedTag(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow(r"This looks like a qualified tag name to me, "),
alloc.reflow(r"but tags cannot be qualified! "),
alloc.reflow(r"Maybe you wanted a qualified name, something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text("?"),
]),
])
}
PrivateTagNotUppercase(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a private tag here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow(r"But after the "),
alloc.keyword("@"),
alloc.reflow(r" symbol I found a lowercase letter. "),
alloc.reflow(r"All tag names (global and private)"),
alloc.reflow(r" must start with an uppercase letter, like "),
alloc.parser_suggestion("@UUID"),
alloc.reflow(" or "),
alloc.parser_suggestion("@Secrets"),
alloc.reflow("."),
]),
])
}
PrivateTagFieldAccess(_row, _col) => alloc.stack(vec![
alloc.reflow("I am very confused by this field access:"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow(r"It looks like a record field access on a private tag.")
]),
]),
_ => todo!(),
}
}
fn to_bad_ident_pattern_report<'b>(
alloc: &'b RocDocAllocator<'b>,
bad_ident: roc_parse::ident::BadIdent,
surroundings: Region,
) -> RocDocBuilder<'b> {
use roc_parse::ident::BadIdent::*;
match bad_ident {
Start(_, _) | Space(_, _, _) => unreachable!("these are handled in the parser"),
WeirdDotAccess(row, col) | StrayDot(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow(r"I trying to parse a record field accessor here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("Something like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow(" that accesses a value from a record."),
]),
])
}
PartStartsWithNumber(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I trying to parse a record field access here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("So I expect to see a lowercase letter next, like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow("."),
]),
])
}
WeirdAccessor(_row, _col) => alloc.stack(vec![
alloc.reflow("I am very confused by this field access"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow("It looks like a field access on an accessor. I parse"),
alloc.parser_suggestion(".client.name"),
alloc.reflow(" as "),
alloc.parser_suggestion("(.client).name"),
alloc.reflow(". Maybe use an anonymous function like "),
alloc.parser_suggestion("(\\r -> r.client.name)"),
alloc.reflow(" instead"),
alloc.reflow("?"),
]),
]),
WeirdDotQualified(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("I was expecting to see an identifier next, like "),
alloc.parser_suggestion("height"),
alloc.reflow(". A complete qualified name looks something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text("."),
]),
])
}
QualifiedTag(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow(r"This looks like a qualified tag name to me, "),
alloc.reflow(r"but tags cannot be qualified! "),
alloc.reflow(r"Maybe you wanted a qualified name, something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text("?"),
]),
])
}
PrivateTagNotUppercase(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a private tag here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow(r"But after the "),
alloc.keyword("@"),
alloc.reflow(r" symbol I found a lowercase letter. "),
alloc.reflow(r"All tag names (global and private)"),
alloc.reflow(r" must start with an uppercase letter, like "),
alloc.parser_suggestion("@UUID"),
alloc.reflow(" or "),
alloc.parser_suggestion("@Secrets"),
alloc.reflow("."),
]),
])
}
PrivateTagFieldAccess(_row, _col) => alloc.stack(vec![
alloc.reflow("I am very confused by this field access:"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow(r"It looks like a record field access on a private tag.")
]),
]),
Underscore(row, col) => {
let region = Region::from_row_col(row, col - 1);
alloc.stack(vec![
alloc.reflow("I am trying to parse an identifier here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![alloc.reflow(
r"Underscores are not allowed in identifiers. Use camelCase instead!",
)]),
])
}
_ => todo!(),
}
}
fn pretty_runtime_error<'b>(
alloc: &'b RocDocAllocator<'b>,
runtime_error: RuntimeError,
@ -432,6 +679,7 @@ fn pretty_runtime_error<'b>(
MalformedBase(Base::Binary) => " binary integer ",
MalformedBase(Base::Octal) => " octal integer ",
MalformedBase(Base::Decimal) => " integer ",
BadIdent(bad_ident) => return to_bad_ident_pattern_report(alloc, bad_ident, region),
Unknown => " ",
QualifiedIdentifier => " qualified ",
};
@ -440,7 +688,7 @@ fn pretty_runtime_error<'b>(
MalformedInt | MalformedFloat | MalformedBase(_) => alloc
.tip()
.append(alloc.reflow("Learn more about number literals at TODO")),
Unknown => alloc.nil(),
Unknown | BadIdent(_) => alloc.nil(),
QualifiedIdentifier => alloc.tip().append(
alloc.reflow("In patterns, only private and global tags can be qualified"),
),
@ -483,124 +731,7 @@ fn pretty_runtime_error<'b>(
unreachable!()
}
RuntimeError::MalformedIdentifier(_box_str, bad_ident, surroundings) => {
// we re-parse the identifier, to learn exactly what is going on
use roc_parse::ident::BadIdent::*;
match bad_ident {
Start(_,_) | Space(_,_,_) => unreachable!("these are handled in the parser"),
WeirdDotAccess(row, col) | StrayDot(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I trying to parse a record field accessor here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("Something like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow(" that accesses a value from a record."),
])
])
}
PartStartsWithNumber(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I trying to parse a record field access here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("So I expect to see a lowercase letter next, like "),
alloc.parser_suggestion(".name"),
alloc.reflow(" or "),
alloc.parser_suggestion(".height"),
alloc.reflow("."),
])
])
}
WeirdAccessor(_row, _col) => {
alloc.stack(vec![
alloc.reflow("I am very confused by this field access"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow("It looks like a field access on an accessor. I parse"),
alloc.parser_suggestion(".client.name"),
alloc.reflow(" as "),
alloc.parser_suggestion("(.client).name"),
alloc.reflow(". Maybe use an anonymous function like "),
alloc.parser_suggestion("(\\r -> r.client.name)"),
alloc.reflow(" instead"),
alloc.reflow("?"),
])
])
}
WeirdDotQualified(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("I was expecting to see an identifier next, like "),
alloc.parser_suggestion("height"),
alloc.reflow(". A complete qualified name looks something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text(".")
])
])
}
QualifiedTag(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a qualified name here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow("This looks like a qualified tag name to me, but tags cannot be qualified! "),
alloc.reflow("Maybe you wanted a qualified name, something like "),
alloc.parser_suggestion("Json.Decode.string"),
alloc.text("?")
])
])
}
PrivateTagNotUppercase(row, col) => {
let region = Region::from_row_col(row, col);
alloc.stack(vec![
alloc.reflow("I am trying to parse a private tag here:"),
alloc.region_with_subregion(surroundings, region),
alloc.concat(vec![
alloc.reflow(r"But after the "),
alloc.keyword("@"),
alloc.reflow(r" symbol I found a lowercase letter. "),
alloc.reflow(r"All tag names (global and private) must start with an uppercase letter, like "),
alloc.parser_suggestion("@UUID"),
alloc.reflow(" or "),
alloc.parser_suggestion("@Secrets"),
alloc.reflow("."),
])
])
}
PrivateTagFieldAccess(_row, _col) => {
alloc.stack(vec![
alloc.reflow("I am very confused by this field access:"),
alloc.region(surroundings),
alloc.concat(vec![
alloc.reflow(r"It looks like a record field access on a private tag."),
])
])
}
_ => todo!(),
}
to_bad_ident_expr_report(alloc, bad_ident, surroundings)
}

View file

@ -5549,4 +5549,27 @@ mod test_reporting {
),
)
}
#[test]
fn closure_underscore_ident() {
report_problem_as(
indoc!(
r#"
\the_answer -> 100
"#
),
indoc!(
r#"
SYNTAX PROBLEM
I am trying to parse an identifier here:
1 \the_answer -> 100
^
Underscores are not allowed in identifiers. Use camelCase instead!
"#
),
)
}
}