Skip to content

Commit

Permalink
Consider binary expr for parenthesized with items parsing (#11012)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes the bug in with items parsing where it would fail to
recognize that the parenthesized expression is part of a large binary
expression.

## Test Plan

Add test cases and verified the snapshots.
  • Loading branch information
dhruvmanila committed Apr 18, 2024
1 parent 6c4d779 commit b7066e6
Show file tree
Hide file tree
Showing 4 changed files with 417 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# It doesn't matter what's inside the parentheses, these tests need to make sure
# all binary expressions parses correctly.
with (a) and b: ...
with (a) is not b: ...
# Make sure precedence works
with (a) or b and c: ...
with (a) and b or c: ...
with (a | b) << c | d: ...
# Postfix should still be parsed first
with (a)[0] + b * c: ...
14 changes: 12 additions & 2 deletions crates/ruff_python_parser/src/parser/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,18 @@ impl<'src> Parser<'src> {
/// [Pratt parsing algorithm]: https://matklad.github.io/2020/04/13/simple-but-powerful-pratt-parsing.html
fn parse_expression_with_precedence(&mut self, previous_precedence: Precedence) -> ParsedExpr {
let start = self.node_start();
let mut lhs = self.parse_lhs_expression(previous_precedence);
let lhs = self.parse_lhs_expression(previous_precedence);
self.parse_expression_with_precedence_recursive(lhs, previous_precedence, start)
}

/// Parses an expression with binding power of at least `previous_precedence` given the
/// left-hand side expression.
pub(super) fn parse_expression_with_precedence_recursive(
&mut self,
mut lhs: ParsedExpr,
previous_precedence: Precedence,
start: TextSize,
) -> ParsedExpr {
let mut progress = ParserProgress::default();

loop {
Expand Down Expand Up @@ -2498,7 +2508,7 @@ enum Associativity {
///
/// See: <https://docs.python.org/3/reference/expressions.html#operator-precedence>
#[derive(Debug, Ord, Eq, PartialEq, PartialOrd, Copy, Clone)]
enum Precedence {
pub(super) enum Precedence {
/// Precedence for an unknown operator.
Unknown,
/// The initital precedence when parsing an expression.
Expand Down
30 changes: 23 additions & 7 deletions crates/ruff_python_parser/src/parser/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::parser::{
use crate::token_set::TokenSet;
use crate::{Mode, ParseErrorType, Tok, TokenKind};

use super::expression::{AllowNamedExpression, AllowStarredExpression};
use super::expression::{AllowNamedExpression, AllowStarredExpression, Precedence};
use super::Parenthesized;

/// Tokens that represent compound statements.
Expand Down Expand Up @@ -2147,15 +2147,31 @@ impl<'src> Parser<'src> {
// expressions.
lhs = self.parse_postfix_expression(lhs, start);

// test_ok ambiguous_lpar_with_items_if_expr
// with (x) if True else y: ...
// with (x for x in iter) if True else y: ...
// with (x async for x in iter) if True else y: ...
// with (x)[0] if True else y: ...
let context_expr = if self.at(TokenKind::If) {
// test_ok ambiguous_lpar_with_items_if_expr
// with (x) if True else y: ...
// with (x for x in iter) if True else y: ...
// with (x async for x in iter) if True else y: ...
// with (x)[0] if True else y: ...
Expr::If(self.parse_if_expression(lhs, start))
} else {
lhs
// test_ok ambiguous_lpar_with_items_binary_expr
// # It doesn't matter what's inside the parentheses, these tests need to make sure
// # all binary expressions parses correctly.
// with (a) and b: ...
// with (a) is not b: ...
// # Make sure precedence works
// with (a) or b and c: ...
// with (a) and b or c: ...
// with (a | b) << c | d: ...
// # Postfix should still be parsed first
// with (a)[0] + b * c: ...
self.parse_expression_with_precedence_recursive(
lhs.into(),
Precedence::Initial,
start,
)
.expr
};

let optional_vars = self
Expand Down

0 comments on commit b7066e6

Please sign in to comment.