Skip to content

Commit

Permalink
add consecutive operators diagnostic to parser
Browse files Browse the repository at this point in the history
  • Loading branch information
wawel37 authored and dean-starkware committed Sep 21, 2024
1 parent 883047e commit 7813ad2
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 0 deletions.
22 changes: 22 additions & 0 deletions crates/cairo-lang-parser/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@ pub struct ParserDiagnostic {
pub span: TextSpan,
pub kind: ParserDiagnosticKind,
}
impl ParserDiagnostic {
/// Converts a `SyntaxKind` to its corresponding operator string.
fn operator_to_string(&self, kind: SyntaxKind) -> String {
match kind {
SyntaxKind::TerminalLT => "<".to_string(),
SyntaxKind::TerminalGT => ">".to_string(),
SyntaxKind::TerminalLE => "<=".to_string(),
SyntaxKind::TerminalGE => ">=".to_string(),
SyntaxKind::TerminalEqEq => "==".to_string(),
SyntaxKind::TerminalNeq => "!=".to_string(),
_ => format!("{:?}", kind),
}
}
}
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub enum ParserDiagnosticKind {
// TODO(spapini): Add tokens from the recovery set to the message.
Expand Down Expand Up @@ -39,6 +53,7 @@ pub enum ParserDiagnosticKind {
AttributesWithoutImplItem,
AttributesWithoutStatement,
DisallowedTrailingSeparatorOr,
ConsecutiveMathOperators { first_op: SyntaxKind, second_op: SyntaxKind },
}
impl DiagnosticEntry for ParserDiagnostic {
type DbType = dyn FilesGroup;
Expand Down Expand Up @@ -124,6 +139,13 @@ Did you mean to write `{identifier}!{left}...{right}'?",
ParserDiagnosticKind::DisallowedTrailingSeparatorOr => {
"A trailing `|` is not allowed in an or-pattern.".to_string()
}
ParserDiagnosticKind::ConsecutiveMathOperators { first_op, second_op } => {
format!(
"Consecutive comparison operators are not allowed: '{}' followed by '{}'",
self.operator_to_string(*first_op),
self.operator_to_string(*second_op)
)
}
}
}

Expand Down
40 changes: 40 additions & 0 deletions crates/cairo-lang-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,29 @@ impl<'a> Parser<'a> {
}
}

/// Checks if the current token is a relational or equality operator (`<`, `>`, `<=`, `>=`,
/// `==`, or `!=`).
///
/// This function is used to determine if the given `SyntaxKind` represents a relational or
/// equality operator, which is commonly used in binary expressions.
///
/// # Parameters:
/// - `kind`: The `SyntaxKind` of the current token.
///
/// # Returns:
/// `true` if the token is a relational or equality operator, otherwise `false`.
fn is_comparison_operator(&self, kind: SyntaxKind) -> bool {
matches!(
kind,
SyntaxKind::TerminalLT
| SyntaxKind::TerminalGT
| SyntaxKind::TerminalLE
| SyntaxKind::TerminalGE
| SyntaxKind::TerminalEqEq
| SyntaxKind::TerminalNeq
)
}

/// Returns a GreenId of a node with an Expr.* kind (see [syntax::node::ast::Expr])
/// or TryParseFailure if such an expression can't be parsed.
///
Expand All @@ -1099,6 +1122,7 @@ impl<'a> Parser<'a> {
lbrace_allowed: LbraceAllowed,
) -> TryParseResult<ExprGreen> {
let mut expr = self.try_parse_atom_or_unary(lbrace_allowed)?;
let mut child_op: Option<SyntaxKind> = None;
while let Some(precedence) = get_post_operator_precedence(self.peek().kind) {
if precedence >= parent_precedence {
return Ok(expr);
Expand All @@ -1112,6 +1136,22 @@ impl<'a> Parser<'a> {
let rbrack = self.parse_token::<TerminalRBrack>();
ExprIndexed::new_green(self.db, expr, lbrack, index_expr, rbrack).into()
} else {
let current_op = self.peek().kind;
if let Some(child_op_kind) = child_op {
if self.is_comparison_operator(child_op_kind)
&& self.is_comparison_operator(current_op)
{
self.diagnostics.add(ParserDiagnostic {
file_id: self.file_id,
kind: ParserDiagnosticKind::ConsecutiveMathOperators {
first_op: child_op_kind,
second_op: current_op,
},
span: TextSpan { start: self.offset, end: self.offset },
});
}
}
child_op = Some(current_op);
let op = self.parse_binary_operator();
let rhs = self.parse_expr_limited(precedence, lbrace_allowed);
ExprBinary::new_green(self.db, expr, op, rhs).into()
Expand Down
35 changes: 35 additions & 0 deletions crates/cairo-lang-parser/src/parser_test_data/partial_trees/expr
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,38 @@ StatementExpr
│ ├── op (kind: TokenMul): '*'
│ └── rhs (kind: TokenLiteralNumber): '5'
└── semicolon (kind: OptionTerminalSemicolonEmpty) []

//! > ==========================================================================

//! > Test consecutive comparison operators

//! > test_runner_name
test_partial_parser_tree(expect_diagnostics: true)

//! > cairo_code
fn f() -> bool {
3 < 1 > 5
}

//! > top_level_kind
StatementExpr

//! > ignored_kinds

//! > expected_diagnostics
error: Consecutive comparison operators are not allowed: '<' followed by '>'
--> dummy_file.cairo:2:9
3 < 1 > 5
^

//! > expected_tree
└── Top level kind: StatementExpr
├── attributes (kind: AttributeList) []
├── expr (kind: ExprBinary)
│ ├── lhs (kind: ExprBinary)
│ │ ├── lhs (kind: TokenLiteralNumber): '3'
│ │ ├── op (kind: TokenLT): '<'
│ │ └── rhs (kind: TokenLiteralNumber): '1'
│ ├── op (kind: TokenGT): '>'
│ └── rhs (kind: TokenLiteralNumber): '5'
└── semicolon (kind: OptionTerminalSemicolonEmpty) []
5 changes: 5 additions & 0 deletions crates/cairo-lang-semantic/src/diagnostic_test_data/tests
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,11 @@ fn bar<T>(_a: T) {}
//! > function_body

//! > expected_diagnostics
error: Consecutive comparison operators are not allowed: '<' followed by '>'
--> lib.cairo:4:19
let _fail = bar<felt252>(1);
^

error: Expected variable or constant, found function.
--> lib.cairo:4:15
let _fail = bar<felt252>(1);
Expand Down
5 changes: 5 additions & 0 deletions crates/cairo-lang-semantic/src/expr/test_data/operators
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ foo
//! > module_code

//! > expected_diagnostics
error: Consecutive comparison operators are not allowed: '>' followed by '>'
--> lib.cairo:6:9
a > a > a;
^

error: Unexpected argument type. Expected: "core::bool", found: "core::integer::u128".
--> lib.cairo:6:13
a > a > a;
Expand Down

0 comments on commit 7813ad2

Please sign in to comment.