diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F522.py b/crates/ruff/resources/test/fixtures/pyflakes/F522.py index 18e3d072f26a1..d229b23e451d0 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F522.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F522.py @@ -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)) \ No newline at end of file + .format(x=2)) # F522 diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F523.py b/crates/ruff/resources/test/fixtures/pyflakes/F523.py index d3dd1b68db7d1..94dbe7bb2911c 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F523.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F523.py @@ -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)) diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index 2ec42610eeb27..8ab8515e4aecc 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -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, ); diff --git a/crates/ruff/src/cst/matchers.rs b/crates/ruff/src/cst/matchers.rs index dad1e689eb142..70cc4b3aa351d 100644 --- a/crates/ruff/src/cst/matchers.rs +++ b/crates/ruff/src/cst/matchers.rs @@ -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 { match libcst_native::parse_module(module_text, None) { @@ -12,13 +14,6 @@ pub(crate) fn match_module(module_text: &str) -> Result { } } -pub(crate) fn match_expression(expression_text: &str) -> Result { - 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 { 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 { + 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, +) -> Result { + // 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, +) -> Result { + // 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) +} diff --git a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs index 2bed494d4af43..f65dd7665d3df 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -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 { let range = expr.range(); - let contents = locator.slice(range); - - let mut expression = match_expression(contents)?; - let comparison = match_comparison(&mut expression)?; - - let left = (*comparison.left).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; - - // 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"), - }; + let source_code = locator.slice(range); + + transform_expression(source_code, stylist, |mut expression| { + let comparison = match_comparison(&mut expression)?; + + let left = (*comparison.left).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; + + // 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 diff --git a/crates/ruff/src/rules/pyflakes/fixes.rs b/crates/ruff/src/rules/pyflakes/fixes.rs index 370541ed6612a..9a1aa46fb498c 100644 --- a/crates/ruff/src/rules/pyflakes/fixes.rs +++ b/crates/ruff/src/rules/pyflakes/fixes.rs @@ -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 { - let module_text = locator.slice(stmt.range()); - let mut tree = match_expression(module_text)?; - let dict = match_dict(&mut tree)?; - - // 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(), - )) + 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 + }); + + 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 { - let module_text = locator.slice(location); - let mut tree = match_expression(module_text)?; - let call = match_call_mut(&mut tree)?; - - // 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 - }); - - Ok(Edit::range_replacement( - tree.codegen_stylist(stylist), - location, - )) + 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; + } + + let is_unused = unused_arguments.contains(&index); + index += 1; + !is_unused + }); + + 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 { - let module_text = locator.slice(location); - let mut tree = match_expression(module_text)?; - let call = match_call_mut(&mut tree)?; - - // 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, - )) + 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 + }); + + Ok(expression) + }) + .map(|output| Edit::range_replacement(output, call.range())) } /// Generate a [`Edit`] to remove the binding from an exception handler. diff --git a/crates/ruff/src/rules/pyflakes/rules/strings.rs b/crates/ruff/src/rules/pyflakes/rules/strings.rs index 17d9be995b99f..1fdfb10e47a3c 100644 --- a/crates/ruff/src/rules/pyflakes/rules/strings.rs +++ b/crates/ruff/src/rules/pyflakes/rules/strings.rs @@ -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 = 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 = args .iter() @@ -817,7 +818,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments( .map(ToString::to_string) .collect::>(), }, - 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(), + )); } } diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap index 9ba19aa183578..8581e62d0b01b 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap @@ -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 + diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F523_F523.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F523_F523.py.snap index c566b9c64e0fb..f84189c4be3e6 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F523_F523.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F523_F523.py.snap @@ -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()) + diff --git a/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs b/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs index a4922f36a2f96..b086532130e45 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs @@ -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 { + fn try_from_call(call: &'a ast::ExprCall, locator: &'a Locator) -> Option { 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); diff --git a/crates/ruff/src/rules/pyupgrade/rules/format_literals.rs b/crates/ruff/src/rules/pyupgrade/rules/format_literals.rs index 90ad01b2281ea..00e5acb4af7c5 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/format_literals.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/format_literals.rs @@ -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>> { +fn generate_arguments<'a>(arguments: &[Arg<'a>], order: &[usize]) -> Result>> { let mut new_arguments: Vec = 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 { - let content = locator.slice(call.range()); - let parenthesized_content = format!("({content})"); - let mut expression = match_expression(&parenthesized_content)?; - - // 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 source_code = locator.slice(call.range()); + + let output = transform_expression_text(source_code, |source_code| { + let mut expression = match_expression(&source_code)?; + + // Fix the call arguments. + let call = match_call_mut(&mut expression)?; + if let Arguments::Reorder(order) = arguments { + call.args = generate_arguments(&call.args, order)?; + } - // 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 string itself. + let item = match_attribute(&mut call.func)?; + let arena = typed_arena::Arena::new(); + remove_specifiers(&mut item.value, &arena); - // Remove the parentheses (first and last characters). - let mut output = expression.codegen_stylist(stylist); - output.remove(0); - output.pop(); + Ok(expression.codegen_stylist(stylist)) + })?; // Ex) `'{' '0}'.format(1)` - if output == content { + if output == source_code { return Err(anyhow!("Unable to identify format literals")); } diff --git a/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs b/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs index 4739395116ec1..f8957b82b244f 100644 --- a/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs +++ b/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs @@ -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 { - // 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"); - } - "repr" => { - formatted_string_expression.conversion = Some("r"); - } - "ascii" => { - formatted_string_expression.conversion = Some("a"); + 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), } - _ => 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