[`flake8-use-pathlib`] Mark `PTH101` fix as unsafe when first argument is a class attribute annotated as `int` by baltasarblanco · Pull Request #25086 · astral-sh/ruff

baltasarblanco · 2026-05-10T12:11:44Z

Summary

Closes #24895 (closed as duplicate of #17699).

The autofix for PTH101 (os.chmod → Path.chmod) currently produces
runtime errors when the first argument is an attribute access whose underlying
type is an int (file descriptor). For example:

class TestFd:
    def __init__(self, fd: int):
        self.fd = fd

    def test_fd(self):
        os.chmod(self.fd, 0o644)  # Currently autofixed to:
        # pathlib.Path(self.fd).chmod(0o644)  → TypeError at runtime

The cause is in is_file_descriptor (in flake8_use_pathlib/helpers.rs),
which extracts a Name from the argument via get_name_expr to look up its
binding type. get_name_expr only handles Expr::Name and Expr::Call —
it returns None for Expr::Attribute, so the helper cannot tell that
self.fd might be an int, and the fix is applied as Safe.

This PR marks the fix as Applicability::Unsafe whenever the first argument
is an Expr::Attribute. The diagnostic is still emitted, but applying the
autofix now requires --unsafe-fixes, preventing the silent regression.

The same demotion applies to attribute accesses that look like strings
(e.g., os.chmod(self.name, 0o644)). This is intentional: the heuristic
cannot statically distinguish int attributes from str/PathLike ones,
so applying it uniformly is the honest behavior.

This PR marks the fix as Applicable::Unsafe if the first argument is annotated with an int type

Scope

The fix is intentionally local to os_chmod.rs:

helpers.rs is unchanged. Extending get_name_expr to handle attributes
would require attribute type resolution, which is closer to ty's domain
than Ruff's.
Other rules in the family that use is_file_descriptor (os.stat,
os.rename, os.unlink, etc.) are likely affected by the same pattern,
but those are out of scope here. Happy to open a tracking issue or follow
up if useful.

This narrow scope also aligns with @ntBre's note in #17699 about the broader
stabilization of these fixes.

Test plan

Added regression cases in
crates/ruff_linter/resources/test/fixtures/flake8_use_pathlib/full_name.py
covering attribute access (both int and str attributes).
Regenerated full_name.py.snap and preview_full_name.py.snap. The new
preview diagnostics carry the note: This is an unsafe fix and may change runtime behavior marker. Existing diagnostics are unchanged in content;
only their reported line numbers shift due to the inserted fixture lines.
cargo test -p ruff_linter → 2759 passed, 0 failed.
cargo clippy --workspace --all-targets --all-features -- -D warnings → clean.
uvx prek run -a → all hooks passed.

Refs #17699

… an attribute access When `os.chmod` is called with an attribute access (e.g., `self.fd`), the linter cannot determine statically whether the attribute is a file descriptor or a path. Apply the fix as unsafe in this case to avoid converting working code into code that raises `TypeError` at runtime. Refs astral-sh#17699

MichaReiser · 2026-05-11T09:31:23Z

I think we could change is_file_descriptor to support at least some attribute expressions like so

if let Expr::Attribute(_) = expr
    && let Some(binding_id) = semantic.lookup_attribute(expr)
{
    return typing::is_int(semantic.binding(binding_id), semantic);
}

But we would still not support

class C:
    def __init__(self, fd: int):
        self.fd = fd

    def f(self):
        os.chmod(self.fd, 0o644)

I'm also not sure if it's worth making all fixes where the type isn't known as Unsafe for attributes. For example, Ruff currently suggests a fix for

import os

fd = compute_path()
os.chmod(fd, 0o644)

even though the type of fd is unknown. I think it's important that whatever we do is consistent between attributes and names. I'm not sure it's worth marking the fix as unsafe for all cases where the type is unknown because that makes the fix significantly less useful. For this PR, I suggest we align the behavior with normal names and bail out if the attribute is known to be an int. We can discuss what we want to do about unannotated names and attributes separately.

ntBre

Thanks! I left one comment on the tests, but I'll otherwise mark this as a draft until Micha's feedback is addressed. Feel free to open it for review again when it's ready for another look.

ntBre · 2026-05-11T16:20:36Z

+# https://github.com/astral-sh/ruff/issues/17699
+# Attribute access in the first argument: the type cannot be statically determined,
+# so the fix should be marked as unsafe (the attribute may be a file descriptor).
+class _AttrHolder:
+    fd: int = 0
+    name: str = ""
+
+_holder = _AttrHolder()
+os.chmod(_holder.fd, 0o644)
+os.chmod(_holder.name, 0o644)
+


