mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-11-25 22:29:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Add autofix functionality for F523 (#3613)

Browse source
This commit is contained in:
Jonathan Plasse 2023-03-21 04:55:23 +01:00 • committed by GitHub
parent 626169e2ef
commit b5edc6dfc9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 283 additions and 87 deletions
Download patch file Download diff file Expand all files Collapse all files

1
crates/ruff/resources/test/fixtures/pyflakes/F523.py vendored
View file

@ -4,6 +4,7 @@
"{1:{0}}".format(1, 2) # No issues
"{1:{0}}".format(1, 2, 3) # F523
"{0}{2}".format(1, 2) # F523, # F524
"{1.arg[1]!r:0{2['arg']}{1}}".format(1, 2, 3, 4) # F523
# With no indexes
"{}".format(1, 2) # F523

31
crates/ruff/src/cst/matchers.rs
View file

@ -1,6 +1,7 @@
use anyhow::{bail, Result};
use libcst_native::{
Call, Comparison, Expr, Expression, Import, ImportFrom, Module, SmallStatement, Statement,
Attribute, Call, Comparison, Dict, Expr, Expression, Import, ImportFrom, Module, SimpleString,
SmallStatement, Statement,
};
pub fn match_module(module_text: &str) -> Result<Module> {
@ -70,3 +71,31 @@ pub fn match_comparison<'a, 'b>(
bail!("Expected Expression::Comparison")
}
}
pub fn match_dict<'a, 'b>(expression: &'a mut Expression<'b>) -> Result<&'a mut Dict<'b>> {
if let Expression::Dict(dict) = expression {
Ok(dict)
} else {
bail!("Expected Expression::Dict")
}
}
pub fn match_attribute<'a, 'b>(
expression: &'a mut Expression<'b>,
) -> Result<&'a mut Attribute<'b>> {
if let Expression::Attribute(attribute) = expression {
Ok(attribute)
} else {
bail!("Expected Expression::Attribute")
}
}
pub fn match_simple_string<'a, 'b>(
expression: &'a mut Expression<'b>,
) -> Result<&'a mut SimpleString<'b>> {
if let Expression::SimpleString(simple_string) = expression {
Ok(simple_string)
} else {
bail!("Expected Expression::SimpleString")
}
}

179
crates/ruff/src/rules/pyflakes/fixes.rs
View file

@ -1,5 +1,5 @@
use anyhow::{bail, Result};
use libcst_native::{Call, Codegen, CodegenState, Dict, DictElement, Expression};
use anyhow::{bail, Ok, Result};
use libcst_native::{Codegen, CodegenState, DictElement, Expression};
use rustpython_parser::ast::{Excepthandler, Expr};
use rustpython_parser::{lexer, Mode, Tok};
@ -7,8 +7,13 @@ use ruff_diagnostics::Fix;
use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::str::raw_contents;
use ruff_python_ast::types::Range;
use rustpython_common::format::{
FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate,
};
use crate::cst::matchers::{match_expr, match_module};
use crate::cst::matchers::{
match_attribute, match_call, match_dict, match_expression, match_simple_string,
};
/// Generate a [`Fix`] to remove unused keys from format dict.
pub fn remove_unused_format_arguments_from_dict(
@ -18,34 +23,15 @@ pub fn remove_unused_format_arguments_from_dict(
stylist: &Stylist,
) -> Result<Fix> {
let module_text = locator.slice(stmt);
let mut tree = match_module(module_text)?;
let mut body = match_expr(&mut tree)?;
let mut tree = match_expression(module_text)?;
let dict = match_dict(&mut tree)?;
let new_dict = {
let Expression::Dict(dict) = &body.value else {
bail!("Expected Expression::Dict")
};
Dict {
lbrace: dict.lbrace.clone(),
lpar: dict.lpar.clone(),
rbrace: dict.rbrace.clone(),
rpar: dict.rpar.clone(),
elements: dict
.elements
.iter()
.filter_map(|e| match e {
DictElement::Simple {
key: Expression::SimpleString(name),
..
} if unused_arguments.contains(&raw_contents(name.value)) => None,
e => Some(e.clone()),
})
.collect(),
}
};
body.value = Expression::Dict(Box::new(new_dict));
dict.elements.retain(|e| {
!matches!(e, DictElement::Simple {
key: Expression::SimpleString(name),
..
} if unused_arguments.contains(&raw_contents(name.value)))
});
let mut state = CodegenState {
default_newline: stylist.line_ending(),
@ -61,7 +47,7 @@ pub fn remove_unused_format_arguments_from_dict(
))
}
/// Generate a [`Fix`] to remove unused keyword arguments from format call.
/// Generate a [`Fix`] to remove unused keyword arguments from a `format` call.
pub fn remove_unused_keyword_arguments_from_format_call(
unused_arguments: &[&str],
location: Range,
@ -69,32 +55,119 @@ pub fn remove_unused_keyword_arguments_from_format_call(
stylist: &Stylist,
) -> Result<Fix> {
let module_text = locator.slice(location);
let mut tree = match_module(module_text)?;
let mut body = match_expr(&mut tree)?;
let mut tree = match_expression(module_text)?;
let call = match_call(&mut tree)?;
let new_call = {
let Expression::Call(call) = &body.value else {
bail!("Expected Expression::Call")
};
call.args
.retain(|e| !matches!(&e.keyword, Some(kw) if unused_arguments.contains(&kw.value)));
Call {
func: call.func.clone(),
lpar: call.lpar.clone(),
rpar: call.rpar.clone(),
whitespace_before_args: call.whitespace_before_args.clone(),
whitespace_after_func: call.whitespace_after_func.clone(),
args: call
.args
.iter()
.filter_map(|e| match &e.keyword {
Some(kw) if unused_arguments.contains(&kw.value) => None,
_ => Some(e.clone()),
})
.collect(),
}
let mut state = CodegenState {
default_newline: stylist.line_ending(),
default_indent: stylist.indentation(),
..CodegenState::default()
};
tree.codegen(&mut state);
body.value = Expression::Call(Box::new(new_call));
Ok(Fix::replacement(
state.to_string(),
location.location,
location.end_location,
))
}
fn unparse_format_part(format_part: FormatPart) -> String {
match format_part {
FormatPart::Literal(literal) => literal,
FormatPart::Field {
field_name,
conversion_spec,
format_spec,
} => {
let mut field_name = field_name;
if let Some(conversion) = conversion_spec {
field_name.push_str(&format!("!{conversion}"));
}
if !format_spec.is_empty() {
field_name.push_str(&format!(":{format_spec}"));
}
format!("{{{field_name}}}")
}
}
}
fn update_field_types(format_string: &FormatString, min_unused: usize) -> String {
format_string
.format_parts
.iter()
.map(|part| match part {
FormatPart::Literal(literal) => FormatPart::Literal(literal.to_string()),
FormatPart::Field {
field_name,
conversion_spec,
format_spec,
} => {
let new_field_name = FieldName::parse(field_name).unwrap(); // This should never fail because we parsed it before
let mut new_field_name_string = match new_field_name.field_type {
FieldType::Auto => String::new(),
FieldType::Index(i) => (i - min_unused).to_string(),
FieldType::Keyword(keyword) => keyword,
};
for field_name_part in &new_field_name.parts {
let field_name_part_string = match field_name_part {
FieldNamePart::Attribute(attribute) => format!(".{attribute}"),
FieldNamePart::Index(i) => format!("[{i}]"),
FieldNamePart::StringIndex(s) => format!("[{s}]"),
};
new_field_name_string.push_str(&field_name_part_string);
}
let new_format_spec = FormatString::from_str(format_spec).unwrap(); // This should never fail because we parsed it before
let new_format_spec_string = update_field_types(&new_format_spec, min_unused);
FormatPart::Field {
field_name: new_field_name_string,
conversion_spec: *conversion_spec,
format_spec: new_format_spec_string,
}
}
})
.map(unparse_format_part)
.collect()
}
/// Generate a [`Fix`] to remove unused positional arguments from a `format` call.
pub fn remove_unused_positional_arguments_from_format_call(
unused_arguments: &[usize],
location: Range,
locator: &Locator,
stylist: &Stylist,
format_string: &FormatString,
) -> Result<Fix> {
let module_text = locator.slice(location);
let mut tree = match_expression(module_text)?;
let call = match_call(&mut tree)?;
let mut index = 0;
call.args.retain(|_| {
index += 1;
!unused_arguments.contains(&(index - 1))
});
let mut min_unused_index = 0;
for index in unused_arguments {
if *index == min_unused_index {
min_unused_index += 1;
} else {
break;
}
}
let mut new_format_string;
if min_unused_index > 0 {
let func = match_attribute(&mut call.func)?;
let simple_string = match_simple_string(&mut func.value)?;
new_format_string = update_field_types(format_string, min_unused_index);
new_format_string = format!(r#""{new_format_string}""#);
simple_string.value = new_format_string.as_str();
}
let mut state = CodegenState {
default_newline: stylist.line_ending(),

8
crates/ruff/src/rules/pyflakes/format.rs
View file

@ -26,6 +26,7 @@ pub(crate) struct FormatSummary {
pub indexes: Vec<usize>,
pub keywords: Vec<String>,
pub has_nested_parts: bool,
pub format_string: FormatString,
}
impl TryFrom<&str> for FormatSummary {
@ -39,7 +40,7 @@ impl TryFrom<&str> for FormatSummary {
let mut keywords = Vec::new();
let mut has_nested_parts = false;
for format_part in format_string.format_parts {
for format_part in &format_string.format_parts {
let FormatPart::Field {
field_name,
format_spec,
@ -47,14 +48,14 @@ impl TryFrom<&str> for FormatSummary {
} = format_part else {
continue;
};
let parsed = FieldName::parse(&field_name)?;
let parsed = FieldName::parse(field_name)?;
match parsed.field_type {
FieldType::Auto => autos.push(autos.len()),
FieldType::Index(i) => indexes.push(i),
FieldType::Keyword(k) => keywords.push(k),
};
let nested = FormatString::from_str(&format_spec)?;
let nested = FormatString::from_str(format_spec)?;
for nested_part in nested.format_parts {
let FormatPart::Field { field_name, .. } = nested_part else {
continue;
@ -74,6 +75,7 @@ impl TryFrom<&str> for FormatSummary {
indexes,
keywords,
has_nested_parts,
format_string,
})
}
}

28
crates/ruff/src/rules/pyflakes/rules/strings.rs
View file

@ -14,6 +14,7 @@ use crate::registry::AsRule;
use super::super::cformat::CFormatSummary;
use super::super::fixes::{
remove_unused_format_arguments_from_dict, remove_unused_keyword_arguments_from_format_call,
remove_unused_positional_arguments_from_format_call,
};
use super::super::format::FormatSummary;
@ -169,13 +170,19 @@ pub struct StringDotFormatExtraPositionalArguments {
pub missing: Vec<String>,
}
impl Violation for StringDotFormatExtraPositionalArguments {
impl AlwaysAutofixableViolation for StringDotFormatExtraPositionalArguments {
#[derive_message_formats]
fn message(&self) -> String {
let StringDotFormatExtraPositionalArguments { missing } = self;
let message = missing.join(", ");
format!("`.format` call has unused arguments at position(s): {message}")
}
fn autofix_title(&self) -> String {
let StringDotFormatExtraPositionalArguments { missing } = self;
let message = missing.join(", ");
format!("Remove extra positional arguments at position(s): {message}")
}
}
#[violation]
@ -505,7 +512,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
return;
}
checker.diagnostics.push(Diagnostic::new(
let mut diagnostic = Diagnostic::new(
StringDotFormatExtraPositionalArguments {
missing: missing
.iter()
@ -513,7 +520,22 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
.collect::<Vec<String>>(),
},
location,
));
);
if checker.patch(diagnostic.kind.rule()) {
match remove_unused_positional_arguments_from_format_call(
&missing,
location,
checker.locator,
checker.stylist,
&summary.format_string,
) {
Ok(fix) => {
diagnostic.amend(fix);
}
Err(e) => error!("Failed to remove unused positional arguments: {e}"),
}
}
checker.diagnostics.push(diagnostic);
}
/// F524

123
crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F523_F523.py.snap
View file

@ -5,92 +5,161 @@ expression: diagnostics
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 1"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 1"
fixable: true
location:
row: 2
column: 0
end_location:
row: 2
column: 18
fix: ~
fix:
content: "\"{0}\".format(1, )"
location:
row: 2
column: 0
end_location:
row: 2
column: 18
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 0, 2"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 0, 2"
fixable: true
location:
row: 3
column: 0
end_location:
row: 3
column: 21
fix: ~
fix:
content: "\"{0}\".format(2, )"
location:
row: 3
column: 0
end_location:
row: 3
column: 21
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 2"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 2"
fixable: true
location:
row: 5
column: 0
end_location:
row: 5
column: 25
fix: ~
fix:
content: "\"{1:{0}}\".format(1, 2, )"
location:
row: 5
column: 0
end_location:
row: 5
column: 25
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 1"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 1"
fixable: true
location:
row: 6
column: 0
end_location:
row: 6
column: 21
fix: ~
fix:
content: "\"{0}{2}\".format(1, )"
location:
row: 6
column: 0
end_location:
row: 6
column: 21
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 0, 3"
suggestion: "Remove extra positional arguments at position(s): 0, 3"
fixable: true
location:
row: 7
column: 0
end_location:
row: 7
column: 48
fix:
content: "\"{0.arg[1]!r:0{1['arg']}{0}}\".format(2, 3, )"
location:
row: 7
column: 0
end_location:
row: 7
column: 48
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 1"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 1"
fixable: true
location:
row: 9
row: 10
column: 0
end_location:
row: 9
row: 10
column: 17
fix: ~
fix:
content: "\"{}\".format(1, )"
location:
row: 10
column: 0
end_location:
row: 10
column: 17
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 1, 2"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 1, 2"
fixable: true
location:
row: 10
row: 11
column: 0
end_location:
row: 10
row: 11
column: 20
fix: ~
fix:
content: "\"{}\".format(1, )"
location:
row: 11
column: 0
end_location:
row: 11
column: 20
parent: ~
- kind:
name: StringDotFormatExtraPositionalArguments
body: "`.format` call has unused arguments at position(s): 2"
suggestion: ~
fixable: false
suggestion: "Remove extra positional arguments at position(s): 2"
fixable: true
location:
row: 12
row: 13
column: 0
end_location:
row: 12
row: 13
column: 23
fix: ~
fix:
content: "\"{:{}}\".format(1, 2, )"
location:
row: 13
column: 0
end_location:
row: 13
column: 23
parent: ~