diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index 6bd5c148d66..37eb944e0c6 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -16,7 +16,11 @@ use noirc_errors::Span; pub use statement::*; pub use structure::*; -use crate::{parser::ParserError, token::IntType, BinaryTypeOperator, CompTime}; +use crate::{ + parser::{ParserError, ParserErrorReason}, + token::IntType, + BinaryTypeOperator, CompTime, +}; use iter_extended::vecmap; /// The parser parses types as 'UnresolvedType's which @@ -152,9 +156,9 @@ impl UnresolvedTypeExpression { expr: Expression, span: Span, ) -> Result { - Self::from_expr_helper(expr).map_err(|err| { + Self::from_expr_helper(expr).map_err(|err_expr| { ParserError::with_reason( - format!("Expression is invalid in an array-length type: '{err}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context."), + ParserErrorReason::InvalidArrayLengthExpression(err_expr), span, ) }) diff --git a/crates/noirc_frontend/src/ast/statement.rs b/crates/noirc_frontend/src/ast/statement.rs index 5e0dd4e4391..d4fabccea70 100644 --- a/crates/noirc_frontend/src/ast/statement.rs +++ b/crates/noirc_frontend/src/ast/statement.rs @@ -1,7 +1,7 @@ use std::fmt::Display; use crate::lexer::token::SpannedToken; -use crate::parser::ParserError; +use crate::parser::{ParserError, ParserErrorReason}; use crate::token::Token; use crate::{Expression, ExpressionKind, IndexExpression, MemberAccessExpression, UnresolvedType}; use iter_extended::vecmap; @@ -59,8 +59,10 @@ impl Statement { | Statement::Error => { // To match rust, statements always require a semicolon, even at the end of a block if semi.is_none() { - let reason = "Expected a ; separating these two statements".to_string(); - emit_error(ParserError::with_reason(reason, span)); + emit_error(ParserError::with_reason( + ParserErrorReason::MissingSeparatingSemi, + span, + )); } self } @@ -83,8 +85,10 @@ impl Statement { // for unneeded expressions like { 1 + 2; 3 } (_, Some(_), false) => Statement::Expression(expr), (_, None, false) => { - let reason = "Expected a ; separating these two statements".to_string(); - emit_error(ParserError::with_reason(reason, span)); + emit_error(ParserError::with_reason( + ParserErrorReason::MissingSeparatingSemi, + span, + )); Statement::Expression(expr) } diff --git a/crates/noirc_frontend/src/parser/errors.rs b/crates/noirc_frontend/src/parser/errors.rs index 7f19ef7f062..7012c0fbda5 100644 --- a/crates/noirc_frontend/src/parser/errors.rs +++ b/crates/noirc_frontend/src/parser/errors.rs @@ -1,18 +1,33 @@ use std::collections::BTreeSet; use crate::lexer::token::Token; -use crate::BinaryOp; +use crate::Expression; +use thiserror::Error; use iter_extended::vecmap; use noirc_errors::CustomDiagnostic as Diagnostic; use noirc_errors::Span; +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum ParserErrorReason { + #[error("Arrays must have at least one element")] + ZeroSizedArray, + #[error("Unexpected '{0}', expected a field name")] + ExpectedFieldName(Token), + #[error("Expected a ; separating these two statements")] + MissingSeparatingSemi, + #[error("constrain keyword is deprecated")] + ConstrainDeprecated, + #[error("Expression is invalid in an array-length type: '{0}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context.")] + InvalidArrayLengthExpression(Expression), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ParserError { expected_tokens: BTreeSet, expected_labels: BTreeSet, found: Token, - reason: Option, + reason: Option, span: Span, } @@ -39,21 +54,11 @@ impl ParserError { error } - pub fn with_reason(reason: String, span: Span) -> ParserError { + pub fn with_reason(reason: ParserErrorReason, span: Span) -> ParserError { let mut error = ParserError::empty(Token::EOF, span); error.reason = Some(reason); error } - - pub fn invalid_constrain_operator(operator: BinaryOp) -> ParserError { - let message = format!( - "Cannot use the {} operator in a constraint statement.", - operator.contents.as_string() - ); - let mut error = ParserError::empty(operator.contents.as_token(), operator.span()); - error.reason = Some(message); - error - } } impl std::fmt::Display for ParserError { @@ -84,7 +89,19 @@ impl std::fmt::Display for ParserError { impl From for Diagnostic { fn from(error: ParserError) -> Diagnostic { match &error.reason { - Some(reason) => Diagnostic::simple_error(reason.clone(), String::new(), error.span), + Some(reason) => { + match reason { + ParserErrorReason::ConstrainDeprecated => Diagnostic::simple_warning( + "Use of deprecated keyword 'constrain'".into(), + "The 'constrain' keyword has been deprecated. Please use the 'assert' function instead.".into(), + error.span, + ), + other => { + + Diagnostic::simple_error(format!("{other}"), String::new(), error.span) + } + } + } None => { let primary = error.to_string(); Diagnostic::simple_error(primary, String::new(), error.span) diff --git a/crates/noirc_frontend/src/parser/mod.rs b/crates/noirc_frontend/src/parser/mod.rs index 788c0eec895..98b7fffbf14 100644 --- a/crates/noirc_frontend/src/parser/mod.rs +++ b/crates/noirc_frontend/src/parser/mod.rs @@ -24,6 +24,7 @@ use acvm::FieldElement; use chumsky::prelude::*; use chumsky::primitive::Container; pub use errors::ParserError; +pub use errors::ParserErrorReason; use noirc_errors::Span; pub use parser::parse_program; @@ -176,7 +177,7 @@ where .try_map(move |peek, span| { if too_far.get_iter().any(|t| t == peek) { // This error will never be shown to the user - Err(ParserError::with_reason(String::new(), span)) + Err(ParserError::empty(Token::EOF, span)) } else { Ok(Recoverable::error(span)) } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 575a9403ea8..4f7c73e609b 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -26,7 +26,7 @@ use super::{ foldl_with_span, parameter_name_recovery, parameter_recovery, parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser, ForRange, NoirParser, - ParsedModule, ParserError, Precedence, SubModule, TopLevelStatement, + ParsedModule, ParserError, ParserErrorReason, Precedence, SubModule, TopLevelStatement, }; use crate::ast::{Expression, ExpressionKind, LetStatement, Statement, UnresolvedType}; use crate::lexer::Lexer; @@ -448,6 +448,10 @@ where { ignore_then_commit(keyword(Keyword::Constrain).labelled("statement"), expr_parser) .map(|expr| Statement::Constrain(ConstrainStatement(expr))) + .validate(|expr, span, emit| { + emit(ParserError::with_reason(ParserErrorReason::ConstrainDeprecated, span)); + expr + }) } fn assertion<'a, P>(expr_parser: P) -> impl NoirParser + 'a @@ -877,10 +881,7 @@ where .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) .validate(|elements, span, emit| { if elements.is_empty() { - emit(ParserError::with_reason( - "Arrays must have at least one element".to_owned(), - span, - )); + emit(ParserError::with_reason(ParserErrorReason::ZeroSizedArray, span)); } ExpressionKind::array(elements) }) @@ -966,8 +967,7 @@ fn field_name() -> impl NoirParser { ident().or(token_kind(TokenKind::Literal).validate(|token, span, emit| match token { Token::Int(_) => Ident::from(Spanned::from(span, token.to_string())), other => { - let reason = format!("Unexpected '{other}', expected a field name"); - emit(ParserError::with_reason(reason, span)); + emit(ParserError::with_reason(ParserErrorReason::ExpectedFieldName(other), span)); Ident::error(span) } })) @@ -1196,10 +1196,12 @@ mod test { ); } - /// This is the standard way to declare a constrain statement + /// Deprecated constrain usage test #[test] fn parse_constrain() { - parse_with(constrain(expression()), "constrain x == y").unwrap(); + let errors = parse_with(constrain(expression()), "constrain x == y").unwrap_err(); + assert_eq!(errors.len(), 1); + assert!(format!("{}", errors.first().unwrap()).contains("deprecated")); // Currently we disallow constrain statements where the outer infix operator // produces a value. This would require an implicit `==` which @@ -1217,7 +1219,9 @@ mod test { for operator in disallowed_operators { let src = format!("constrain x {} y;", operator.as_string()); - parse_with(constrain(expression()), &src).unwrap_err(); + let errors = parse_with(constrain(expression()), &src).unwrap_err(); + assert_eq!(errors.len(), 2); + assert!(format!("{}", errors.first().unwrap()).contains("deprecated")); } // These are general cases which should always work. @@ -1226,7 +1230,7 @@ mod test { // The first (inner) `==` is a predicate which returns 0/1 // The outer layer is an infix `==` which is // associated with the Constrain statement - parse_all( + let errors = parse_all_failing( constrain(expression()), vec![ "constrain ((x + y) == k) + z == y", @@ -1236,8 +1240,11 @@ mod test { "constrain x + x ^ x == y | m", ], ); + assert_eq!(errors.len(), 5); + assert!(errors.iter().all(|err| { format!("{}", err).contains("deprecated") })); } + /// This is the standard way to declare an assert statement #[test] fn parse_assert() { parse_with(assertion(expression()), "assert(x == y)").unwrap(); @@ -1533,9 +1540,9 @@ mod test { ("let = ", 2, "let $error: unspecified = Error"), ("let", 3, "let $error: unspecified = Error"), ("foo = one two three", 1, "foo = plain::one"), - ("constrain", 1, "constrain Error"), + ("constrain", 2, "constrain Error"), ("assert", 1, "constrain Error"), - ("constrain x ==", 1, "constrain (plain::x == Error)"), + ("constrain x ==", 2, "constrain (plain::x == Error)"), ("assert(x ==)", 1, "constrain (plain::x == Error)"), ];