From 7b91a162c6c7286438c9932deee70638d6f41b88 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 9 May 2023 15:40:12 -0400 Subject: [PATCH] Remove `current_` prefix from some Context methods (#4325) --- crates/ruff/src/checkers/ast/mod.rs | 34 ++++++++----------- .../rules/hardcoded_sql_expression.rs | 2 +- .../rules/setattr_with_constant.rs | 2 +- .../ruff/src/rules/flake8_datetimez/rules.rs | 2 +- .../flake8_pytest_style/rules/assertion.rs | 4 +-- .../flake8_simplify/rules/ast_unary_op.rs | 4 +-- .../rules/open_file_with_context_handler.rs | 6 ++-- .../src/rules/pandas_vet/rules/check_attr.rs | 2 +- .../pandas_vet/rules/inplace_argument.rs | 4 +-- .../pyupgrade/rules/outdated_version_block.rs | 4 +-- .../rules/unnecessary_builtin_import.rs | 4 +-- .../rules/unnecessary_future_import.rs | 4 +-- .../pyupgrade/rules/useless_metaclass_type.rs | 4 +-- crates/ruff_python_semantic/src/context.rs | 15 ++++---- 14 files changed, 45 insertions(+), 46 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 075cada62f..827763d28a 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1571,15 +1571,11 @@ where test, body, orelse, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self.settings.rules.enabled(Rule::IfWithSameArms) { - flake8_simplify::rules::if_with_same_arms( - self, - stmt, - self.ctx.current_stmt_parent(), - ); + flake8_simplify::rules::if_with_same_arms(self, stmt, self.ctx.stmt_parent()); } if self.settings.rules.enabled(Rule::NeedlessBool) { flake8_simplify::rules::needless_bool(self, stmt); @@ -1595,14 +1591,14 @@ where test, body, orelse, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self.settings.rules.enabled(Rule::IfElseBlockInsteadOfIfExp) { flake8_simplify::rules::use_ternary_operator( self, stmt, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self @@ -1616,7 +1612,7 @@ where test, body, orelse, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self.settings.rules.enabled(Rule::TypeCheckWithoutTypeError) { @@ -1625,7 +1621,7 @@ where body, test, orelse, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self.settings.rules.enabled(Rule::OutdatedVersionBlock) { @@ -1683,7 +1679,7 @@ where self, stmt, body, - self.ctx.current_stmt_parent(), + self.ctx.stmt_parent(), ); } if self.settings.rules.enabled(Rule::RedefinedLoopName) { @@ -1741,7 +1737,7 @@ where flake8_simplify::rules::convert_for_loop_to_any_all( self, stmt, - self.ctx.current_sibling_stmt(), + self.ctx.sibling_stmt(), ); } if self.settings.rules.enabled(Rule::InDictKeys) { @@ -2750,7 +2746,7 @@ where flake8_comprehensions::rules::unnecessary_generator_set( self, expr, - self.ctx.current_expr_parent(), + self.ctx.expr_parent(), func, args, keywords, @@ -2760,7 +2756,7 @@ where flake8_comprehensions::rules::unnecessary_generator_dict( self, expr, - self.ctx.current_expr_parent(), + self.ctx.expr_parent(), func, args, keywords, @@ -2865,7 +2861,7 @@ where flake8_comprehensions::rules::unnecessary_map( self, expr, - self.ctx.current_expr_parent(), + self.ctx.expr_parent(), func, args, ); @@ -3384,7 +3380,7 @@ where if self.is_stub { if self.settings.rules.enabled(Rule::DuplicateUnionMember) && self.ctx.in_type_definition - && self.ctx.current_expr_parent().map_or(true, |parent| { + && self.ctx.expr_parent().map_or(true, |parent| { !matches!( parent.node, ExprKind::BinOp { @@ -4536,7 +4532,7 @@ impl<'a> Checker<'a> { } fn handle_node_store(&mut self, id: &'a str, expr: &Expr) { - let parent = self.ctx.current_stmt(); + let parent = self.ctx.stmt(); if self.settings.rules.enabled(Rule::UndefinedLocal) { pyflakes::rules::undefined_local(self, id); @@ -4878,7 +4874,7 @@ impl<'a> Checker<'a> { self.ctx.stmt_id = stmt_id; self.ctx.visible_scope = visibility; - match &self.ctx.current_stmt().node { + match &self.ctx.stmt().node { StmtKind::FunctionDef { body, args, .. } | StmtKind::AsyncFunctionDef { body, args, .. } => { self.visit_arguments(args); @@ -4958,7 +4954,7 @@ impl<'a> Checker<'a> { self.ctx.stmt_id = stmt_id; if let StmtKind::For { target, body, .. } - | StmtKind::AsyncFor { target, body, .. } = &self.ctx.current_stmt().node + | StmtKind::AsyncFor { target, body, .. } = &self.ctx.stmt().node { if self.settings.rules.enabled(Rule::UnusedLoopControlVariable) { flake8_bugbear::rules::unused_loop_control_variable(self, target, body); diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs index 72216ff9d4..ee10134b0a 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs @@ -60,7 +60,7 @@ fn unparse_string_format_expression(checker: &mut Checker, expr: &Expr) -> Optio op: Operator::Add | Operator::Mod, .. } => { - let Some(parent) = checker.ctx.current_expr_parent() else { + let Some(parent) = checker.ctx.expr_parent() else { if any_over_expr(expr, &has_string_literal) { return Some(unparse_expr(expr, checker.stylist)); } diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs index edf5345b3b..63c9cf5b47 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/setattr_with_constant.rs @@ -74,7 +74,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar // We can only replace a `setattr` call (which is an `Expr`) with an assignment // (which is a `Stmt`) if the `Expr` is already being used as a `Stmt` // (i.e., it's directly within an `StmtKind::Expr`). - if let StmtKind::Expr { value: child } = &checker.ctx.current_stmt().node { + if let StmtKind::Expr { value: child } = &checker.ctx.stmt().node { if expr == child.as_ref() { let mut diagnostic = Diagnostic::new(SetAttrWithConstant, expr.range()); diff --git a/crates/ruff/src/rules/flake8_datetimez/rules.rs b/crates/ruff/src/rules/flake8_datetimez/rules.rs index 7cdca81647..ec38cd7a84 100644 --- a/crates/ruff/src/rules/flake8_datetimez/rules.rs +++ b/crates/ruff/src/rules/flake8_datetimez/rules.rs @@ -327,7 +327,7 @@ pub fn call_datetime_strptime_without_zone( } }; - let (Some(grandparent), Some(parent)) = (checker.ctx.current_expr_grandparent(), checker.ctx.current_expr_parent()) else { + let (Some(grandparent), Some(parent)) = (checker.ctx.expr_grandparent(), checker.ctx.expr_parent()) else { checker.diagnostics.push(Diagnostic::new( CallDatetimeStrptimeWithoutZone, location, diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs index 5aae6a40df..bf4986cbfe 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs @@ -191,8 +191,8 @@ pub fn unittest_assertion( if let Ok(unittest_assert) = UnittestAssert::try_from(attr.as_str()) { // We're converting an expression to a statement, so avoid applying the fix if // the assertion is part of a larger expression. - let fixable = matches!(checker.ctx.current_stmt().node, StmtKind::Expr { .. }) - && checker.ctx.current_expr_parent().is_none() + let fixable = matches!(checker.ctx.stmt().node, StmtKind::Expr { .. }) + && checker.ctx.expr_parent().is_none() && !checker.ctx.scope().kind.is_lambda() && !has_comments_in(expr.range(), checker.locator); let mut diagnostic = Diagnostic::new( diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs index 2af4d4862d..7bdf1e2b29 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs @@ -88,7 +88,7 @@ pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop, if !matches!(&ops[..], [Cmpop::Eq]) { return; } - if is_exception_check(checker.ctx.current_stmt()) { + if is_exception_check(checker.ctx.stmt()) { return; } @@ -138,7 +138,7 @@ pub fn negation_with_not_equal_op( if !matches!(&ops[..], [Cmpop::NotEq]) { return; } - if is_exception_check(checker.ctx.current_stmt()) { + if is_exception_check(checker.ctx.stmt()) { return; } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs b/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs index 2e435dc6dc..33466ac876 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs @@ -18,7 +18,7 @@ impl Violation for OpenFileWithContextHandler { /// Return `true` if the current expression is nested in an `await /// exit_stack.enter_async_context` call. fn match_async_exit_stack(checker: &Checker) -> bool { - let Some(expr) = checker.ctx.current_expr_grandparent() else { + let Some(expr) = checker.ctx.expr_grandparent() else { return false; }; let ExprKind::Await { value } = &expr.node else { @@ -56,7 +56,7 @@ fn match_async_exit_stack(checker: &Checker) -> bool { /// Return `true` if the current expression is nested in an /// `exit_stack.enter_context` call. fn match_exit_stack(checker: &Checker) -> bool { - let Some(expr) = checker.ctx.current_expr_parent() else { + let Some(expr) = checker.ctx.expr_parent() else { return false; }; let ExprKind::Call { func, .. } = &expr.node else { @@ -97,7 +97,7 @@ pub fn open_file_with_context_handler(checker: &mut Checker, func: &Expr) { { if checker.ctx.is_builtin("open") { // Ex) `with open("foo.txt") as f: ...` - if matches!(checker.ctx.current_stmt().node, StmtKind::With { .. }) { + if matches!(checker.ctx.stmt().node, StmtKind::With { .. }) { return; } diff --git a/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs b/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs index 0f4f11e47b..22636ea6f6 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/check_attr.rs @@ -60,7 +60,7 @@ pub fn check_attr(checker: &mut Checker, attr: &str, value: &Expr, attr_expr: &E }; // Avoid flagging on function calls (e.g., `df.values()`). - if let Some(parent) = checker.ctx.current_expr_parent() { + if let Some(parent) = checker.ctx.expr_parent() { if matches!(parent.node, ExprKind::Call { .. }) { return; } diff --git a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs index d40d9299aa..fdcd70d880 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs @@ -82,8 +82,8 @@ pub fn inplace_argument( // but we don't currently restore expression stacks when parsing deferred nodes, // and so the parent is lost. let fixable = !seen_star - && matches!(checker.ctx.current_stmt().node, StmtKind::Expr { .. }) - && checker.ctx.current_expr_parent().is_none() + && matches!(checker.ctx.stmt().node, StmtKind::Expr { .. }) + && checker.ctx.expr_parent().is_none() && !checker.ctx.scope().kind.is_lambda(); let mut diagnostic = Diagnostic::new(PandasUseOfInplaceArgument { fixable }, keyword.range()); diff --git a/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs b/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs index 2368b61fcc..29e2d346a8 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/outdated_version_block.rs @@ -161,8 +161,8 @@ fn fix_py2_block( // of its parent, so avoid passing in the parent at all. Otherwise, // `delete_stmt` will erroneously include a `pass`. let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); - let defined_by = checker.ctx.current_stmt(); - let defined_in = checker.ctx.current_stmt_parent(); + let defined_by = checker.ctx.stmt(); + let defined_in = checker.ctx.stmt_parent(); return match delete_stmt( defined_by, if block.starter == Tok::If { diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs index a0c1cc0ceb..af1498b6ab 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs @@ -106,8 +106,8 @@ pub fn unnecessary_builtin_import( if checker.patch(diagnostic.kind.rule()) { let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); - let defined_by = checker.ctx.current_stmt(); - let defined_in = checker.ctx.current_stmt_parent(); + let defined_by = checker.ctx.stmt(); + let defined_in = checker.ctx.stmt_parent(); let unused_imports: Vec = unused_imports .iter() .map(|alias| format!("{module}.{}", alias.node.name)) diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs index ea27a55b03..67991d30fe 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs @@ -86,8 +86,8 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo if checker.patch(diagnostic.kind.rule()) { let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); - let defined_by = checker.ctx.current_stmt(); - let defined_in = checker.ctx.current_stmt_parent(); + let defined_by = checker.ctx.stmt(); + let defined_in = checker.ctx.stmt_parent(); let unused_imports: Vec = unused_imports .iter() .map(|alias| format!("__future__.{}", alias.node.name)) diff --git a/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs b/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs index b0a3104e62..be871babb6 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs @@ -51,8 +51,8 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, }; if checker.patch(diagnostic.kind.rule()) { let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); - let defined_by = checker.ctx.current_stmt(); - let defined_in = checker.ctx.current_stmt_parent(); + let defined_by = checker.ctx.stmt(); + let defined_in = checker.ctx.stmt_parent(); match actions::delete_stmt( defined_by, defined_in, diff --git a/crates/ruff_python_semantic/src/context.rs b/crates/ruff_python_semantic/src/context.rs index 29bc3a4fe5..26d9d0a61f 100644 --- a/crates/ruff_python_semantic/src/context.rs +++ b/crates/ruff_python_semantic/src/context.rs @@ -25,6 +25,8 @@ pub struct Context<'a> { // Stack of all visited statements, along with the identifier of the current statement. pub stmts: Nodes<'a>, pub stmt_id: Option, + // Stack of current expressions. + pub exprs: Vec<&'a Expr>, // Stack of all scopes, along with the identifier of the current scope. pub scopes: Scopes<'a>, pub scope_id: ScopeId, @@ -34,7 +36,6 @@ pub struct Context<'a> { // Map from binding index to indexes of bindings that shadow it in other scopes. pub shadowed_bindings: std::collections::HashMap, BuildNoHashHasher>, - pub exprs: Vec<&'a Expr>, // Body iteration; used to peek at siblings. pub body: &'a [Stmt], pub body_index: usize, @@ -311,10 +312,12 @@ impl<'a> Context<'a> { self.stmt_id = self.stmts.parent_id(node_id); } + /// Push an [`Expr`] onto the stack. pub fn push_expr(&mut self, expr: &'a Expr) { self.exprs.push(expr); } + /// Pop the current [`Expr`] off the stack. pub fn pop_expr(&mut self) { self.exprs .pop() @@ -336,25 +339,25 @@ impl<'a> Context<'a> { } /// Return the current `Stmt`. - pub fn current_stmt(&self) -> &'a Stmt { + pub fn stmt(&self) -> &'a Stmt { let node_id = self.stmt_id.expect("No current statement"); self.stmts[node_id] } /// Return the parent `Stmt` of the current `Stmt`, if any. - pub fn current_stmt_parent(&self) -> Option<&'a Stmt> { + pub fn stmt_parent(&self) -> Option<&'a Stmt> { let node_id = self.stmt_id.expect("No current statement"); let parent_id = self.stmts.parent_id(node_id)?; Some(self.stmts[parent_id]) } /// Return the parent `Expr` of the current `Expr`. - pub fn current_expr_parent(&self) -> Option<&'a Expr> { + pub fn expr_parent(&self) -> Option<&'a Expr> { self.exprs.iter().rev().nth(1).copied() } /// Return the grandparent `Expr` of the current `Expr`. - pub fn current_expr_grandparent(&self) -> Option<&'a Expr> { + pub fn expr_grandparent(&self) -> Option<&'a Expr> { self.exprs.iter().rev().nth(2).copied() } @@ -364,7 +367,7 @@ impl<'a> Context<'a> { } /// Return the `Stmt` that immediately follows the current `Stmt`, if any. - pub fn current_sibling_stmt(&self) -> Option<&'a Stmt> { + pub fn sibling_stmt(&self) -> Option<&'a Stmt> { self.body.get(self.body_index + 1) }