diff --git a/crates/ruff/resources/test/fixtures/pandas_vet/PD002.py b/crates/ruff/resources/test/fixtures/pandas_vet/PD002.py index 99dc33a327..4d1fc96b59 100644 --- a/crates/ruff/resources/test/fixtures/pandas_vet/PD002.py +++ b/crates/ruff/resources/test/fixtures/pandas_vet/PD002.py @@ -4,7 +4,9 @@ x = pd.DataFrame() x.drop(["a"], axis=1, inplace=True) -x.drop(["a"], axis=1, inplace=True) +x.y.drop(["a"], axis=1, inplace=True) + +x["y"].drop(["a"], axis=1, inplace=True) x.drop( inplace=True, @@ -23,6 +25,7 @@ x.drop(["a"], axis=1, **kwargs, inplace=True) x.drop(["a"], axis=1, inplace=True, **kwargs) f(x.drop(["a"], axis=1, inplace=True)) -x.apply(lambda x: x.sort_values('a', inplace=True)) +x.apply(lambda x: x.sort_values("a", inplace=True)) import torch -torch.m.ReLU(inplace=True) # safe because this isn't a pandas call + +torch.m.ReLU(inplace=True) # safe because this isn't a pandas call diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 8d85f2968c..fa49c24332 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -668,21 +668,17 @@ where } if !self.is_stub { if self.enabled(Rule::DjangoModelWithoutDunderStr) { - if let Some(diagnostic) = - flake8_django::rules::model_without_dunder_str(self, bases, body, stmt) - { - self.diagnostics.push(diagnostic); - } + flake8_django::rules::model_without_dunder_str(self, class_def); } } if self.enabled(Rule::GlobalStatement) { pylint::rules::global_statement(self, name); } if self.enabled(Rule::UselessObjectInheritance) { - pyupgrade::rules::useless_object_inheritance(self, class_def, stmt); + pyupgrade::rules::useless_object_inheritance(self, class_def); } if self.enabled(Rule::UnnecessaryClassParentheses) { - pyupgrade::rules::unnecessary_class_parentheses(self, class_def, stmt); + pyupgrade::rules::unnecessary_class_parentheses(self, class_def); } if self.enabled(Rule::AmbiguousClassName) { if let Some(diagnostic) = @@ -2756,17 +2752,12 @@ where flake8_debugger::rules::debugger_call(self, expr, func); } if self.enabled(Rule::PandasUseOfInplaceArgument) { - self.diagnostics.extend( - pandas_vet::rules::inplace_argument(self, expr, func, args, keywords) - .into_iter(), - ); + pandas_vet::rules::inplace_argument(self, expr, func, args, keywords); } pandas_vet::rules::call(self, func); if self.enabled(Rule::PandasUseOfPdMerge) { - if let Some(diagnostic) = pandas_vet::rules::use_of_pd_merge(func) { - self.diagnostics.push(diagnostic); - }; + pandas_vet::rules::use_of_pd_merge(self, func); } if self.enabled(Rule::CallDatetimeWithoutTzinfo) { flake8_datetimez::rules::call_datetime_without_tzinfo( diff --git a/crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs b/crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs index 31d0ea65a9..6c30cb52c1 100644 --- a/crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs +++ b/crates/ruff/src/rules/flake8_django/rules/model_without_dunder_str.rs @@ -52,21 +52,20 @@ impl Violation for DjangoModelWithoutDunderStr { /// DJ008 pub(crate) fn model_without_dunder_str( - checker: &Checker, - bases: &[Expr], - body: &[Stmt], - class_location: &Stmt, -) -> Option { + checker: &mut Checker, + ast::StmtClassDef { + name, bases, body, .. + }: &ast::StmtClassDef, +) { if !is_non_abstract_model(bases, body, checker.semantic()) { - return None; + return; } - if !has_dunder_method(body) { - return Some(Diagnostic::new( - DjangoModelWithoutDunderStr, - class_location.range(), - )); + if has_dunder_method(body) { + return; } - None + checker + .diagnostics + .push(Diagnostic::new(DjangoModelWithoutDunderStr, name.range())); } fn has_dunder_method(body: &[Stmt]) -> bool { diff --git a/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap index 1af01e2856..2aae3d948b 100644 --- a/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap +++ b/crates/ruff/src/rules/flake8_django/snapshots/ruff__rules__flake8_django__tests__DJ008_DJ008.py.snap @@ -1,58 +1,26 @@ --- source: crates/ruff/src/rules/flake8_django/mod.rs --- -DJ008.py:6:1: DJ008 Model does not define `__str__` method +DJ008.py:6:7: DJ008 Model does not define `__str__` method + | +5 | # Models without __str__ +6 | class TestModel1(models.Model): + | ^^^^^^^^^^ DJ008 +7 | new_field = models.CharField(max_length=10) + | + +DJ008.py:21:7: DJ008 Model does not define `__str__` method | - 5 | # Models without __str__ - 6 | / class TestModel1(models.Model): - 7 | | new_field = models.CharField(max_length=10) - 8 | | - 9 | | class Meta: -10 | | verbose_name = "test model" -11 | | verbose_name_plural = "test models" -12 | | -13 | | @property -14 | | def my_brand_new_property(self): -15 | | return 1 -16 | | -17 | | def my_beautiful_method(self): -18 | | return 2 - | |________________^ DJ008 +21 | class TestModel2(Model): + | ^^^^^^^^^^ DJ008 +22 | new_field = models.CharField(max_length=10) | -DJ008.py:21:1: DJ008 Model does not define `__str__` method +DJ008.py:36:7: DJ008 Model does not define `__str__` method | -21 | / class TestModel2(Model): -22 | | new_field = models.CharField(max_length=10) -23 | | -24 | | class Meta: -25 | | verbose_name = "test model" -26 | | verbose_name_plural = "test models" -27 | | -28 | | @property -29 | | def my_brand_new_property(self): -30 | | return 1 -31 | | -32 | | def my_beautiful_method(self): -33 | | return 2 - | |________________^ DJ008 - | - -DJ008.py:36:1: DJ008 Model does not define `__str__` method - | -36 | / class TestModel3(Model): -37 | | new_field = models.CharField(max_length=10) -38 | | -39 | | class Meta: -40 | | abstract = False -41 | | -42 | | @property -43 | | def my_brand_new_property(self): -44 | | return 1 -45 | | -46 | | def my_beautiful_method(self): -47 | | return 2 - | |________________^ DJ008 +36 | class TestModel3(Model): + | ^^^^^^^^^^ DJ008 +37 | new_field = models.CharField(max_length=10) | diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs index 7cf8ff64f9..dc840d42f5 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs @@ -1,9 +1,8 @@ use std::fmt; -use anyhow::Result; -use ruff_text_size::{TextLen, TextRange, TextSize}; +use ruff_text_size::{TextLen, TextRange}; use rustpython_parser::ast::Decorator; -use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Expr, Keyword, Ranged, Stmt}; +use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Expr, Ranged, Stmt}; use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; @@ -11,7 +10,6 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; use ruff_python_ast::helpers::collect_arg_names; use ruff_python_ast::identifier::Identifier; -use ruff_python_ast::source_code::Locator; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; use ruff_python_semantic::analyze::visibility::is_abstract; @@ -25,21 +23,6 @@ use super::helpers::{ get_mark_decorators, is_pytest_fixture, is_pytest_yield_fixture, keyword_is_literal, }; -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum Parentheses { - None, - Empty, -} - -impl fmt::Display for Parentheses { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self { - Parentheses::None => fmt.write_str(""), - Parentheses::Empty => fmt.write_str("()"), - } - } -} - #[violation] pub struct PytestFixtureIncorrectParenthesesStyle { expected: Parentheses, @@ -196,8 +179,23 @@ impl AlwaysAutofixableViolation for PytestUnnecessaryAsyncioMarkOnFixture { } } -#[derive(Default)] +#[derive(Debug, PartialEq, Eq)] +enum Parentheses { + None, + Empty, +} + +impl fmt::Display for Parentheses { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + Parentheses::None => fmt.write_str(""), + Parentheses::Empty => fmt.write_str("()"), + } + } +} + /// Visitor that skips functions +#[derive(Debug, Default)] struct SkipFunctionsVisitor<'a> { has_return_with_value: bool, has_yield_from: bool, @@ -245,7 +243,7 @@ where } } -fn get_fixture_decorator<'a>( +fn fixture_decorator<'a>( decorators: &'a [Decorator], semantic: &SemanticModel, ) -> Option<&'a Decorator> { @@ -271,16 +269,6 @@ fn pytest_fixture_parentheses( checker.diagnostics.push(diagnostic); } -pub(crate) fn fix_extraneous_scope_function( - locator: &Locator, - stmt_at: TextSize, - expr_range: TextRange, - args: &[Expr], - keywords: &[Keyword], -) -> Result { - remove_argument(locator, stmt_at, expr_range, args, keywords, false) -} - /// PT001, PT002, PT003 fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &Decorator) { match &decorator.expression { @@ -290,28 +278,31 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D keywords, range: _, }) => { - if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) - && !checker.settings.flake8_pytest_style.fixture_parentheses - && args.is_empty() - && keywords.is_empty() - { - let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end())); - pytest_fixture_parentheses( - checker, - decorator, - fix, - Parentheses::None, - Parentheses::Empty, - ); + if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) { + if !checker.settings.flake8_pytest_style.fixture_parentheses + && args.is_empty() + && keywords.is_empty() + { + let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end())); + pytest_fixture_parentheses( + checker, + decorator, + fix, + Parentheses::None, + Parentheses::Empty, + ); + } } - if checker.enabled(Rule::PytestFixturePositionalArgs) && !args.is_empty() { - checker.diagnostics.push(Diagnostic::new( - PytestFixturePositionalArgs { - function: func_name.to_string(), - }, - decorator.range(), - )); + if checker.enabled(Rule::PytestFixturePositionalArgs) { + if !args.is_empty() { + checker.diagnostics.push(Diagnostic::new( + PytestFixturePositionalArgs { + function: func_name.to_string(), + }, + decorator.range(), + )); + } } if checker.enabled(Rule::PytestExtraneousScopeFunction) { @@ -324,16 +315,16 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D let mut diagnostic = Diagnostic::new(PytestExtraneousScopeFunction, scope_keyword.range()); if checker.patch(diagnostic.kind.rule()) { - let expr_range = diagnostic.range(); - #[allow(deprecated)] - diagnostic.try_set_fix_from_edit(|| { - fix_extraneous_scope_function( + diagnostic.try_set_fix(|| { + remove_argument( checker.locator, - decorator.start(), - expr_range, + func.end(), + scope_keyword.range, args, keywords, + false, ) + .map(Fix::suggested) }); } checker.diagnostics.push(diagnostic); @@ -342,20 +333,20 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D } } _ => { - if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) - && checker.settings.flake8_pytest_style.fixture_parentheses - { - let fix = Fix::automatic(Edit::insertion( - Parentheses::Empty.to_string(), - decorator.end(), - )); - pytest_fixture_parentheses( - checker, - decorator, - fix, - Parentheses::Empty, - Parentheses::None, - ); + if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) { + if checker.settings.flake8_pytest_style.fixture_parentheses { + let fix = Fix::automatic(Edit::insertion( + Parentheses::Empty.to_string(), + decorator.end(), + )); + pytest_fixture_parentheses( + checker, + decorator, + fix, + Parentheses::Empty, + Parentheses::None, + ); + } } } } @@ -511,7 +502,7 @@ pub(crate) fn fixture( decorators: &[Decorator], body: &[Stmt], ) { - let decorator = get_fixture_decorator(decorators, checker.semantic()); + let decorator = fixture_decorator(decorators, checker.semantic()); if let Some(decorator) = decorator { if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) || checker.enabled(Rule::PytestFixturePositionalArgs) diff --git a/crates/ruff/src/rules/pandas_vet/fixes.rs b/crates/ruff/src/rules/pandas_vet/fixes.rs deleted file mode 100644 index 8a3d368f07..0000000000 --- a/crates/ruff/src/rules/pandas_vet/fixes.rs +++ /dev/null @@ -1,44 +0,0 @@ -use ruff_text_size::TextRange; -use rustpython_parser::ast::{self, Expr, Keyword, Ranged}; - -use ruff_diagnostics::{Edit, Fix}; -use ruff_python_ast::source_code::Locator; - -use crate::autofix::edits::remove_argument; - -fn match_name(expr: &Expr) -> Option<&str> { - if let Expr::Call(ast::ExprCall { func, .. }) = expr { - if let Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() { - if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() { - return Some(id); - } - } - } - None -} - -/// Remove the `inplace` argument from a function call and replace it with an -/// assignment. -pub(super) fn convert_inplace_argument_to_assignment( - locator: &Locator, - expr: &Expr, - violation_range: TextRange, - args: &[Expr], - keywords: &[Keyword], -) -> Option { - // Add the assignment. - let name = match_name(expr)?; - let insert_assignment = Edit::insertion(format!("{name} = "), expr.start()); - - // Remove the `inplace` argument. - let remove_argument = remove_argument( - locator, - expr.start(), - violation_range, - args, - keywords, - false, - ) - .ok()?; - Some(Fix::suggested_edits(insert_assignment, [remove_argument])) -} diff --git a/crates/ruff/src/rules/pandas_vet/mod.rs b/crates/ruff/src/rules/pandas_vet/mod.rs index d3ac303cbc..2032749fe2 100644 --- a/crates/ruff/src/rules/pandas_vet/mod.rs +++ b/crates/ruff/src/rules/pandas_vet/mod.rs @@ -1,5 +1,4 @@ //! Rules from [pandas-vet](https://pypi.org/project/pandas-vet/). -pub(crate) mod fixes; pub(crate) mod helpers; pub(crate) mod rules; 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 cf5983213d..98a44a58d6 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs @@ -1,12 +1,15 @@ -use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged}; +use ruff_text_size::TextRange; +use rustpython_parser::ast::{Expr, Keyword, Ranged}; -use ruff_diagnostics::{AutofixKind, Diagnostic, Violation}; +use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::is_const_true; +use ruff_python_ast::source_code::Locator; use ruff_python_semantic::{BindingKind, Import}; +use crate::autofix::edits::remove_argument; use crate::checkers::ast::Checker; use crate::registry::AsRule; -use crate::rules::pandas_vet::fixes::convert_inplace_argument_to_assignment; /// ## What it does /// Checks for `inplace=True` usages in `pandas` function and method @@ -50,23 +53,17 @@ impl Violation for PandasUseOfInplaceArgument { /// PD002 pub(crate) fn inplace_argument( - checker: &Checker, + checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr], keywords: &[Keyword], -) -> Option { - let mut seen_star = false; - let mut is_checkable = false; - let mut is_pandas = false; - +) { + // If the function was imported from another module, and it's _not_ Pandas, abort. if let Some(call_path) = checker.semantic().resolve_call_path(func) { - is_checkable = true; - - let module = call_path[0]; - is_pandas = checker - .semantic() - .find_binding(module) + if !call_path + .first() + .and_then(|module| checker.semantic().find_binding(module)) .map_or(false, |binding| { matches!( binding.kind, @@ -74,23 +71,20 @@ pub(crate) fn inplace_argument( qualified_name: "pandas" }) ) - }); + }) + { + return; + } } + let mut seen_star = false; for keyword in keywords.iter().rev() { let Some(arg) = &keyword.arg else { seen_star = true; continue; }; if arg == "inplace" { - let is_true_literal = match &keyword.value { - Expr::Constant(ast::ExprConstant { - value: Constant::Bool(boolean), - .. - }) => *boolean, - _ => false, - }; - if is_true_literal { + if is_const_true(&keyword.value) { let mut diagnostic = Diagnostic::new(PandasUseOfInplaceArgument, keyword.range()); if checker.patch(diagnostic.kind.rule()) { // Avoid applying the fix if: @@ -110,7 +104,7 @@ pub(crate) fn inplace_argument( if let Some(fix) = convert_inplace_argument_to_assignment( checker.locator, expr, - diagnostic.range(), + keyword.range(), args, keywords, ) { @@ -119,18 +113,35 @@ pub(crate) fn inplace_argument( } } - // Without a static type system, only module-level functions could potentially be - // non-pandas calls. If they're not, `inplace` should be considered safe. - if is_checkable && !is_pandas { - return None; - } - - return Some(diagnostic); + checker.diagnostics.push(diagnostic); } // Duplicate keywords is a syntax error, so we can stop here. break; } } - None +} + +/// Remove the `inplace` argument from a function call and replace it with an +/// assignment. +fn convert_inplace_argument_to_assignment( + locator: &Locator, + expr: &Expr, + expr_range: TextRange, + args: &[Expr], + keywords: &[Keyword], +) -> Option { + // Add the assignment. + let call = expr.as_call_expr()?; + let attr = call.func.as_attribute_expr()?; + let insert_assignment = Edit::insertion( + format!("{name} = ", name = locator.slice(attr.value.range())), + expr.start(), + ); + + // Remove the `inplace` argument. + let remove_argument = + remove_argument(locator, call.func.end(), expr_range, args, keywords, false).ok()?; + + Some(Fix::suggested_edits(insert_assignment, [remove_argument])) } diff --git a/crates/ruff/src/rules/pandas_vet/rules/pd_merge.rs b/crates/ruff/src/rules/pandas_vet/rules/pd_merge.rs index b6c4120d7d..873d5c7f68 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/pd_merge.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/pd_merge.rs @@ -1,5 +1,6 @@ use rustpython_parser::ast::{self, Expr, Ranged}; +use crate::checkers::ast::Checker; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -17,13 +18,14 @@ impl Violation for PandasUseOfPdMerge { } /// PD015 -pub(crate) fn use_of_pd_merge(func: &Expr) -> Option { +pub(crate) fn use_of_pd_merge(checker: &mut Checker, func: &Expr) { if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func { if let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() { if id == "pd" && attr == "merge" { - return Some(Diagnostic::new(PandasUseOfPdMerge, func.range())); + checker + .diagnostics + .push(Diagnostic::new(PandasUseOfPdMerge, func.range())); } } } - None } diff --git a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD002_PD002.py.snap b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD002_PD002.py.snap index b837655f46..513c426e64 100644 --- a/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD002_PD002.py.snap +++ b/crates/ruff/src/rules/pandas_vet/snapshots/ruff__rules__pandas_vet__tests__PD002_PD002.py.snap @@ -8,7 +8,7 @@ PD002.py:5:23: PD002 [*] `inplace=True` should be avoided; it has inconsistent b 5 | x.drop(["a"], axis=1, inplace=True) | ^^^^^^^^^^^^ PD002 6 | -7 | x.drop(["a"], axis=1, inplace=True) +7 | x.y.drop(["a"], axis=1, inplace=True) | = help: Assign to variable; remove `inplace` arg @@ -19,17 +19,17 @@ PD002.py:5:23: PD002 [*] `inplace=True` should be avoided; it has inconsistent b 5 |-x.drop(["a"], axis=1, inplace=True) 5 |+x = x.drop(["a"], axis=1) 6 6 | -7 7 | x.drop(["a"], axis=1, inplace=True) +7 7 | x.y.drop(["a"], axis=1, inplace=True) 8 8 | -PD002.py:7:23: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:7:25: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior | 5 | x.drop(["a"], axis=1, inplace=True) 6 | -7 | x.drop(["a"], axis=1, inplace=True) - | ^^^^^^^^^^^^ PD002 +7 | x.y.drop(["a"], axis=1, inplace=True) + | ^^^^^^^^^^^^ PD002 8 | -9 | x.drop( +9 | x["y"].drop(["a"], axis=1, inplace=True) | = help: Assign to variable; remove `inplace` arg @@ -37,104 +37,124 @@ PD002.py:7:23: PD002 [*] `inplace=True` should be avoided; it has inconsistent b 4 4 | 5 5 | x.drop(["a"], axis=1, inplace=True) 6 6 | -7 |-x.drop(["a"], axis=1, inplace=True) - 7 |+x = x.drop(["a"], axis=1) +7 |-x.y.drop(["a"], axis=1, inplace=True) + 7 |+x.y = x.y.drop(["a"], axis=1) 8 8 | -9 9 | x.drop( -10 10 | inplace=True, +9 9 | x["y"].drop(["a"], axis=1, inplace=True) +10 10 | -PD002.py:10:5: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:9:28: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior | - 9 | x.drop( -10 | inplace=True, - | ^^^^^^^^^^^^ PD002 -11 | columns=["a"], -12 | axis=1, + 7 | x.y.drop(["a"], axis=1, inplace=True) + 8 | + 9 | x["y"].drop(["a"], axis=1, inplace=True) + | ^^^^^^^^^^^^ PD002 +10 | +11 | x.drop( | = help: Assign to variable; remove `inplace` arg ℹ Suggested fix 6 6 | -7 7 | x.drop(["a"], axis=1, inplace=True) +7 7 | x.y.drop(["a"], axis=1, inplace=True) 8 8 | -9 |-x.drop( -10 |- inplace=True, - 9 |+x = x.drop( -11 10 | columns=["a"], -12 11 | axis=1, -13 12 | ) +9 |-x["y"].drop(["a"], axis=1, inplace=True) + 9 |+x["y"] = x["y"].drop(["a"], axis=1) +10 10 | +11 11 | x.drop( +12 12 | inplace=True, -PD002.py:17:9: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:12:5: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior | -15 | if True: -16 | x.drop( -17 | inplace=True, +11 | x.drop( +12 | inplace=True, + | ^^^^^^^^^^^^ PD002 +13 | columns=["a"], +14 | axis=1, + | + = help: Assign to variable; remove `inplace` arg + +ℹ Suggested fix +8 8 | +9 9 | x["y"].drop(["a"], axis=1, inplace=True) +10 10 | +11 |-x.drop( +12 |- inplace=True, + 11 |+x = x.drop( +13 12 | columns=["a"], +14 13 | axis=1, +15 14 | ) + +PD002.py:19:9: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior + | +17 | if True: +18 | x.drop( +19 | inplace=True, | ^^^^^^^^^^^^ PD002 -18 | columns=["a"], -19 | axis=1, +20 | columns=["a"], +21 | axis=1, | = help: Assign to variable; remove `inplace` arg ℹ Suggested fix -13 13 | ) -14 14 | -15 15 | if True: -16 |- x.drop( -17 |- inplace=True, - 16 |+ x = x.drop( -18 17 | columns=["a"], -19 18 | axis=1, -20 19 | ) +15 15 | ) +16 16 | +17 17 | if True: +18 |- x.drop( +19 |- inplace=True, + 18 |+ x = x.drop( +20 19 | columns=["a"], +21 20 | axis=1, +22 21 | ) -PD002.py:22:33: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:24:33: PD002 [*] `inplace=True` should be avoided; it has inconsistent behavior | -20 | ) -21 | -22 | x.drop(["a"], axis=1, **kwargs, inplace=True) +22 | ) +23 | +24 | x.drop(["a"], axis=1, **kwargs, inplace=True) | ^^^^^^^^^^^^ PD002 -23 | x.drop(["a"], axis=1, inplace=True, **kwargs) -24 | f(x.drop(["a"], axis=1, inplace=True)) +25 | x.drop(["a"], axis=1, inplace=True, **kwargs) +26 | f(x.drop(["a"], axis=1, inplace=True)) | = help: Assign to variable; remove `inplace` arg ℹ Suggested fix -19 19 | axis=1, -20 20 | ) -21 21 | -22 |-x.drop(["a"], axis=1, **kwargs, inplace=True) - 22 |+x = x.drop(["a"], axis=1, **kwargs) -23 23 | x.drop(["a"], axis=1, inplace=True, **kwargs) -24 24 | f(x.drop(["a"], axis=1, inplace=True)) -25 25 | +21 21 | axis=1, +22 22 | ) +23 23 | +24 |-x.drop(["a"], axis=1, **kwargs, inplace=True) + 24 |+x = x.drop(["a"], axis=1, **kwargs) +25 25 | x.drop(["a"], axis=1, inplace=True, **kwargs) +26 26 | f(x.drop(["a"], axis=1, inplace=True)) +27 27 | -PD002.py:23:23: PD002 `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:25:23: PD002 `inplace=True` should be avoided; it has inconsistent behavior | -22 | x.drop(["a"], axis=1, **kwargs, inplace=True) -23 | x.drop(["a"], axis=1, inplace=True, **kwargs) +24 | x.drop(["a"], axis=1, **kwargs, inplace=True) +25 | x.drop(["a"], axis=1, inplace=True, **kwargs) | ^^^^^^^^^^^^ PD002 -24 | f(x.drop(["a"], axis=1, inplace=True)) +26 | f(x.drop(["a"], axis=1, inplace=True)) | = help: Assign to variable; remove `inplace` arg -PD002.py:24:25: PD002 `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:26:25: PD002 `inplace=True` should be avoided; it has inconsistent behavior | -22 | x.drop(["a"], axis=1, **kwargs, inplace=True) -23 | x.drop(["a"], axis=1, inplace=True, **kwargs) -24 | f(x.drop(["a"], axis=1, inplace=True)) +24 | x.drop(["a"], axis=1, **kwargs, inplace=True) +25 | x.drop(["a"], axis=1, inplace=True, **kwargs) +26 | f(x.drop(["a"], axis=1, inplace=True)) | ^^^^^^^^^^^^ PD002 -25 | -26 | x.apply(lambda x: x.sort_values('a', inplace=True)) +27 | +28 | x.apply(lambda x: x.sort_values("a", inplace=True)) | = help: Assign to variable; remove `inplace` arg -PD002.py:26:38: PD002 `inplace=True` should be avoided; it has inconsistent behavior +PD002.py:28:38: PD002 `inplace=True` should be avoided; it has inconsistent behavior | -24 | f(x.drop(["a"], axis=1, inplace=True)) -25 | -26 | x.apply(lambda x: x.sort_values('a', inplace=True)) +26 | f(x.drop(["a"], axis=1, inplace=True)) +27 | +28 | x.apply(lambda x: x.sort_values("a", inplace=True)) | ^^^^^^^^^^^^ PD002 -27 | import torch -28 | torch.m.ReLU(inplace=True) # safe because this isn't a pandas call +29 | import torch | = help: Assign to variable; remove `inplace` arg diff --git a/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs b/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs index 01592e3c29..a94eed542e 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs @@ -69,7 +69,7 @@ fn generate_fix( Edit::range_replacement("capture_output=True".to_string(), first.range()), [remove_argument( locator, - func.start(), + func.end(), second.range(), args, keywords, diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_class_parentheses.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_class_parentheses.rs index 5fcb60fa8c..69a27b27b4 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_class_parentheses.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_class_parentheses.rs @@ -1,11 +1,10 @@ use std::ops::Add; use ruff_text_size::{TextRange, TextSize}; -use rustpython_parser::ast::{self, Stmt}; +use rustpython_parser::ast::{self, Ranged}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -44,16 +43,12 @@ impl AlwaysAutofixableViolation for UnnecessaryClassParentheses { } /// UP039 -pub(crate) fn unnecessary_class_parentheses( - checker: &mut Checker, - class_def: &ast::StmtClassDef, - stmt: &Stmt, -) { +pub(crate) fn unnecessary_class_parentheses(checker: &mut Checker, class_def: &ast::StmtClassDef) { if !class_def.bases.is_empty() || !class_def.keywords.is_empty() { return; } - let offset = stmt.identifier().start(); + let offset = class_def.name.end(); let contents = checker.locator.after(offset); // Find the open and closing parentheses between the class name and the colon, if they exist. diff --git a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs index aa6ec0e31b..b3ac4c2899 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs @@ -1,8 +1,7 @@ -use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; +use rustpython_parser::ast::{self, Expr, Ranged}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; use crate::autofix::edits::remove_argument; use crate::checkers::ast::Checker; @@ -47,11 +46,7 @@ impl AlwaysAutofixableViolation for UselessObjectInheritance { } /// UP004 -pub(crate) fn useless_object_inheritance( - checker: &mut Checker, - class_def: &ast::StmtClassDef, - stmt: &Stmt, -) { +pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast::StmtClassDef) { for expr in &class_def.bases { let Expr::Name(ast::ExprName { id, .. }) = expr else { continue; @@ -73,7 +68,7 @@ pub(crate) fn useless_object_inheritance( diagnostic.try_set_fix(|| { let edit = remove_argument( checker.locator, - stmt.identifier().start(), + class_def.name.end(), expr.range(), &class_def.bases, &class_def.keywords,