[ty] Skip eagerly evaluated scopes for attribute storing by Glyphack · Pull Request #20856 · astral-sh/ruff

Glyphack · 2025-10-14T07:00:32Z

Summary

This PR adds support for storing attributes in comprehension scopes (any eager scope.)

For example in the following code we infer type of z correctly:

class C:
    def __init__(self):
        [None for self.z in range(1)]
reveal_type(C().z) # previously [unresolved-attribute] but now shows Unknown | int

The fix works by adjusting the following logics:

To identify if an attriute is an assignment to self or cls we need to check the scope is a method. To allow comprehension scopes here we skip any eager scope in the check.
Also at this stage the code checks if self or the first method argument is shadowed by another binding that eager scope to prevent this:

class D:
    g: int

class C:
    def __init__(self):
        [[None for self.g in range(1)] for self in [D()]]
reveal_type(C().g) # [unresolved-attribute]

When determining scopes that attributes might be defined after collecting all the methods of the class the code also returns any decendant scope that is eager and only has eager parents until the method scope.

When checking reachability of a attribute definition if the attribute is defined in an eager scope we use the reachability of the first non eager scope which must be a method. This allows attributes to be marked as reachable and be seen.

There are also which I didn't add support for:

class C:
    def __init__(self):
        def f():
            [None for self.z in range(1)]
        f()

reveal_type(C().z) # [unresolved-attribute]

In the above example we will not even return the comprehension scope as an attribute scope because there is a non eager scope (f function) between the comprehension and the __init__ method

Test Plan

github-actions · 2025-10-15T19:02:40Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

github-actions · 2025-10-15T19:03:59Z

`mypy_primer` results

Changes were detected when running on open source projects

attrs (https://github.com/python-attrs/attrs)
- tests/test_hooks.py:154:48: error[missing-argument] No argument provided for required parameter `y`
- tests/test_next_gen.py:142:14: error[unresolved-attribute] Object of type `dataclasses.Field` has no attribute `validator`
- tests/test_next_gen.py:380:14: error[unresolved-attribute] Object of type `dataclasses.Field` has no attribute `validator`
- Found 584 diagnostics
+ Found 581 diagnostics

No memory usage changes detected ✅

Glyphack · 2025-10-15T19:34:07Z

crates/ty_python_semantic/src/semantic_index/builder.rs

@@ -184,29 +184,34 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
        self.current_scope_info().file_scope_id
    }

-    /// Returns the scope ID of the surrounding class body scope if the current scope
+    /// Returns the scope ID of the method scope if the current scope
    /// is a method inside a class body. Returns `None` otherwise, e.g. if the current
    /// scope is a function body outside of a class, or if the current scope is not a
    /// function body.
    fn is_method_of_class(&self) -> Option<FileScopeId> {


This function is also used at https://github.com/Glyphack/ruff/blob/8d53802bc7e9566a9087fd3e3a460345e40a7e6d/crates/ty_python_semantic/src/semantic_index/builder.rs#L1681 and I think this new behavior should be there as well.
I'm not happy with the name though, this now returns either method or an eager child scope of the class.

Glyphack · 2025-10-15T19:39:09Z

There is no ecosystem change detected by mypy primer? With the amount of custom logic added I expected to have more effect. Do you think I should simplify the logic even more since there's not much impact?

Other solution could be only support children eager scopes and don't go into decendants. This would turn a lot of loops into if statements.

sharkdp · 2025-10-16T08:15:21Z

There is no ecosystem change detected by mypy primer? With the amount of custom logic added I expected to have more effect. Do you think I should simplify the logic even more since there's not much impact?

I would expect the impact to be bigger when combined with type-of-self-in-method bodies, no? Maybe we could test that opening a draft PR that merges both. Not strictly necessary though.

Other solution could be only support children eager scopes and don't go into decendants. This would turn a lot of loops into if statements.

I don't really see a reason not to support that if you already implemented it correctly.

sharkdp · 2025-10-16T08:16:16Z

Thank you for working on this? Could you please also add some tests with generic methods like described here? I'd like to see that we correctly skip the type params scope when detecting methods in classes.

Glyphack · 2025-10-16T15:10:01Z

Thanks. I didn't think of the impact there, I was still hoping to reduce the false positives like this. So let's keep the current implementation.

I went over the tests again added more examples of what is supported.

sharkdp · 2025-10-22T15:00:55Z

I merged this branch into the type-of-self PR and compared it with the type-of-self PR alone. This seems to remove 4 diagnostics from the ecosystem:

type-of-self:

type-of-self + this:

I did this to evaluate whether I need to prioritize this, or if it can land separately afterwards. After seeing these results, I'm going with the latter.

But we'll come back to this eventually.

carljm · 2025-11-06T14:18:38Z

Just rebased this (and updated some tests) to get an updated ecosystem report.

carljm · 2025-11-06T15:14:52Z

crates/ty_python_semantic/resources/mdtest/attributes.md

+If the comprehension is inside another scope like function then that attribute is not inferred.
+
+```py
+class C:
+    def __init__(self):
+        def f():
+            # error: [unresolved-attribute]
+            [... for self.a in [1]]
+
+        def g():
+            # error: [unresolved-attribute]
+            [... for self.b in [1]]
+        g()
+
+c_instance = C()
+
+# This attribute is in the function f and is not reachable
+# error: [unresolved-attribute]
+reveal_type(c_instance.a)  # revealed: Unknown
+
+# TODO: Even though g method is called and is reachable we do not record this attribute assignment
+# error: [unresolved-attribute]
+reveal_type(c_instance.b)  # revealed: Unknown
+```


It's not totally clear to me if this is the right behavior or not. We can't guarantee that these attributes will ever be initialized. We can definitely say that they might be.

For reference, this behavior matches pyright. Mypy finds both attributes and gives them type Any. Pyrefly finds both attributes and gives them type int. No type checker attempts to distinguish these cases based on trying to determine whether the nested function is ever called; this is quite complex to do in general and I doubt we should attempt it either.

Overall I think the current behavior in this PR is fine and this case is probably rare (and it doesn't show up in the ecosystem report.) I don't think we need TODOs to revisit it; we'll revisit it only if there are real user reports.

I haven't worked with call graph analysis and didn't think about it's complexity. If an issue was reported for this I would follow it up.

crates/ty_python_semantic/resources/mdtest/attributes.md

crates/ty_python_semantic/src/semantic_index/builder.rs

crates/ty_python_semantic/src/semantic_index.rs

Co-authored-by: Carl Meyer <[email protected]>

carljm · 2025-11-11T22:45:40Z

Thank you!

Glyphack changed the title ~~Skip eagerly evaluated scopes for attribute storing~~ [ty] Skip eagerly evaluated scopes for attribute storing Oct 14, 2025

AlexWaygood added the ty Multi-file analysis & type inference label Oct 14, 2025

Glyphack force-pushed the comprehension-assign-attr-unresolved branch from dc32d77 to d263c50 Compare October 15, 2025 19:00

Glyphack force-pushed the comprehension-assign-attr-unresolved branch from d263c50 to 8d53802 Compare October 15, 2025 19:08

Glyphack commented Oct 15, 2025

View reviewed changes

Glyphack marked this pull request as ready for review October 15, 2025 19:37

Glyphack requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners October 15, 2025 19:37

AlexWaygood removed their request for review October 16, 2025 08:59

sharkdp mentioned this pull request Oct 22, 2025

[ty] Infer type for implicit self parameters in method bodies #20922

Merged

Glyphack added 2 commits November 6, 2025 09:12

Skip eagerly evaluated scopes for attribute storing

7ec67f2

Update tests

aed27db

carljm force-pushed the comprehension-assign-attr-unresolved branch from 7b7c842 to aed27db Compare November 6, 2025 14:12

update tests

a70e4dd

carljm approved these changes Nov 6, 2025

View reviewed changes

Glyphack and others added 5 commits November 7, 2025 07:49

Update crates/ty_python_semantic/resources/mdtest/attributes.md

c778090

Co-authored-by: Carl Meyer <[email protected]>

Update crates/ty_python_semantic/src/semantic_index/builder.rs

5b73551

Co-authored-by: Carl Meyer <[email protected]>

Remove unused test

39233bf

Fix clippy

fe8bcb6

Merge branch 'main' into comprehension-assign-attr-unresolved

7f12f07

carljm merged commit 988c38c into astral-sh:main Nov 11, 2025
41 checks passed

carljm mentioned this pull request Nov 11, 2025

instance attributes bound in nested scopes are reported as unresolved astral-sh/ty#147

Closed

Glyphack deleted the comprehension-assign-attr-unresolved branch February 9, 2026 21:43

Comments

Conversation

Glyphack commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Glyphack Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Glyphack commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sharkdp commented Oct 16, 2025

Uh oh!

sharkdp commented Oct 16, 2025

Uh oh!

Glyphack commented Oct 16, 2025

Uh oh!

sharkdp commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carljm commented Nov 6, 2025

Uh oh!

carljm Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Glyphack Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carljm commented Nov 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Glyphack commented Oct 14, 2025 •

edited

Loading

github-actions bot commented Oct 15, 2025 •

edited

Loading

github-actions bot commented Oct 15, 2025 •

edited

Loading

`mypy_primer` results

Glyphack commented Oct 15, 2025 •

edited

Loading

sharkdp commented Oct 22, 2025 •

edited

Loading