diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index a26e9ddd90..d95965da52 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -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) diff --git a/compiler/fmt/src/pattern.rs b/compiler/fmt/src/pattern.rs index 3f4fb2667f..f768b26813 100644 --- a/compiler/fmt/src/pattern.rs +++ b/compiler/fmt/src/pattern.rs @@ -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); diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 9cff30eaed..6d63379acc 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -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, diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 553aed97c7..b7fa39d244 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -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( diff --git a/compiler/parse/src/ident.rs b/compiler/parse/src/ident.rs index 3101d27744..bb3143b53d 100644 --- a/compiler/parse/src/ident.rs +++ b/compiler/parse/src/ident.rs @@ -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! diff --git a/compiler/parse/src/pattern.rs b/compiler/parse/src/pattern.rs index 76fc15c65d..ca87f598be 100644 --- a/compiler/parse/src/pattern.rs +++ b/compiler/parse/src/pattern.rs @@ -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, )) } diff --git a/compiler/parse/tests/test_parse.rs b/compiler/parse/tests/test_parse.rs index 16a3fa46fd..fec5017711 100644 --- a/compiler/parse/tests/test_parse.rs +++ b/compiler/parse/tests/test_parse.rs @@ -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] diff --git a/compiler/problem/src/can.rs b/compiler/problem/src/can.rs index e6ec70e65c..f28d3b1c5e 100644 --- a/compiler/problem/src/can.rs +++ b/compiler/problem/src/can.rs @@ -167,4 +167,5 @@ pub enum MalformedPatternProblem { MalformedBase(Base), Unknown, QualifiedIdentifier, + BadIdent(roc_parse::ident::BadIdent), } diff --git a/compiler/reporting/src/error/canonicalize.rs b/compiler/reporting/src/error/canonicalize.rs index f8dc2f5a39..85fc4da8d1 100644 --- a/compiler/reporting/src/error/canonicalize.rs +++ b/compiler/reporting/src/error/canonicalize.rs @@ -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,126 +731,9 @@ 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::*; + to_bad_ident_expr_report(alloc, bad_ident, surroundings) - - 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!(), - } - - } RuntimeError::MalformedClosure(_) => todo!(""), RuntimeError::InvalidFloat(sign @ FloatErrorKind::PositiveInfinity, region, _raw_str) diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index f9c30413af..a904d86526 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -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! + "# + ), + ) + } }