mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 20:10:09 +00:00
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`
This commit is contained in:
parent
4c53bfe896
commit
8c40886f87
7 changed files with 165 additions and 231 deletions
|
@ -1,14 +1,15 @@
|
||||||
//! Interface for generating autofix edits from higher-level actions (e.g., "remove an argument").
|
//! Interface for generating autofix edits from higher-level actions (e.g., "remove an argument").
|
||||||
|
|
||||||
use anyhow::{bail, Result};
|
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_diagnostics::Edit;
|
||||||
|
use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, Keyword, Ranged, Stmt};
|
||||||
use ruff_python_codegen::Stylist;
|
use ruff_python_codegen::Stylist;
|
||||||
use ruff_python_index::Indexer;
|
use ruff_python_index::Indexer;
|
||||||
|
use ruff_python_parser::{lexer, Mode};
|
||||||
use ruff_python_trivia::{has_leading_content, is_python_whitespace, PythonWhitespace};
|
use ruff_python_trivia::{has_leading_content, is_python_whitespace, PythonWhitespace};
|
||||||
use ruff_source_file::{Locator, NewlineWithTrailingNewline};
|
use ruff_source_file::{Locator, NewlineWithTrailingNewline};
|
||||||
|
use ruff_text_size::{TextLen, TextRange, TextSize};
|
||||||
|
|
||||||
use crate::autofix::codemods;
|
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
|
/// Generic function to remove arguments or keyword arguments in function
|
||||||
/// calls and class definitions. (For classes `args` should be considered
|
/// calls and class definitions. (For classes `args` should be considered
|
||||||
/// `bases`)
|
/// `bases`)
|
||||||
///
|
///
|
||||||
/// Supports the removal of parentheses when this is the only (kw)arg left.
|
/// Supports the removal of parentheses when this is the only (kw)arg left.
|
||||||
/// For this behavior, set `remove_parentheses` to `true`.
|
/// For this behavior, set `remove_parentheses` to `true`.
|
||||||
///
|
pub(crate) fn remove_argument<T: Ranged>(
|
||||||
/// TODO(charlie): Migrate this signature to take [`Arguments`] rather than
|
argument: &T,
|
||||||
/// separate args and keywords.
|
arguments: &Arguments,
|
||||||
pub(crate) fn remove_argument(
|
parentheses: Parentheses,
|
||||||
locator: &Locator,
|
locator: &Locator,
|
||||||
call_at: TextSize,
|
|
||||||
expr_range: TextRange,
|
|
||||||
args: &[Expr],
|
|
||||||
keywords: &[Keyword],
|
|
||||||
remove_parentheses: bool,
|
|
||||||
) -> Result<Edit> {
|
) -> Result<Edit> {
|
||||||
// TODO(sbrugman): Preserve trailing comments.
|
// 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;
|
if arguments
|
||||||
let mut fix_end = None;
|
.args
|
||||||
|
.iter()
|
||||||
let n_arguments = keywords.len() + args.len();
|
.map(Expr::start)
|
||||||
if n_arguments == 0 {
|
.chain(arguments.keywords.iter().map(Keyword::start))
|
||||||
bail!("No arguments or keywords to remove");
|
.any(|location| location > argument.start())
|
||||||
}
|
{
|
||||||
|
// Case 1: argument or keyword is _not_ the last node, so delete from the start of the
|
||||||
if n_arguments == 1 {
|
// argument to the end of the subsequent comma.
|
||||||
// Case 1: there is only one argument.
|
let mut seen_comma = false;
|
||||||
let mut count = 0u32;
|
for (tok, range) in lexer::lex_starts_at(
|
||||||
for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, call_at).flatten() {
|
locator.slice(arguments.range()),
|
||||||
if tok.is_lpar() {
|
Mode::Module,
|
||||||
if count == 0 {
|
arguments.start(),
|
||||||
fix_start = Some(if remove_parentheses {
|
)
|
||||||
range.start()
|
.flatten()
|
||||||
} else {
|
{
|
||||||
range.start() + TextSize::from(1)
|
if seen_comma {
|
||||||
});
|
if tok.is_non_logical_newline() {
|
||||||
}
|
// Also delete any non-logical newlines after the comma.
|
||||||
count = count.saturating_add(1);
|
continue;
|
||||||
}
|
}
|
||||||
|
fix_end = Some(if tok.is_newline() {
|
||||||
if tok.is_rpar() {
|
|
||||||
count = count.saturating_sub(1);
|
|
||||||
if count == 0 {
|
|
||||||
fix_end = Some(if remove_parentheses {
|
|
||||||
range.end()
|
range.end()
|
||||||
} else {
|
} else {
|
||||||
range.end() - TextSize::from(1)
|
range.start()
|
||||||
});
|
});
|
||||||
break;
|
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()
|
match (fix_start, fix_end) {
|
||||||
.map(Expr::start)
|
(Some(start), Some(end)) => Ok(Edit::deletion(start, end)),
|
||||||
.chain(keywords.iter().map(Keyword::start))
|
_ => {
|
||||||
.any(|location| location > expr_range.start())
|
bail!("No fix could be constructed")
|
||||||
{
|
|
||||||
// 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;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Case 3: argument or keyword is the last node, so we have to find the last
|
// Only one argument; remove it (but preserve parentheses, if needed).
|
||||||
// comma in the stmt.
|
Ok(match parentheses {
|
||||||
for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, call_at).flatten() {
|
Parentheses::Remove => Edit::deletion(arguments.start(), arguments.end()),
|
||||||
if range.start() == expr_range.start() {
|
Parentheses::Preserve => {
|
||||||
fix_end = Some(expr_range.end());
|
Edit::replacement("()".to_string(), arguments.start(), arguments.end())
|
||||||
break;
|
|
||||||
}
|
}
|
||||||
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
|
|
||||||
use ruff_python_ast::Ranged;
|
use ruff_python_ast::Ranged;
|
||||||
use ruff_python_parser::parse_suite;
|
use ruff_python_parser::parse_suite;
|
||||||
use ruff_text_size::TextSize;
|
|
||||||
|
|
||||||
use ruff_source_file::Locator;
|
use ruff_source_file::Locator;
|
||||||
|
use ruff_text_size::TextSize;
|
||||||
|
|
||||||
use crate::autofix::edits::{next_stmt_break, trailing_semicolon};
|
use crate::autofix::edits::{next_stmt_break, trailing_semicolon};
|
||||||
|
|
||||||
|
|
|
@ -426,7 +426,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
pyupgrade::rules::super_call_with_parameters(checker, expr, func, args);
|
pyupgrade::rules::super_call_with_parameters(checker, expr, func, args);
|
||||||
}
|
}
|
||||||
if checker.enabled(Rule::UnnecessaryEncodeUTF8) {
|
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) {
|
if checker.enabled(Rule::RedundantOpenModes) {
|
||||||
pyupgrade::rules::redundant_open_modes(checker, expr);
|
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);
|
pyupgrade::rules::replace_universal_newlines(checker, func, keywords);
|
||||||
}
|
}
|
||||||
if checker.enabled(Rule::ReplaceStdoutStderr) {
|
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) {
|
if checker.enabled(Rule::OSErrorAlias) {
|
||||||
pyupgrade::rules::os_error_alias_call(checker, func);
|
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);
|
flake8_debugger::rules::debugger_call(checker, expr, func);
|
||||||
}
|
}
|
||||||
if checker.enabled(Rule::PandasUseOfInplaceArgument) {
|
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);
|
pandas_vet::rules::call(checker, func);
|
||||||
if checker.enabled(Rule::PandasUseOfDotReadTable) {
|
if checker.enabled(Rule::PandasUseOfDotReadTable) {
|
||||||
|
|
|
@ -1,9 +1,5 @@
|
||||||
use std::fmt;
|
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::{AlwaysAutofixableViolation, Violation};
|
||||||
use ruff_diagnostics::{Diagnostic, Edit, Fix};
|
use ruff_diagnostics::{Diagnostic, Edit, Fix};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
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::identifier::Identifier;
|
||||||
use ruff_python_ast::visitor;
|
use ruff_python_ast::visitor;
|
||||||
use ruff_python_ast::visitor::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::analyze::visibility::is_abstract;
|
||||||
use ruff_python_semantic::SemanticModel;
|
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::checkers::ast::Checker;
|
||||||
use crate::registry::{AsRule, Rule};
|
use crate::registry::{AsRule, Rule};
|
||||||
|
|
||||||
|
@ -476,18 +475,13 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D
|
||||||
match &decorator.expression {
|
match &decorator.expression {
|
||||||
Expr::Call(ast::ExprCall {
|
Expr::Call(ast::ExprCall {
|
||||||
func,
|
func,
|
||||||
arguments:
|
arguments,
|
||||||
Arguments {
|
|
||||||
args,
|
|
||||||
keywords,
|
|
||||||
range: _,
|
|
||||||
},
|
|
||||||
range: _,
|
range: _,
|
||||||
}) => {
|
}) => {
|
||||||
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) {
|
if checker.enabled(Rule::PytestFixtureIncorrectParenthesesStyle) {
|
||||||
if !checker.settings.flake8_pytest_style.fixture_parentheses
|
if !checker.settings.flake8_pytest_style.fixture_parentheses
|
||||||
&& args.is_empty()
|
&& arguments.args.is_empty()
|
||||||
&& keywords.is_empty()
|
&& arguments.keywords.is_empty()
|
||||||
{
|
{
|
||||||
let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end()));
|
let fix = Fix::automatic(Edit::deletion(func.end(), decorator.end()));
|
||||||
pytest_fixture_parentheses(
|
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 checker.enabled(Rule::PytestFixturePositionalArgs) {
|
||||||
if !args.is_empty() {
|
if !arguments.args.is_empty() {
|
||||||
checker.diagnostics.push(Diagnostic::new(
|
checker.diagnostics.push(Diagnostic::new(
|
||||||
PytestFixturePositionalArgs {
|
PytestFixturePositionalArgs {
|
||||||
function: func_name.to_string(),
|
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 checker.enabled(Rule::PytestExtraneousScopeFunction) {
|
||||||
if let Some(scope_keyword) = find_keyword(keywords, "scope") {
|
if let Some(keyword) = find_keyword(&arguments.keywords, "scope") {
|
||||||
if keyword_is_literal(scope_keyword, "function") {
|
if keyword_is_literal(keyword, "function") {
|
||||||
let mut diagnostic =
|
let mut diagnostic =
|
||||||
Diagnostic::new(PytestExtraneousScopeFunction, scope_keyword.range());
|
Diagnostic::new(PytestExtraneousScopeFunction, keyword.range());
|
||||||
if checker.patch(diagnostic.kind.rule()) {
|
if checker.patch(diagnostic.kind.rule()) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
remove_argument(
|
edits::remove_argument(
|
||||||
|
keyword,
|
||||||
|
arguments,
|
||||||
|
edits::Parentheses::Preserve,
|
||||||
checker.locator(),
|
checker.locator(),
|
||||||
func.end(),
|
|
||||||
scope_keyword.range,
|
|
||||||
args,
|
|
||||||
keywords,
|
|
||||||
false,
|
|
||||||
)
|
)
|
||||||
.map(Fix::suggested)
|
.map(Fix::suggested)
|
||||||
});
|
});
|
||||||
|
|
|
@ -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_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
use ruff_python_ast::helpers::is_const_true;
|
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_python_semantic::{BindingKind, Import};
|
||||||
use ruff_source_file::Locator;
|
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::checkers::ast::Checker;
|
||||||
use crate::registry::AsRule;
|
use crate::registry::AsRule;
|
||||||
|
|
||||||
|
@ -52,15 +50,9 @@ impl Violation for PandasUseOfInplaceArgument {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// PD002
|
/// PD002
|
||||||
pub(crate) fn inplace_argument(
|
pub(crate) fn inplace_argument(checker: &mut Checker, call: &ast::ExprCall) {
|
||||||
checker: &mut Checker,
|
|
||||||
expr: &Expr,
|
|
||||||
func: &Expr,
|
|
||||||
args: &[Expr],
|
|
||||||
keywords: &[Keyword],
|
|
||||||
) {
|
|
||||||
// If the function was imported from another module, and it's _not_ Pandas, abort.
|
// 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
|
if !call_path
|
||||||
.first()
|
.first()
|
||||||
.and_then(|module| checker.semantic().find_binding(module))
|
.and_then(|module| checker.semantic().find_binding(module))
|
||||||
|
@ -78,7 +70,7 @@ pub(crate) fn inplace_argument(
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut seen_star = false;
|
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 {
|
let Some(arg) = &keyword.arg else {
|
||||||
seen_star = true;
|
seen_star = true;
|
||||||
continue;
|
continue;
|
||||||
|
@ -101,13 +93,9 @@ pub(crate) fn inplace_argument(
|
||||||
&& checker.semantic().expr_parent().is_none()
|
&& checker.semantic().expr_parent().is_none()
|
||||||
&& !checker.semantic().scope().kind.is_lambda()
|
&& !checker.semantic().scope().kind.is_lambda()
|
||||||
{
|
{
|
||||||
if let Some(fix) = convert_inplace_argument_to_assignment(
|
if let Some(fix) =
|
||||||
checker.locator(),
|
convert_inplace_argument_to_assignment(checker.locator(), call, keyword)
|
||||||
expr,
|
{
|
||||||
keyword.range(),
|
|
||||||
args,
|
|
||||||
keywords,
|
|
||||||
) {
|
|
||||||
diagnostic.set_fix(fix);
|
diagnostic.set_fix(fix);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -126,22 +114,19 @@ pub(crate) fn inplace_argument(
|
||||||
/// assignment.
|
/// assignment.
|
||||||
fn convert_inplace_argument_to_assignment(
|
fn convert_inplace_argument_to_assignment(
|
||||||
locator: &Locator,
|
locator: &Locator,
|
||||||
expr: &Expr,
|
call: &ast::ExprCall,
|
||||||
expr_range: TextRange,
|
keyword: &Keyword,
|
||||||
args: &[Expr],
|
|
||||||
keywords: &[Keyword],
|
|
||||||
) -> Option<Fix> {
|
) -> Option<Fix> {
|
||||||
// Add the assignment.
|
// Add the assignment.
|
||||||
let call = expr.as_call_expr()?;
|
|
||||||
let attr = call.func.as_attribute_expr()?;
|
let attr = call.func.as_attribute_expr()?;
|
||||||
let insert_assignment = Edit::insertion(
|
let insert_assignment = Edit::insertion(
|
||||||
format!("{name} = ", name = locator.slice(attr.value.range())),
|
format!("{name} = ", name = locator.slice(attr.value.range())),
|
||||||
expr.start(),
|
call.start(),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Remove the `inplace` argument.
|
// Remove the `inplace` argument.
|
||||||
let remove_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]))
|
Some(Fix::suggested_edits(insert_assignment, [remove_argument]))
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,12 +1,12 @@
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use ruff_python_ast::{Expr, Keyword, Ranged};
|
|
||||||
|
|
||||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
use ruff_python_ast::helpers::find_keyword;
|
use ruff_python_ast::helpers::find_keyword;
|
||||||
|
use ruff_python_ast::{self as ast, Keyword, Ranged};
|
||||||
use ruff_source_file::Locator;
|
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::checkers::ast::Checker;
|
||||||
use crate::registry::AsRule;
|
use crate::registry::AsRule;
|
||||||
|
|
||||||
|
@ -53,12 +53,10 @@ impl AlwaysAutofixableViolation for ReplaceStdoutStderr {
|
||||||
|
|
||||||
/// Generate a [`Edit`] for a `stdout` and `stderr` [`Keyword`] pair.
|
/// Generate a [`Edit`] for a `stdout` and `stderr` [`Keyword`] pair.
|
||||||
fn generate_fix(
|
fn generate_fix(
|
||||||
locator: &Locator,
|
|
||||||
func: &Expr,
|
|
||||||
args: &[Expr],
|
|
||||||
keywords: &[Keyword],
|
|
||||||
stdout: &Keyword,
|
stdout: &Keyword,
|
||||||
stderr: &Keyword,
|
stderr: &Keyword,
|
||||||
|
call: &ast::ExprCall,
|
||||||
|
locator: &Locator,
|
||||||
) -> Result<Fix> {
|
) -> Result<Fix> {
|
||||||
let (first, second) = if stdout.start() < stderr.start() {
|
let (first, second) = if stdout.start() < stderr.start() {
|
||||||
(stdout, stderr)
|
(stdout, stderr)
|
||||||
|
@ -68,34 +66,26 @@ fn generate_fix(
|
||||||
Ok(Fix::suggested_edits(
|
Ok(Fix::suggested_edits(
|
||||||
Edit::range_replacement("capture_output=True".to_string(), first.range()),
|
Edit::range_replacement("capture_output=True".to_string(), first.range()),
|
||||||
[remove_argument(
|
[remove_argument(
|
||||||
|
second,
|
||||||
|
&call.arguments,
|
||||||
|
Parentheses::Preserve,
|
||||||
locator,
|
locator,
|
||||||
func.end(),
|
|
||||||
second.range(),
|
|
||||||
args,
|
|
||||||
keywords,
|
|
||||||
false,
|
|
||||||
)?],
|
)?],
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// UP022
|
/// UP022
|
||||||
pub(crate) fn replace_stdout_stderr(
|
pub(crate) fn replace_stdout_stderr(checker: &mut Checker, call: &ast::ExprCall) {
|
||||||
checker: &mut Checker,
|
|
||||||
expr: &Expr,
|
|
||||||
func: &Expr,
|
|
||||||
args: &[Expr],
|
|
||||||
keywords: &[Keyword],
|
|
||||||
) {
|
|
||||||
if checker
|
if checker
|
||||||
.semantic()
|
.semantic()
|
||||||
.resolve_call_path(func)
|
.resolve_call_path(&call.func)
|
||||||
.is_some_and(|call_path| matches!(call_path.as_slice(), ["subprocess", "run"]))
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["subprocess", "run"]))
|
||||||
{
|
{
|
||||||
// Find `stdout` and `stderr` kwargs.
|
// Find `stdout` and `stderr` kwargs.
|
||||||
let Some(stdout) = find_keyword(keywords, "stdout") else {
|
let Some(stdout) = find_keyword(&call.arguments.keywords, "stdout") else {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
let Some(stderr) = find_keyword(keywords, "stderr") else {
|
let Some(stderr) = find_keyword(&call.arguments.keywords, "stderr") else {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -112,11 +102,9 @@ pub(crate) fn replace_stdout_stderr(
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut diagnostic = Diagnostic::new(ReplaceStdoutStderr, expr.range());
|
let mut diagnostic = Diagnostic::new(ReplaceStdoutStderr, call.range());
|
||||||
if checker.patch(diagnostic.kind.rule()) {
|
if checker.patch(diagnostic.kind.rule()) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| generate_fix(stdout, stderr, call, checker.locator()));
|
||||||
generate_fix(checker.locator(), func, args, keywords, stdout, stderr)
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
checker.diagnostics.push(diagnostic);
|
checker.diagnostics.push(diagnostic);
|
||||||
}
|
}
|
||||||
|
|
|
@ -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_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
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_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::checkers::ast::Checker;
|
||||||
use crate::registry::Rule;
|
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
|
/// Return the encoding argument to an `encode` call, if it can be determined to be a
|
||||||
/// UTF-8-equivalent encoding.
|
/// UTF-8-equivalent encoding.
|
||||||
fn match_encoding_arg<'a>(args: &'a [Expr], kwargs: &'a [Keyword]) -> Option<EncodingArg<'a>> {
|
fn match_encoding_arg(arguments: &Arguments) -> Option<EncodingArg> {
|
||||||
match (args.len(), kwargs.len()) {
|
match (arguments.args.as_slice(), arguments.keywords.as_slice()) {
|
||||||
// Ex `"".encode()`
|
// Ex `"".encode()`
|
||||||
(0, 0) => return Some(EncodingArg::Empty),
|
([], []) => return Some(EncodingArg::Empty),
|
||||||
// Ex `"".encode(encoding)`
|
// Ex `"".encode(encoding)`
|
||||||
(1, 0) => {
|
([arg], []) => {
|
||||||
let arg = &args[0];
|
|
||||||
if is_utf8_encoding_arg(arg) {
|
if is_utf8_encoding_arg(arg) {
|
||||||
return Some(EncodingArg::Positional(arg));
|
return Some(EncodingArg::Positional(arg));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Ex `"".encode(kwarg=kwarg)`
|
// Ex `"".encode(kwarg=kwarg)`
|
||||||
(0, 1) => {
|
([], [keyword]) => {
|
||||||
let kwarg = &kwargs[0];
|
if keyword.arg.as_ref().is_some_and(|arg| arg == "encoding") {
|
||||||
if kwarg.arg.as_ref().is_some_and(|arg| arg == "encoding") {
|
if is_utf8_encoding_arg(&keyword.value) {
|
||||||
if is_utf8_encoding_arg(&kwarg.value) {
|
return Some(EncodingArg::Keyword(keyword));
|
||||||
return Some(EncodingArg::Keyword(kwarg));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -122,7 +119,7 @@ fn match_encoding_arg<'a>(args: &'a [Expr], kwargs: &'a [Keyword]) -> Option<Enc
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Return a [`Fix`] replacing the call to encode with a byte string.
|
/// Return a [`Fix`] replacing the call to encode with a byte string.
|
||||||
fn replace_with_bytes_literal(locator: &Locator, expr: &Expr) -> Fix {
|
fn replace_with_bytes_literal<T: Ranged>(locator: &Locator, expr: &T) -> Fix {
|
||||||
// Build up a replacement string by prefixing all string tokens with `b`.
|
// Build up a replacement string by prefixing all string tokens with `b`.
|
||||||
let contents = locator.slice(expr.range());
|
let contents = locator.slice(expr.range());
|
||||||
let mut replacement = String::with_capacity(contents.len() + 1);
|
let mut replacement = String::with_capacity(contents.len() + 1);
|
||||||
|
@ -149,14 +146,8 @@ fn replace_with_bytes_literal(locator: &Locator, expr: &Expr) -> Fix {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// UP012
|
/// UP012
|
||||||
pub(crate) fn unnecessary_encode_utf8(
|
pub(crate) fn unnecessary_encode_utf8(checker: &mut Checker, call: &ast::ExprCall) {
|
||||||
checker: &mut Checker,
|
let Some(variable) = match_encoded_variable(&call.func) else {
|
||||||
expr: &Expr,
|
|
||||||
func: &Expr,
|
|
||||||
args: &[Expr],
|
|
||||||
kwargs: &[Keyword],
|
|
||||||
) {
|
|
||||||
let Some(variable) = match_encoded_variable(func) else {
|
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
match variable {
|
match variable {
|
||||||
|
@ -165,17 +156,17 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
..
|
..
|
||||||
}) => {
|
}) => {
|
||||||
// Ex) `"str".encode()`, `"str".encode("utf-8")`
|
// 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() {
|
if literal.is_ascii() {
|
||||||
// Ex) Convert `"foo".encode()` to `b"foo"`.
|
// Ex) Convert `"foo".encode()` to `b"foo"`.
|
||||||
let mut diagnostic = Diagnostic::new(
|
let mut diagnostic = Diagnostic::new(
|
||||||
UnnecessaryEncodeUTF8 {
|
UnnecessaryEncodeUTF8 {
|
||||||
reason: Reason::BytesLiteral,
|
reason: Reason::BytesLiteral,
|
||||||
},
|
},
|
||||||
expr.range(),
|
call.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
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);
|
checker.diagnostics.push(diagnostic);
|
||||||
} else if let EncodingArg::Keyword(kwarg) = encoding_arg {
|
} else if let EncodingArg::Keyword(kwarg) = encoding_arg {
|
||||||
|
@ -185,17 +176,15 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
UnnecessaryEncodeUTF8 {
|
UnnecessaryEncodeUTF8 {
|
||||||
reason: Reason::DefaultArgument,
|
reason: Reason::DefaultArgument,
|
||||||
},
|
},
|
||||||
expr.range(),
|
call.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
remove_argument(
|
remove_argument(
|
||||||
|
kwarg,
|
||||||
|
&call.arguments,
|
||||||
|
Parentheses::Preserve,
|
||||||
checker.locator(),
|
checker.locator(),
|
||||||
func.end(),
|
|
||||||
kwarg.range(),
|
|
||||||
args,
|
|
||||||
kwargs,
|
|
||||||
false,
|
|
||||||
)
|
)
|
||||||
.map(Fix::automatic)
|
.map(Fix::automatic)
|
||||||
});
|
});
|
||||||
|
@ -207,17 +196,15 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
UnnecessaryEncodeUTF8 {
|
UnnecessaryEncodeUTF8 {
|
||||||
reason: Reason::DefaultArgument,
|
reason: Reason::DefaultArgument,
|
||||||
},
|
},
|
||||||
expr.range(),
|
call.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
remove_argument(
|
remove_argument(
|
||||||
|
arg,
|
||||||
|
&call.arguments,
|
||||||
|
Parentheses::Preserve,
|
||||||
checker.locator(),
|
checker.locator(),
|
||||||
func.end(),
|
|
||||||
arg.range(),
|
|
||||||
args,
|
|
||||||
kwargs,
|
|
||||||
false,
|
|
||||||
)
|
)
|
||||||
.map(Fix::automatic)
|
.map(Fix::automatic)
|
||||||
});
|
});
|
||||||
|
@ -228,7 +215,7 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
}
|
}
|
||||||
// Ex) `f"foo{bar}".encode("utf-8")`
|
// Ex) `f"foo{bar}".encode("utf-8")`
|
||||||
Expr::JoinedStr(_) => {
|
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 {
|
if let EncodingArg::Keyword(kwarg) = encoding_arg {
|
||||||
// Ex) Convert `f"unicode text©".encode(encoding="utf-8")` to
|
// Ex) Convert `f"unicode text©".encode(encoding="utf-8")` to
|
||||||
// `f"unicode text©".encode()`.
|
// `f"unicode text©".encode()`.
|
||||||
|
@ -236,17 +223,15 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
UnnecessaryEncodeUTF8 {
|
UnnecessaryEncodeUTF8 {
|
||||||
reason: Reason::DefaultArgument,
|
reason: Reason::DefaultArgument,
|
||||||
},
|
},
|
||||||
expr.range(),
|
call.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
remove_argument(
|
remove_argument(
|
||||||
|
kwarg,
|
||||||
|
&call.arguments,
|
||||||
|
Parentheses::Preserve,
|
||||||
checker.locator(),
|
checker.locator(),
|
||||||
func.end(),
|
|
||||||
kwarg.range(),
|
|
||||||
args,
|
|
||||||
kwargs,
|
|
||||||
false,
|
|
||||||
)
|
)
|
||||||
.map(Fix::automatic)
|
.map(Fix::automatic)
|
||||||
});
|
});
|
||||||
|
@ -258,17 +243,15 @@ pub(crate) fn unnecessary_encode_utf8(
|
||||||
UnnecessaryEncodeUTF8 {
|
UnnecessaryEncodeUTF8 {
|
||||||
reason: Reason::DefaultArgument,
|
reason: Reason::DefaultArgument,
|
||||||
},
|
},
|
||||||
expr.range(),
|
call.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
if checker.patch(Rule::UnnecessaryEncodeUTF8) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
remove_argument(
|
remove_argument(
|
||||||
|
arg,
|
||||||
|
&call.arguments,
|
||||||
|
Parentheses::Preserve,
|
||||||
checker.locator(),
|
checker.locator(),
|
||||||
func.end(),
|
|
||||||
arg.range(),
|
|
||||||
args,
|
|
||||||
kwargs,
|
|
||||||
false,
|
|
||||||
)
|
)
|
||||||
.map(Fix::automatic)
|
.map(Fix::automatic)
|
||||||
});
|
});
|
||||||
|
|
|
@ -1,9 +1,8 @@
|
||||||
use ruff_python_ast::{self as ast, Expr, Ranged};
|
|
||||||
|
|
||||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
|
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
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::checkers::ast::Checker;
|
||||||
use crate::registry::AsRule;
|
use crate::registry::AsRule;
|
||||||
|
|
||||||
|
@ -51,8 +50,8 @@ pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast:
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
for expr in &arguments.args {
|
for base in &arguments.args {
|
||||||
let Expr::Name(ast::ExprName { id, .. }) = expr else {
|
let Expr::Name(ast::ExprName { id, .. }) = base else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
if id != "object" {
|
if id != "object" {
|
||||||
|
@ -66,19 +65,12 @@ pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast:
|
||||||
UselessObjectInheritance {
|
UselessObjectInheritance {
|
||||||
name: class_def.name.to_string(),
|
name: class_def.name.to_string(),
|
||||||
},
|
},
|
||||||
expr.range(),
|
base.range(),
|
||||||
);
|
);
|
||||||
if checker.patch(diagnostic.kind.rule()) {
|
if checker.patch(diagnostic.kind.rule()) {
|
||||||
diagnostic.try_set_fix(|| {
|
diagnostic.try_set_fix(|| {
|
||||||
let edit = remove_argument(
|
remove_argument(base, arguments, Parentheses::Remove, checker.locator())
|
||||||
checker.locator(),
|
.map(Fix::automatic)
|
||||||
class_def.name.end(),
|
|
||||||
expr.range(),
|
|
||||||
&arguments.args,
|
|
||||||
&arguments.keywords,
|
|
||||||
true,
|
|
||||||
)?;
|
|
||||||
Ok(Fix::automatic(edit))
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
checker.diagnostics.push(diagnostic);
|
checker.diagnostics.push(diagnostic);
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue