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

FURB157 has incorrect fixes when replacing strings with integers #14204

Closed
dscorbett opened this issue Nov 8, 2024 · 1 comment · Fixed by #14216
Closed

FURB157 has incorrect fixes when replacing strings with integers #14204

dscorbett opened this issue Nov 8, 2024 · 1 comment · Fixed by #14216
Assignees
Labels
bug Something isn't working fixes Related to suggested fixes for violations

Comments

@dscorbett
Copy link

The fixes for verbose-decimal-constructor (FURB157) are sometimes incorrect in Ruff 0.7.3. Two of them change behavior and one introduces a syntax error.

First, negative zero should be kept as a string. int doesn’t have signed zeroes but Decimal does.

$ cat furb157.py
from decimal import Decimal
print(Decimal("-0"))

$ python furb157.py
-0

$ ruff check --isolated --preview --select FURB157 furb157.py --fix
Found 1 error (1 fixed, 0 remaining).

$ cat furb157.py
from decimal import Decimal
print(Decimal(-0))

$ python furb157.py
0

Second, the empty string should not be normalized to zero. The empty string signals an InvalidOperation, which can be trapped and converted to NaN or can cause an error. “Empty” here refers to the string after normalizing away white space and underscores.

$ cat furb157.py
from decimal import Decimal, ExtendedContext
print(Decimal("_", ExtendedContext))
print(Decimal(" "))

$ python furb157.py
NaN
Traceback (most recent call last):
  File "furb157.py", line 3, in <module>
    print(Decimal(" "))
          ^^^^^^^^^^^
decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]

$ ruff check --isolated --preview --select FURB157 furb157.py --fix
Found 2 errors (2 fixed, 0 remaining).

$ cat furb157.py
cat furb157.py
from decimal import Decimal, ExtendedContext
print(Decimal(0, ExtendedContext))
print(Decimal(0))

$ python furb157.py
0
0

Third, decimal int literals with too many digits cause syntax errors. The minimum maximum digit count is 640. If the integer-valued string argument to Decimal has over that many digits, it should stay a string.

$ { printf 'from decimal import Decimal\nDecimal("1'; printf 0%0.s {1..640}; printf '")\n'; } >furb157.py

$ PYTHONINTMAXSTRDIGITS=640 python furb157.py

$ ruff check --isolated --preview --select FURB157 furb157.py --fix
Found 1 error (1 fixed, 0 remaining).

$ PYTHONINTMAXSTRDIGITS=640 python furb157.py 2>&1 | tail -n 1
SyntaxError: Exceeds the limit (640 digits) for integer string conversion: value has 641 digits; use sys.set_int_max_str_digits() to increase the limit - Consider hexadecimal for huge integer literals to avoid decimal conversion limits.
@dylwil3
Copy link
Collaborator

dylwil3 commented Nov 8, 2024

Fantastic, thank you!

@dylwil3 dylwil3 self-assigned this Nov 8, 2024
@AlexWaygood AlexWaygood added bug Something isn't working fixes Related to suggested fixes for violations labels Nov 8, 2024
charliermarsh pushed a commit that referenced this issue Nov 9, 2024
…r (FURB157)` (#14216)

This PR accounts for further subtleties in `Decimal` parsing:

- Strings which are empty modulo underscores and surrounding whitespace
are skipped
- `Decimal("-0")` is skipped
- `Decimal("{integer literal that is longer than 640 digits}")` are
skipped (see linked issue for explanation)

NB: The snapshot did not need to be updated since the new test cases are
"Ok" instances and added below the diff.

Closes #14204
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants