From be654041feb32724920a94cc826139c1a4557fd7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 2 Jan 2025 17:28:13 -0500 Subject: [PATCH] Avoid syntax error when removing int over multiple lines --- .../resources/test/fixtures/ruff/RUF046.py | 6 ++ .../ruff/rules/unnecessary_cast_to_int.rs | 73 ++++++++++++++----- ...uff__tests__preview__RUF046_RUF046.py.snap | 42 ++++++++++- 3 files changed, 103 insertions(+), 18 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py index e857c0a67fa3c2..2d44fe6a107277 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py @@ -154,3 +154,9 @@ async def f(): int(1 @ 1) int(1. if ... else .2) + +int(1 + + 1) + +int(round(1, + 0)) diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs index d4d29388907df9..82e6b97e38402c 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs @@ -1,14 +1,19 @@ -use crate::checkers::ast::Checker; -use crate::rules::ruff::rules::unnecessary_round::{ - rounded_and_ndigits, InferredType, NdigitsValue, RoundedValue, -}; use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{Arguments, Expr, ExprCall}; use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType}; use ruff_python_semantic::SemanticModel; +use ruff_python_trivia::CommentRanges; +use ruff_source_file::LineRanges; use ruff_text_size::Ranged; +use crate::checkers::ast::Checker; +use crate::rules::ruff::rules::unnecessary_round::{ + rounded_and_ndigits, InferredType, NdigitsValue, RoundedValue, +}; +use crate::Locator; + /// ## What it does /// Checks for `int` conversions of values that are already integers. /// @@ -76,31 +81,48 @@ pub(crate) fn unnecessary_cast_to_int(checker: &mut Checker, call: &ExprCall) { return; }; - let fix = unwrap_int_expression(checker, call, argument, applicability); - let diagnostic = Diagnostic::new(UnnecessaryCastToInt, call.range); + let fix = unwrap_int_expression( + call, + argument, + applicability, + checker.semantic(), + checker.locator(), + checker.comment_ranges(), + checker.source(), + ); + let diagnostic = Diagnostic::new(UnnecessaryCastToInt, call.range()); checker.diagnostics.push(diagnostic.with_fix(fix)); } /// Creates a fix that replaces `int(expression)` with `expression`. fn unwrap_int_expression( - checker: &Checker, call: &ExprCall, argument: &Expr, applicability: Applicability, + semantic: &SemanticModel, + locator: &Locator, + comment_ranges: &CommentRanges, + source: &str, ) -> Fix { - let (locator, semantic) = (checker.locator(), checker.semantic()); - - let argument_expr = locator.slice(argument.range()); - - let has_parent_expr = semantic.current_expression_parent().is_some(); - let new_content = if has_parent_expr || argument.is_named_expr() { - format!("({argument_expr})") + let content = if let Some(range) = parenthesized_range( + argument.into(), + (&call.arguments).into(), + comment_ranges, + source, + ) { + locator.slice(range).to_string() } else { - argument_expr.to_string() + let parenthesize = semantic.current_expression_parent().is_some() + || argument.is_named_expr() + || locator.count_lines(argument.range()) > 0; + if parenthesize && !has_own_parentheses(argument) { + format!("({})", locator.slice(argument.range())) + } else { + locator.slice(argument.range()).to_string() + } }; - - let edit = Edit::range_replacement(new_content, call.range); + let edit = Edit::range_replacement(content, call.range()); Fix::applicable_edit(edit, applicability) } @@ -206,3 +228,20 @@ fn round_applicability(checker: &Checker, arguments: &Arguments) -> Option None, } } + +/// Returns `true` if the given [`Expr`] has its own parentheses (e.g., `()`, `[]`, `{}`). +fn has_own_parentheses(expr: &Expr) -> bool { + match expr { + Expr::ListComp(_) + | Expr::SetComp(_) + | Expr::DictComp(_) + | Expr::Subscript(_) + | Expr::List(_) + | Expr::Set(_) + | Expr::Dict(_) + | Expr::Call(_) => true, + Expr::Generator(generator) => generator.parenthesized, + Expr::Tuple(tuple) => tuple.parenthesized, + _ => false, + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF046_RUF046.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF046_RUF046.py.snap index 015d78d21bcf89..f5ccbf8d2af069 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF046_RUF046.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF046_RUF046.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs -snapshot_kind: text --- RUF046.py:10:1: RUF046 [*] Value being casted is already an integer | @@ -976,3 +975,44 @@ RUF046.py:76:1: RUF046 [*] Value being casted is already an integer 77 77 | 78 78 | 79 79 | ### No errors + +RUF046.py:158:1: RUF046 [*] Value being casted is already an integer + | +156 | int(1. if ... else .2) +157 | +158 | / int(1 + +159 | | 1) + | |______^ RUF046 +160 | +161 | int(round(1, + | + = help: Remove unnecessary conversion to `int` + +ℹ Safe fix +155 155 | +156 156 | int(1. if ... else .2) +157 157 | +158 |-int(1 + + 158 |+(1 + +159 159 | 1) +160 160 | +161 161 | int(round(1, + +RUF046.py:161:1: RUF046 [*] Value being casted is already an integer + | +159 | 1) +160 | +161 | / int(round(1, +162 | | 0)) + | |_____________^ RUF046 + | + = help: Remove unnecessary conversion to `int` + +ℹ Safe fix +158 158 | int(1 + +159 159 | 1) +160 160 | +161 |-int(round(1, +162 |- 0)) + 161 |+round(1, + 162 |+ 0)