From a39dd76d95e575eb124dac81b16f11336ea62d57 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 9 Aug 2023 11:09:00 +0200 Subject: [PATCH] Add `enter` and `leave_node` methods to Preoder visitor (#6422) --- .../ruff_python_ast/src/visitor/preorder.rs | 1200 +++++++++-------- .../src/comments/visitor.rs | 203 +-- 2 files changed, 686 insertions(+), 717 deletions(-) diff --git a/crates/ruff_python_ast/src/visitor/preorder.rs b/crates/ruff_python_ast/src/visitor/preorder.rs index 6d3c5334da04a..1e18bf25442b9 100644 --- a/crates/ruff_python_ast/src/visitor/preorder.rs +++ b/crates/ruff_python_ast/src/visitor/preorder.rs @@ -1,3 +1,4 @@ +use crate::node::AnyNodeRef; use crate::{ self as ast, Alias, Arguments, BoolOp, CmpOp, Comprehension, Constant, Decorator, ElifElseClause, ExceptHandler, Expr, Keyword, MatchCase, Mod, Operator, Parameter, @@ -7,64 +8,89 @@ use crate::{ /// Visitor that traverses all nodes recursively in pre-order. pub trait PreorderVisitor<'a> { + #[allow(clippy::inline_always)] + #[inline(always)] + fn enter_node(&mut self, _node: AnyNodeRef<'a>) -> TraversalSignal { + TraversalSignal::Traverse + } + + #[inline(always)] + fn leave_node(&mut self, _node: AnyNodeRef<'a>) {} + + #[inline] fn visit_mod(&mut self, module: &'a Mod) { walk_module(self, module); } + #[inline] fn visit_stmt(&mut self, stmt: &'a Stmt) { walk_stmt(self, stmt); } + #[inline] fn visit_annotation(&mut self, expr: &'a Expr) { walk_annotation(self, expr); } + #[inline] fn visit_expr(&mut self, expr: &'a Expr) { walk_expr(self, expr); } + #[inline] fn visit_decorator(&mut self, decorator: &'a Decorator) { walk_decorator(self, decorator); } + #[inline] fn visit_constant(&mut self, _constant: &'a Constant) {} + #[inline] fn visit_bool_op(&mut self, bool_op: &'a BoolOp) { walk_bool_op(self, bool_op); } + #[inline] fn visit_operator(&mut self, operator: &'a Operator) { walk_operator(self, operator); } + #[inline] fn visit_unary_op(&mut self, unary_op: &'a UnaryOp) { walk_unary_op(self, unary_op); } + #[inline] fn visit_cmp_op(&mut self, cmp_op: &'a CmpOp) { walk_cmp_op(self, cmp_op); } + #[inline] fn visit_comprehension(&mut self, comprehension: &'a Comprehension) { walk_comprehension(self, comprehension); } + #[inline] fn visit_except_handler(&mut self, except_handler: &'a ExceptHandler) { walk_except_handler(self, except_handler); } + #[inline] fn visit_format_spec(&mut self, format_spec: &'a Expr) { walk_format_spec(self, format_spec); } + #[inline] fn visit_arguments(&mut self, arguments: &'a Arguments) { walk_arguments(self, arguments); } + #[inline] fn visit_parameters(&mut self, parameters: &'a Parameters) { walk_parameters(self, parameters); } + #[inline] fn visit_parameter(&mut self, arg: &'a Parameter) { walk_parameter(self, arg); } @@ -73,38 +99,47 @@ pub trait PreorderVisitor<'a> { walk_parameter_with_default(self, parameter_with_default); } + #[inline] fn visit_keyword(&mut self, keyword: &'a Keyword) { walk_keyword(self, keyword); } + #[inline] fn visit_alias(&mut self, alias: &'a Alias) { walk_alias(self, alias); } + #[inline] fn visit_with_item(&mut self, with_item: &'a WithItem) { walk_with_item(self, with_item); } + #[inline] fn visit_type_params(&mut self, type_params: &'a TypeParams) { walk_type_params(self, type_params); } + #[inline] fn visit_type_param(&mut self, type_param: &'a TypeParam) { walk_type_param(self, type_param); } + #[inline] fn visit_match_case(&mut self, match_case: &'a MatchCase) { walk_match_case(self, match_case); } + #[inline] fn visit_pattern(&mut self, pattern: &'a Pattern) { walk_pattern(self, pattern); } + #[inline] fn visit_body(&mut self, body: &'a [Stmt]) { walk_body(self, body); } + #[inline] fn visit_elif_else_clause(&mut self, elif_else_clause: &'a ElifElseClause) { walk_elif_else_clause(self, elif_else_clause); } @@ -114,12 +149,17 @@ pub fn walk_module<'a, V>(visitor: &mut V, module: &'a Mod) where V: PreorderVisitor<'a> + ?Sized, { - match module { - Mod::Module(ast::ModModule { body, range: _ }) => { - visitor.visit_body(body); + let node = AnyNodeRef::from(module); + if visitor.enter_node(node).is_traverse() { + match module { + Mod::Module(ast::ModModule { body, range: _ }) => { + visitor.visit_body(body); + } + Mod::Expression(ast::ModExpression { body, range: _ }) => visitor.visit_expr(body), } - Mod::Expression(ast::ModExpression { body, range: _ }) => visitor.visit_expr(body), } + + visitor.leave_node(node); } pub fn walk_body<'a, V>(visitor: &mut V, body: &'a [Stmt]) @@ -135,613 +175,680 @@ pub fn walk_stmt<'a, V>(visitor: &mut V, stmt: &'a Stmt) where V: PreorderVisitor<'a> + ?Sized, { - match stmt { - Stmt::Expr(ast::StmtExpr { value, range: _ }) => visitor.visit_expr(value), + let node = AnyNodeRef::from(stmt); + + if visitor.enter_node(node).is_traverse() { + match stmt { + Stmt::Expr(ast::StmtExpr { value, range: _ }) => visitor.visit_expr(value), + + Stmt::FunctionDef(ast::StmtFunctionDef { + parameters, + body, + decorator_list, + returns, + type_params, + .. + }) => { + for decorator in decorator_list { + visitor.visit_decorator(decorator); + } - Stmt::FunctionDef(ast::StmtFunctionDef { - parameters, - body, - decorator_list, - returns, - type_params, - .. - }) => { - for decorator in decorator_list { - visitor.visit_decorator(decorator); - } + if let Some(type_params) = type_params { + visitor.visit_type_params(type_params); + } - if let Some(type_params) = type_params { - visitor.visit_type_params(type_params); - } + visitor.visit_parameters(parameters); - visitor.visit_parameters(parameters); + for expr in returns { + visitor.visit_annotation(expr); + } - for expr in returns { - visitor.visit_annotation(expr); + visitor.visit_body(body); } - visitor.visit_body(body); - } + Stmt::ClassDef(ast::StmtClassDef { + arguments, + body, + decorator_list, + type_params, + .. + }) => { + for decorator in decorator_list { + visitor.visit_decorator(decorator); + } - Stmt::ClassDef(ast::StmtClassDef { - arguments, - body, - decorator_list, - type_params, - .. - }) => { - for decorator in decorator_list { - visitor.visit_decorator(decorator); - } + if let Some(type_params) = type_params { + visitor.visit_type_params(type_params); + } - if let Some(type_params) = type_params { - visitor.visit_type_params(type_params); - } + if let Some(arguments) = arguments { + visitor.visit_arguments(arguments); + } - if let Some(arguments) = arguments { - visitor.visit_arguments(arguments); + visitor.visit_body(body); } - visitor.visit_body(body); - } + Stmt::Return(ast::StmtReturn { value, range: _ }) => { + if let Some(expr) = value { + visitor.visit_expr(expr); + } + } - Stmt::Return(ast::StmtReturn { value, range: _ }) => { - if let Some(expr) = value { - visitor.visit_expr(expr); + Stmt::Delete(ast::StmtDelete { targets, range: _ }) => { + for expr in targets { + visitor.visit_expr(expr); + } } - } - Stmt::Delete(ast::StmtDelete { targets, range: _ }) => { - for expr in targets { - visitor.visit_expr(expr); + Stmt::TypeAlias(ast::StmtTypeAlias { + range: _, + name, + type_params, + value, + }) => { + visitor.visit_expr(name); + if let Some(type_params) = type_params { + visitor.visit_type_params(type_params); + } + visitor.visit_expr(value); } - } - Stmt::TypeAlias(ast::StmtTypeAlias { - range: _, - name, - type_params, - value, - }) => { - visitor.visit_expr(name); - if let Some(type_params) = type_params { - visitor.visit_type_params(type_params); - } - visitor.visit_expr(value); - } + Stmt::Assign(ast::StmtAssign { + targets, + value, + range: _, + }) => { + for expr in targets { + visitor.visit_expr(expr); + } - Stmt::Assign(ast::StmtAssign { - targets, - value, - range: _, - }) => { - for expr in targets { - visitor.visit_expr(expr); + visitor.visit_expr(value); } - visitor.visit_expr(value); - } - - Stmt::AugAssign(ast::StmtAugAssign { - target, - op, - value, - range: _, - }) => { - visitor.visit_expr(target); - visitor.visit_operator(op); - visitor.visit_expr(value); - } + Stmt::AugAssign(ast::StmtAugAssign { + target, + op, + value, + range: _, + }) => { + visitor.visit_expr(target); + visitor.visit_operator(op); + visitor.visit_expr(value); + } - Stmt::AnnAssign(ast::StmtAnnAssign { - target, - annotation, - value, - range: _, - simple: _, - }) => { - visitor.visit_expr(target); - visitor.visit_annotation(annotation); - if let Some(expr) = value { - visitor.visit_expr(expr); + Stmt::AnnAssign(ast::StmtAnnAssign { + target, + annotation, + value, + range: _, + simple: _, + }) => { + visitor.visit_expr(target); + visitor.visit_annotation(annotation); + if let Some(expr) = value { + visitor.visit_expr(expr); + } } - } - Stmt::For(ast::StmtFor { - target, - iter, - body, - orelse, - .. - }) => { - visitor.visit_expr(target); - visitor.visit_expr(iter); - visitor.visit_body(body); - visitor.visit_body(orelse); - } + Stmt::For(ast::StmtFor { + target, + iter, + body, + orelse, + .. + }) => { + visitor.visit_expr(target); + visitor.visit_expr(iter); + visitor.visit_body(body); + visitor.visit_body(orelse); + } - Stmt::While(ast::StmtWhile { - test, - body, - orelse, - range: _, - }) => { - visitor.visit_expr(test); - visitor.visit_body(body); - visitor.visit_body(orelse); - } + Stmt::While(ast::StmtWhile { + test, + body, + orelse, + range: _, + }) => { + visitor.visit_expr(test); + visitor.visit_body(body); + visitor.visit_body(orelse); + } - Stmt::If(ast::StmtIf { - test, - body, - elif_else_clauses, - range: _, - }) => { - visitor.visit_expr(test); - visitor.visit_body(body); - for clause in elif_else_clauses { - visitor.visit_elif_else_clause(clause); + Stmt::If(ast::StmtIf { + test, + body, + elif_else_clauses, + range: _, + }) => { + visitor.visit_expr(test); + visitor.visit_body(body); + for clause in elif_else_clauses { + visitor.visit_elif_else_clause(clause); + } } - } - Stmt::With(ast::StmtWith { - items, - body, - is_async: _, - range: _, - }) => { - for with_item in items { - visitor.visit_with_item(with_item); + Stmt::With(ast::StmtWith { + items, + body, + is_async: _, + range: _, + }) => { + for with_item in items { + visitor.visit_with_item(with_item); + } + visitor.visit_body(body); } - visitor.visit_body(body); - } - Stmt::Match(ast::StmtMatch { - subject, - cases, - range: _, - }) => { - visitor.visit_expr(subject); - for match_case in cases { - visitor.visit_match_case(match_case); + Stmt::Match(ast::StmtMatch { + subject, + cases, + range: _, + }) => { + visitor.visit_expr(subject); + for match_case in cases { + visitor.visit_match_case(match_case); + } } - } - Stmt::Raise(ast::StmtRaise { - exc, - cause, - range: _, - }) => { - if let Some(expr) = exc { - visitor.visit_expr(expr); - }; - if let Some(expr) = cause { - visitor.visit_expr(expr); - }; - } + Stmt::Raise(ast::StmtRaise { + exc, + cause, + range: _, + }) => { + if let Some(expr) = exc { + visitor.visit_expr(expr); + }; + if let Some(expr) = cause { + visitor.visit_expr(expr); + }; + } - Stmt::Try(ast::StmtTry { - body, - handlers, - orelse, - finalbody, - range: _, - }) - | Stmt::TryStar(ast::StmtTryStar { - body, - handlers, - orelse, - finalbody, - range: _, - }) => { - visitor.visit_body(body); - for except_handler in handlers { - visitor.visit_except_handler(except_handler); - } - visitor.visit_body(orelse); - visitor.visit_body(finalbody); - } + Stmt::Try(ast::StmtTry { + body, + handlers, + orelse, + finalbody, + range: _, + }) + | Stmt::TryStar(ast::StmtTryStar { + body, + handlers, + orelse, + finalbody, + range: _, + }) => { + visitor.visit_body(body); + for except_handler in handlers { + visitor.visit_except_handler(except_handler); + } + visitor.visit_body(orelse); + visitor.visit_body(finalbody); + } - Stmt::Assert(ast::StmtAssert { - test, - msg, - range: _, - }) => { - visitor.visit_expr(test); - if let Some(expr) = msg { - visitor.visit_expr(expr); + Stmt::Assert(ast::StmtAssert { + test, + msg, + range: _, + }) => { + visitor.visit_expr(test); + if let Some(expr) = msg { + visitor.visit_expr(expr); + } } - } - Stmt::Import(ast::StmtImport { names, range: _ }) => { - for alias in names { - visitor.visit_alias(alias); + Stmt::Import(ast::StmtImport { names, range: _ }) => { + for alias in names { + visitor.visit_alias(alias); + } } - } - Stmt::ImportFrom(ast::StmtImportFrom { - range: _, - module: _, - names, - level: _, - }) => { - for alias in names { - visitor.visit_alias(alias); + Stmt::ImportFrom(ast::StmtImportFrom { + range: _, + module: _, + names, + level: _, + }) => { + for alias in names { + visitor.visit_alias(alias); + } } + + Stmt::Pass(_) + | Stmt::Break(_) + | Stmt::Continue(_) + | Stmt::Global(_) + | Stmt::Nonlocal(_) + | Stmt::LineMagic(_) => {} } + } + + visitor.leave_node(node); +} + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum TraversalSignal { + Traverse, + Skip, +} - Stmt::Pass(_) - | Stmt::Break(_) - | Stmt::Continue(_) - | Stmt::Global(_) - | Stmt::Nonlocal(_) - | Stmt::LineMagic(_) => {} +impl TraversalSignal { + const fn is_traverse(self) -> bool { + matches!(self, TraversalSignal::Traverse) } } pub fn walk_annotation<'a, V: PreorderVisitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) { - visitor.visit_expr(expr); + let node = AnyNodeRef::from(expr); + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(expr); + } + + visitor.leave_node(node); } pub fn walk_decorator<'a, V>(visitor: &mut V, decorator: &'a Decorator) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_expr(&decorator.expression); + let node = AnyNodeRef::from(decorator); + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(&decorator.expression); + } + + visitor.leave_node(node); } pub fn walk_expr<'a, V>(visitor: &mut V, expr: &'a Expr) where V: PreorderVisitor<'a> + ?Sized, { - match expr { - Expr::BoolOp(ast::ExprBoolOp { - op, - values, - range: _, - }) => match values.as_slice() { - [left, rest @ ..] => { - visitor.visit_expr(left); - visitor.visit_bool_op(op); - for expr in rest { - visitor.visit_expr(expr); + let node = AnyNodeRef::from(expr); + if visitor.enter_node(node).is_traverse() { + match expr { + Expr::BoolOp(ast::ExprBoolOp { + op, + values, + range: _, + }) => match values.as_slice() { + [left, rest @ ..] => { + visitor.visit_expr(left); + visitor.visit_bool_op(op); + for expr in rest { + visitor.visit_expr(expr); + } } + [] => { + visitor.visit_bool_op(op); + } + }, + + Expr::NamedExpr(ast::ExprNamedExpr { + target, + value, + range: _, + }) => { + visitor.visit_expr(target); + visitor.visit_expr(value); } - [] => { - visitor.visit_bool_op(op); - } - }, - - Expr::NamedExpr(ast::ExprNamedExpr { - target, - value, - range: _, - }) => { - visitor.visit_expr(target); - visitor.visit_expr(value); - } - Expr::BinOp(ast::ExprBinOp { - left, - op, - right, - range: _, - }) => { - visitor.visit_expr(left); - visitor.visit_operator(op); - visitor.visit_expr(right); - } + Expr::BinOp(ast::ExprBinOp { + left, + op, + right, + range: _, + }) => { + visitor.visit_expr(left); + visitor.visit_operator(op); + visitor.visit_expr(right); + } - Expr::UnaryOp(ast::ExprUnaryOp { - op, - operand, - range: _, - }) => { - visitor.visit_unary_op(op); - visitor.visit_expr(operand); - } + Expr::UnaryOp(ast::ExprUnaryOp { + op, + operand, + range: _, + }) => { + visitor.visit_unary_op(op); + visitor.visit_expr(operand); + } - Expr::Lambda(ast::ExprLambda { - parameters, - body, - range: _, - }) => { - visitor.visit_parameters(parameters); - visitor.visit_expr(body); - } + Expr::Lambda(ast::ExprLambda { + parameters, + body, + range: _, + }) => { + visitor.visit_parameters(parameters); + visitor.visit_expr(body); + } - Expr::IfExp(ast::ExprIfExp { - test, - body, - orelse, - range: _, - }) => { - // `body if test else orelse` - visitor.visit_expr(body); - visitor.visit_expr(test); - visitor.visit_expr(orelse); - } + Expr::IfExp(ast::ExprIfExp { + test, + body, + orelse, + range: _, + }) => { + // `body if test else orelse` + visitor.visit_expr(body); + visitor.visit_expr(test); + visitor.visit_expr(orelse); + } - Expr::Dict(ast::ExprDict { - keys, - values, - range: _, - }) => { - for (key, value) in keys.iter().zip(values) { - if let Some(key) = key { - visitor.visit_expr(key); + Expr::Dict(ast::ExprDict { + keys, + values, + range: _, + }) => { + for (key, value) in keys.iter().zip(values) { + if let Some(key) = key { + visitor.visit_expr(key); + } + visitor.visit_expr(value); } - visitor.visit_expr(value); } - } - Expr::Set(ast::ExprSet { elts, range: _ }) => { - for expr in elts { - visitor.visit_expr(expr); + Expr::Set(ast::ExprSet { elts, range: _ }) => { + for expr in elts { + visitor.visit_expr(expr); + } } - } - Expr::ListComp(ast::ExprListComp { - elt, - generators, - range: _, - }) => { - visitor.visit_expr(elt); - for comprehension in generators { - visitor.visit_comprehension(comprehension); + Expr::ListComp(ast::ExprListComp { + elt, + generators, + range: _, + }) => { + visitor.visit_expr(elt); + for comprehension in generators { + visitor.visit_comprehension(comprehension); + } } - } - Expr::SetComp(ast::ExprSetComp { - elt, - generators, - range: _, - }) => { - visitor.visit_expr(elt); - for comprehension in generators { - visitor.visit_comprehension(comprehension); + Expr::SetComp(ast::ExprSetComp { + elt, + generators, + range: _, + }) => { + visitor.visit_expr(elt); + for comprehension in generators { + visitor.visit_comprehension(comprehension); + } } - } - Expr::DictComp(ast::ExprDictComp { - key, - value, - generators, - range: _, - }) => { - visitor.visit_expr(key); - visitor.visit_expr(value); + Expr::DictComp(ast::ExprDictComp { + key, + value, + generators, + range: _, + }) => { + visitor.visit_expr(key); + visitor.visit_expr(value); - for comprehension in generators { - visitor.visit_comprehension(comprehension); + for comprehension in generators { + visitor.visit_comprehension(comprehension); + } } - } - Expr::GeneratorExp(ast::ExprGeneratorExp { - elt, - generators, - range: _, - }) => { - visitor.visit_expr(elt); - for comprehension in generators { - visitor.visit_comprehension(comprehension); + Expr::GeneratorExp(ast::ExprGeneratorExp { + elt, + generators, + range: _, + }) => { + visitor.visit_expr(elt); + for comprehension in generators { + visitor.visit_comprehension(comprehension); + } } - } - Expr::Await(ast::ExprAwait { value, range: _ }) - | Expr::YieldFrom(ast::ExprYieldFrom { value, range: _ }) => visitor.visit_expr(value), + Expr::Await(ast::ExprAwait { value, range: _ }) + | Expr::YieldFrom(ast::ExprYieldFrom { value, range: _ }) => visitor.visit_expr(value), - Expr::Yield(ast::ExprYield { value, range: _ }) => { - if let Some(expr) = value { - visitor.visit_expr(expr); + Expr::Yield(ast::ExprYield { value, range: _ }) => { + if let Some(expr) = value { + visitor.visit_expr(expr); + } } - } - Expr::Compare(ast::ExprCompare { - left, - ops, - comparators, - range: _, - }) => { - visitor.visit_expr(left); + Expr::Compare(ast::ExprCompare { + left, + ops, + comparators, + range: _, + }) => { + visitor.visit_expr(left); - for (op, comparator) in ops.iter().zip(comparators) { - visitor.visit_cmp_op(op); - visitor.visit_expr(comparator); + for (op, comparator) in ops.iter().zip(comparators) { + visitor.visit_cmp_op(op); + visitor.visit_expr(comparator); + } } - } - Expr::Call(ast::ExprCall { - func, - arguments, - range: _, - }) => { - visitor.visit_expr(func); - visitor.visit_arguments(arguments); - } + Expr::Call(ast::ExprCall { + func, + arguments, + range: _, + }) => { + visitor.visit_expr(func); + visitor.visit_arguments(arguments); + } - Expr::FormattedValue(ast::ExprFormattedValue { - value, format_spec, .. - }) => { - visitor.visit_expr(value); + Expr::FormattedValue(ast::ExprFormattedValue { + value, format_spec, .. + }) => { + visitor.visit_expr(value); - if let Some(expr) = format_spec { - visitor.visit_format_spec(expr); + if let Some(expr) = format_spec { + visitor.visit_format_spec(expr); + } } - } - Expr::FString(ast::ExprFString { values, range: _ }) => { - for expr in values { - visitor.visit_expr(expr); + Expr::FString(ast::ExprFString { values, range: _ }) => { + for expr in values { + visitor.visit_expr(expr); + } } - } - Expr::Constant(ast::ExprConstant { - value, - range: _, - kind: _, - }) => visitor.visit_constant(value), - - Expr::Attribute(ast::ExprAttribute { - value, - attr: _, - ctx: _, - range: _, - }) => { - visitor.visit_expr(value); - } - - Expr::Subscript(ast::ExprSubscript { - value, - slice, - ctx: _, - range: _, - }) => { - visitor.visit_expr(value); - visitor.visit_expr(slice); - } - Expr::Starred(ast::ExprStarred { - value, - ctx: _, - range: _, - }) => { - visitor.visit_expr(value); - } - - Expr::Name(ast::ExprName { - id: _, - ctx: _, - range: _, - }) => {} + Expr::Constant(ast::ExprConstant { + value, + range: _, + kind: _, + }) => visitor.visit_constant(value), + + Expr::Attribute(ast::ExprAttribute { + value, + attr: _, + ctx: _, + range: _, + }) => { + visitor.visit_expr(value); + } - Expr::List(ast::ExprList { - elts, - ctx: _, - range: _, - }) => { - for expr in elts { - visitor.visit_expr(expr); + Expr::Subscript(ast::ExprSubscript { + value, + slice, + ctx: _, + range: _, + }) => { + visitor.visit_expr(value); + visitor.visit_expr(slice); } - } - Expr::Tuple(ast::ExprTuple { - elts, - ctx: _, - range: _, - }) => { - for expr in elts { - visitor.visit_expr(expr); + Expr::Starred(ast::ExprStarred { + value, + ctx: _, + range: _, + }) => { + visitor.visit_expr(value); } - } - Expr::Slice(ast::ExprSlice { - lower, - upper, - step, - range: _, - }) => { - if let Some(expr) = lower { - visitor.visit_expr(expr); + Expr::Name(ast::ExprName { + id: _, + ctx: _, + range: _, + }) => {} + + Expr::List(ast::ExprList { + elts, + ctx: _, + range: _, + }) => { + for expr in elts { + visitor.visit_expr(expr); + } } - if let Some(expr) = upper { - visitor.visit_expr(expr); + Expr::Tuple(ast::ExprTuple { + elts, + ctx: _, + range: _, + }) => { + for expr in elts { + visitor.visit_expr(expr); + } } - if let Some(expr) = step { - visitor.visit_expr(expr); + + Expr::Slice(ast::ExprSlice { + lower, + upper, + step, + range: _, + }) => { + if let Some(expr) = lower { + visitor.visit_expr(expr); + } + if let Some(expr) = upper { + visitor.visit_expr(expr); + } + if let Some(expr) = step { + visitor.visit_expr(expr); + } } + Expr::LineMagic(_) => (), } - Expr::LineMagic(_) => (), } + + visitor.leave_node(node); } pub fn walk_comprehension<'a, V>(visitor: &mut V, comprehension: &'a Comprehension) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_expr(&comprehension.target); - visitor.visit_expr(&comprehension.iter); + let node = AnyNodeRef::from(comprehension); + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(&comprehension.target); + visitor.visit_expr(&comprehension.iter); - for expr in &comprehension.ifs { - visitor.visit_expr(expr); + for expr in &comprehension.ifs { + visitor.visit_expr(expr); + } } + + visitor.leave_node(node); } pub fn walk_elif_else_clause<'a, V>(visitor: &mut V, elif_else_clause: &'a ElifElseClause) where V: PreorderVisitor<'a> + ?Sized, { - if let Some(test) = &elif_else_clause.test { - visitor.visit_expr(test); + let node = AnyNodeRef::from(elif_else_clause); + if visitor.enter_node(node).is_traverse() { + if let Some(test) = &elif_else_clause.test { + visitor.visit_expr(test); + } + visitor.visit_body(&elif_else_clause.body); } - visitor.visit_body(&elif_else_clause.body); + + visitor.leave_node(node); } pub fn walk_except_handler<'a, V>(visitor: &mut V, except_handler: &'a ExceptHandler) where V: PreorderVisitor<'a> + ?Sized, { - match except_handler { - ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { - range: _, - type_, - name: _, - body, - }) => { - if let Some(expr) = type_ { - visitor.visit_expr(expr); - } - visitor.visit_body(body); + let node = AnyNodeRef::from(except_handler); + if visitor.enter_node(node).is_traverse() { + match except_handler { + ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + range: _, + type_, + name: _, + body, + }) => { + if let Some(expr) = type_ { + visitor.visit_expr(expr); + } + visitor.visit_body(body); + } } } + visitor.leave_node(node); } pub fn walk_format_spec<'a, V: PreorderVisitor<'a> + ?Sized>( visitor: &mut V, format_spec: &'a Expr, ) { - visitor.visit_expr(format_spec); + let node = AnyNodeRef::from(format_spec); + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(format_spec); + } + + visitor.leave_node(node); } pub fn walk_arguments<'a, V>(visitor: &mut V, arguments: &'a Arguments) where V: PreorderVisitor<'a> + ?Sized, { - for arg in &arguments.args { - visitor.visit_expr(arg); - } + let node = AnyNodeRef::from(arguments); + if visitor.enter_node(node).is_traverse() { + for arg in &arguments.args { + visitor.visit_expr(arg); + } - for keyword in &arguments.keywords { - visitor.visit_keyword(keyword); + for keyword in &arguments.keywords { + visitor.visit_keyword(keyword); + } } + + visitor.leave_node(node); } pub fn walk_parameters<'a, V>(visitor: &mut V, parameters: &'a Parameters) where V: PreorderVisitor<'a> + ?Sized, { - for arg in parameters.posonlyargs.iter().chain(¶meters.args) { - visitor.visit_parameter_with_default(arg); - } + let node = AnyNodeRef::from(parameters); + if visitor.enter_node(node).is_traverse() { + for arg in parameters.posonlyargs.iter().chain(¶meters.args) { + visitor.visit_parameter_with_default(arg); + } - if let Some(arg) = ¶meters.vararg { - visitor.visit_parameter(arg); - } + if let Some(arg) = ¶meters.vararg { + visitor.visit_parameter(arg); + } - for arg in ¶meters.kwonlyargs { - visitor.visit_parameter_with_default(arg); - } + for arg in ¶meters.kwonlyargs { + visitor.visit_parameter_with_default(arg); + } - if let Some(arg) = ¶meters.kwarg { - visitor.visit_parameter(arg); + if let Some(arg) = ¶meters.kwarg { + visitor.visit_parameter(arg); + } } + + visitor.leave_node(node); } pub fn walk_parameter<'a, V>(visitor: &mut V, parameter: &'a Parameter) where V: PreorderVisitor<'a> + ?Sized, { - if let Some(expr) = ¶meter.annotation { - visitor.visit_annotation(expr); + let node = AnyNodeRef::from(parameter); + + if visitor.enter_node(node).is_traverse() { + if let Some(expr) = ¶meter.annotation { + visitor.visit_annotation(expr); + } } + visitor.leave_node(node); } pub fn walk_parameter_with_default<'a, V>( @@ -750,10 +857,15 @@ pub fn walk_parameter_with_default<'a, V>( ) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_parameter(¶meter_with_default.parameter); - if let Some(expr) = ¶meter_with_default.default { - visitor.visit_expr(expr); + let node = AnyNodeRef::from(parameter_with_default); + if visitor.enter_node(node).is_traverse() { + visitor.visit_parameter(¶meter_with_default.parameter); + if let Some(expr) = ¶meter_with_default.default { + visitor.visit_expr(expr); + } } + + visitor.leave_node(node); } #[inline] @@ -761,124 +873,149 @@ pub fn walk_keyword<'a, V>(visitor: &mut V, keyword: &'a Keyword) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_expr(&keyword.value); + let node = AnyNodeRef::from(keyword); + + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(&keyword.value); + } + visitor.leave_node(node); } pub fn walk_with_item<'a, V>(visitor: &mut V, with_item: &'a WithItem) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_expr(&with_item.context_expr); + let node = AnyNodeRef::from(with_item); + if visitor.enter_node(node).is_traverse() { + visitor.visit_expr(&with_item.context_expr); - if let Some(expr) = &with_item.optional_vars { - visitor.visit_expr(expr); + if let Some(expr) = &with_item.optional_vars { + visitor.visit_expr(expr); + } } + visitor.leave_node(node); } pub fn walk_type_params<'a, V>(visitor: &mut V, type_params: &'a TypeParams) where V: PreorderVisitor<'a> + ?Sized, { - for type_param in &type_params.type_params { - visitor.visit_type_param(type_param); + let node = AnyNodeRef::from(type_params); + if visitor.enter_node(node).is_traverse() { + for type_param in &type_params.type_params { + visitor.visit_type_param(type_param); + } } + visitor.leave_node(node); } pub fn walk_type_param<'a, V>(visitor: &mut V, type_param: &'a TypeParam) where V: PreorderVisitor<'a> + ?Sized, { - match type_param { - TypeParam::TypeVar(TypeParamTypeVar { - bound, - name: _, - range: _, - }) => { - if let Some(expr) = bound { - visitor.visit_expr(expr); + let node = AnyNodeRef::from(type_param); + if visitor.enter_node(node).is_traverse() { + match type_param { + TypeParam::TypeVar(TypeParamTypeVar { + bound, + name: _, + range: _, + }) => { + if let Some(expr) = bound { + visitor.visit_expr(expr); + } } + TypeParam::TypeVarTuple(_) | TypeParam::ParamSpec(_) => {} } - TypeParam::TypeVarTuple(_) | TypeParam::ParamSpec(_) => {} } + visitor.leave_node(node); } pub fn walk_match_case<'a, V>(visitor: &mut V, match_case: &'a MatchCase) where V: PreorderVisitor<'a> + ?Sized, { - visitor.visit_pattern(&match_case.pattern); - if let Some(expr) = &match_case.guard { - visitor.visit_expr(expr); + let node = AnyNodeRef::from(match_case); + if visitor.enter_node(node).is_traverse() { + visitor.visit_pattern(&match_case.pattern); + if let Some(expr) = &match_case.guard { + visitor.visit_expr(expr); + } + visitor.visit_body(&match_case.body); } - visitor.visit_body(&match_case.body); + visitor.leave_node(node); } pub fn walk_pattern<'a, V>(visitor: &mut V, pattern: &'a Pattern) where V: PreorderVisitor<'a> + ?Sized, { - match pattern { - Pattern::MatchValue(ast::PatternMatchValue { value, range: _ }) => { - visitor.visit_expr(value); - } - - Pattern::MatchSingleton(ast::PatternMatchSingleton { value, range: _ }) => { - visitor.visit_constant(value); - } + let node = AnyNodeRef::from(pattern); + if visitor.enter_node(node).is_traverse() { + match pattern { + Pattern::MatchValue(ast::PatternMatchValue { value, range: _ }) => { + visitor.visit_expr(value); + } - Pattern::MatchSequence(ast::PatternMatchSequence { patterns, range: _ }) => { - for pattern in patterns { - visitor.visit_pattern(pattern); + Pattern::MatchSingleton(ast::PatternMatchSingleton { value, range: _ }) => { + visitor.visit_constant(value); } - } - Pattern::MatchMapping(ast::PatternMatchMapping { - keys, - patterns, - range: _, - rest: _, - }) => { - for (key, pattern) in keys.iter().zip(patterns) { - visitor.visit_expr(key); - visitor.visit_pattern(pattern); + Pattern::MatchSequence(ast::PatternMatchSequence { patterns, range: _ }) => { + for pattern in patterns { + visitor.visit_pattern(pattern); + } } - } - Pattern::MatchClass(ast::PatternMatchClass { - cls, - patterns, - kwd_attrs: _, - kwd_patterns, - range: _, - }) => { - visitor.visit_expr(cls); - for pattern in patterns { - visitor.visit_pattern(pattern); + Pattern::MatchMapping(ast::PatternMatchMapping { + keys, + patterns, + range: _, + rest: _, + }) => { + for (key, pattern) in keys.iter().zip(patterns) { + visitor.visit_expr(key); + visitor.visit_pattern(pattern); + } } - for pattern in kwd_patterns { - visitor.visit_pattern(pattern); + Pattern::MatchClass(ast::PatternMatchClass { + cls, + patterns, + kwd_attrs: _, + kwd_patterns, + range: _, + }) => { + visitor.visit_expr(cls); + for pattern in patterns { + visitor.visit_pattern(pattern); + } + + for pattern in kwd_patterns { + visitor.visit_pattern(pattern); + } } - } - Pattern::MatchStar(_) => {} + Pattern::MatchStar(_) => {} - Pattern::MatchAs(ast::PatternMatchAs { - pattern, - range: _, - name: _, - }) => { - if let Some(pattern) = pattern { - visitor.visit_pattern(pattern); + Pattern::MatchAs(ast::PatternMatchAs { + pattern, + range: _, + name: _, + }) => { + if let Some(pattern) = pattern { + visitor.visit_pattern(pattern); + } } - } - Pattern::MatchOr(ast::PatternMatchOr { patterns, range: _ }) => { - for pattern in patterns { - visitor.visit_pattern(pattern); + Pattern::MatchOr(ast::PatternMatchOr { patterns, range: _ }) => { + for pattern in patterns { + visitor.visit_pattern(pattern); + } } } } + visitor.leave_node(node); } pub fn walk_bool_op<'a, V>(_visitor: &mut V, _bool_op: &'a BoolOp) @@ -909,8 +1046,11 @@ where } #[inline] -pub fn walk_alias<'a, V>(_visitor: &mut V, _alias: &'a Alias) +pub fn walk_alias<'a, V>(visitor: &mut V, alias: &'a Alias) where V: PreorderVisitor<'a> + ?Sized, { + let node = AnyNodeRef::from(alias); + visitor.enter_node(node); + visitor.leave_node(node); } diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs index 0b18ea76d44e9..a70b62d0fd2e7 100644 --- a/crates/ruff_python_formatter/src/comments/visitor.rs +++ b/crates/ruff_python_formatter/src/comments/visitor.rs @@ -1,10 +1,6 @@ use std::iter::Peekable; -use ruff_python_ast::{ - Alias, Arguments, Comprehension, Decorator, ElifElseClause, ExceptHandler, Expr, Keyword, - MatchCase, Mod, Parameter, ParameterWithDefault, Parameters, Pattern, Ranged, Stmt, TypeParam, - TypeParams, WithItem, -}; +use ruff_python_ast::{Mod, Ranged, Stmt}; use ruff_text_size::{TextRange, TextSize}; use ruff_formatter::{SourceCode, SourceCodeSlice}; @@ -48,14 +44,22 @@ impl<'a> CommentsVisitor<'a> { self.finish() } - fn start_node(&mut self, node: N) -> TraversalSignal - where - N: Into>, - { - self.start_node_impl(node.into()) + // Try to skip the subtree if + // * there are no comments + // * if the next comment comes after this node (meaning, this nodes subtree contains no comments) + fn can_skip(&mut self, node_end: TextSize) -> bool { + self.comment_ranges + .peek() + .map_or(true, |next_comment| next_comment.start() >= node_end) + } + + fn finish(self) -> CommentsMap<'a> { + self.builder.finish() } +} - fn start_node_impl(&mut self, node: AnyNodeRef<'a>) -> TraversalSignal { +impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> { + fn enter_node(&mut self, node: AnyNodeRef<'ast>) -> TraversalSignal { let node_range = node.range(); let enclosing_node = self.parents.last().copied().unwrap_or(node); @@ -95,23 +99,7 @@ impl<'a> CommentsVisitor<'a> { } } - // Try to skip the subtree if - // * there are no comments - // * if the next comment comes after this node (meaning, this nodes subtree contains no comments) - fn can_skip(&mut self, node_end: TextSize) -> bool { - self.comment_ranges - .peek() - .map_or(true, |next_comment| next_comment.start() >= node_end) - } - - fn finish_node(&mut self, node: N) - where - N: Into>, - { - self.finish_node_impl(node.into()); - } - - fn finish_node_impl(&mut self, node: AnyNodeRef<'a>) { + fn leave_node(&mut self, node: AnyNodeRef<'ast>) { // We are leaving this node, pop it from the parent stack. self.parents.pop(); @@ -146,19 +134,6 @@ impl<'a> CommentsVisitor<'a> { self.preceding_node = Some(node); } - fn finish(self) -> CommentsMap<'a> { - self.builder.finish() - } -} - -impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> { - fn visit_mod(&mut self, module: &'ast Mod) { - if self.start_node(module).is_traverse() { - walk_module(self, module); - } - self.finish_node(module); - } - fn visit_body(&mut self, body: &'ast [Stmt]) { match body { [] => { @@ -178,140 +153,6 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> { } } } - - fn visit_stmt(&mut self, stmt: &'ast Stmt) { - if self.start_node(stmt).is_traverse() { - walk_stmt(self, stmt); - } - self.finish_node(stmt); - } - - fn visit_annotation(&mut self, expr: &'ast Expr) { - if self.start_node(expr).is_traverse() { - walk_expr(self, expr); - } - self.finish_node(expr); - } - - fn visit_decorator(&mut self, decorator: &'ast Decorator) { - if self.start_node(decorator).is_traverse() { - walk_decorator(self, decorator); - } - self.finish_node(decorator); - } - - fn visit_expr(&mut self, expr: &'ast Expr) { - if self.start_node(expr).is_traverse() { - walk_expr(self, expr); - } - self.finish_node(expr); - } - - fn visit_comprehension(&mut self, comprehension: &'ast Comprehension) { - if self.start_node(comprehension).is_traverse() { - walk_comprehension(self, comprehension); - } - self.finish_node(comprehension); - } - - fn visit_except_handler(&mut self, except_handler: &'ast ExceptHandler) { - if self.start_node(except_handler).is_traverse() { - walk_except_handler(self, except_handler); - } - self.finish_node(except_handler); - } - - fn visit_format_spec(&mut self, format_spec: &'ast Expr) { - if self.start_node(format_spec).is_traverse() { - walk_expr(self, format_spec); - } - self.finish_node(format_spec); - } - - fn visit_arguments(&mut self, arguments: &'ast Arguments) { - if self.start_node(arguments).is_traverse() { - walk_arguments(self, arguments); - } - self.finish_node(arguments); - } - - fn visit_parameters(&mut self, parameters: &'ast Parameters) { - if self.start_node(parameters).is_traverse() { - walk_parameters(self, parameters); - } - self.finish_node(parameters); - } - - fn visit_parameter(&mut self, arg: &'ast Parameter) { - if self.start_node(arg).is_traverse() { - walk_parameter(self, arg); - } - self.finish_node(arg); - } - - fn visit_parameter_with_default(&mut self, parameter_with_default: &'ast ParameterWithDefault) { - if self.start_node(parameter_with_default).is_traverse() { - walk_parameter_with_default(self, parameter_with_default); - } - self.finish_node(parameter_with_default); - } - - fn visit_keyword(&mut self, keyword: &'ast Keyword) { - if self.start_node(keyword).is_traverse() { - walk_keyword(self, keyword); - } - self.finish_node(keyword); - } - - fn visit_alias(&mut self, alias: &'ast Alias) { - if self.start_node(alias).is_traverse() { - walk_alias(self, alias); - } - self.finish_node(alias); - } - - fn visit_with_item(&mut self, with_item: &'ast WithItem) { - if self.start_node(with_item).is_traverse() { - walk_with_item(self, with_item); - } - - self.finish_node(with_item); - } - - fn visit_match_case(&mut self, match_case: &'ast MatchCase) { - if self.start_node(match_case).is_traverse() { - walk_match_case(self, match_case); - } - self.finish_node(match_case); - } - - fn visit_pattern(&mut self, pattern: &'ast Pattern) { - if self.start_node(pattern).is_traverse() { - walk_pattern(self, pattern); - } - self.finish_node(pattern); - } - - fn visit_elif_else_clause(&mut self, elif_else_clause: &'ast ElifElseClause) { - if self.start_node(elif_else_clause).is_traverse() { - walk_elif_else_clause(self, elif_else_clause); - } - self.finish_node(elif_else_clause); - } - - fn visit_type_params(&mut self, type_params: &'ast TypeParams) { - if self.start_node(type_params).is_traverse() { - walk_type_params(self, type_params); - } - self.finish_node(type_params); - } - - fn visit_type_param(&mut self, type_param: &'ast TypeParam) { - if self.start_node(type_param).is_traverse() { - walk_type_param(self, type_param); - } - self.finish_node(type_param); - } } fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentLinePosition { @@ -663,18 +504,6 @@ impl<'a> CommentPlacement<'a> { } } -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -enum TraversalSignal { - Traverse, - Skip, -} - -impl TraversalSignal { - const fn is_traverse(self) -> bool { - matches!(self, TraversalSignal::Traverse) - } -} - #[derive(Clone, Debug, Default)] struct CommentsBuilder<'a> { comments: CommentsMap<'a>,