diff --git a/crates/monty/src/parse.rs b/crates/monty/src/parse.rs index 88951de3..116d70b6 100644 --- a/crates/monty/src/parse.rs +++ b/crates/monty/src/parse.rs @@ -1250,7 +1250,7 @@ impl<'a> Parser<'a> { match ast { AstExpr::Name(ast::ExprName { id, range, .. }) => Ok(self.identifier(&id, range)), other => Err(ParseError::syntax( - format!("Expected name, got {other:?}"), + format!("Expected name, got {}", describe_expr_kind(&other)), self.convert_range(other.range()), )), } @@ -1351,7 +1351,7 @@ impl<'a> Parser<'a> { Ok(UnpackTarget::Tuple { targets, position }) } other => Err(ParseError::syntax( - format!("invalid unpacking target: {other:?}"), + format!("invalid unpacking target: {}", describe_expr_kind(&other)), self.convert_range(other.range()), )), } @@ -1611,6 +1611,48 @@ fn convert_conversion_flag(flag: RuffConversionFlag) -> ConversionFlag { } } +/// Short human-readable name for an `AstExpr` variant, for use in +/// user-facing parse errors. Avoids the Rust `Debug` formatting of the +/// node, which would leak internal field names, ranges, and struct +/// layout of `ruff_python_ast` into the error message. +fn describe_expr_kind(expr: &AstExpr) -> &'static str { + match expr { + AstExpr::Name(_) => "name", + AstExpr::Starred(_) => "starred expression", + AstExpr::Attribute(_) => "attribute", + AstExpr::Subscript(_) => "subscript", + AstExpr::Call(_) => "function call", + AstExpr::Tuple(_) => "tuple", + AstExpr::List(_) => "list", + AstExpr::Set(_) => "set", + AstExpr::Dict(_) => "dict", + AstExpr::NumberLiteral(_) => "number literal", + AstExpr::StringLiteral(_) => "string literal", + AstExpr::BytesLiteral(_) => "bytes literal", + AstExpr::BooleanLiteral(_) => "boolean literal", + AstExpr::NoneLiteral(_) => "None", + AstExpr::EllipsisLiteral(_) => "...", + AstExpr::FString(_) => "f-string", + AstExpr::TString(_) => "t-string", + AstExpr::Lambda(_) => "lambda", + AstExpr::If(_) => "conditional expression", + AstExpr::BoolOp(_) => "boolean expression", + AstExpr::BinOp(_) => "binary expression", + AstExpr::UnaryOp(_) => "unary expression", + AstExpr::Compare(_) => "comparison", + AstExpr::Named(_) => "named expression", + AstExpr::Yield(_) => "yield expression", + AstExpr::YieldFrom(_) => "yield from expression", + AstExpr::Await(_) => "await expression", + AstExpr::ListComp(_) => "list comprehension", + AstExpr::SetComp(_) => "set comprehension", + AstExpr::DictComp(_) => "dict comprehension", + AstExpr::Generator(_) => "generator expression", + AstExpr::Slice(_) => "slice", + AstExpr::IpyEscapeCommand(_) => "IPython escape command", + } +} + /// Source code location for a parsed node, stored as raw byte offsets. /// /// `CodeRange` is written by the parser for every AST node and must therefore diff --git a/crates/monty/tests/parse_errors.rs b/crates/monty/tests/parse_errors.rs index ef575065..7fa49126 100644 --- a/crates/monty/tests/parse_errors.rs +++ b/crates/monty/tests/parse_errors.rs @@ -1,5 +1,6 @@ use std::fmt::Write; +use insta::assert_snapshot; use monty::{ExcType, MontyException, MontyRun}; /// Helper to extract the exception type from a parse error. @@ -450,3 +451,51 @@ fn long_source_line_does_not_overflow_column() { let result = run.run_no_limits(vec![]); assert!(result.is_ok(), "long line should run: {result:?}"); } + +// === Parse error messages must not leak ruff_python_ast Debug formatting === +// +// These snapshot the full error message for each trigger so any future +// regression that reintroduces Debug formatting of AST nodes (struct +// names, `node_index`, `range`, `ctx: Store`, etc.) fails the snapshot +// diff loudly. + +#[test] +fn starred_name_target_has_clean_message() { + // `*a = [1, 2]`: Ruff parses the LHS as a bare starred target, which + // Monty rejects at `parse_identifier`. + let result = MontyRun::new("*a = [1, 2]".to_owned(), "test.py", vec![]); + let exc = result.expect_err("expected parse error"); + assert_eq!(exc.exc_type(), ExcType::SyntaxError); + assert_snapshot!(exc.message().expect("has message"), @"Expected name, got starred expression"); +} + +#[test] +fn starred_attribute_target_has_clean_message() { + // `*x.y = 1`: starred target wrapping an attribute. Same rejection + // path, different inner node shape. + let result = MontyRun::new("*x.y = 1".to_owned(), "test.py", vec![]); + let exc = result.expect_err("expected parse error"); + assert_eq!(exc.exc_type(), ExcType::SyntaxError); + assert_snapshot!(exc.message().expect("has message"), @"Expected name, got starred expression"); +} + +#[test] +fn starred_subscript_target_has_clean_message() { + // `*x[0] = 1`: starred target wrapping a subscript. + let result = MontyRun::new("*x[0] = 1".to_owned(), "test.py", vec![]); + let exc = result.expect_err("expected parse error"); + assert_eq!(exc.exc_type(), ExcType::SyntaxError); + assert_snapshot!(exc.message().expect("has message"), @"Expected name, got starred expression"); +} + +#[test] +fn for_loop_attribute_target_has_clean_message() { + // `for x.y in [1]: pass`: attribute as a for-loop target. CPython + // accepts this; Monty currently rejects at `parse_unpack_target_impl`. + // That rejection of valid Python is a separate issue; this test locks + // only that the error message does not leak `ExprAttribute` Debug. + let result = MontyRun::new("for x.y in [1]: pass".to_owned(), "test.py", vec![]); + let exc = result.expect_err("expected parse error"); + assert_eq!(exc.exc_type(), ExcType::SyntaxError); + assert_snapshot!(exc.message().expect("has message"), @"invalid unpacking target: attribute"); +}