Skip to content

Commit b8ced6c

Browse files
KotlinIslandclaude
andcommitted
stop the postfix !/^ operators from clobbering conversions and xor
in `.py` mode a glued `a^-b` parsed as the basedpython postfix propagate operator rather than infix xor, and an f-string conversion after a ternary (`f"{a if b else c!s}"`) lost the interpolation context so its `!` was eaten as force-unwrap. both turned valid python into parse errors, which tipped the formatter ecosystem check past its allowed input-error count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d2f1cf2 commit b8ced6c

2 files changed

Lines changed: 103 additions & 10 deletions

File tree

crates/ruff_python_parser/src/parser/expression.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ impl<'src> Parser<'src> {
289289
let parsed_expr = self.parse_simple_expression(context);
290290

291291
if self.at(TokenKind::If) {
292-
Expr::If(self.parse_if_expression(parsed_expr.expr, start)).into()
292+
Expr::If(self.parse_if_expression(parsed_expr.expr, start, context)).into()
293293
} else {
294294
parsed_expr
295295
}
@@ -1108,15 +1108,18 @@ impl<'src> Parser<'src> {
11081108
// basedpython postfix `^` propagate. `^` is otherwise the infix
11091109
// bitwise-xor operator, so it is only postfix when no operand
11101110
// follows; an operand on the right means it is xor (left for the
1111-
// binary loop). exception: a `^` *glued* to its operand (no
1112-
// whitespace, as one writes `expr^`) followed by a unary-capable
1113-
// arithmetic token (`+ - ~ * **`) is postfix-then-binary
1114-
// (`p(a)^ + b` → `(p(a)^) + b`), not xor-of-unary — the spaced
1115-
// form `a ^ -b` stays xor.
1111+
// binary loop). exception: in basedpython a `^` *glued* to its
1112+
// operand (no whitespace, as one writes `expr^`) followed by a
1113+
// unary-capable arithmetic token (`+ - ~ * **`) is
1114+
// postfix-then-binary (`p(a)^ + b` → `(p(a)^) + b`), not
1115+
// xor-of-unary. that disambiguation must not apply in `.py` mode,
1116+
// where glued `a^-b` is plain `a ^ (-b)` — stealing it would make
1117+
// valid python a syntax error.
11161118
TokenKind::CircumFlex
11171119
if self.options.mode != Mode::Ipython
11181120
&& (!(EXPR_SET.contains(self.peek()) || self.peek().is_soft_keyword())
1119-
|| (lhs.range().end() == self.current_token_range().start()
1121+
|| (self.options.is_basedpython
1122+
&& lhs.range().end() == self.current_token_range().start()
11201123
&& matches!(
11211124
self.peek(),
11221125
TokenKind::Plus
@@ -4342,18 +4345,34 @@ impl<'src> Parser<'src> {
43424345
/// If the parser isn't positioned at an `if` token.
43434346
///
43444347
/// See: <https://docs.python.org/3/reference/expressions.html#conditional-expressions>
4345-
pub(super) fn parse_if_expression(&mut self, body: Expr, start: TextSize) -> ast::ExprIf {
4348+
pub(super) fn parse_if_expression(
4349+
&mut self,
4350+
body: Expr,
4351+
start: TextSize,
4352+
context: ExpressionContext,
4353+
) -> ast::ExprIf {
43464354
self.bump(TokenKind::If);
43474355

43484356
let test = self.parse_simple_expression(ExpressionContext::default());
43494357

43504358
self.expect(TokenKind::Else);
43514359

4360+
// the `else` value is the tail of the conditional, so a trailing
4361+
// interpolation conversion (`f"{a if b else c!s}"`) lands here — carry
4362+
// only the interpolation flag so the `!` stays the conversion flag
4363+
// rather than being eaten as a postfix force-unwrap. the other context
4364+
// flags (excluded `in`/`for`, starred precedence, …) must not leak into
4365+
// the `else` value, which otherwise parses as an ordinary expression.
4366+
let orelse_context = if context.is_in_interpolation() {
4367+
ExpressionContext::default().with_in_interpolation()
4368+
} else {
4369+
ExpressionContext::default()
4370+
};
43524371
// `a if b else a if b else ...` recurses through `orelse` at the
43534372
// conditional layer, which is not covered by the `parse_lhs_expression`
43544373
// guard (that scope is released once each atom is parsed). Guard here.
43554374
let orelse = if let Some(orelse) =
4356-
self.with_recursion(Self::parse_conditional_expression_or_higher)
4375+
self.with_recursion(|p| p.parse_conditional_expression_or_higher_impl(orelse_context))
43574376
{
43584377
orelse
43594378
} else {

crates/ruff_python_parser/src/parser/tests.rs

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use ruff_python_ast::{ModModule, Stmt};
1+
use ruff_python_ast::{Expr, ModModule, Operator, Stmt, UnaryOp};
22

33
use crate::{Mode, ParseErrorType, ParseOptions, Parsed, parse, parse_expression, parse_module};
44

@@ -414,6 +414,80 @@ fn test_top_star_subscript_in_py_errors() {
414414
);
415415
}
416416

417+
#[test]
418+
fn glued_circumflex_before_unary_is_xor_in_py() {
419+
// `a^-b`, `a^+b`, `a^~b` are valid standard Python — `a ^ (-b)` and friends.
420+
// basedpython reads a glued `^` before a unary sign as the postfix propagate
421+
// operator (`(a^) - b`), but that disambiguation must stay off in `.py`
422+
// mode: stealing it turns valid python into a parse error, which the
423+
// formatter ecosystem check (it parses `.py` with basedpython disabled)
424+
// counts as a syntax error and trips over.
425+
for source in ["a^-b\n", "a^+b\n", "a^~b\n"] {
426+
let parsed = crate::parse_unchecked(source, ParseOptions::from(Mode::Module));
427+
assert!(
428+
parsed.errors().is_empty(),
429+
"expected {source:?} to parse cleanly in .py mode, got: {:?}",
430+
parsed.errors()
431+
);
432+
let module = parsed.try_into_module().unwrap();
433+
let Some(Stmt::Expr(stmt)) = module.suite().first() else {
434+
panic!("expected an expression statement for {source:?}");
435+
};
436+
let Expr::BinOp(binop) = &*stmt.value else {
437+
panic!(
438+
"expected `a ^ <unary>` for {source:?}, got {:?}",
439+
stmt.value
440+
);
441+
};
442+
assert_eq!(binop.op, Operator::BitXor, "operator for {source:?}");
443+
assert!(
444+
matches!(&*binop.right, Expr::UnaryOp(_)),
445+
"rhs of {source:?} should be the unary operand, got {:?}",
446+
binop.right
447+
);
448+
}
449+
450+
// the same glued source keeps its basedpython meaning in `.by`: a postfix
451+
// propagate followed by a binary subtract, i.e. `(a^) - b`
452+
let parsed = parse_basedpython_module("a^-b\n");
453+
let Some(Stmt::Expr(stmt)) = parsed.syntax().body.first() else {
454+
panic!("expected an expression statement");
455+
};
456+
let Expr::BinOp(binop) = &*stmt.value else {
457+
panic!("expected a binary op, got {:?}", stmt.value);
458+
};
459+
assert_eq!(binop.op, Operator::Sub);
460+
assert!(
461+
matches!(&*binop.left, Expr::UnaryOp(unary) if unary.op == UnaryOp::Propagate),
462+
"lhs should be the postfix propagate, got {:?}",
463+
binop.left
464+
);
465+
}
466+
467+
#[test]
468+
fn fstring_conversion_after_ternary_is_not_force_unwrap() {
469+
// the `!s` / `!r` in an interpolation is the conversion flag, not the
470+
// basedpython postfix force-unwrap. for a conditional the conversion lands
471+
// on the `else` tail, which must inherit the interpolation context so the
472+
// `!` isn't eaten as force-unwrap. regression: poetry's
473+
// `f"{p if not W else g(p)!s}"` became a spurious `.py` syntax error and
474+
// tipped the formatter ecosystem check over its allowed error count.
475+
for source in [
476+
r#"f"{a if b else c!s}""#,
477+
r#"f"{a if b else c!r:>{w}}""#,
478+
r#"f"{a if b else c if d else e!s}""#,
479+
r#"f"{x!r}""#,
480+
r#"f"{a + b!s}""#,
481+
] {
482+
let parsed = crate::parse_unchecked(source, ParseOptions::from(Mode::Module));
483+
assert!(
484+
parsed.errors().is_empty(),
485+
"expected {source:?} to parse cleanly in .py mode, got: {:?}",
486+
parsed.errors()
487+
);
488+
}
489+
}
490+
417491
#[test]
418492
fn recursion_limit_nested_parens() {
419493
let src = format!("{}1{}", "(".repeat(1_000), ")".repeat(1_000));

0 commit comments

Comments
 (0)