Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruff] Parenthesize arguments to int when removing int would change semantics in unnecessary-cast-to-int (RUF046) #15277

Merged
merged 10 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,37 @@ 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
dylwil3 marked this conversation as resolved.
Show resolved Hide resolved

int((round) # Comment
(42)
)

int((round # Comment
)(42)
)

int( # Comment
( # Comment
(round
) # Comment
)(42)
)

45 changes: 39 additions & 6 deletions crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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, comment_ranges, source) {
format!("({})", locator.slice(argument.range()))
} else {
locator.slice(argument.range()).to_string()
Expand Down Expand Up @@ -229,16 +229,49 @@ 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, comment_ranges: &CommentRanges, 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)
// ```
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) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add couple of test cases for subscript expressions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, type inference is not robust enough to ever apply this rule to a subscript. So I can't write a test that would trigger this branch. On the other hand, I think I should leave this match arm in case type inference is later improved to cover this case.

// Same as above
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,
_ => false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -1016,3 +1018,180 @@ 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
181 |
182 | int((round) # Comment
|
= 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 |
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 |-)
dylwil3 marked this conversation as resolved.
Show resolved Hide resolved
196 194 |
Loading