mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 02:38:25 +00:00
Fix AST visitor traversal order (#5221)
## Summary According to the AST visitor documentation, the AST visitor "visits all nodes in the AST recursively in evaluation-order". However, the current traversal fails to meet this specification in a few places. ### Function traversal ```python order = [] @(order.append("decorator") or (lambda x: x)) def f( posonly: order.append("posonly annotation") = order.append("posonly default"), /, arg: order.append("arg annotation") = order.append("arg default"), *args: order.append("vararg annotation"), kwarg: order.append("kwarg annotation") = order.append("kwarg default"), **kwargs: order.append("kwarg annotation") ) -> order.append("return annotation"): pass print(order) ``` Executing the above snippet using CPython 3.10.6 prints the following result (formatted for readability): ```python [ 'decorator', 'posonly default', 'arg default', 'kwarg default', 'arg annotation', 'posonly annotation', 'vararg annotation', 'kwarg annotation', 'kwarg annotation', 'return annotation', ] ``` Here we can see that decorators are evaluated first, followed by argument defaults, and annotations are last. The current traversal of a function's AST does not align with this order. ### Annotated assignment traversal ```python order = [] x: order.append("annotation") = order.append("expression") print(order) ``` Executing the above snippet using CPython 3.10.6 prints the following result: ```python ['expression', 'annotation'] ``` Here we can see that an annotated assignments annotation gets evaluated after the assignment's expression. The current traversal of an annotated assignment's AST does not align with this order. ## Why? I'm slowly working on #3946 and porting over some of the logic and tests from ssort. ssort is very sensitive to AST traversal order, so ensuring the utmost correctness here is important. ## Test Plan There doesn't seem to be existing tests for the AST visitor, so I didn't bother adding tests for these very subtle changes. However, this behavior will be captured in the tests for the PR which addresses #3946.
This commit is contained in:
parent
d7c7484618
commit
9b5fb8f38f
1 changed files with 7 additions and 7 deletions
|
@ -99,10 +99,10 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) {
|
|||
returns,
|
||||
..
|
||||
}) => {
|
||||
visitor.visit_arguments(args);
|
||||
for decorator in decorator_list {
|
||||
visitor.visit_decorator(decorator);
|
||||
}
|
||||
visitor.visit_arguments(args);
|
||||
for expr in returns {
|
||||
visitor.visit_annotation(expr);
|
||||
}
|
||||
|
@ -115,10 +115,10 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) {
|
|||
returns,
|
||||
..
|
||||
}) => {
|
||||
visitor.visit_arguments(args);
|
||||
for decorator in decorator_list {
|
||||
visitor.visit_decorator(decorator);
|
||||
}
|
||||
visitor.visit_arguments(args);
|
||||
for expr in returns {
|
||||
visitor.visit_annotation(expr);
|
||||
}
|
||||
|
@ -131,15 +131,15 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) {
|
|||
decorator_list,
|
||||
..
|
||||
}) => {
|
||||
for decorator in decorator_list {
|
||||
visitor.visit_decorator(decorator);
|
||||
}
|
||||
for expr in bases {
|
||||
visitor.visit_expr(expr);
|
||||
}
|
||||
for keyword in keywords {
|
||||
visitor.visit_keyword(keyword);
|
||||
}
|
||||
for decorator in decorator_list {
|
||||
visitor.visit_decorator(decorator);
|
||||
}
|
||||
visitor.visit_body(body);
|
||||
}
|
||||
Stmt::Return(ast::StmtReturn {
|
||||
|
@ -180,10 +180,10 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) {
|
|||
value,
|
||||
..
|
||||
}) => {
|
||||
visitor.visit_annotation(annotation);
|
||||
if let Some(expr) = value {
|
||||
visitor.visit_expr(expr);
|
||||
}
|
||||
visitor.visit_annotation(annotation);
|
||||
visitor.visit_expr(target);
|
||||
}
|
||||
Stmt::For(ast::StmtFor {
|
||||
|
@ -633,10 +633,10 @@ pub fn walk_arg_with_default<'a, V: Visitor<'a> + ?Sized>(
|
|||
visitor: &mut V,
|
||||
arg_with_default: &'a ArgWithDefault,
|
||||
) {
|
||||
visitor.visit_arg(&arg_with_default.def);
|
||||
if let Some(expr) = &arg_with_default.default {
|
||||
visitor.visit_expr(expr);
|
||||
}
|
||||
visitor.visit_arg(&arg_with_default.def);
|
||||
}
|
||||
|
||||
pub fn walk_keyword<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, keyword: &'a Keyword) {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue