From 55b3d681ef9558d359912ea0d42e76e08ad65145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=B4=EA=B4=91=ED=9A=A8?= <87641474+widehyo1@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:12:02 +0900 Subject: [PATCH] ast: match CPython argument validation order (#8414) Validate parameter annotations while converting ast.arguments, before merging positional and keyword-only defaults. This gives invalid annotation contexts precedence over default-list shape errors, as in CPython. Merge positional defaults before keyword-only defaults so an excess positional-default error also takes precedence over a keyword-only default length mismatch. Remove the expected-failure markers from the FunctionDef and Lambda AST validator tests that now pass. Assisted-by: Codex:gpt-5.6-sol --- Lib/test/test_ast/test_ast.py | 2 -- crates/vm/src/stdlib/_ast/parameter.rs | 34 +++++++++++++++++++++++++- crates/vm/src/stdlib/_ast/validate.rs | 5 +++- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 1450f440d..a3ce57034 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -2055,7 +2055,6 @@ class ASTValidatorTests(unittest.TestCase): kw_defaults=[None, ast.Name("x", ast.Store())]), "must have Load context") - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError not raised def test_funcdef(self): a = ast.arguments([], [], None, [], [], None, []) f = ast.FunctionDef("x", a, [], [], None, None, []) @@ -2267,7 +2266,6 @@ class ASTValidatorTests(unittest.TestCase): u = ast.UnaryOp(ast.Not(), ast.Name("x", ast.Store())) self.expr(u, "must have Load context") - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError not raised def test_lambda(self): a = ast.arguments([], [], None, [], [], None, []) self.expr(ast.Lambda(a, ast.Name("x", ast.Store())), diff --git a/crates/vm/src/stdlib/_ast/parameter.rs b/crates/vm/src/stdlib/_ast/parameter.rs index 477ceeb62..1ce4973b4 100644 --- a/crates/vm/src/stdlib/_ast/parameter.rs +++ b/crates/vm/src/stdlib/_ast/parameter.rs @@ -88,14 +88,23 @@ impl Node for ast::Parameters { "arguments", )?; + validate_parameter_annotations( + vm, + &posonlyargs, + &args, + vararg.as_deref(), + &kwonlyargs, + kwarg.as_deref(), + )?; + let ParameterDefaults { runtime_defaults, defaults, _range: _, } = defaults; - let kwonlyargs = merge_keyword_parameter_defaults(vm, kwonlyargs, kw_defaults)?; let (posonlyargs, args) = merge_positional_parameter_defaults(vm, posonlyargs, args, defaults)?; + let kwonlyargs = merge_keyword_parameter_defaults(vm, kwonlyargs, kw_defaults)?; Ok(Self { node_index: Default::default(), @@ -114,6 +123,29 @@ impl Node for ast::Parameters { } } +fn validate_parameter_annotations( + vm: &VirtualMachine, + posonlyargs: &PositionalParameters, + args: &PositionalParameters, + vararg: Option<&ast::Parameter>, + kwonlyargs: &KeywordParameters, + kwarg: Option<&ast::Parameter>, +) -> PyResult<()> { + for parameter in posonlyargs.args.iter().chain(&args.args) { + super::validate::validate_parameter_annotation(vm, parameter)?; + } + if let Some(parameter) = vararg { + super::validate::validate_parameter_annotation(vm, parameter)?; + } + for parameter in &kwonlyargs.keywords { + super::validate::validate_parameter_annotation(vm, parameter)?; + } + if let Some(parameter) = kwarg { + super::validate::validate_parameter_annotation(vm, parameter)?; + } + Ok(()) +} + // product impl Node for ast::Parameter { fn ast_to_object(self, vm: &VirtualMachine, source_file: &SourceFile) -> PyObjectRef { diff --git a/crates/vm/src/stdlib/_ast/validate.rs b/crates/vm/src/stdlib/_ast/validate.rs index 081197d6b..e936c9d3f 100644 --- a/crates/vm/src/stdlib/_ast/validate.rs +++ b/crates/vm/src/stdlib/_ast/validate.rs @@ -56,7 +56,10 @@ fn validate_keywords(vm: &VirtualMachine, keywords: &[ast::Keyword]) -> PyResult Ok(()) } -fn validate_parameter_annotation(vm: &VirtualMachine, parameter: &ast::Parameter) -> PyResult<()> { +pub(super) fn validate_parameter_annotation( + vm: &VirtualMachine, + parameter: &ast::Parameter, +) -> PyResult<()> { if let Some(annotation) = ¶meter.annotation { validate_expr(vm, annotation, ast::ExprContext::Load)?; }