From 1b750149c0e9198d0a8a57b8db272d6687c5efce Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 7 Mar 2021 18:50:44 +0100 Subject: [PATCH] error messages for bad operators --- compiler/parse/src/expr.rs | 76 ++-- compiler/reporting/src/error/parse.rs | 76 ++++ compiler/reporting/tests/test_reporting.rs | 382 +++++++++++++-------- 3 files changed, 346 insertions(+), 188 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index f6527ea3ba..754648b3e2 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -436,46 +436,44 @@ fn parse_expr_help<'a>( arena: &'a Bump, state: State<'a>, ) -> ParseResult<'a, Expr<'a>, EExpr<'a>> { - let (_, (loc_expr1, opt_operator), state) = and!( - // First parse the body without operators, then try to parse possible operators after. - move |arena, state| loc_parse_expr_body_without_operators_help(min_indent, arena, state), - // Parse the operator, with optional spaces before it. - // - // Since spaces can only wrap an Expr, not an BinOp, we have to first - // parse the spaces and then attach them retroactively to the expression - // preceding the operator (the one we parsed before considering operators). - optional(and!( - and!( - space0_e(min_indent, EExpr::Space, EExpr::IndentEnd), - loc!(operator()) - ), - // The spaces *after* the operator can be attached directly to - // the expression following the operator. - space0_before_e( - loc!(move |arena, state| parse_expr_help(min_indent, arena, state)), - min_indent, - EExpr::Space, - EExpr::IndentEnd, - ) - )) - ) - .parse(arena, state)?; + let (_, loc_expr1, state) = + loc_parse_expr_body_without_operators_help(min_indent, arena, state)?; - match opt_operator { - Some(((spaces_before_op, loc_op), loc_expr2)) => { - let loc_expr1 = if spaces_before_op.is_empty() { - loc_expr1 - } else { - // Attach the spaces retroactively to the expression preceding the operator. - arena - .alloc(loc_expr1.value) - .with_spaces_after(spaces_before_op, loc_expr1.region) - }; - let tuple = arena.alloc((loc_expr1, loc_op, loc_expr2)); + let initial = state.clone(); - Ok((MadeProgress, Expr::BinOp(tuple), state)) + match space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state) { + Err((_, _, state)) => Ok((MadeProgress, loc_expr1.value, state)), + Ok((_, spaces_before_op, state)) => { + let parser = and!( + loc!(operator()), + // The spaces *after* the operator can be attached directly to + // the expression following the operator. + space0_before_e( + loc!(move |arena, state| parse_expr_help(min_indent, arena, state)), + min_indent, + EExpr::Space, + EExpr::IndentEnd, + ) + ); + + match parser.parse(arena, state) { + Ok((_, (loc_op, loc_expr2), state)) => { + let loc_expr1 = if spaces_before_op.is_empty() { + loc_expr1 + } else { + // Attach the spaces retroactively to the expression preceding the operator. + arena + .alloc(loc_expr1.value) + .with_spaces_after(spaces_before_op, loc_expr1.region) + }; + let tuple = arena.alloc((loc_expr1, loc_op, loc_expr2)); + + Ok((MadeProgress, Expr::BinOp(tuple), state)) + } + Err((NoProgress, _, _)) => Ok((MadeProgress, loc_expr1.value, initial)), + Err((MadeProgress, fail, state)) => Err((MadeProgress, fail, state)), + } } - None => Ok((MadeProgress, loc_expr1.value, state)), } } @@ -2240,6 +2238,10 @@ where (b'|', b'|') => good!(BinOp::Or, 2), (b'/', b'/') => good!(BinOp::DoubleSlash, 2), (b'%', b'%') => good!(BinOp::DoublePercent, 2), + (b'-', b'>') => { + // makes no progress, so it does not interfere with `_ if isGood -> ...` + Err((NoProgress, to_error(b"->", state.line, state.column), state)) + } _ => bad_made_progress!(&state.bytes[0..2]), } } diff --git a/compiler/reporting/src/error/parse.rs b/compiler/reporting/src/error/parse.rs index 92b5556d53..c8bf4e577d 100644 --- a/compiler/reporting/src/error/parse.rs +++ b/compiler/reporting/src/error/parse.rs @@ -221,6 +221,82 @@ fn to_expr_report<'a>( } } + EExpr::BadOperator(op, row, col) => { + let surroundings = Region::from_rows_cols(start_row, start_col, *row, *col); + let region = Region::from_rows_cols(*row, *col, *row, *col + op.len() as u16); + + let suggestion = match *op { + b"|" => vec![ + alloc.reflow("Maybe you want "), + alloc.parser_suggestion("||"), + alloc.reflow(" or "), + alloc.parser_suggestion("|>"), + alloc.reflow(" instead?"), + ], + b"++" => vec![ + alloc.reflow("To concatenate two lists or strings, try using "), + alloc.parser_suggestion("List.concat"), + alloc.reflow(" or "), + alloc.parser_suggestion("Str.concat"), + alloc.reflow(" instead."), + ], + b":" => vec![alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The has-type operator "), + alloc.parser_suggestion(":"), + alloc.reflow(" can only occur in a definition's type signature, like "), + ]), + alloc + .vcat(vec![ + alloc.text("increment : I64 -> I64"), + alloc.text("increment = \\x -> x + 1"), + ]) + .indent(4), + ])], + b"->" => vec![alloc.stack(vec![ + alloc.concat(vec![ + alloc.reflow("The arrow "), + alloc.parser_suggestion("->"), + alloc.reflow(" is only used to define cases in a "), + alloc.keyword("when"), + alloc.reflow("."), + ]), + alloc + .vcat(vec![ + alloc.text("when color is"), + alloc.text("Red -> \"stop!\"").indent(4), + alloc.text("Green -> \"go!\"").indent(4), + ]) + .indent(4), + ])], + b"!" => vec![ + alloc.reflow("The boolean negation operator "), + alloc.parser_suggestion("!"), + alloc.reflow(" must occur immediately before an expression, like "), + alloc.parser_suggestion("!(List.isEmpty primes)"), + alloc.reflow(". There cannot be a space between the "), + alloc.parser_suggestion("!"), + alloc.reflow(" and the expression after it."), + ], + _ => vec![ + alloc.reflow("I have no specific suggestion for this operator, "), + alloc.reflow("see TODO for the full list of operators in Roc."), + ], + }; + + let doc = alloc.stack(vec![ + alloc.reflow(r"This looks like an operator, but it's not one I recognize!"), + alloc.region_with_subregion(surroundings, region), + alloc.concat(suggestion), + ]); + + Report { + filename, + doc, + title: "UNKNOWN OPERATOR".to_string(), + } + } + EExpr::Ident(_row, _col) => unreachable!("another branch would be taken"), EExpr::QualifiedTag(row, col) => { diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 00883dacc9..7a0a4cf8ea 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -812,20 +812,20 @@ mod test_reporting { indoc!( r#" ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - + The 3rd branch of this `if` does not match all the previous branches: - + 1│ if True then 2 else if False then 2 else "foo" ^^^^^ - + The 3rd branch is a string of type: - + Str - + But all the previous branches have type: - + Num a - + I need all branches in an `if` to have the same type! "# ), @@ -3159,12 +3159,12 @@ mod test_reporting { indoc!( r#" ── ARGUMENTS BEFORE EQUALS ───────────────────────────────────────────────────── - + I am in the middle of parsing a definition, but I got stuck here: - + 1│ f x y = x ^^^ - + Looks like you are trying to define a function. In roc, functions are always written as a lambda, like increment = \n -> n + 1. "# @@ -4020,12 +4020,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am trying to parse a qualified name here: - + 1│ Foo.Bar ^ - + This looks like a qualified tag name to me, but tags cannot be qualified! Maybe you wanted a qualified name, something like Json.Decode.string? @@ -4045,12 +4045,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am trying to parse a qualified name here: - + 1│ Foo.Bar. ^ - + I was expecting to see an identifier next, like height. A complete qualified name looks something like Json.Decode.string. "# @@ -4069,12 +4069,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I trying to parse a record field accessor here: - + 1│ foo.bar. ^ - + Something like .name or .height that accesses a value from a record. "# ), @@ -4092,12 +4092,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am trying to parse a qualified name here: - + 1│ @Foo.Bar ^ - + This looks like a qualified tag name to me, but tags cannot be qualified! Maybe you wanted a qualified name, something like Json.Decode.string? @@ -4163,32 +4163,6 @@ mod test_reporting { ) } - #[test] - fn invalid_operator() { - // NOTE: VERY BAD ERROR MESSAGE - report_problem_as( - indoc!( - r#" - main = - 5 ** 3 - "# - ), - indoc!( - r#" - ── MISSING EXPRESSION ────────────────────────────────────────────────────────── - - I am partway through parsing a definition, but I got stuck here: - - 1│ main = - 2│ 5 ** 3 - ^ - - I was expecting to see an expression like 42 or "hello". - "# - ), - ) - } - #[test] fn tag_union_open() { report_problem_as( @@ -4718,12 +4692,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TYPE ───────────────────────────────────────────────────────────── - + I am partway through parsing a type, but I got stuck here: - + 1│ f : I64, I64 ^ - + Note: I may be confused by indentation "# ), @@ -4905,13 +4879,13 @@ mod test_reporting { indoc!( r#" ── MISSING EXPRESSION ────────────────────────────────────────────────────────── - + I am partway through parsing a definition, but I got stuck here: - + 1│ when Just 4 is 2│ Just 4 | -> ^ - + I was expecting to see an expression like 42 or "hello". "# ), @@ -4966,31 +4940,31 @@ mod test_reporting { r#" when 5 is 1 -> 2 - _ + _ "# ), indoc!( r#" ── MISSING ARROW ─────────────────────────────────────────────────────────────── - + I am partway through parsing a `when` expression, but got stuck here: - + 2│ 1 -> 2 - 3│ _ + 3│ _ ^ - + I was expecting to see an arrow next. - + Note: Sometimes I get confused by indentation, so try to make your `when` look something like this: - + when List.first plants is Ok n -> n - + Err _ -> 200 - + Notice the indentation. All patterns are aligned, and each branch is indented a bit more than the corresponding pattern. That is important! "# @@ -5009,13 +4983,13 @@ mod test_reporting { indoc!( r#" ── UNFINISHED ARGUMENT LIST ──────────────────────────────────────────────────── - + I am in the middle of parsing a function argument list, but I got stuck at this comma: - + 1│ \a,,b -> 1 ^ - + I was expecting an argument pattern before this, so try adding an argument before the comma and see if that helps? "# @@ -5034,13 +5008,13 @@ mod test_reporting { indoc!( r#" ── UNFINISHED ARGUMENT LIST ──────────────────────────────────────────────────── - + I am in the middle of parsing a function argument list, but I got stuck at this comma: - + 1│ \,b -> 1 ^ - + I was expecting an argument pattern before this, so try adding an argument before the comma and see if that helps? "# @@ -5062,23 +5036,23 @@ mod test_reporting { indoc!( r#" ── UNFINISHED WHEN ───────────────────────────────────────────────────────────── - + I was partway through parsing a `when` expression, but I got stuck here: - + 3│ _ -> 2 ^ - + I suspect this is a pattern that is not indented enough? (by 2 spaces) - + Note: Here is an example of a valid `when` expression for reference. - + when List.first plants is Ok n -> n - + Err _ -> 200 - + Notice the indentation. All patterns are aligned, and each branch is indented a bit more than the corresponding pattern. That is important! "# @@ -5093,7 +5067,7 @@ mod test_reporting { indoc!( r#" x = - if 5 == 5 + if 5 == 5 then 2 else 3 x @@ -5102,12 +5076,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED IF ─────────────────────────────────────────────────────────────── - + I was partway through parsing an `if` expression, but I got stuck here: - - 2│ if 5 == 5 + + 2│ if 5 == 5 ^ - + I was expecting to see the `then` keyword next. "# ), @@ -5126,12 +5100,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED IF ─────────────────────────────────────────────────────────────── - + I was partway through parsing an `if` expression, but I got stuck here: - + 1│ if 5 == 5 then 2 ^ - + I was expecting to see the `else` keyword next. "# ), @@ -5149,12 +5123,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED LIST ───────────────────────────────────────────────────────────── - + I am partway through started parsing a list, but I got stuck here: - + 1│ [ 1, 2, , 3 ] ^ - + I was expecting to see a list entry before this comma, so try adding a list entry and see if that helps? "# @@ -5167,21 +5141,21 @@ mod test_reporting { report_problem_as( indoc!( r#" - [ 1, 2, + [ 1, 2, "# ), indoc!( r#" ── UNFINISHED LIST ───────────────────────────────────────────────────────────── - + I am partway through started parsing a list, but I got stuck here: - - 1│ [ 1, 2, + + 1│ [ 1, 2, ^ - + I was expecting to see a closing square bracket before this, so try adding a ] and see if that helps? - + Note: When I get stuck like this, it usually means that there is a missing parenthesis or bracket somewhere earlier. It could also be a stray keyword or operator. @@ -5195,7 +5169,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - x = [ 1, 2, + x = [ 1, 2, ] x @@ -5204,16 +5178,16 @@ mod test_reporting { indoc!( r#" ── UNFINISHED LIST ───────────────────────────────────────────────────────────── - + I cannot find the end of this list: - - 1│ x = [ 1, 2, + + 1│ x = [ 1, 2, ^ - + You could change it to something like [ 1, 2, 3 ] or even just []. Anything where there is an open and a close square bracket, and where the elements of the list are separated by commas. - + Note: I may be confused by indentation "# ), @@ -5231,15 +5205,15 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + This float literal contains an invalid digit: - + 1│ 1.1.1 ^^^^^ - + Floating point literals can only contain the digits 0-9, or use scientific notation 10e4 - + Tip: Learn more about number literals at TODO "# ), @@ -5253,15 +5227,15 @@ mod test_reporting { indoc!( r#" ── WEIRD CODE POINT ──────────────────────────────────────────────────────────── - + I am partway through parsing a unicode code point, but I got stuck here: - + 1│ "abc\u(zzzz)def" ^ - + I was expecting a hexadecimal number, like \u(1100) or \u(00FF). - + Learn more about working with unicode in roc at TODO "# ), @@ -5275,15 +5249,15 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + This string interpolation is invalid: - + 1│ "abc\(32)def" ^^ - + I was expecting an identifier, like \u(message) or \u(LoremIpsum.text). - + Learn more about string interpolation at TODO "# ), @@ -5297,12 +5271,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + This unicode code point is invalid: - + 1│ "abc\u(110000)def" ^^^^^^ - + Learn more about working with unicode in roc at TODO "# ), @@ -5316,15 +5290,15 @@ mod test_reporting { indoc!( r#" ── WEIRD ESCAPE ──────────────────────────────────────────────────────────────── - + I was partway through parsing a string literal, but I got stuck here: - + 1│ "abc\qdef" ^^ - + This is not an escape sequence I recognize. After a backslash, I am looking for one of these: - + - A newline: \n - A caret return: \r - A tab: \t @@ -5344,12 +5318,12 @@ mod test_reporting { indoc!( r#" ── ENDLESS STRING ────────────────────────────────────────────────────────────── - + I cannot find the end of this string: - + 1│ "there is no end ^ - + You could change it to something like "to be or not to be" or even just "". "# @@ -5364,12 +5338,12 @@ mod test_reporting { indoc!( r#" ── ENDLESS STRING ────────────────────────────────────────────────────────────── - + I cannot find the end of this block string: - + 1│ """there is no end ^ - + You could change it to something like """to be or not to be""" or even just """""". "# @@ -5390,20 +5364,20 @@ mod test_reporting { indoc!( r#" ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - + This expression is used in an unexpected way: - + 3│ foo.if ^^^^^^ - + This `foo` value is a: - + {} - + But you are trying to use it as: - + { if : a }b - + "# ), @@ -5421,9 +5395,9 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + The Num module does not expose a if value: - + 1│ Num.if ^^^^^^ "# @@ -5442,12 +5416,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I trying to parse a record field accessor here: - + 1│ Num.add . 23 ^ - + Something like .name or .height that accesses a value from a record. "# ), @@ -5465,12 +5439,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am trying to parse a private tag here: - + 1│ Num.add @foo 23 ^ - + But after the `@` symbol I found a lowercase letter. All tag names (global and private) must start with an uppercase letter, like @UUID or @Secrets. @@ -5490,12 +5464,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am very confused by this field access: - + 1│ @UUID.bar ^^^^^^^^^ - + It looks like a record field access on a private tag. "# ), @@ -5513,12 +5487,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I am very confused by this field access - + 1│ .foo.bar ^^^^^^^^ - + It looks like a field access on an accessor. I parse.client.name as (.client).name. Maybe use an anonymous function like (\r -> r.client.name) instead? @@ -5538,12 +5512,12 @@ mod test_reporting { indoc!( r#" ── SYNTAX PROBLEM ────────────────────────────────────────────────────────────── - + I trying to parse a record field access here: - + 1│ foo.100 ^ - + So I expect to see a lowercase letter next, like .name or .height. "# ), @@ -5561,12 +5535,12 @@ mod test_reporting { 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! "# ), @@ -5622,4 +5596,110 @@ mod test_reporting { ), ) } + + #[test] + fn invalid_operator() { + report_problem_as( + indoc!( + r#" + main = + 5 ** 3 + "# + ), + indoc!( + r#" + ── UNKNOWN OPERATOR ──────────────────────────────────────────────────────────── + + This looks like an operator, but it's not one I recognize! + + 1│ main = + 2│ 5 ** 3 + ^^ + + I have no specific suggestion for this operator, see TODO for the full + list of operators in Roc. + "# + ), + ) + } + + #[test] + fn double_plus() { + report_problem_as( + indoc!( + r#" + main = + [] ++ [] + "# + ), + indoc!( + r#" + ── UNKNOWN OPERATOR ──────────────────────────────────────────────────────────── + + This looks like an operator, but it's not one I recognize! + + 1│ main = + 2│ [] ++ [] + ^^ + + To concatenate two lists or strings, try using List.concat or + Str.concat instead. + "# + ), + ) + } + + #[test] + fn inline_hastype() { + report_problem_as( + indoc!( + r#" + main = + 5 : I64 + "# + ), + indoc!( + r#" + ── UNKNOWN OPERATOR ──────────────────────────────────────────────────────────── + + This looks like an operator, but it's not one I recognize! + + 1│ main = + 2│ 5 : I64 + ^ + + The has-type operator : can only occur in a definition's type + signature, like + + increment : I64 -> I64 + increment = \x -> x + 1 + "# + ), + ) + } + + #[test] + fn wild_case_arrow() { + // this is still bad, but changing the order and progress of other parsers should improve it + // down the line + report_problem_as( + indoc!( + r#" + main = 5 -> 3 + "# + ), + indoc!( + r#" + ── MISSING EXPRESSION ────────────────────────────────────────────────────────── + + I am partway through parsing a definition, but I got stuck here: + + 1│ main = 5 -> 3 + ^ + + I was expecting to see an expression like 42 or "hello". + "# + ), + ) + } }