Skip to content

[ruff] Add unsafe fix for os-path-commonprefix (RUF071)#23852

Merged
amyreese merged 3 commits intoastral-sh:mainfrom
anishgirianish:add-unsafe-fix-ruf071
Mar 11, 2026
Merged

[ruff] Add unsafe fix for os-path-commonprefix (RUF071)#23852
amyreese merged 3 commits intoastral-sh:mainfrom
anishgirianish:add-unsafe-fix-ruf071

Conversation

@anishgirianish
Copy link
Copy Markdown
Contributor

Summary

Adds an unsafe autofix for RUF071 (os-path-commonprefix) that replaces os.path.commonprefix() calls with os.path.commonpath().

Test Plan

  • cargo nextest run -p ruff_linter -- rule_ospathcommonprefix

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 10, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@amyreese amyreese added fixes Related to suggested fixes for violations preview Related to preview mode features labels Mar 10, 2026
@anishgirianish
Copy link
Copy Markdown
Contributor Author

Thank you so much for the feedback @ntBre @amyreese! I've updated the PR:

  • Changed from AlwaysFixableViolation to Violation with FixAvailability::Sometimes
  • Uses try_set_fix with get_or_import_symbol to correctly handle all import styles (including from os.path import commonprefix)

I tried a simpler text replacement to preserve the user's import style, but that breaks the from os.path import commonprefix case; the call becomes commonpath(...) with no matching import. Went with the get_or_import_symbol approach instead.

I would like to request your re-review whenever you get a chance.

Thank you

@anishgirianish anishgirianish requested a review from ntBre March 11, 2026 14:35
Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

LGTM!

I'm still slightly disappointed that we can't preserve the import style, but that seems like a general issue with get_or_import_symbol and doesn't need to block this PR! Using manual range replacements to achieve that effect seems much worse.

@amyreese amyreese merged commit 62ae61e into astral-sh:main Mar 11, 2026
43 checks passed
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 preview Related to preview mode features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants