From 2a4d8351325d97e3a2dd4a0fcc489cce15561dc1 Mon Sep 17 00:00:00 2001 From: Junhson Jean-Baptiste Date: Fri, 21 Mar 2025 00:10:37 -0400 Subject: [PATCH] Use the common `OperatorPrecedence` for the parser (#16747) ## Summary This change continues to resolve #16071 (and continues the work started in #16162). Specifically, this PR changes the code in the parser so that it uses the `OperatorPrecedence` struct from `ruff_python_ast` instead of its own version. This is part of an effort to get rid of the redundant definitions of `OperatorPrecedence` throughout the codebase. Note that this PR only makes this change for `ruff_python_parser` -- we still want to make a similar change for the formatter (namely the `OperatorPrecedence` defined in the expression part of the formatter, the pattern one is different). I separated the work to keep the PRs small and easily reviewable. ## Test Plan Because this is an internal change, I didn't add any additional tests. Existing tests do pass. --- .../src/operator_precedence.rs | 15 +++ .../src/parser/expression.rs | 98 +------------------ .../valid_syntax@expressions__bin_op.py.snap | 1 - 3 files changed, 19 insertions(+), 95 deletions(-) diff --git a/crates/ruff_python_ast/src/operator_precedence.rs b/crates/ruff_python_ast/src/operator_precedence.rs index b39c017cd4..750ef7f719 100644 --- a/crates/ruff_python_ast/src/operator_precedence.rs +++ b/crates/ruff_python_ast/src/operator_precedence.rs @@ -131,6 +131,12 @@ impl OperatorPrecedence { pub fn from_expr(expr: &Expr) -> Self { Self::from(&ExprRef::from(expr)) } + + /// Returns `true` if the precedence is right-associative i.e., the operations are evaluated + /// from right to left. + pub fn is_right_associative(self) -> bool { + matches!(self, OperatorPrecedence::Exponent) + } } impl From<&Expr> for OperatorPrecedence { @@ -177,3 +183,12 @@ impl From for OperatorPrecedence { } } } + +impl From for OperatorPrecedence { + fn from(unary_op: UnaryOp) -> Self { + match unary_op { + UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert => Self::PosNegBitNot, + UnaryOp::Not => Self::Not, + } + } +} diff --git a/crates/ruff_python_parser/src/parser/expression.rs b/crates/ruff_python_parser/src/parser/expression.rs index a090d1525e..4e1fd6dff1 100644 --- a/crates/ruff_python_parser/src/parser/expression.rs +++ b/crates/ruff_python_parser/src/parser/expression.rs @@ -7,7 +7,7 @@ use rustc_hash::{FxBuildHasher, FxHashSet}; use ruff_python_ast::name::Name; use ruff_python_ast::{ self as ast, BoolOp, CmpOp, ConversionFlag, Expr, ExprContext, FStringElement, FStringElements, - IpyEscapeKind, Number, Operator, StringFlags, UnaryOp, + IpyEscapeKind, Number, Operator, OperatorPrecedence, StringFlags, UnaryOp, }; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -228,7 +228,7 @@ impl<'src> Parser<'src> { /// /// [Python grammar]: https://docs.python.org/3/reference/grammar.html fn parse_simple_expression(&mut self, context: ExpressionContext) -> ParsedExpr { - self.parse_binary_expression_or_higher(OperatorPrecedence::Initial, context) + self.parse_binary_expression_or_higher(OperatorPrecedence::None, context) } /// Parses a binary expression using the [Pratt parsing algorithm]. @@ -360,7 +360,7 @@ impl<'src> Parser<'src> { TokenKind::Star => { let starred_expr = self.parse_starred_expression(context); - if left_precedence > OperatorPrecedence::Initial + if left_precedence > OperatorPrecedence::None || !context.is_starred_expression_allowed() { self.add_error(ParseErrorType::InvalidStarredExpressionUsage, &starred_expr); @@ -393,7 +393,7 @@ impl<'src> Parser<'src> { TokenKind::Yield => { let expr = self.parse_yield_expression(); - if left_precedence > OperatorPrecedence::Initial + if left_precedence > OperatorPrecedence::None || !context.is_yield_expression_allowed() { self.add_error(ParseErrorType::InvalidYieldExpressionUsage, &expr); @@ -2596,57 +2596,6 @@ impl Ranged for ParsedExpr { } } -/// Represents the precedence levels for various operators and expressions of Python. -/// Variants at the top have lower precedence and variants at the bottom have -/// higher precedence. -/// -/// Note: Some expressions like if-else, named expression (`:=`), lambda, subscription, -/// slicing, call and attribute reference expressions, that are mentioned in the link -/// below are better handled in other parts of the parser. -/// -/// See: -#[derive(Debug, Ord, Eq, PartialEq, PartialOrd, Copy, Clone)] -pub(super) enum OperatorPrecedence { - /// The initial precedence when parsing an expression. - Initial, - /// Precedence of boolean `or` operator. - Or, - /// Precedence of boolean `and` operator. - And, - /// Precedence of boolean `not` unary operator. - Not, - /// Precedence of comparisons operators (`<`, `<=`, `>`, `>=`, `!=`, `==`), - /// memberships tests (`in`, `not in`) and identity tests (`is` `is not`). - ComparisonsMembershipIdentity, - /// Precedence of `Bitwise OR` (`|`) operator. - BitOr, - /// Precedence of `Bitwise XOR` (`^`) operator. - BitXor, - /// Precedence of `Bitwise AND` (`&`) operator. - BitAnd, - /// Precedence of left and right shift operators (`<<`, `>>`). - LeftRightShift, - /// Precedence of addition (`+`) and subtraction (`-`) operators. - AddSub, - /// Precedence of multiplication (`*`), matrix multiplication (`@`), division (`/`), floor - /// division (`//`) and remainder operators (`%`). - MulDivRemain, - /// Precedence of positive (`+`), negative (`-`), `Bitwise NOT` (`~`) unary operators. - PosNegBitNot, - /// Precedence of exponentiation operator (`**`). - Exponent, - /// Precedence of `await` expression. - Await, -} - -impl OperatorPrecedence { - /// Returns `true` if the precedence is right-associative i.e., the operations are evaluated - /// from right to left. - fn is_right_associative(self) -> bool { - matches!(self, OperatorPrecedence::Exponent) - } -} - #[derive(Debug)] enum BinaryLikeOperator { Boolean(BoolOp), @@ -2678,45 +2627,6 @@ impl BinaryLikeOperator { } } -impl From for OperatorPrecedence { - #[inline] - fn from(op: BoolOp) -> Self { - match op { - BoolOp::And => OperatorPrecedence::And, - BoolOp::Or => OperatorPrecedence::Or, - } - } -} - -impl From for OperatorPrecedence { - #[inline] - fn from(op: UnaryOp) -> Self { - match op { - UnaryOp::Not => OperatorPrecedence::Not, - _ => OperatorPrecedence::PosNegBitNot, - } - } -} - -impl From for OperatorPrecedence { - #[inline] - fn from(op: Operator) -> Self { - match op { - Operator::Add | Operator::Sub => OperatorPrecedence::AddSub, - Operator::Mult - | Operator::Div - | Operator::FloorDiv - | Operator::Mod - | Operator::MatMult => OperatorPrecedence::MulDivRemain, - Operator::BitAnd => OperatorPrecedence::BitAnd, - Operator::BitOr => OperatorPrecedence::BitOr, - Operator::BitXor => OperatorPrecedence::BitXor, - Operator::LShift | Operator::RShift => OperatorPrecedence::LeftRightShift, - Operator::Pow => OperatorPrecedence::Exponent, - } - } -} - /// Represents the precedence used for parsing the value part of a starred expression. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum StarredExpressionPrecedence { diff --git a/crates/ruff_python_parser/tests/snapshots/valid_syntax@expressions__bin_op.py.snap b/crates/ruff_python_parser/tests/snapshots/valid_syntax@expressions__bin_op.py.snap index 70015f4751..a6cc4528a1 100644 --- a/crates/ruff_python_parser/tests/snapshots/valid_syntax@expressions__bin_op.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/valid_syntax@expressions__bin_op.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_parser/tests/fixtures.rs input_file: crates/ruff_python_parser/resources/valid/expressions/bin_op.py -snapshot_kind: text --- ## AST