diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 3c5c8d0bb..7142064dd 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -604,7 +604,6 @@ class AST_Tests(unittest.TestCase): ): compile(e, "", "eval") - @unittest.expectedFailure # TODO: RUSTPYTHON; TypeError: expected some sort of expr, but got None def test_empty_yield_from(self): # Issue 16546: yield from value is not optional. empty_yield_from = ast.parse("def f():\n yield from g()") @@ -1039,7 +1038,6 @@ class AST_Tests(unittest.TestCase): with self.assertRaisesRegex(ValueError, f"^{e}$"): compile(tree, "", "exec") - @unittest.expectedFailure # TODO: RUSTPYTHON; TypeError: expected some sort of expr, but got None def test_none_checks(self) -> None: tests = [ (ast.alias, "name", "import spam as SPAM"), diff --git a/crates/vm/src/stdlib/_ast.rs b/crates/vm/src/stdlib/_ast.rs index bde6916a6..104ad39ee 100644 --- a/crates/vm/src/stdlib/_ast.rs +++ b/crates/vm/src/stdlib/_ast.rs @@ -73,6 +73,33 @@ fn get_node_field(vm: &VirtualMachine, obj: &PyObject, field: &'static str, typ: .ok_or_else(|| vm.new_type_error(format!(r#"required field "{field}" missing from {typ}"#))) } +/// Read a required scalar field, rejecting both attribute absence and `None` value +/// with CPython-compatible error messages. Pairs with `get_node_field_opt` (which +/// returns `Option::None` for the same conditions): both filter `None`, but diverge +/// on whether to raise or return `None`. +/// +/// Errors: +/// - Attribute absent: `TypeError("required field \"X\" missing from Y")` (via `get_node_field`). +/// - Attribute present but `None`: `ValueError("field 'X' is required for Y")`, +/// matching CPython's `Python/ast.c` validator output. +/// +/// Use for required scalar fields where `None` is invalid (e.g. `comprehension.target`, +/// `keyword.value`, `match_case.pattern`). Do NOT use for fields where `None` is +/// legitimate (e.g. `Constant.value` representing the `None` literal — use plain +/// `get_node_field`); or for optional fields (use `get_node_field_opt`). +fn get_node_field_required( + vm: &VirtualMachine, + obj: &PyObject, + field: &'static str, + typ: &str, +) -> PyResult { + let value = get_node_field(vm, obj, field, typ)?; + if vm.is_none(&value) { + return Err(vm.new_value_error(format!("field '{field}' is required for {typ}"))); + } + Ok(value) +} + fn get_node_field_opt( vm: &VirtualMachine, obj: &PyObject, diff --git a/crates/vm/src/stdlib/_ast/expression.rs b/crates/vm/src/stdlib/_ast/expression.rs index f40275f90..2b32a33f3 100644 --- a/crates/vm/src/stdlib/_ast/expression.rs +++ b/crates/vm/src/stdlib/_ast/expression.rs @@ -802,7 +802,7 @@ impl Node for ast::ExprYieldFrom { value: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "value", "YieldFrom")?, + get_node_field_required(vm, &object, "value", "YieldFrom")?, )?, range: range_from_object(vm, source_file, object, "YieldFrom")?, }) @@ -1316,12 +1316,12 @@ impl Node for ast::Comprehension { target: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "target", "comprehension")?, + get_node_field_required(vm, &object, "target", "comprehension")?, )?, iter: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "iter", "comprehension")?, + get_node_field_required(vm, &object, "iter", "comprehension")?, )?, ifs: Node::ast_from_object( vm, diff --git a/crates/vm/src/stdlib/_ast/other.rs b/crates/vm/src/stdlib/_ast/other.rs index 5c0803ac5..5009c588c 100644 --- a/crates/vm/src/stdlib/_ast/other.rs +++ b/crates/vm/src/stdlib/_ast/other.rs @@ -92,7 +92,7 @@ impl Node for ast::Alias { name: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "name", "alias")?, + get_node_field_required(vm, &object, "name", "alias")?, )?, asname: get_node_field_opt(vm, &object, "asname")? .map(|obj| Node::ast_from_object(vm, source_file, obj)) @@ -140,7 +140,7 @@ impl Node for ast::WithItem { context_expr: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "context_expr", "withitem")?, + get_node_field_required(vm, &object, "context_expr", "withitem")?, )?, optional_vars: get_node_field_opt(vm, &object, "optional_vars")? .map(|obj| Node::ast_from_object(vm, source_file, obj)) diff --git a/crates/vm/src/stdlib/_ast/parameter.rs b/crates/vm/src/stdlib/_ast/parameter.rs index 15ff237e5..b0c807a29 100644 --- a/crates/vm/src/stdlib/_ast/parameter.rs +++ b/crates/vm/src/stdlib/_ast/parameter.rs @@ -143,7 +143,7 @@ impl Node for ast::Parameter { name: Node::ast_from_object( _vm, source_file, - get_node_field(_vm, &_object, "arg", "arg")?, + get_node_field_required(_vm, &_object, "arg", "arg")?, )?, annotation: get_node_field_opt(_vm, &_object, "annotation")? .map(|obj| Node::ast_from_object(_vm, source_file, obj)) @@ -189,7 +189,7 @@ impl Node for ast::Keyword { value: Node::ast_from_object( _vm, source_file, - get_node_field(_vm, &_object, "value", "keyword")?, + get_node_field_required(_vm, &_object, "value", "keyword")?, )?, range: range_from_object(_vm, source_file, _object, "keyword")?, }) diff --git a/crates/vm/src/stdlib/_ast/pattern.rs b/crates/vm/src/stdlib/_ast/pattern.rs index 621c849e8..11f758f02 100644 --- a/crates/vm/src/stdlib/_ast/pattern.rs +++ b/crates/vm/src/stdlib/_ast/pattern.rs @@ -34,7 +34,7 @@ impl Node for ast::MatchCase { pattern: Node::ast_from_object( vm, source_file, - get_node_field(vm, &object, "pattern", "match_case")?, + get_node_field_required(vm, &object, "pattern", "match_case")?, )?, guard: get_node_field_opt(vm, &object, "guard")? .map(|obj| Node::ast_from_object(vm, source_file, obj))