chore(parser)!: deprecate constrain keyword for assert (#1286)

* chore(parser)!: deprecate constrain

* chore(parser): typo in error

* add shorter message

* chore(parser): constrain tests expect deprecated

* chore(parser): rm type annotation

---------

Co-authored-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>

Author: joss-aztec

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<UnresolvedTypeExpression, ParserError> {
-        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,
             )
         })

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)
                     }
 

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<Token>,
     expected_labels: BTreeSet<String>,
     found: Token,
-    reason: Option<String>,
+    reason: Option<ParserErrorReason>,
     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<ParserError> 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)

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))
             }

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<Statement> + '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> {
     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)"),
         ];