Skip to content

[ruff] Add os-path-commonprefix (RUF071)#23814

Merged
ntBre merged 4 commits intoastral-sh:mainfrom
anishgirianish:add-ruf071-os-path-commonprefix
Mar 9, 2026
Merged

[ruff] Add os-path-commonprefix (RUF071)#23814
ntBre merged 4 commits intoastral-sh:mainfrom
anishgirianish:add-ruf071-os-path-commonprefix

Conversation

@anishgirianish
Copy link
Copy Markdown
Contributor

@anishgirianish anishgirianish commented Mar 8, 2026

Summary

Adds a new rule os-path-commonprefix (RUF071) that detects calls to os.path.commonprefix(), which performs character-by-character string comparison instead of path-component comparison — a well-known footgun. os.path.commonpath() is the correct alternative.

Test Plan

cargo nextest run -p ruff_linter -- rule_ospathcommonprefix

Closes #22981

@astral-sh-bot astral-sh-bot bot requested a review from ntBre March 8, 2026 23:23
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 8, 2026 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+3 -0 violations, +0 -0 fixes in 2 projects; 54 projects unchanged)

python-poetry/poetry (+2 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ src/poetry/utils/authenticator.py:415:67: RUF071 `os.path.commonprefix()` compares strings character-by-character
+ src/poetry/utils/authenticator.py:437:35: RUF071 `os.path.commonprefix()` compares strings character-by-character

astropy/astropy (+1 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ astropy/tests/runner.py:414:26: RUF071 `os.path.commonprefix()` compares strings character-by-character

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF071 3 3 0 0 0

@ntBre ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Mar 9, 2026
ntBre
ntBre approved these changes Mar 9, 2026
View reviewed changes
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.

Thanks, this looks great to me! I just had one suggestion on the diagnostic structure.

Would you be interested in adding a fix too? We could leave that for a follow-up PR, but I think it should be straightforward to autofix this, although I think it will have to be an unsafe fix.

@ntBre ntBre changed the title [ruff] Add os-path-commonprefix (RUF071) [ruff] Add os-path-commonprefix (RUF071) Mar 9, 2026
@ntBre ntBre changed the title [ruff] Add os-path-commonprefix (RUF071) [ruff] Add os-path-commonprefix (RUF071) Mar 9, 2026
@anishgirianish
Copy link
Copy Markdown
Contributor Author

anishgirianish commented Mar 9, 2026

Thanks, this looks great to me! I just had one suggestion on the diagnostic structure.

Would you be interested in adding a fix too? We could leave that for a follow-up PR, but I think it should be straightforward to autofix this, although I think it will have to be an unsafe fix.

Fo sure will do. thank you so much for the review

@ntBre ntBre enabled auto-merge (squash) March 9, 2026 16:30
@ntBre ntBre merged commit e4dfddc into astral-sh:main Mar 9, 2026
42 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Mar 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntBre ntBre ntBre approved these changes

Assignees

@ntBre ntBre

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rule to advise os.path.commonprefix is wrong and you should use os.path.commonpath

2 participants

@anishgirianish @ntBre