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

UP031 (printf-string-formatting) difference between Ruff and pyupgrade #8107

Closed
tdulcet opened this issue Oct 21, 2023 · 2 comments · Fixed by #9037
Closed

UP031 (printf-string-formatting) difference between Ruff and pyupgrade #8107

tdulcet opened this issue Oct 21, 2023 · 2 comments · Fixed by #9037
Assignees
Labels
fixes Related to suggested fixes for violations

Comments

@tdulcet
Copy link

tdulcet commented Oct 21, 2023

I ran Ruff on medium sized project and then pyupgrade. Ruff missed one UP031 autofix that pyupgrade fixed. Both Ruff and pyupgrade missed 96 UP031 (a known problem) and 23 UP032 (a few of these may be caused by #8106).

Ruff command:

ruff --target-version py310 --select UP --line-length 320 --unsafe-fixes --fix .

pyupgrade command:

pyupgrade --py310-plus **/*.py

Diff:

-       path = "%s-%s-%s.pem" % (
+       path = "{}-{}-{}.pem".format(
		safe_domain_name(cn), # common name, which should be filename safe because it is IDNA-encoded, but in case of a malformed cert make sure it's ok to use as a filename
		cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date
		hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix
		)

(There were some additional differences due to Ruff not supporting the newer pyupgrade rules.)

Ruff version:

ruff 0.1.1
@dhruvmanila
Copy link
Member

Thanks for reporting! It's because of the comments which is a known limitation:

// Avoid fix if there are comments within the right-hand side:
// ```
// "%s" % (
// 0, # 0
// )
// ```

Relevant issue: #6336

\cc @zanieb Do we want to downgrade the applicability level as suggested (#6342 (comment)) now that we support them?

@dhruvmanila dhruvmanila added fixes Related to suggested fixes for violations needs-decision Awaiting a decision from a maintainer labels Oct 30, 2023
@charliermarsh
Copy link
Member

Yeah, I'd suggest flagging the diagnostic, and using an unsafe fix personally.

@dhruvmanila dhruvmanila removed the needs-decision Awaiting a decision from a maintainer label Nov 20, 2023
@charliermarsh charliermarsh self-assigned this Dec 7, 2023
charliermarsh added a commit that referenced this issue Dec 7, 2023
#9037)

## Summary

This was added in #6364 (as a
follow-on to #6342), but I don't
think it applies in the same way, because we don't _remove_ the
right-hand side when converting from `%`-style formatting to `.format`
calls.

Closes #8107.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants