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

UP032 complains about format args performing function calls #10258

Closed
ThiefMaster opened this issue Mar 6, 2024 · 1 comment · Fixed by #10265
Closed

UP032 complains about format args performing function calls #10258

ThiefMaster opened this issue Mar 6, 2024 · 1 comment · Fixed by #10265
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ThiefMaster
Copy link
Contributor

x = 'CASE WHEN {0} > {1} THEN {0} ELSE {1} END'.format(process(arg1), process(arg2))

In this case one can assume the format() call is intentional to avoid calling process() more than once per arg.

$ ruff version
ruff 0.3.1

$ ruff check --isolated --select UP032 --preview --no-cache --output-format concise rufftest/ruff_sample.py
rufftest/ruff_sample.py:1:5: UP032 Use f-string instead of `format` call
Found 1 error.

Of course it could be refactored like this, but the rule triggering there feels incorrect.

arg1 = process(arg1)
arg2 = process(arg2)
return f'CASE WHEN {arg1} > {arg2} THEN {arg1} ELSE {arg2} END'
@charliermarsh
Copy link
Member

Good call, let's fix this.

@charliermarsh charliermarsh added the good first issue Good for newcomers label Mar 7, 2024
@charliermarsh charliermarsh self-assigned this Mar 7, 2024
charliermarsh added a commit that referenced this issue Mar 7, 2024
## Summary

Given a format string like `"{x} {x}".format(x=foo())`, we should avoid
converting to an f-string, since doing so would require repeating the
function call (`f"{foo()} {foo()}"`), which could introduce side
effects.

Closes #10258.
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
## Summary

Given a format string like `"{x} {x}".format(x=foo())`, we should avoid
converting to an f-string, since doing so would require repeating the
function call (`f"{foo()} {foo()}"`), which could introduce side
effects.

Closes astral-sh#10258.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants