Skip to content

Comments

[ruff] Do not simplify round() calls (RUF046)#14832

Merged
MichaReiser merged 5 commits intoastral-sh:mainfrom
InSyncWithFoo:RUF046-1
Dec 9, 2024
Merged

[ruff] Do not simplify round() calls (RUF046)#14832
MichaReiser merged 5 commits intoastral-sh:mainfrom
InSyncWithFoo:RUF046-1

Conversation

@InSyncWithFoo
Copy link
Contributor

Summary

Part 1 of the big change introduced in #14828. This temporarily causes all fixes for round(...) to be considered unsafe, but they will eventually be enhanced.

Test Plan

cargo nextest run and cargo insta test.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks for splitting the PR!

I think the rule is now over eager in removing int calls from rount

@InSyncWithFoo
Copy link
Contributor Author

InSyncWithFoo commented Dec 8, 2024

For reference, here's the logic table for round() cases:

number ndigits round() Error/fix
Literal int Not given int Safe
Literal int Literal int int Safe
Literal int None int Safe
Literal int Other Other -
Literal float Not given int Safe
Literal float Literal int float -
Literal float None int Safe
Literal float Other Other -
Inferred int Not given int Unsafe
Inferred int Literal int int Unsafe
Inferred int None int Unsafe
Inferred int Other Other -
Inferred float Not given int Unsafe
Inferred float Literal int float -
Inferred float None int Unsafe
Inferred float Other Other -
Other Not given Likely int Unsafe
Other Literal int Other -
Other None Likely int Unsafe
Other Other Other -

@MichaReiser
Copy link
Member

MichaReiser commented Dec 9, 2024

Nice table: From testing a few cases, it seems that round(4, ndigits=10) always returns an int

>>> round(4, 10)
4
>>> type(_)
<class 'int'>

But we should not create a diagnostic if ndigits isn't an integer, e.g. round(4, 10.4)

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Dec 9, 2024
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice. This looks good now, except that we can also provide a safe fix if the number is an int, and ndigits is any integer.

@InSyncWithFoo
Copy link
Contributor Author

Logic changed and table updated. round(0, integer) and round(inferred_integer, integer) are now both fixable, safely and unsafely correspondingly.

@MichaReiser
Copy link
Member

Perfect, thank you

@MichaReiser MichaReiser merged commit c62ba48 into astral-sh:main Dec 9, 2024
@InSyncWithFoo InSyncWithFoo deleted the RUF046-1 branch December 9, 2024 16:39
dcreager added a commit that referenced this pull request Dec 10, 2024
* main:
  [`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) (#14887)
  Improve mdtests style (#14884)
  Reference `suppress-dummy-regex-options` in documentation of rules supporting it (#14888)
  [`flake8-bugbear`] `itertools.batched()` without explicit `strict` (`B911`) (#14408)
  [`ruff`] Mark autofix for `RUF052` as always unsafe (#14824)
  [red-knot] Improve type inference for except handlers (#14838)
  More typos found by codespell (#14880)
  [red-knot] move standalone expression_ty to TypeInferenceBuilder::file_expression_ty (#14879)
  [`ruff`] Do not simplify `round()` calls (`RUF046`) (#14832)
  Stop referring to early ruff versions (#14862)
  Fix a typo in `class.rs` (#14877)
  [`flake8-pyi`] Also remove `self` and `cls`'s annotation (`PYI034`) (#14801)
  [`pyupgrade`] Remove unreachable code in `UP015` implementation (#14871)
  [`flake8-bugbear`] Skip `B028` if `warnings.warn` is called with `*args` or `**kwargs` (#14870)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants