Parenthesize expressions prior to LibCST parsing (#6742)

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

This PR adds a utility for transforming expressions via LibCST that
automatically wraps the expression in parentheses, applies a
user-provided transformation, then strips the parentheses from the
generated code. LibCST can't parse arbitrary expression ranges, since
some expressions may require parenthesization in order to be parsed
properly. For example:

```python
option = (
    '{name}={value}'
    .format(nam=name, value=value)
)
```

In this case, the expression range is:

```python
'{name}={value}'
    .format(nam=name, value=value)
```

Which isn't valid on its own. So, instead, we add "fake" parentheses
around the expression.

We were already doing this in a few places, so this is mostly
formalizing and DRYing up that pattern.

Closes https://github.com/astral-sh/ruff/issues/6720.
This commit is contained in:
Charlie Marsh 2023-08-22 13:45:05 -04:00 committed by GitHub
parent 5c1f7fd5dd
commit 214eb707a6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 304 additions and 266 deletions

View file

@ -2,6 +2,5 @@
"{bar}{}".format(1, bar=2, spam=3) # F522
"{bar:{spam}}".format(bar=2, spam=3) # No issues
"{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
# Not fixable
(''
.format(x=2))
.format(x=2)) # F522

View file

@ -28,6 +28,6 @@
"{1}{3}".format(1, 2, 3, 4) # F523, # F524
"{1} {8}".format(0, 1) # F523, # F524
# Not fixable
# Multiline
(''
.format(2))

View file

