From f307e214c36e5d2196318c728c79904e713bcd63 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sun, 19 Feb 2023 09:47:06 +0100 Subject: [PATCH] Remove parse error on bytearray literals for trace, todo & error, parse as String instead. This has been bothering me and the more I thought of it the more I disliked the idea of a warning. The rationale being that in this very context, there's absolutely no ambiguity. So it is only frustrating that the parser is even able to make the exact suggestion of what should be fixed, but still fails. I can imagine it is going to be very common for people to type: ``` trace "foo" ``` ...yet terribly frustrating if they have to remember each time that this should actually be a string. Because of the `trace`, `todo` and `error` keywords, we know exactly the surrounding context and what to expect here. So we can work it nicely. However, the formatter will re-format it to: ``` trace @"foo" ``` Just for the sake of remaining consistent with the type-system. This way, we still only manipulate `String` in the AST, but we conveniently parse a double-quote utf-8 literal when coupled with one of the specific keywords. I believe that's the best of both worlds. --- crates/aiken-lang/src/parser.rs | 87 ++++++++------------------- crates/aiken-lang/src/parser/error.rs | 33 ---------- crates/aiken-lang/src/tests/format.rs | 43 ++++++++++++- 3 files changed, 66 insertions(+), 97 deletions(-) diff --git a/crates/aiken-lang/src/parser.rs b/crates/aiken-lang/src/parser.rs index 1e14b1e0..abd82341 100644 --- a/crates/aiken-lang/src/parser.rs +++ b/crates/aiken-lang/src/parser.rs @@ -533,46 +533,22 @@ pub fn anon_fn_param_parser() -> impl Parser Option<(ParseError, String)> { +// Interpret bytearray string literals written as utf-8 strings, as strings. +// +// This is mostly convenient so that todo & error works with either @"..." or plain "...". +// In this particular context, there's actually no ambiguity about the right-hand-side, so +// we can provide this syntactic sugar. +fn flexible_string_literal(expr: expr::UntypedExpr) -> expr::UntypedExpr { match expr { expr::UntypedExpr::ByteArray { - bytes, preferred_format: ByteArrayFormatPreference::Utf8String, + bytes, location, - .. - } => { - let literal = String::from_utf8(bytes.clone()).unwrap(); - - Some(( - ParseError::unexpected_bytearray_literal(*location, token, literal.clone()), - literal, - )) - } - _ => None, - } -} - -fn validate_error_todo( - token: Token, - reason: Option, - emit: &mut dyn FnMut(ParseError), -) -> Option { - if let Some(reason) = reason { - match unexpected_bytearray_literal(token, &reason) { - None => Some(reason), - Some((err, value)) => { - emit(err); - Some(expr::UntypedExpr::String { - location: reason.location(), - value, - }) - } - } - } else { - reason + } => expr::UntypedExpr::String { + location, + value: String::from_utf8(bytes).unwrap(), + }, + _ => expr, } } @@ -582,35 +558,22 @@ pub fn expr_seq_parser() -> impl Parser (text, then), - Some((err, value)) => { - emit(err); - ( - expr::UntypedExpr::String { - location: text.location(), - value, - }, - then, - ) - } - } - }) .map_with_span(|(text, then_), span| expr::UntypedExpr::Trace { kind: TraceKind::Trace, location: span, then: Box::new(then_), - text: Box::new(text), + text: Box::new(flexible_string_literal(text)), }), just(Token::ErrorTerm) .ignore_then(expr_parser(r.clone()).or_not()) - .validate(|reason, _span, emit| validate_error_todo(Token::ErrorTerm, reason, emit)) - .map_with_span(|reason, span| expr::UntypedExpr::error(span, reason)), + .map_with_span(|reason, span| { + expr::UntypedExpr::error(span, reason.map(flexible_string_literal)) + }), just(Token::Todo) .ignore_then(expr_parser(r.clone()).or_not()) - .validate(|reason, _span, emit| validate_error_todo(Token::Todo, reason, emit)) - .map_with_span(|reason, span| expr::UntypedExpr::todo(span, reason)), + .map_with_span(|reason, span| { + expr::UntypedExpr::todo(span, reason.map(flexible_string_literal)) + }), expr_parser(r.clone()) .then(r.repeated()) .foldl(|current, next| current.append_in_sequence(next)), @@ -984,18 +947,18 @@ pub fn expr_parser( .then_ignore(one_of(Token::RArrow).not().rewind()) .or_not(), ) - .validate(|reason, _span, emit| validate_error_todo(Token::Todo, reason, emit)) - .map_with_span(|reason, span| expr::UntypedExpr::todo(span, reason)), + .map_with_span(|reason, span| { + expr::UntypedExpr::todo(span, reason.map(flexible_string_literal)) + }), just(Token::ErrorTerm) .ignore_then( r.clone() .then_ignore(just(Token::RArrow).not().rewind()) .or_not(), ) - .validate(|reason, _span, emit| { - validate_error_todo(Token::ErrorTerm, reason, emit) - }) - .map_with_span(|reason, span| expr::UntypedExpr::error(span, reason)), + .map_with_span(|reason, span| { + expr::UntypedExpr::error(span, reason.map(flexible_string_literal)) + }), ))) .map_with_span( |(((patterns, alternative_patterns_opt), guard), then), span| ast::UntypedClause { diff --git a/crates/aiken-lang/src/parser/error.rs b/crates/aiken-lang/src/parser/error.rs index 7893244b..639ce3cc 100644 --- a/crates/aiken-lang/src/parser/error.rs +++ b/crates/aiken-lang/src/parser/error.rs @@ -55,16 +55,6 @@ impl ParseError { label: None, } } - - pub fn unexpected_bytearray_literal(span: Span, keyword: Token, literal: String) -> Self { - Self { - kind: ErrorKind::UnexpectedByteArrayLiteral(keyword, literal), - span, - while_parsing: None, - expected: HashSet::new(), - label: Some("use @"), - } - } } impl PartialEq for ParseError { @@ -159,29 +149,6 @@ pub enum ErrorKind { , bad = "✖️".red() }))] InvalidWhenClause, - - #[error( - "I noticed a {} literal where I would expect a {}.", - "ByteArray".bold().bright_blue(), - "String".bold().bright_blue(), - )] - #[diagnostic(url("https://aiken-lang.org/language-tour/primitive-types#string"))] - #[diagnostic(help("{}", formatdoc! { - r#"The keyword {keyword} accepts a {type_String} as argument. A {type_String} literal is denoted by {syntax}. Simple double-quotes are interpreted as {type_ByteArray} of UTF-8 encoded bytes. Juggling between {type_ByteArray} and {type_String} can be a little confusing. - - In this instance, you probably meant to write the following: - - ╰─▶ {keyword} {symbol_at}{symbol_doublequotes}{literal}{symbol_doublequotes} - "#, - type_String = "String".bold().bright_blue(), - type_ByteArray = "ByteArray".bold().bright_blue(), - symbol_at = "@".purple(), - symbol_doublequotes = "\"".purple(), - syntax = format!("{}{}", "@".purple(), "\"...\"".purple()), - keyword = format!("{}", .0).replace('"', "").bold(), - literal = .1.purple(), - }))] - UnexpectedByteArrayLiteral(Token, String), } #[derive(Debug, PartialEq, Eq, Hash, Diagnostic, thiserror::Error)] diff --git a/crates/aiken-lang/src/tests/format.rs b/crates/aiken-lang/src/tests/format.rs index 2fa00f8c..0e3b82a1 100644 --- a/crates/aiken-lang/src/tests/format.rs +++ b/crates/aiken-lang/src/tests/format.rs @@ -344,7 +344,7 @@ fn test_nested_function_calls() { ), when output.datum is { InlineDatum(_) -> True - _ -> error "expected inline datum" + _ -> error @"expected inline datum" }, ] |> list.and @@ -382,7 +382,33 @@ fn format_trace_todo_error() { } "#}; - assert_fmt(src, src); + let out = indoc! {r#" + fn foo_1() { + todo + } + + fn foo_2() { + todo @"my custom message" + } + + fn foo_3() { + when x is { + Foo -> True + _ -> error + } + } + + fn foo_4() { + if 14 == 42 { + error @"I don't think so" + } else { + trace @"been there" + True + } + } + "#}; + + assert_fmt(src, out); } #[test] @@ -508,3 +534,16 @@ fn test_bytearray_literals() { assert_fmt(src, src); } + +#[test] +fn test_string_literal() { + let src = indoc! {r#" + const foo_const: String = @"foo" + + fn foo() { + let foo_var: String = @"foo" + } + "#}; + + assert_fmt(src, src); +}