From 8c40886f8720a46e4deadcf9140e4650f1a538d6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 2 Aug 2023 12:38:43 -0400 Subject: [PATCH] Use `Arguments` node to power `remove_argument` method (#6278) ## Summary Internal refactor to take advantage of the new `Arguments` node, to power our `remove_argument` fix action. ## Test Plan `cargo test` --- crates/ruff/src/autofix/edits.rs | 166 +++++++++--------- .../src/checkers/ast/analyze/expression.rs | 6 +- .../flake8_pytest_style/rules/fixture.rs | 38 ++-- .../pandas_vet/rules/inplace_argument.rs | 39 ++-- .../pyupgrade/rules/replace_stdout_stderr.rs | 38 ++-- .../rules/unnecessary_encode_utf8.rs | 87 ++++----- .../rules/useless_object_inheritance.rs | 22 +-- 7 files changed, 165 insertions(+), 231 deletions(-) diff --git a/crates/ruff/src/autofix/edits.rs b/crates/ruff/src/autofix/edits.rs index db3eaf477e..19e2daa3f2 100644 --- a/crates/ruff/src/autofix/edits.rs +++ b/crates/ruff/src/autofix/edits.rs @@ -1,14 +1,15 @@ //! Interface for generating autofix edits from higher-level actions (e.g., "remove an argument"). + use anyhow::{bail, Result}; -use ruff_python_ast::{self as ast, ExceptHandler, Expr, Keyword, Ranged, Stmt}; -use ruff_python_parser::{lexer, Mode}; -use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_diagnostics::Edit; +use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, Keyword, Ranged, Stmt}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; +use ruff_python_parser::{lexer, Mode}; use ruff_python_trivia::{has_leading_content, is_python_whitespace, PythonWhitespace}; use ruff_source_file::{Locator, NewlineWithTrailingNewline}; +use ruff_text_size::{TextLen, TextRange, TextSize}; use crate::autofix::codemods; @@ -68,108 +69,101 @@ pub(crate) fn remove_unused_imports<'a>( } } +#[derive(Debug, Copy, Clone)] +pub(crate) enum Parentheses { + /// Remove parentheses, if the removed argument is the only argument left. + Remove, + /// Preserve parentheses, even if the removed argument is the only argument + Preserve, +} + /// Generic function to remove arguments or keyword arguments in function /// calls and class definitions. (For classes `args` should be considered /// `bases`) /// /// Supports the removal of parentheses when this is the only (kw)arg left. /// For this behavior, set `remove_parentheses` to `true`. -/// -/// TODO(charlie): Migrate this signature to take [`Arguments`] rather than -/// separate args and keywords. -pub(crate) fn remove_argument( +pub(crate) fn remove_argument( + argument: &T, + arguments: &Arguments, + parentheses: Parentheses, locator: &Locator, - call_at: TextSize, - expr_range: TextRange, - args: &[Expr], - keywords: &[Keyword], - remove_parentheses: bool, ) -> Result { // TODO(sbrugman): Preserve trailing comments. - let contents = locator.after(call_at); + if arguments.keywords.len() + arguments.args.len() > 1 { + let mut fix_start = None; + let mut fix_end = None; - let mut fix_start = None; - let mut fix_end = None; - - let n_arguments = keywords.len() + args.len(); - if n_arguments == 0 { - bail!("No arguments or keywords to remove"); - } - - if n_arguments == 1 { - // Case 1: there is only one argument. - let mut count = 0u32; - for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, call_at).flatten() { - if tok.is_lpar() { - if count == 0 { - fix_start = Some(if remove_parentheses { - range.start() - } else { - range.start() + TextSize::from(1) - }); - } - count = count.saturating_add(1); - } - - if tok.is_rpar() { - count = count.saturating_sub(1); - if count == 0 { - fix_end = Some(if remove_parentheses { + if arguments + .args + .iter() + .map(Expr::start) + .chain(arguments.keywords.iter().map(Keyword::start)) + .any(|location| location > argument.start()) + { + // Case 1: argument or keyword is _not_ the last node, so delete from the start of the + // argument to the end of the subsequent comma. + let mut seen_comma = false; + for (tok, range) in lexer::lex_starts_at( + locator.slice(arguments.range()), + Mode::Module, + arguments.start(), + ) + .flatten() + { + if seen_comma { + if tok.is_non_logical_newline() { + // Also delete any non-logical newlines after the comma. + continue; + } + fix_end = Some(if tok.is_newline() { range.end() } else { - range.end() - TextSize::from(1) + range.start() }); break; } + if range.start() == argument.start() { + fix_start = Some(range.start()); + } + if fix_start.is_some() && tok.is_comma() { + seen_comma = true; + } + } + } else { + // Case 2: argument or keyword is the last node, so delete from the start of the + // previous comma to the end of the argument. + for (tok, range) in lexer::lex_starts_at( + locator.slice(arguments.range()), + Mode::Module, + arguments.start(), + ) + .flatten() + { + if range.start() == argument.start() { + fix_end = Some(argument.end()); + break; + } + if tok.is_comma() { + fix_start = Some(range.start()); + } } } - } else if args - .iter() - .map(Expr::start) - .chain(keywords.iter().map(Keyword::start)) - .any(|location| location > expr_range.start()) - { - // Case 2: argument or keyword is _not_ the last node. - let mut seen_comma = false; - for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, call_at).flatten() { - if seen_comma { - if tok.is_non_logical_newline() { - // Also delete any non-logical newlines after the comma. - continue; - } - fix_end = Some(if tok.is_newline() { - range.end() - } else { - range.start() - }); - break; - } - if range.start() == expr_range.start() { - fix_start = Some(range.start()); - } - if fix_start.is_some() && tok.is_comma() { - seen_comma = true; + + match (fix_start, fix_end) { + (Some(start), Some(end)) => Ok(Edit::deletion(start, end)), + _ => { + bail!("No fix could be constructed") } } } else { - // Case 3: argument or keyword is the last node, so we have to find the last - // comma in the stmt. - for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, call_at).flatten() { - if range.start() == expr_range.start() { - fix_end = Some(expr_range.end()); - break; + // Only one argument; remove it (but preserve parentheses, if needed). + Ok(match parentheses { + Parentheses::Remove => Edit::deletion(arguments.start(), arguments.end()), + Parentheses::Preserve => { + Edit::replacement("()".to_string(), arguments.start(), arguments.end()) } - if tok.is_comma() { - fix_start = Some(range.start()); - } - } - } - - match (fix_start, fix_end) { - (Some(start), Some(end)) => Ok(Edit::deletion(start, end)), - _ => { - bail!("No fix could be constructed") - } + }) } } @@ -297,11 +291,11 @@ fn next_stmt_break(semicolon: TextSize, locator: &Locator) -> TextSize { #[cfg(test)] mod tests { use anyhow::Result; + use ruff_python_ast::Ranged; use ruff_python_parser::parse_suite; - use ruff_text_size::TextSize; - use ruff_source_file::Locator; + use ruff_text_size::TextSize; use crate::autofix::edits::{next_stmt_break, trailing_semicolon}; diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index 34bd9c6535..7cb5268c12 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -426,7 +426,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { pyupgrade::rules::super_call_with_parameters(checker, expr, func, args); } if checker.enabled(Rule::UnnecessaryEncodeUTF8) { - pyupgrade::rules::unnecessary_encode_utf8(checker, expr, func, args, keywords); + pyupgrade::rules::unnecessary_encode_utf8(checker, call); } if checker.enabled(Rule::RedundantOpenModes) { pyupgrade::rules::redundant_open_modes(checker, expr); @@ -441,7 +441,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { pyupgrade::rules::replace_universal_newlines(checker, func, keywords); } if checker.enabled(Rule::ReplaceStdoutStderr) { - pyupgrade::rules::replace_stdout_stderr(checker, expr, func, args, keywords); + pyupgrade::rules::replace_stdout_stderr(checker, call); } if checker.enabled(Rule::OSErrorAlias) { pyupgrade::rules::os_error_alias_call(checker, func); @@ -677,7 +677,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { flake8_debugger::rules::debugger_call(checker, expr, func); } if checker.enabled(Rule::PandasUseOfInplaceArgument) { - pandas_vet::rules::inplace_argument(checker, expr, func, args, keywords); + pandas_vet::rules::inplace_argument(checker, call); } pandas_vet::rules::call(checker, func); if checker.enabled(Rule::PandasUseOfDotReadTable) { 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 8e8302c51d..47572b674a 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,5 @@ use std::fmt; -use ruff_python_ast::{self as ast, Expr, ParameterWithDefault, Parameters, Ranged, Stmt}; -use ruff_python_ast::{Arguments, Decorator}; -use ruff_text_size::{TextLen, TextRange}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; @@ -12,10 +8,13 @@ use ruff_python_ast::helpers::{find_keyword, includes_arg_name}; use ruff_python_ast::identifier::Identifier; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::Decorator; +use ruff_python_ast::{self as ast, Expr, ParameterWithDefault, Parameters, Ranged, Stmt}; use ruff_python_semantic::analyze::visibility::is_abstract; use ruff_python_semantic::SemanticModel; +use ruff_text_size::{TextLen, TextRange}; -use crate::autofix::edits::remove_argument; +use crate::autofix::edits; use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; @@ -476,18 +475,13 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D match &decorator.expression { Expr::Call(ast::ExprCall { func, - arguments: - Arguments { - args, - keywords, - range: _, - }, + arguments, range: _, }) => { if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) { if !checker.settings.flake8_pytest_style.fixture_parentheses - && args.is_empty() - && keywords.is_empty() + && arguments.args.is_empty() + && arguments.keywords.is_empty() { let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end())); pytest_fixture_parentheses( @@ -501,7 +495,7 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D } if checker.enabled(Rule::PytestFixturePositionalArgs) { - if !args.is_empty() { + if !arguments.args.is_empty() { checker.diagnostics.push(Diagnostic::new( PytestFixturePositionalArgs { function: func_name.to_string(), @@ -512,19 +506,17 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D } if checker.enabled(Rule::PytestExtraneousScopeFunction) { - if let Some(scope_keyword) = find_keyword(keywords, "scope") { - if keyword_is_literal(scope_keyword, "function") { + if let Some(keyword) = find_keyword(&arguments.keywords, "scope") { + if keyword_is_literal(keyword, "function") { let mut diagnostic = - Diagnostic::new(PytestExtraneousScopeFunction, scope_keyword.range()); + Diagnostic::new(PytestExtraneousScopeFunction, keyword.range()); if checker.patch(diagnostic.kind.rule()) { diagnostic.try_set_fix(|| { - remove_argument( + edits::remove_argument( + keyword, + arguments, + edits::Parentheses::Preserve, checker.locator(), - func.end(), - scope_keyword.range, - args, - keywords, - false, ) .map(Fix::suggested) }); 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 7deb387c8c..4ecd4f4f01 100644 --- a/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs +++ b/crates/ruff/src/rules/pandas_vet/rules/inplace_argument.rs @@ -1,13 +1,11 @@ -use ruff_python_ast::{Expr, Keyword, Ranged}; -use ruff_text_size::TextRange; - 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::{self as ast, Keyword, Ranged}; use ruff_python_semantic::{BindingKind, Import}; use ruff_source_file::Locator; -use crate::autofix::edits::remove_argument; +use crate::autofix::edits::{remove_argument, Parentheses}; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -52,15 +50,9 @@ impl Violation for PandasUseOfInplaceArgument { } /// PD002 -pub(crate) fn inplace_argument( - checker: &mut Checker, - expr: &Expr, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], -) { +pub(crate) fn inplace_argument(checker: &mut Checker, call: &ast::ExprCall) { // 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) { + if let Some(call_path) = checker.semantic().resolve_call_path(&call.func) { if !call_path .first() .and_then(|module| checker.semantic().find_binding(module)) @@ -78,7 +70,7 @@ pub(crate) fn inplace_argument( } let mut seen_star = false; - for keyword in keywords.iter().rev() { + for keyword in call.arguments.keywords.iter().rev() { let Some(arg) = &keyword.arg else { seen_star = true; continue; @@ -101,13 +93,9 @@ pub(crate) fn inplace_argument( && checker.semantic().expr_parent().is_none() && !checker.semantic().scope().kind.is_lambda() { - if let Some(fix) = convert_inplace_argument_to_assignment( - checker.locator(), - expr, - keyword.range(), - args, - keywords, - ) { + if let Some(fix) = + convert_inplace_argument_to_assignment(checker.locator(), call, keyword) + { diagnostic.set_fix(fix); } } @@ -126,22 +114,19 @@ pub(crate) fn inplace_argument( /// assignment. fn convert_inplace_argument_to_assignment( locator: &Locator, - expr: &Expr, - expr_range: TextRange, - args: &[Expr], - keywords: &[Keyword], + call: &ast::ExprCall, + keyword: &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(), + call.start(), ); // Remove the `inplace` argument. let remove_argument = - remove_argument(locator, call.func.end(), expr_range, args, keywords, false).ok()?; + remove_argument(keyword, &call.arguments, Parentheses::Preserve, locator).ok()?; Some(Fix::suggested_edits(insert_assignment, [remove_argument])) } 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 f52a3a70c4..2be2f1d7d1 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/replace_stdout_stderr.rs @@ -1,12 +1,12 @@ use anyhow::Result; -use ruff_python_ast::{Expr, Keyword, Ranged}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::find_keyword; +use ruff_python_ast::{self as ast, Keyword, Ranged}; use ruff_source_file::Locator; -use crate::autofix::edits::remove_argument; +use crate::autofix::edits::{remove_argument, Parentheses}; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -53,12 +53,10 @@ impl AlwaysAutofixableViolation for ReplaceStdoutStderr { /// Generate a [`Edit`] for a `stdout` and `stderr` [`Keyword`] pair. fn generate_fix( - locator: &Locator, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], stdout: &Keyword, stderr: &Keyword, + call: &ast::ExprCall, + locator: &Locator, ) -> Result { let (first, second) = if stdout.start() < stderr.start() { (stdout, stderr) @@ -68,34 +66,26 @@ fn generate_fix( Ok(Fix::suggested_edits( Edit::range_replacement("capture_output=True".to_string(), first.range()), [remove_argument( + second, + &call.arguments, + Parentheses::Preserve, locator, - func.end(), - second.range(), - args, - keywords, - false, )?], )) } /// UP022 -pub(crate) fn replace_stdout_stderr( - checker: &mut Checker, - expr: &Expr, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], -) { +pub(crate) fn replace_stdout_stderr(checker: &mut Checker, call: &ast::ExprCall) { if checker .semantic() - .resolve_call_path(func) + .resolve_call_path(&call.func) .is_some_and(|call_path| matches!(call_path.as_slice(), ["subprocess", "run"])) { // Find `stdout` and `stderr` kwargs. - let Some(stdout) = find_keyword(keywords, "stdout") else { + let Some(stdout) = find_keyword(&call.arguments.keywords, "stdout") else { return; }; - let Some(stderr) = find_keyword(keywords, "stderr") else { + let Some(stderr) = find_keyword(&call.arguments.keywords, "stderr") else { return; }; @@ -112,11 +102,9 @@ pub(crate) fn replace_stdout_stderr( return; } - let mut diagnostic = Diagnostic::new(ReplaceStdoutStderr, expr.range()); + let mut diagnostic = Diagnostic::new(ReplaceStdoutStderr, call.range()); if checker.patch(diagnostic.kind.rule()) { - diagnostic.try_set_fix(|| { - generate_fix(checker.locator(), func, args, keywords, stdout, stderr) - }); + diagnostic.try_set_fix(|| generate_fix(stdout, stderr, call, checker.locator())); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs index 3f1b1dbdd2..31396177b4 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_encode_utf8.rs @@ -1,12 +1,11 @@ -use ruff_python_ast::{self as ast, Constant, Expr, Keyword, Ranged}; -use ruff_python_parser::{lexer, Mode, Tok}; -use ruff_text_size::TextRange; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Arguments, Constant, Expr, Keyword, Ranged}; +use ruff_python_parser::{lexer, Mode, Tok}; use ruff_source_file::Locator; +use ruff_text_size::TextRange; -use crate::autofix::edits::remove_argument; +use crate::autofix::edits::{remove_argument, Parentheses}; use crate::checkers::ast::Checker; use crate::registry::Rule; @@ -95,23 +94,21 @@ enum EncodingArg<'a> { /// Return the encoding argument to an `encode` call, if it can be determined to be a /// UTF-8-equivalent encoding. -fn match_encoding_arg<'a>(args: &'a [Expr], kwargs: &'a [Keyword]) -> Option> { - match (args.len(), kwargs.len()) { +fn match_encoding_arg(arguments: &Arguments) -> Option { + match (arguments.args.as_slice(), arguments.keywords.as_slice()) { // Ex `"".encode()` - (0, 0) => return Some(EncodingArg::Empty), + ([], []) => return Some(EncodingArg::Empty), // Ex `"".encode(encoding)` - (1, 0) => { - let arg = &args[0]; + ([arg], []) => { if is_utf8_encoding_arg(arg) { return Some(EncodingArg::Positional(arg)); } } // Ex `"".encode(kwarg=kwarg)` - (0, 1) => { - let kwarg = &kwargs[0]; - if kwarg.arg.as_ref().is_some_and(|arg| arg == "encoding") { - if is_utf8_encoding_arg(&kwarg.value) { - return Some(EncodingArg::Keyword(kwarg)); + ([], [keyword]) => { + if keyword.arg.as_ref().is_some_and(|arg| arg == "encoding") { + if is_utf8_encoding_arg(&keyword.value) { + return Some(EncodingArg::Keyword(keyword)); } } } @@ -122,7 +119,7 @@ fn match_encoding_arg<'a>(args: &'a [Expr], kwargs: &'a [Keyword]) -> Option Fix { +fn replace_with_bytes_literal(locator: &Locator, expr: &T) -> Fix { // Build up a replacement string by prefixing all string tokens with `b`. let contents = locator.slice(expr.range()); let mut replacement = String::with_capacity(contents.len() + 1); @@ -149,14 +146,8 @@ fn replace_with_bytes_literal(locator: &Locator, expr: &Expr) -> Fix { } /// UP012 -pub(crate) fn unnecessary_encode_utf8( - checker: &mut Checker, - expr: &Expr, - func: &Expr, - args: &[Expr], - kwargs: &[Keyword], -) { - let Some(variable) = match_encoded_variable(func) else { +pub(crate) fn unnecessary_encode_utf8(checker: &mut Checker, call: &ast::ExprCall) { + let Some(variable) = match_encoded_variable(&call.func) else { return; }; match variable { @@ -165,17 +156,17 @@ pub(crate) fn unnecessary_encode_utf8( .. }) => { // Ex) `"str".encode()`, `"str".encode("utf-8")` - if let Some(encoding_arg) = match_encoding_arg(args, kwargs) { + if let Some(encoding_arg) = match_encoding_arg(&call.arguments) { if literal.is_ascii() { // Ex) Convert `"foo".encode()` to `b"foo"`. let mut diagnostic = Diagnostic::new( UnnecessaryEncodeUTF8 { reason: Reason::BytesLiteral, }, - expr.range(), + call.range(), ); if checker.patch(Rule::UnnecessaryEncodeUTF8) { - diagnostic.set_fix(replace_with_bytes_literal(checker.locator(), expr)); + diagnostic.set_fix(replace_with_bytes_literal(checker.locator(), call)); } checker.diagnostics.push(diagnostic); } else if let EncodingArg::Keyword(kwarg) = encoding_arg { @@ -185,17 +176,15 @@ pub(crate) fn unnecessary_encode_utf8( UnnecessaryEncodeUTF8 { reason: Reason::DefaultArgument, }, - expr.range(), + call.range(), ); if checker.patch(Rule::UnnecessaryEncodeUTF8) { diagnostic.try_set_fix(|| { remove_argument( + kwarg, + &call.arguments, + Parentheses::Preserve, checker.locator(), - func.end(), - kwarg.range(), - args, - kwargs, - false, ) .map(Fix::automatic) }); @@ -207,17 +196,15 @@ pub(crate) fn unnecessary_encode_utf8( UnnecessaryEncodeUTF8 { reason: Reason::DefaultArgument, }, - expr.range(), + call.range(), ); if checker.patch(Rule::UnnecessaryEncodeUTF8) { diagnostic.try_set_fix(|| { remove_argument( + arg, + &call.arguments, + Parentheses::Preserve, checker.locator(), - func.end(), - arg.range(), - args, - kwargs, - false, ) .map(Fix::automatic) }); @@ -228,7 +215,7 @@ pub(crate) fn unnecessary_encode_utf8( } // Ex) `f"foo{bar}".encode("utf-8")` Expr::JoinedStr(_) => { - if let Some(encoding_arg) = match_encoding_arg(args, kwargs) { + if let Some(encoding_arg) = match_encoding_arg(&call.arguments) { if let EncodingArg::Keyword(kwarg) = encoding_arg { // Ex) Convert `f"unicode text©".encode(encoding="utf-8")` to // `f"unicode text©".encode()`. @@ -236,17 +223,15 @@ pub(crate) fn unnecessary_encode_utf8( UnnecessaryEncodeUTF8 { reason: Reason::DefaultArgument, }, - expr.range(), + call.range(), ); if checker.patch(Rule::UnnecessaryEncodeUTF8) { diagnostic.try_set_fix(|| { remove_argument( + kwarg, + &call.arguments, + Parentheses::Preserve, checker.locator(), - func.end(), - kwarg.range(), - args, - kwargs, - false, ) .map(Fix::automatic) }); @@ -258,17 +243,15 @@ pub(crate) fn unnecessary_encode_utf8( UnnecessaryEncodeUTF8 { reason: Reason::DefaultArgument, }, - expr.range(), + call.range(), ); if checker.patch(Rule::UnnecessaryEncodeUTF8) { diagnostic.try_set_fix(|| { remove_argument( + arg, + &call.arguments, + Parentheses::Preserve, checker.locator(), - func.end(), - arg.range(), - args, - kwargs, - false, ) .map(Fix::automatic) }); 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 e357a73866..7298309a06 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs @@ -1,9 +1,8 @@ -use ruff_python_ast::{self as ast, Expr, Ranged}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr, Ranged}; -use crate::autofix::edits::remove_argument; +use crate::autofix::edits::{remove_argument, Parentheses}; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -51,8 +50,8 @@ pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast: return; }; - for expr in &arguments.args { - let Expr::Name(ast::ExprName { id, .. }) = expr else { + for base in &arguments.args { + let Expr::Name(ast::ExprName { id, .. }) = base else { continue; }; if id != "object" { @@ -66,19 +65,12 @@ pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast: UselessObjectInheritance { name: class_def.name.to_string(), }, - expr.range(), + base.range(), ); if checker.patch(diagnostic.kind.rule()) { diagnostic.try_set_fix(|| { - let edit = remove_argument( - checker.locator(), - class_def.name.end(), - expr.range(), - &arguments.args, - &arguments.keywords, - true, - )?; - Ok(Fix::automatic(edit)) + remove_argument(base, arguments, Parentheses::Remove, checker.locator()) + .map(Fix::automatic) }); } checker.diagnostics.push(diagnostic);