From a3f34cdd4859c3c28ceb4070688265b6be1a3751 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 14:38:43 -0600 Subject: [PATCH 01/10] add test fixture --- .../resources/test/fixtures/ruff/RUF046.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py index 2d44fe6a10727..5a49963fa8c12 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py @@ -160,3 +160,22 @@ async def f(): int(round(1, 0)) + +# function calls may need to retain parentheses +# if the parentheses for the call itself +# lie on the next line. +# See https://github.com/astral-sh/ruff/issues/15263 +int(round +(1)) + +int(round # a comment +# and another comment +(10) +) + +int(round (17)) # this is safe without parens + +int( round ( + 17 + )) # this is also safe without parens + From cef146fc30b0d2aa7238adfdaab5e5b46501d1dd Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 14:59:17 -0600 Subject: [PATCH 02/10] keep parens around calls if newlines between func and opening parens --- .../ruff/rules/unnecessary_cast_to_int.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) 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 f2f2b8c0a480a..7ae26a4d5c9d3 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 @@ -4,7 +4,7 @@ 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_python_trivia::{lines_after_ignoring_trivia, CommentRanges}; use ruff_source_file::LineRanges; use ruff_text_size::Ranged; @@ -114,7 +114,7 @@ fn unwrap_int_expression( let parenthesize = semantic.current_expression_parent().is_some() || argument.is_named_expr() || locator.count_lines(argument.range()) > 0; - if parenthesize && !has_own_parentheses(argument) { + if parenthesize && !has_own_parentheses(argument, source) { format!("({})", locator.slice(argument.range())) } else { locator.slice(argument.range()).to_string() @@ -229,16 +229,33 @@ fn round_applicability(arguments: &Arguments, semantic: &SemanticModel) -> Optio } /// Returns `true` if the given [`Expr`] has its own parentheses (e.g., `()`, `[]`, `{}`). -fn has_own_parentheses(expr: &Expr) -> bool { +fn has_own_parentheses(expr: &Expr, source: &str) -> bool { match expr { Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) - | Expr::Subscript(_) | Expr::List(_) | Expr::Set(_) - | Expr::Dict(_) - | Expr::Call(_) => true, + | Expr::Dict(_) => true, + Expr::Call(call_expr) => { + // A call where the function and parenthesized + // argument(s) appear on separate lines + // requires outer parentheses. That is: + // ``` + // (f + // (10)) + // ``` + // is different than + // ``` + // f + // (10) + // ``` + lines_after_ignoring_trivia(call_expr.func.end(), source) == 0 + } + Expr::Subscript(subscript_expr) => { + // Same as above + lines_after_ignoring_trivia(subscript_expr.value.end(), source) == 0 + } Expr::Generator(generator) => generator.parenthesized, Expr::Tuple(tuple) => tuple.parenthesized, _ => false, From febee966488c42ce30dd3caec843f032a3f8713a Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 15:00:25 -0600 Subject: [PATCH 03/10] update snapshot --- ...uff__tests__preview__RUF046_RUF046.py.snap | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) 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 69af6673a344f..965beb418cfb1 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 @@ -1005,6 +1005,8 @@ RUF046.py:161:1: RUF046 [*] Value being cast to `int` is already an integer 161 | / int(round(1, 162 | | 0)) | |_____________^ RUF046 +163 | +164 | # function calls may need to retain parentheses | = help: Remove unnecessary `int` call @@ -1016,3 +1018,99 @@ RUF046.py:161:1: RUF046 [*] Value being cast to `int` is already an integer 162 |- 0)) 161 |+round(1, 162 |+ 0) +163 163 | +164 164 | # function calls may need to retain parentheses +165 165 | # if the parentheses for the call itself + +RUF046.py:168:1: RUF046 [*] Value being cast to `int` is already an integer + | +166 | # lie on the next line. +167 | # See https://github.com/astral-sh/ruff/issues/15263 +168 | / int(round +169 | | (1)) + | |____^ RUF046 +170 | +171 | int(round # a comment + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +165 165 | # if the parentheses for the call itself +166 166 | # lie on the next line. +167 167 | # See https://github.com/astral-sh/ruff/issues/15263 +168 |-int(round + 168 |+(round +169 169 | (1)) +170 170 | +171 171 | int(round # a comment + +RUF046.py:171:1: RUF046 [*] Value being cast to `int` is already an integer + | +169 | (1)) +170 | +171 | / int(round # a comment +172 | | # and another comment +173 | | (10) +174 | | ) + | |_^ RUF046 +175 | +176 | int(round (17)) # this is safe without parens + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +168 168 | int(round +169 169 | (1)) +170 170 | +171 |-int(round # a comment + 171 |+(round # a comment +172 172 | # and another comment +173 |-(10) +174 |-) + 173 |+(10)) +175 174 | +176 175 | int(round (17)) # this is safe without parens +177 176 | + +RUF046.py:176:1: RUF046 [*] Value being cast to `int` is already an integer + | +174 | ) +175 | +176 | int(round (17)) # this is safe without parens + | ^^^^^^^^^^^^^^^ RUF046 +177 | +178 | int( round ( + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +173 173 | (10) +174 174 | ) +175 175 | +176 |-int(round (17)) # this is safe without parens + 176 |+round (17) # this is safe without parens +177 177 | +178 178 | int( round ( +179 179 | 17 + +RUF046.py:178:1: RUF046 [*] Value being cast to `int` is already an integer + | +176 | int(round (17)) # this is safe without parens +177 | +178 | / int( round ( +179 | | 17 +180 | | )) # this is also safe without parens + | |______________^ RUF046 + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +175 175 | +176 176 | int(round (17)) # this is safe without parens +177 177 | +178 |-int( round ( + 178 |+round ( +179 179 | 17 +180 |- )) # this is also safe without parens + 180 |+ ) # this is also safe without parens +181 181 | From cf826b797e14071aa4a3ff4a86f8e7ef5210439e Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 20:13:54 -0600 Subject: [PATCH 04/10] more test cases --- .../resources/test/fixtures/ruff/RUF046.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py index 5a49963fa8c12..5054172b1f1f1 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py @@ -179,3 +179,18 @@ async def f(): 17 )) # this is also safe without parens +int((round) # Comment +(42) +) + +int((round # Comment +)(42) +) + +int( # Comment +( # Comment + (round +) # Comment +)(42) +) + From 1a78ab9fe1c7f175a1ed94e7ada97d7908b036b7 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 20:14:26 -0600 Subject: [PATCH 05/10] use parenthesized range around func name --- .../ruff/rules/unnecessary_cast_to_int.rs | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) 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 7ae26a4d5c9d3..267316b2e4eb4 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 @@ -114,7 +114,7 @@ fn unwrap_int_expression( let parenthesize = semantic.current_expression_parent().is_some() || argument.is_named_expr() || locator.count_lines(argument.range()) > 0; - if parenthesize && !has_own_parentheses(argument, source) { + if parenthesize && !has_own_parentheses(argument, comment_ranges, source) { format!("({})", locator.slice(argument.range())) } else { locator.slice(argument.range()).to_string() @@ -229,7 +229,7 @@ fn round_applicability(arguments: &Arguments, semantic: &SemanticModel) -> Optio } /// Returns `true` if the given [`Expr`] has its own parentheses (e.g., `()`, `[]`, `{}`). -fn has_own_parentheses(expr: &Expr, source: &str) -> bool { +fn has_own_parentheses(expr: &Expr, comment_ranges: &CommentRanges, source: &str) -> bool { match expr { Expr::ListComp(_) | Expr::SetComp(_) @@ -250,11 +250,27 @@ fn has_own_parentheses(expr: &Expr, source: &str) -> bool { // f // (10) // ``` - lines_after_ignoring_trivia(call_expr.func.end(), source) == 0 + let func_end = parenthesized_range( + call_expr.func.as_ref().into(), + call_expr.into(), + comment_ranges, + source, + ) + .unwrap_or(call_expr.func.range()) + .end(); + lines_after_ignoring_trivia(func_end, source) == 0 } Expr::Subscript(subscript_expr) => { // Same as above - lines_after_ignoring_trivia(subscript_expr.value.end(), source) == 0 + let subscript_end = parenthesized_range( + subscript_expr.value.as_ref().into(), + subscript_expr.into(), + comment_ranges, + source, + ) + .unwrap_or(subscript_expr.value.range()) + .end(); + lines_after_ignoring_trivia(subscript_end, source) == 0 } Expr::Generator(generator) => generator.parenthesized, Expr::Tuple(tuple) => tuple.parenthesized, From f06cb775fe9785a96ff3eda63859259ae8c0b9a6 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Sun, 5 Jan 2025 20:14:33 -0600 Subject: [PATCH 06/10] update snapshot --- ...uff__tests__preview__RUF046_RUF046.py.snap | 83 ++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) 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 965beb418cfb1..78e13df760573 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 @@ -1101,6 +1101,8 @@ RUF046.py:178:1: RUF046 [*] Value being cast to `int` is already an integer 179 | | 17 180 | | )) # this is also safe without parens | |______________^ RUF046 +181 | +182 | int((round) # Comment | = help: Remove unnecessary `int` call @@ -1113,4 +1115,83 @@ RUF046.py:178:1: RUF046 [*] Value being cast to `int` is already an integer 179 179 | 17 180 |- )) # this is also safe without parens 180 |+ ) # this is also safe without parens -181 181 | +181 181 | +182 182 | int((round) # Comment +183 183 | (42) + +RUF046.py:182:1: RUF046 [*] Value being cast to `int` is already an integer + | +180 | )) # this is also safe without parens +181 | +182 | / int((round) # Comment +183 | | (42) +184 | | ) + | |_^ RUF046 +185 | +186 | int((round # Comment + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +179 179 | 17 +180 180 | )) # this is also safe without parens +181 181 | +182 |-int((round) # Comment +183 |-(42) +184 |-) + 182 |+((round) # Comment + 183 |+(42)) +185 184 | +186 185 | int((round # Comment +187 186 | )(42) + +RUF046.py:186:1: RUF046 [*] Value being cast to `int` is already an integer + | +184 | ) +185 | +186 | / int((round # Comment +187 | | )(42) +188 | | ) + | |_^ RUF046 +189 | +190 | int( # Comment + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +183 183 | (42) +184 184 | ) +185 185 | +186 |-int((round # Comment + 186 |+(round # Comment +187 187 | )(42) +188 |-) +189 188 | +190 189 | int( # Comment +191 190 | ( # Comment + +RUF046.py:190:1: RUF046 [*] Value being cast to `int` is already an integer + | +188 | ) +189 | +190 | / int( # Comment +191 | | ( # Comment +192 | | (round +193 | | ) # Comment +194 | | )(42) +195 | | ) + | |_^ RUF046 + | + = help: Remove unnecessary `int` call + +ℹ Safe fix +187 187 | )(42) +188 188 | ) +189 189 | +190 |-int( # Comment +191 190 | ( # Comment +192 191 | (round +193 192 | ) # Comment +194 193 | )(42) +195 |-) +196 194 | From d7ddf2e7e465e8b541ea37b3084959d2a7568bb6 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Tue, 7 Jan 2025 11:59:15 -0600 Subject: [PATCH 07/10] update test fixture --- .../resources/test/fixtures/ruff/RUF046.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py index 5054172b1f1f1..6fd21d779b8f6 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py @@ -187,10 +187,22 @@ async def f(): )(42) ) -int( # Comment +int( # Unsafe fix because of this comment ( # Comment (round ) # Comment )(42) ) +int( + round( + 42 + ) # unsafe fix because of this comment +) + +int( + round( + 42 + ) +# unsafe fix because of this comment +) From a11d4693b420d899eb60282f07c3bf0d76c10895 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Tue, 7 Jan 2025 11:59:47 -0600 Subject: [PATCH 08/10] unsafe if deletes comments --- .../ruff/rules/unnecessary_cast_to_int.rs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) 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 267316b2e4eb4..d5da8cedb3879 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 @@ -6,7 +6,7 @@ use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, Reso use ruff_python_semantic::SemanticModel; use ruff_python_trivia::{lines_after_ignoring_trivia, CommentRanges}; use ruff_source_file::LineRanges; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::unnecessary_round::{ @@ -97,7 +97,7 @@ pub(crate) fn unnecessary_cast_to_int(checker: &mut Checker, call: &ExprCall) { fn unwrap_int_expression( call: &ExprCall, argument: &Expr, - applicability: Applicability, + mut applicability: Applicability, semantic: &SemanticModel, locator: &Locator, comment_ranges: &CommentRanges, @@ -120,6 +120,22 @@ fn unwrap_int_expression( locator.slice(argument.range()).to_string() } }; + + // We are deleting the complement of the argument range within + // the call range, so we have to check both ends for comments. + // For example: + // ``` + // int( # comment + // round( + // 42.1 + // ) # comment + // ) + // ``` + let call_to_arg_start = TextRange::new(call.start(), argument.start()); + let arg_to_call_end = TextRange::new(argument.end(), call.end()); + if comment_ranges.intersects(call_to_arg_start) || comment_ranges.intersects(arg_to_call_end) { + applicability = Applicability::Unsafe; + } let edit = Edit::range_replacement(content, call.range()); Fix::applicable_edit(edit, applicability) } From 99a6db2f7929cabaa8c0b7b3c6d2eaf2956439e4 Mon Sep 17 00:00:00 2001 From: dylwil3 Date: Tue, 7 Jan 2025 11:59:55 -0600 Subject: [PATCH 09/10] update snapshot --- ...uff__tests__preview__RUF046_RUF046.py.snap | 73 +++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) 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 78e13df760573..26d34f017ab5c 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 @@ -1154,7 +1154,7 @@ RUF046.py:186:1: RUF046 [*] Value being cast to `int` is already an integer 188 | | ) | |_^ RUF046 189 | -190 | int( # Comment +190 | int( # Unsafe fix because of this comment | = help: Remove unnecessary `int` call @@ -1167,31 +1167,92 @@ RUF046.py:186:1: RUF046 [*] Value being cast to `int` is already an integer 187 187 | )(42) 188 |-) 189 188 | -190 189 | int( # Comment +190 189 | int( # Unsafe fix because of this comment 191 190 | ( # Comment RUF046.py:190:1: RUF046 [*] Value being cast to `int` is already an integer | 188 | ) 189 | -190 | / int( # Comment +190 | / int( # Unsafe fix because of this comment 191 | | ( # Comment 192 | | (round 193 | | ) # Comment 194 | | )(42) 195 | | ) | |_^ RUF046 +196 | +197 | int( | = help: Remove unnecessary `int` call -ℹ Safe fix +ℹ Unsafe fix 187 187 | )(42) 188 188 | ) 189 189 | -190 |-int( # Comment +190 |-int( # Unsafe fix because of this comment 191 190 | ( # Comment 192 191 | (round 193 192 | ) # Comment 194 193 | )(42) 195 |-) -196 194 | +196 194 | +197 195 | int( +198 196 | round( + +RUF046.py:197:1: RUF046 [*] Value being cast to `int` is already an integer + | +195 | ) +196 | +197 | / int( +198 | | round( +199 | | 42 +200 | | ) # unsafe fix because of this comment +201 | | ) + | |_^ RUF046 +202 | +203 | int( + | + = help: Remove unnecessary `int` call + +ℹ Unsafe fix +194 194 | )(42) +195 195 | ) +196 196 | +197 |-int( +198 |- round( + 197 |+round( +199 198 | 42 +200 |- ) # unsafe fix because of this comment +201 |-) + 199 |+ ) +202 200 | +203 201 | int( +204 202 | round( + +RUF046.py:203:1: RUF046 [*] Value being cast to `int` is already an integer + | +201 | ) +202 | +203 | / int( +204 | | round( +205 | | 42 +206 | | ) +207 | | # unsafe fix because of this comment +208 | | ) + | |_^ RUF046 + | + = help: Remove unnecessary `int` call + +ℹ Unsafe fix +200 200 | ) # unsafe fix because of this comment +201 201 | ) +202 202 | +203 |-int( +204 |- round( + 203 |+round( +205 204 | 42 +206 |- ) +207 |-# unsafe fix because of this comment +208 |-) + 205 |+ ) From 9dcb4d6fb12652f16eaf11171256847bf2f75e02 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 7 Jan 2025 16:39:06 -0500 Subject: [PATCH 10/10] Remove mut --- .../ruff/rules/unnecessary_cast_to_int.rs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) 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 d5da8cedb3879..4c67155629e94 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 @@ -97,7 +97,7 @@ pub(crate) fn unnecessary_cast_to_int(checker: &mut Checker, call: &ExprCall) { fn unwrap_int_expression( call: &ExprCall, argument: &Expr, - mut applicability: Applicability, + applicability: Applicability, semantic: &SemanticModel, locator: &Locator, comment_ranges: &CommentRanges, @@ -121,21 +121,29 @@ fn unwrap_int_expression( } }; - // We are deleting the complement of the argument range within - // the call range, so we have to check both ends for comments. + // Since we're deleting the complement of the argument range within + // the call range, we have to check both ends for comments. + // // For example: - // ``` + // ```python // int( # comment // round( // 42.1 // ) # comment // ) // ``` - let call_to_arg_start = TextRange::new(call.start(), argument.start()); - let arg_to_call_end = TextRange::new(argument.end(), call.end()); - if comment_ranges.intersects(call_to_arg_start) || comment_ranges.intersects(arg_to_call_end) { - applicability = Applicability::Unsafe; - } + let applicability = { + let call_to_arg_start = TextRange::new(call.start(), argument.start()); + let arg_to_call_end = TextRange::new(argument.end(), call.end()); + if comment_ranges.intersects(call_to_arg_start) + || comment_ranges.intersects(arg_to_call_end) + { + Applicability::Unsafe + } else { + applicability + } + }; + let edit = Edit::range_replacement(content, call.range()); Fix::applicable_edit(edit, applicability) }