Could you move any additions to the file to the bottom of the file (or a new one)? This avoids shifting all of the other snapshots down and increasing the size of the diff.

…kup_attribute` Extend `is_file_descriptor` to resolve attribute expressions that `SemanticModel::lookup_attribute` maps to a binding — concretely, class attributes accessed via the class itself (e.g., `MyClass.fd`). When the resolved binding is `int`, the PTH rule bails out, mirroring the behavior for `Name` expressions with `int` annotations. Test additions are placed at the bottom of `full_name.py` to keep the snapshot diff focused on the new cases. Refs astral-sh#17699

baltasarblanco · 2026-05-11T18:43:03Z

Hi! Updated:

Reverted the Applicability::Unsafe change in os_chmod.rs.
Extended is_file_descriptor to use SemanticModel::lookup_attribute for resolvable attribute expressions. When the resolved binding is int, the diagnostic is suppressed — matching the behavior for int-annotated Name.
Moved the test additions to the bottom of full_name.py (thank you @ntBre :) , the snapshot diff is much smaller now).

Let me know if you need anything!

astral-sh-bot · 2026-05-11T18:58:26Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

…t is a class attribute annotated as `int` (astral-sh#25086)

astral-sh-bot Bot assigned ntBre May 10, 2026

astral-sh-bot Bot requested a review from ntBre May 10, 2026 12:11

MichaReiser added preview Related to preview mode features rule Implementing or modifying a lint rule labels May 11, 2026

ntBre reviewed May 11, 2026

View reviewed changes

ntBre marked this pull request as draft May 11, 2026 16:22

baltasarblanco marked this pull request as ready for review May 11, 2026 18:49

astral-sh-bot Bot requested a review from ntBre May 11, 2026 18:49

MichaReiser changed the title ~~[flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is an attribute access~~ [flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is a class attribute annotated as int May 15, 2026

MichaReiser changed the title ~~[flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is a class attribute annotated as int~~ [flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is a class attribute annotated as int` May 15, 2026

MichaReiser changed the title ~~[flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is a class attribute annotated as int`~~ [flake8-use-pathlib] Mark PTH101 fix as unsafe when first argument is a class attribute annotated as int May 15, 2026

MichaReiser added the bug Something isn't working label May 15, 2026

MichaReiser merged commit 6e9d3ab into astral-sh:main May 15, 2026
44 checks passed

BrewTestBot mentioned this pull request May 21, 2026

ruff 0.15.14 Homebrew/homebrew-core#284070

Merged

thejchap pushed a commit to thejchap/ruff that referenced this pull request May 23, 2026

[flake8-use-pathlib] Mark PTH101 fix as unsafe when first argumen…

6710e10

…t is a class attribute annotated as `int` (astral-sh#25086)

anishgirianish pushed a commit to anishgirianish/ruff that referenced this pull request May 28, 2026

[flake8-use-pathlib] Mark PTH101 fix as unsafe when first argumen…

18dd6b2

…t is a class attribute annotated as `int` (astral-sh#25086)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`flake8-use-pathlib`] Mark `PTH101` fix as unsafe when first argument is a class attribute annotated as `int`#25086

[`flake8-use-pathlib`] Mark `PTH101` fix as unsafe when first argument is a class attribute annotated as `int`#25086
MichaReiser merged 2 commits into
astral-sh:mainfrom
baltasarblanco:feat/17699-pth101-unsafe-fix-on-attribute

baltasarblanco commented May 10, 2026 •

edited by MichaReiser

Loading

Uh oh!

MichaReiser commented May 11, 2026

Uh oh!

ntBre left a comment

Uh oh!

ntBre May 11, 2026

Uh oh!

baltasarblanco commented May 11, 2026 •

edited

Loading

Uh oh!

astral-sh-bot Bot commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

baltasarblanco commented May 10, 2026 • edited by MichaReiser Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope

Test plan

Uh oh!

MichaReiser commented May 11, 2026

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre May 11, 2026

Choose a reason for hiding this comment

Uh oh!

baltasarblanco commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astral-sh-bot Bot commented May 11, 2026

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

baltasarblanco commented May 10, 2026 •

edited by MichaReiser

Loading

baltasarblanco commented May 11, 2026 •

edited

Loading

`ruff-ecosystem` results