@ -381,34 +381,34 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Ok(summary) => {
if checker.enabled(Rule::StringDotFormatExtraNamedArguments) {
pyflakes::rules::string_dot_format_extra_named_arguments(
checker, &summary, keywords, location,
checker, call, &summary, keywords,
);
}
if checker
.enabled(Rule::StringDotFormatExtraPositionalArguments)
{
pyflakes::rules::string_dot_format_extra_positional_arguments(
checker, &summary, args, location,
checker, call, &summary, args,
);
}
if checker.enabled(Rule::StringDotFormatMissingArguments) {
pyflakes::rules::string_dot_format_missing_argument(
checker, &summary, args, keywords, location,
checker, call, &summary, args, keywords,
);
}
if checker.enabled(Rule::StringDotFormatMixingAutomatic) {
pyflakes::rules::string_dot_format_mixing_automatic(
checker, &summary, location,
checker, call, &summary,
);
}
if checker.enabled(Rule::FormatLiterals) {
pyupgrade::rules::format_literals(checker, &summary, call);
pyupgrade::rules::format_literals(checker, call, &summary);
}
if checker.enabled(Rule::FString) {
pyupgrade::rules::f_strings(
checker,
call,
&summary,
expr,
value,
checker.settings.line_length,
);

View file

@ -1,9 +1,11 @@
use crate::autofix::codemods::CodegenStylist;
use anyhow::{bail, Result};
use libcst_native::{
Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FunctionDef,
GeneratorExp, If, Import, ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda,
ListComp, Module, Name, SmallStatement, Statement, Suite, Tuple, With,
};
use ruff_python_codegen::Stylist;
pub(crate) fn match_module(module_text: &str) -> Result<Module> {
match libcst_native::parse_module(module_text, None) {
@ -12,13 +14,6 @@ pub(crate) fn match_module(module_text: &str) -> Result<Module> {
}
}
pub(crate) fn match_expression(expression_text: &str) -> Result<Expression> {
match libcst_native::parse_expression(expression_text) {
Ok(expression) => Ok(expression),
Err(_) => bail!("Failed to extract expression from source"),
}
}
pub(crate) fn match_statement(statement_text: &str) -> Result<Statement> {
match libcst_native::parse_statement(statement_text) {
Ok(statement) => Ok(statement),
@ -205,3 +200,59 @@ pub(crate) fn match_if<'a, 'b>(statement: &'a mut Statement<'b>) -> Result<&'a m
bail!("Expected Statement::Compound")
}
}
/// Given the source code for an expression, return the parsed [`Expression`].
///
/// If the expression is not guaranteed to be valid as a standalone expression (e.g., if it may
/// span multiple lines and/or require parentheses), use [`transform_expression`] instead.
pub(crate) fn match_expression(expression_text: &str) -> Result<Expression> {
match libcst_native::parse_expression(expression_text) {
Ok(expression) => Ok(expression),
Err(_) => bail!("Failed to extract expression from source"),
}
}
/// Run a transformation function over an expression.
///
/// Passing an expression to [`match_expression`] directly can lead to parse errors if the
/// expression is not a valid standalone expression (e.g., it was parenthesized in the original
/// source). This method instead wraps the expression in "fake" parentheses, runs the
/// transformation, then removes the "fake" parentheses.
pub(crate) fn transform_expression(
source_code: &str,
stylist: &Stylist,
func: impl FnOnce(Expression) -> Result<Expression>,
) -> Result<String> {
// Wrap the expression in parentheses.
let source_code = format!("({source_code})");
let expression = match_expression(&source_code)?;
// Run the function on the expression.
let expression = func(expression)?;
// Codegen the expression.
let mut source_code = expression.codegen_stylist(stylist);
// Drop the outer parentheses.
source_code.drain(0..1);
source_code.drain(source_code.len() - 1..source_code.len());
Ok(source_code)
}
/// Like [`transform_expression`], but operates on the source code of the expression, rather than
/// the parsed [`Expression`]. This _shouldn't_ exist, but does to accommodate lifetime issues.
pub(crate) fn transform_expression_text(
source_code: &str,
func: impl FnOnce(String) -> Result<String>,
) -> Result<String> {
// Wrap the expression in parentheses.
let source_code = format!("({source_code})");
// Run the function on the expression.
let mut transformed = func(source_code)?;
// Drop the outer parentheses.
transformed.drain(0..1);
transformed.drain(transformed.len() - 1..transformed.len());
Ok(transformed)
}

View file

@ -8,10 +8,9 @@ use ruff_python_codegen::Stylist;
use ruff_python_stdlib::str::{self};
use ruff_source_file::Locator;
use crate::autofix::codemods::CodegenStylist;
use crate::autofix::snippet::SourceCodeSnippet;
use crate::checkers::ast::Checker;
use crate::cst::matchers::{match_comparison, match_expression};
use crate::cst::matchers::{match_comparison, transform_expression};
use crate::registry::AsRule;
/// ## What it does
@ -96,68 +95,69 @@ fn is_constant_like(expr: &Expr) -> bool {
/// Generate a fix to reverse a comparison.
fn reverse_comparison(expr: &Expr, locator: &Locator, stylist: &Stylist) -> Result<String> {
let range = expr.range();
let contents = locator.slice(range);
let source_code = locator.slice(range);
let mut expression = match_expression(contents)?;
let comparison = match_comparison(&mut expression)?;
transform_expression(source_code, stylist, |mut expression| {
let comparison = match_comparison(&mut expression)?;
let left = (*comparison.left).clone();
let left = (*comparison.left).clone();
// Copy the right side to the left side.
comparison.left = Box::new(comparison.comparisons[0].comparator.clone());
// Copy the right side to the left side.
comparison.left = Box::new(comparison.comparisons[0].comparator.clone());
// Copy the left side to the right side.
comparison.comparisons[0].comparator = left;
// Copy the left side to the right side.
comparison.comparisons[0].comparator = left;
// Reverse the operator.
let op = comparison.comparisons[0].operator.clone();
comparison.comparisons[0].operator = match op {
CompOp::LessThan {
whitespace_before,
whitespace_after,
} => CompOp::GreaterThan {
whitespace_before,
whitespace_after,
},
CompOp::GreaterThan {
whitespace_before,
whitespace_after,
} => CompOp::LessThan {
whitespace_before,
whitespace_after,
},
CompOp::LessThanEqual {
whitespace_before,
whitespace_after,
} => CompOp::GreaterThanEqual {
whitespace_before,
whitespace_after,
},
CompOp::GreaterThanEqual {
whitespace_before,
whitespace_after,
} => CompOp::LessThanEqual {
whitespace_before,
whitespace_after,
},
CompOp::Equal {
whitespace_before,
whitespace_after,
} => CompOp::Equal {
whitespace_before,
whitespace_after,
},
CompOp::NotEqual {
whitespace_before,
whitespace_after,
} => CompOp::NotEqual {
whitespace_before,
whitespace_after,
},
_ => panic!("Expected comparison operator"),
};
// Reverse the operator.
let op = comparison.comparisons[0].operator.clone();
comparison.comparisons[0].operator = match op {
CompOp::LessThan {
whitespace_before,
whitespace_after,
} => CompOp::GreaterThan {
whitespace_before,
whitespace_after,
},
CompOp::GreaterThan {
whitespace_before,
whitespace_after,
} => CompOp::LessThan {
whitespace_before,
whitespace_after,
},
CompOp::LessThanEqual {
whitespace_before,
whitespace_after,
} => CompOp::GreaterThanEqual {
whitespace_before,
whitespace_after,
},
CompOp::GreaterThanEqual {
whitespace_before,
whitespace_after,
} => CompOp::LessThanEqual {
whitespace_before,
whitespace_after,
},
CompOp::Equal {
whitespace_before,
whitespace_after,
} => CompOp::Equal {
whitespace_before,
whitespace_after,
},
CompOp::NotEqual {
whitespace_before,
whitespace_after,
} => CompOp::NotEqual {
whitespace_before,
whitespace_after,
},
_ => panic!("Expected comparison operator"),
};
Ok(expression.codegen_stylist(stylist))
Ok(expression)
})
}
/// SIM300

View file

@ -1,93 +1,88 @@
use anyhow::{Context, Ok, Result};
use ruff_python_ast::{Expr, Ranged};
use ruff_text_size::TextRange;
use ruff_diagnostics::Edit;
use ruff_python_ast::{self as ast, Ranged};
use ruff_python_codegen::Stylist;
use ruff_python_semantic::Binding;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_source_file::Locator;
use crate::autofix::codemods::CodegenStylist;
use crate::cst::matchers::{match_call_mut, match_dict, match_expression};
use crate::cst::matchers::{match_call_mut, match_dict, transform_expression};
/// Generate a [`Edit`] to remove unused keys from format dict.
pub(super) fn remove_unused_format_arguments_from_dict(
unused_arguments: &[usize],
stmt: &Expr,
dict: &ast::ExprDict,
locator: &Locator,
stylist: &Stylist,
) -> Result<Edit> {
let module_text = locator.slice(stmt.range());
let mut tree = match_expression(module_text)?;
let dict = match_dict(&mut tree)?;
let source_code = locator.slice(dict.range());
transform_expression(source_code, stylist, |mut expression| {
let dict = match_dict(&mut expression)?;
// Remove the elements at the given indexes.
let mut index = 0;
dict.elements.retain(|_| {
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
// Remove the elements at the given indexes.
let mut index = 0;
dict.elements.retain(|_| {
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
Ok(Edit::range_replacement(
tree.codegen_stylist(stylist),
stmt.range(),
))
Ok(expression)
})
.map(|output| Edit::range_replacement(output, dict.range()))
}
/// Generate a [`Edit`] to remove unused keyword arguments from a `format` call.
pub(super) fn remove_unused_keyword_arguments_from_format_call(
unused_arguments: &[usize],
location: TextRange,
call: &ast::ExprCall,
locator: &Locator,
stylist: &Stylist,
) -> Result<Edit> {
let module_text = locator.slice(location);
let mut tree = match_expression(module_text)?;
let call = match_call_mut(&mut tree)?;
let source_code = locator.slice(call.range());
transform_expression(source_code, stylist, |mut expression| {
let call = match_call_mut(&mut expression)?;
// Remove the keyword arguments at the given indexes.
let mut index = 0;
call.args.retain(|arg| {
if arg.keyword.is_none() {
return true;
}
// Remove the keyword arguments at the given indexes.
let mut index = 0;
call.args.retain(|arg| {
if arg.keyword.is_none() {
return true;
}
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
Ok(Edit::range_replacement(
tree.codegen_stylist(stylist),
location,
))
Ok(expression)
})
.map(|output| Edit::range_replacement(output, call.range()))
}
/// Generate a [`Edit`] to remove unused positional arguments from a `format` call.
pub(crate) fn remove_unused_positional_arguments_from_format_call(
unused_arguments: &[usize],
location: TextRange,
call: &ast::ExprCall,
locator: &Locator,
stylist: &Stylist,
) -> Result<Edit> {
let module_text = locator.slice(location);
let mut tree = match_expression(module_text)?;
let call = match_call_mut(&mut tree)?;
let source_code = locator.slice(call.range());
transform_expression(source_code, stylist, |mut expression| {
let call = match_call_mut(&mut expression)?;
// Remove any unused arguments.
let mut index = 0;
call.args.retain(|_| {
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
// Remove any unused arguments.
let mut index = 0;
call.args.retain(|_| {
let is_unused = unused_arguments.contains(&index);
index += 1;
!is_unused
});
Ok(Edit::range_replacement(
tree.codegen_stylist(stylist),
location,
))
Ok(expression)
})
.map(|output| Edit::range_replacement(output, call.range()))
}
/// Generate a [`Edit`] to remove the binding from an exception handler.

View file

@ -1,6 +1,6 @@
use std::string::ToString;
use ruff_python_ast::{self as ast, Constant, Expr, Identifier, Keyword};
use ruff_python_ast::{self as ast, Constant, Expr, Identifier, Keyword, Ranged};
use ruff_text_size::TextRange;
use rustc_hash::FxHashSet;
@ -570,15 +570,16 @@ pub(crate) fn percent_format_extra_named_arguments(
if summary.num_positional > 0 {
return;
}
let Expr::Dict(ast::ExprDict { keys, .. }) = &right else {
let Expr::Dict(dict) = &right else {
return;
};
// If any of the keys are spread, abort.
if keys.iter().any(Option::is_none) {
if dict.keys.iter().any(Option::is_none) {
return;
}
let missing: Vec<(usize, &str)> = keys
let missing: Vec<(usize, &str)> = dict
.keys
.iter()
.enumerate()
.filter_map(|(index, key)| match key {
@ -613,7 +614,7 @@ pub(crate) fn percent_format_extra_named_arguments(
diagnostic.try_set_fix(|| {
let edit = remove_unused_format_arguments_from_dict(
&indexes,
right,
dict,
checker.locator(),
checker.stylist(),
)?;
@ -739,9 +740,9 @@ pub(crate) fn percent_format_star_requires_sequence(
/// F522
pub(crate) fn string_dot_format_extra_named_arguments(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
keywords: &[Keyword],
location: TextRange,
) {
// If there are any **kwargs, abort.
if has_star_star_kwargs(keywords) {
@ -773,14 +774,14 @@ pub(crate) fn string_dot_format_extra_named_arguments(
.collect();
let mut diagnostic = Diagnostic::new(
StringDotFormatExtraNamedArguments { missing: names },
location,
call.range(),
);
if checker.patch(diagnostic.kind.rule()) {
let indexes: Vec<usize> = missing.iter().map(|(index, _)| *index).collect();
diagnostic.try_set_fix(|| {
let edit = remove_unused_keyword_arguments_from_format_call(
&indexes,
location,
call,
checker.locator(),
checker.stylist(),
)?;
@ -793,9 +794,9 @@ pub(crate) fn string_dot_format_extra_named_arguments(
/// F523
pub(crate) fn string_dot_format_extra_positional_arguments(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
args: &[Expr],
location: TextRange,
) {
let missing: Vec<usize> = args
.iter()
@ -817,7 +818,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
.map(ToString::to_string)
.collect::<Vec<String>>(),
},
location,
call.range(),
);
if checker.patch(diagnostic.kind.rule()) {
// We can only fix if the positional arguments we're removing don't require re-indexing
@ -849,7 +850,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
diagnostic.try_set_fix(|| {
let edit = remove_unused_positional_arguments_from_format_call(
&missing,
location,
call,
checker.locator(),
checker.stylist(),
)?;
@ -863,10 +864,10 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
/// F524
pub(crate) fn string_dot_format_missing_argument(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
args: &[Expr],
keywords: &[Keyword],
location: TextRange,
) {
if has_star_args(args) || has_star_star_kwargs(keywords) {
return;
@ -898,7 +899,7 @@ pub(crate) fn string_dot_format_missing_argument(
if !missing.is_empty() {
checker.diagnostics.push(Diagnostic::new(
StringDotFormatMissingArguments { missing },
location,
call.range(),
));
}
}
@ -906,12 +907,13 @@ pub(crate) fn string_dot_format_missing_argument(
/// F525
pub(crate) fn string_dot_format_mixing_automatic(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
location: TextRange,
) {
if !(summary.autos.is_empty() || summary.indices.is_empty()) {
checker
.diagnostics
.push(Diagnostic::new(StringDotFormatMixingAutomatic, location));
checker.diagnostics.push(Diagnostic::new(
StringDotFormatMixingAutomatic,
call.range(),
));
}
}

View file

@ -33,7 +33,7 @@ F522.py:2:1: F522 [*] `.format` call has unused named argument(s): spam
2 |+"{bar}{}".format(1, bar=2, ) # F522
3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues
4 4 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
5 5 | # Not fixable
5 5 | (''
F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham
|
@ -41,8 +41,8 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham
3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues
4 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F522
5 | # Not fixable
6 | (''
5 | (''
6 | .format(x=2)) # F522
|
= help: Remove extra named arguments: eggs, ham
@ -52,19 +52,25 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham
3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues
4 |-"{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
4 |+"{bar:{spam}}".format(bar=2, spam=3, ) # F522
5 5 | # Not fixable
6 6 | (''
7 7 | .format(x=2))
5 5 | (''
6 6 | .format(x=2)) # F522
F522.py:6:2: F522 `.format` call has unused named argument(s): x
F522.py:5:2: F522 [*] `.format` call has unused named argument(s): x
|
3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues
4 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
5 | # Not fixable
6 | (''
5 | (''
| __^
7 | | .format(x=2))
6 | | .format(x=2)) # F522
| |_____________^ F522
|
= help: Remove extra named arguments: x
Fix
3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues
4 4 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
5 5 | (''
6 |- .format(x=2)) # F522
6 |+ .format()) # F522

View file

@ -243,13 +243,13 @@ F523.py:29:1: F523 `.format` call has unused arguments at position(s): 0
29 | "{1} {8}".format(0, 1) # F523, # F524
| ^^^^^^^^^^^^^^^^^^^^^^ F523
30 |
31 | # Not fixable
31 | # Multiline
|
= help: Remove extra positional arguments at position(s): 0
F523.py:32:2: F523 `.format` call has unused arguments at position(s): 0
F523.py:32:2: F523 [*] `.format` call has unused arguments at position(s): 0
|
31 | # Not fixable
31 | # Multiline
32 | (''
| __^
33 | | .format(2))
@ -257,4 +257,11 @@ F523.py:32:2: F523 `.format` call has unused arguments at position(s): 0
|
= help: Remove extra positional arguments at position(s): 0
Fix
30 30 |
31 31 | # Multiline
32 32 | (''
33 |-.format(2))
33 |+.format())

View file

@ -1,18 +1,18 @@
use std::borrow::Cow;
use anyhow::{Context, Result};
use ruff_python_ast::{self as ast, Arguments, Constant, Expr, Keyword, Ranged};
use ruff_python_literal::format::{
FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate,
};
use ruff_python_parser::{lexer, Mode, Tok};
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{leading_quote, trailing_quote};
use ruff_python_ast::{self as ast, Constant, Expr, Keyword, Ranged};
use ruff_python_literal::format::{
FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate,
};
use ruff_python_parser::{lexer, Mode, Tok};
use ruff_source_file::Locator;
use ruff_text_size::TextRange;
use crate::checkers::ast::Checker;
use crate::line_width::LineLength;
@ -67,39 +67,34 @@ struct FormatSummaryValues<'a> {
}
impl<'a> FormatSummaryValues<'a> {
fn try_from_expr(expr: &'a Expr, locator: &'a Locator) -> Option<Self> {
fn try_from_call(call: &'a ast::ExprCall, locator: &'a Locator) -> Option<Self> {
let mut extracted_args: Vec<&Expr> = Vec::new();
let mut extracted_kwargs: FxHashMap<&str, &Expr> = FxHashMap::default();
if let Expr::Call(ast::ExprCall {
arguments: Arguments { args, keywords, .. },
..
}) = expr
{
for arg in args {
if matches!(arg, Expr::Starred(..))
|| contains_quotes(locator.slice(arg.range()))
|| locator.contains_line_break(arg.range())
{
return None;
}
extracted_args.push(arg);
for arg in &call.arguments.args {
if matches!(arg, Expr::Starred(..))
|| contains_quotes(locator.slice(arg.range()))
|| locator.contains_line_break(arg.range())
{
return None;
}
for keyword in keywords {
let Keyword {
arg,
value,
range: _,
} = keyword;
let Some(key) = arg else {
return None;
};
if contains_quotes(locator.slice(value.range()))
|| locator.contains_line_break(value.range())
{
return None;
}
extracted_kwargs.insert(key, value);
extracted_args.push(arg);
}
for keyword in &call.arguments.keywords {
let Keyword {
arg,
value,
range: _,
} = keyword;
let Some(key) = arg else {
return None;
};
if contains_quotes(locator.slice(value.range()))
|| locator.contains_line_break(value.range())
{
return None;
}
extracted_kwargs.insert(key, value);
}
if extracted_args.is_empty() && extracted_kwargs.is_empty() {
@ -309,8 +304,8 @@ fn try_convert_to_f_string(
/// UP032
pub(crate) fn f_strings(
checker: &mut Checker,
call: &ast::ExprCall,
summary: &FormatSummary,
expr: &Expr,
template: &Expr,
line_length: LineLength,
) {
@ -318,14 +313,7 @@ pub(crate) fn f_strings(
return;
}
let Expr::Call(ast::ExprCall {
func, arguments, ..
}) = expr
else {
return;
};
let Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() else {
let Expr::Attribute(ast::ExprAttribute { value, .. }) = call.func.as_ref() else {
return;
};
@ -339,14 +327,14 @@ pub(crate) fn f_strings(
return;
};
let Some(mut summary) = FormatSummaryValues::try_from_expr(expr, checker.locator()) else {
let Some(mut summary) = FormatSummaryValues::try_from_call(call, checker.locator()) else {
return;
};
let mut patches: Vec<(TextRange, String)> = vec![];
let mut lex = lexer::lex_starts_at(
checker.locator().slice(func.range()),
checker.locator().slice(call.func.range()),
Mode::Expression,
expr.start(),
call.start(),
)
.flatten();
let end = loop {
@ -384,8 +372,8 @@ pub(crate) fn f_strings(
return;
}
let mut contents = String::with_capacity(checker.locator().slice(expr.range()).len());
let mut prev_end = expr.start();
let mut contents = String::with_capacity(checker.locator().slice(call.range()).len());
let mut prev_end = call.start();
for (range, fstring) in patches {
contents.push_str(
checker
@ -415,7 +403,7 @@ pub(crate) fn f_strings(
// If necessary, add a space between any leading keyword (`return`, `yield`, `assert`, etc.)
// and the string. For example, `return"foo"` is valid, but `returnf"foo"` is not.
let existing = checker.locator().slice(TextRange::up_to(expr.start()));
let existing = checker.locator().slice(TextRange::up_to(call.start()));
if existing
.chars()
.last()
@ -424,7 +412,7 @@ pub(crate) fn f_strings(
contents.insert(0, ' ');
}
let mut diagnostic = Diagnostic::new(FString, expr.range());
let mut diagnostic = Diagnostic::new(FString, call.range());
// Avoid autofix if there are comments within the call:
// ```
@ -436,11 +424,11 @@ pub(crate) fn f_strings(
&& !checker
.indexer()
.comment_ranges()
.intersects(arguments.range())
.intersects(call.arguments.range())
{
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
contents,
expr.range(),
call.range(),
)));
};
checker.diagnostics.push(diagnostic);

View file

@ -2,16 +2,18 @@ use anyhow::{anyhow, Result};
use libcst_native::{Arg, Expression};
use once_cell::sync::Lazy;
use regex::Regex;
use ruff_python_ast::{self as ast, Expr, Ranged};
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Ranged};
use ruff_python_codegen::Stylist;
use ruff_source_file::Locator;
use crate::autofix::codemods::CodegenStylist;
use crate::checkers::ast::Checker;
use crate::cst::matchers::{match_attribute, match_call_mut, match_expression};
use crate::cst::matchers::{
match_attribute, match_call_mut, match_expression, transform_expression_text,
};
use crate::registry::AsRule;
use crate::rules::pyflakes::format::FormatSummary;
@ -58,8 +60,8 @@ impl Violation for FormatLiterals {
/// UP030
pub(crate) fn format_literals(
checker: &mut Checker,
summary: &FormatSummary,
call: &ast::ExprCall,
summary: &FormatSummary,
) {
// The format we expect is, e.g.: `"{0} {1}".format(...)`
if summary.has_nested_parts {
@ -112,10 +114,8 @@ pub(crate) fn format_literals(
let mut diagnostic = Diagnostic::new(FormatLiterals, call.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
Ok(Fix::suggested(Edit::range_replacement(
generate_call(call, arguments, checker.locator(), checker.stylist())?,
call.range(),
)))
generate_call(call, arguments, checker.locator(), checker.stylist())
.map(|suggestion| Fix::suggested(Edit::range_replacement(suggestion, call.range())))
});
}
checker.diagnostics.push(diagnostic);
@ -165,7 +165,7 @@ fn remove_specifiers<'a>(value: &mut Expression<'a>, arena: &'a typed_arena::Are
}
/// Return the corrected argument vector.
fn generate_arguments<'a>(arguments: &[Arg<'a>], order: &'a [usize]) -> Result<Vec<Arg<'a>>> {
fn generate_arguments<'a>(arguments: &[Arg<'a>], order: &[usize]) -> Result<Vec<Arg<'a>>> {
let mut new_arguments: Vec<Arg> = Vec::with_capacity(arguments.len());
for (idx, given) in order.iter().enumerate() {
// We need to keep the formatting in the same order but move the values.
@ -205,28 +205,27 @@ fn generate_call(
locator: &Locator,
stylist: &Stylist,
) -> Result<String> {
let content = locator.slice(call.range());
let parenthesized_content = format!("({content})");
let mut expression = match_expression(&parenthesized_content)?;
let source_code = locator.slice(call.range());
// Fix the call arguments.
let call = match_call_mut(&mut expression)?;
if let Arguments::Reorder(order) = arguments {
call.args = generate_arguments(&call.args, order)?;
}
let output = transform_expression_text(source_code, |source_code| {
let mut expression = match_expression(&source_code)?;
// Fix the string itself.
let item = match_attribute(&mut call.func)?;
let arena = typed_arena::Arena::new();
remove_specifiers(&mut item.value, &arena);
// Fix the call arguments.
let call = match_call_mut(&mut expression)?;
if let Arguments::Reorder(order) = arguments {
call.args = generate_arguments(&call.args, order)?;
}
// Remove the parentheses (first and last characters).
let mut output = expression.codegen_stylist(stylist);
output.remove(0);
output.pop();
// Fix the string itself.
let item = match_attribute(&mut call.func)?;
let arena = typed_arena::Arena::new();
remove_specifiers(&mut item.value, &arena);
Ok(expression.codegen_stylist(stylist))
})?;
// Ex) `'{' '0}'.format(1)`
if output == content {
if output == source_code {
return Err(anyhow!("Unable to identify format literals"));
}

View file

@ -2,16 +2,15 @@ use anyhow::{bail, Result};
use libcst_native::{
ConcatenatedString, Expression, FormattedStringContent, FormattedStringExpression,
};
use ruff_python_ast::{self as ast, Arguments, Expr, Ranged};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, Expr, Ranged};
use ruff_python_codegen::Stylist;
use ruff_source_file::Locator;
use crate::autofix::codemods::CodegenStylist;
use crate::checkers::ast::Checker;
use crate::cst::matchers::{match_call_mut, match_expression, match_name};
use crate::cst::matchers::{match_call_mut, match_name, transform_expression};
use crate::registry::AsRule;
/// ## What it does
@ -139,36 +138,28 @@ fn convert_call_to_conversion_flag(
locator: &Locator,
stylist: &Stylist,
) -> Result<Fix> {
// Parenthesize the expression, to support implicit concatenation.
let range = expr.range();
let content = locator.slice(range);
let parenthesized_content = format!("({content})");
let mut expression = match_expression(&parenthesized_content)?;
// Replace the formatted call expression at `index` with a conversion flag.
let formatted_string_expression = match_part(index, &mut expression)?;
let call = match_call_mut(&mut formatted_string_expression.expression)?;
let name = match_name(&call.func)?;
match name.value {
"str" => {
formatted_string_expression.conversion = Some("s");
let source_code = locator.slice(expr.range());
transform_expression(source_code, stylist, |mut expression| {
// Replace the formatted call expression at `index` with a conversion flag.
let formatted_string_expression = match_part(index, &mut expression)?;
let call = match_call_mut(&mut formatted_string_expression.expression)?;
let name = match_name(&call.func)?;
match name.value {
"str" => {
formatted_string_expression.conversion = Some("s");
}
"repr" => {
formatted_string_expression.conversion = Some("r");
}
"ascii" => {
formatted_string_expression.conversion = Some("a");
}
_ => bail!("Unexpected function call: `{:?}`", name.value),
}
"repr" => {
formatted_string_expression.conversion = Some("r");
}
"ascii" => {
formatted_string_expression.conversion = Some("a");
}
_ => bail!("Unexpected function call: `{:?}`", name.value),
}
formatted_string_expression.expression = call.args[0].value.clone();
// Remove the parentheses (first and last characters).
let mut content = expression.codegen_stylist(stylist);
content.remove(0);
content.pop();
Ok(Fix::automatic(Edit::range_replacement(content, range)))
formatted_string_expression.expression = call.args[0].value.clone();
Ok(expression)
})
.map(|output| Fix::automatic(Edit::range_replacement(output, expr.range())))
}
/// Return the [`FormattedStringContent`] at the given index in an f-string or implicit