Skip to content

[flake8-bugbear] Clarify RUF071 fix safety for non-path string comparisons#24149

Merged
MichaReiser merged 3 commits intoastral-sh:mainfrom
chinar-amrutkar:fix/ruf071-docs-clarify-non-path-usage
Mar 27, 2026
Merged

[flake8-bugbear] Clarify RUF071 fix safety for non-path string comparisons#24149
MichaReiser merged 3 commits intoastral-sh:mainfrom
chinar-amrutkar:fix/ruf071-docs-clarify-non-path-usage

Conversation

@chinar-amrutkar
Copy link
Copy Markdown
Contributor

Summary

Fixes #24028

Added documentation explaining that os.path.commonprefix is valid for non-path string comparisons (e.g., finding a common prefix among version numbers or identifiers), and that os.path.commonpath cannot be used as a drop-in replacement in those cases since it raises ValueError on non-path strings.

Added a "Fix safety" section explaining why the fix is marked as unsafe, with concrete examples showing the semantic difference between commonprefix (character-by-character) and commonpath (path-component-level).

Test Plan

Existing tests pass. Documentation change only (rule definition in os_path_commonprefix.rs).

@astral-sh-bot astral-sh-bot bot requested a review from ntBre March 23, 2026 22:33
@ntBre ntBre added the documentation Improvements or additions to documentation label Mar 24, 2026
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 also looks good to me overall, but I had a couple of suggestions.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 24, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@chinar-amrutkar
Copy link
Copy Markdown
Contributor Author

@ntBre Thanks for the review! I have addressed all three points:

  1. Fixed the ValueError claim - commonpath returns "" not raises.
  2. Replaced the specific suppression mentions with a link to the general suppression docs.
  3. Removed the redundant non-path note from the "Why is this bad?" section since the "Fix safety" section already covers it.

@chinar-amrutkar chinar-amrutkar requested a review from ntBre March 24, 2026 21:41
chinar-amrutkar and others added 2 commits March 27, 2026 15:45
Add documentation explaining that os.path.commonprefix is valid for
non-path string comparisons (e.g., version numbers, identifiers)
and that RUF071 should be ignored in such cases.

Adds a 'Fix safety' section documenting the semantic differences
between commonprefix and commonpath with examples.

Fixes astral-sh#24028
- Remove redundant non-path note (redundant with Fix safety section)
- Link to general suppression docs instead of specific mechanisms
- Fix incorrect ValueError claim: commonpath returns empty string
@MichaReiser MichaReiser force-pushed the fix/ruf071-docs-clarify-non-path-usage branch from b00938b to eb2125f Compare March 27, 2026 14:45
@MichaReiser MichaReiser enabled auto-merge (squash) March 27, 2026 14:46
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 27, 2026

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 86.59%. The percentage of expected errors that received a diagnostic held steady at 80.96%. The number of fully passing files held steady at 68/132.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 27, 2026

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 27, 2026

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-await 40 0 0
invalid-return-type 1 0 0
Total 41 0 0

Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.

Full report with detailed diff (timing results)

@MichaReiser MichaReiser merged commit 627e2a0 into astral-sh:main Mar 27, 2026
41 checks passed
@chinar-amrutkar chinar-amrutkar deleted the fix/ruf071-docs-clarify-non-path-usage branch March 27, 2026 19:20
carljm added a commit that referenced this pull request Mar 31, 2026
* main: (40 commits)
  [ty] resolve union-likes in emitting union attribute errors (#24263)
  [ty] Improve support for `Callable` type context (#23888)
  [ty] Propagate type context through `await` expressions (#24256)
  [`pyflakes`] Flag annotated variable redeclarations as `F811` in preview mode (#24244)
  [ty] Preserve `Divergent` when materializing recursive aliases (#24245)
  Fix W391 fixes for consecutive empty notebook cells (#24236)
  [flake8-bugbear] Clarify RUF071 fix safety for non-path string comparisons (#24149)
  [ty] Ban type qualifiers in PEP-695 type aliases (#24242)
  [ty] Include keyword-prefixed symbols in completions for attributes (#24232)
  [ty] Add tests for TypedDict method overloads on unions (#24230)
  [ty] report unused bindings as unnecessary hint diagnostics (#23305)
  Remove unused `non_root` variable (#24238)
  Extend F507 to flag %-format strings with zero placeholders (#24215)
  [`flake8-simplify`] Suppress `SIM105` for `except*` before Python 3.12 (#23869)
  Ignore pre-initialization references in SIM113 (#24235)
  Parenthesize expression in RUF050 fix (#24234)
  Publish playgrounds using the `release-playground` environment (#24223)
  [ty] Fix instance-attribute lookup in methods of protocol classes (#24213)
  [ty] Used shared expression cache during generic call inference (#24219)
  [ty] make `Type::BoundMethod` include instances of same-named methods bound to a subclass (#24039)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os-path-commonprefix (RUF071): update documentation

3 participants