Skip to content

Conversation

Glyphack
Copy link
Contributor

@Glyphack Glyphack commented Jul 27, 2025

Summary

closes astral-sh/ty#692

If the expression (or any child expressions) is not definitely bound the reachability constraint evaluation is determined as ambiguous.

This fixes the infinite cycles panic in the following code:

from typing import Literal

class Toggle:
    def __init__(self: "Toggle"):
        if not self.x:
            self.x: Literal[True] = True

Credit of this solution is for David.

Test Plan

  • Added a test case with too many cycle iterations panic.
  • Previous tests.

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Jul 27, 2025
@AlexWaygood AlexWaygood changed the title Evaluate reachability of non-definitely-bound to Ambiguous [ty] Evaluate reachability of non-definitely-bound to Ambiguous Jul 27, 2025
Copy link
Contributor

github-actions bot commented Jul 27, 2025

mypy_primer results

Changes were detected when running on open source projects
trio (https://github.com/python-trio/trio)
+ src/trio/_tests/test_util.py:255:12: error[unresolved-attribute] Type `ModuleType` has no attribute `some_func`
+ src/trio/_tests/test_util.py:256:12: error[unresolved-attribute] Type `ModuleType` has no attribute `some_func`
+ src/trio/_tests/test_util.py:258:12: error[unresolved-attribute] Type `ModuleType` has no attribute `not_funclike`
+ src/trio/_tests/test_util.py:259:12: error[unresolved-attribute] Type `ModuleType` has no attribute `_private`
+ src/trio/_tests/test_util.py:260:12: error[unresolved-attribute] Type `ModuleType` has no attribute `_private`
+ src/trio/_tests/test_util.py:261:12: error[unresolved-attribute] Type `ModuleType` has no attribute `_private`
+ src/trio/_tests/test_util.py:263:12: error[unresolved-attribute] Type `ModuleType` has no attribute `only_has_name`
+ src/trio/_tests/test_util.py:264:12: error[unresolved-attribute] Type `ModuleType` has no attribute `only_has_name`
+ src/trio/_tests/test_util.py:265:24: error[unresolved-attribute] Type `ModuleType` has no attribute `only_has_name`
+ src/trio/_tests/test_util.py:267:12: error[unresolved-attribute] Type `ModuleType` has no attribute `SomeClass`
+ src/trio/_tests/test_util.py:267:12: error[unresolved-attribute] Type `type` has no attribute `method`
+ src/trio/_tests/test_util.py:268:12: error[unresolved-attribute] Type `ModuleType` has no attribute `SomeClass`
+ src/trio/_tests/test_util.py:268:12: error[unresolved-attribute] Type `type` has no attribute `method`
+ src/trio/_tests/test_util.py:269:12: error[unresolved-attribute] Type `ModuleType` has no attribute `SomeClass`
+ src/trio/_tests/test_util.py:269:12: error[unresolved-attribute] Type `type` has no attribute `method`
+ src/trio/_tests/test_util.py:271:5: error[unresolved-attribute] Type `ModuleType` has no attribute `some_func`
+ src/trio/_tests/test_util.py:272:5: error[unresolved-attribute] Type `ModuleType` has no attribute `some_func`
+ src/trio/_tests/test_util.py:273:5: error[unresolved-attribute] Type `ModuleType` has no attribute `_private`
+ src/trio/_tests/test_util.py:274:5: error[unresolved-attribute] Type `ModuleType` has no attribute `SomeClass`
- Found 656 diagnostics
+ Found 675 diagnostics

static-frame (https://github.com/static-frame/static-frame)
+ static_frame/core/util.py:1386:32: warning[possibly-unresolved-reference] Name `rows` used when possibly not defined
- Found 1788 diagnostics
+ Found 1789 diagnostics

dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:65:12: warning[possibly-unbound-attribute] Attribute `vulnerability_budget` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:66:12: warning[possibly-unbound-attribute] Attribute `vulnerabilities_request_limit` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:80:12: warning[possibly-unbound-attribute] Attribute `is_first_vulnerability` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:81:12: warning[possibly-unbound-attribute] Attribute `vulnerability_budget` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:82:12: warning[possibly-unbound-attribute] Attribute `vulnerabilities_request_limit` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:83:12: warning[possibly-unbound-attribute] Attribute `vulnerabilities_request_limit` on type `IASTEnvironment | None` is possibly unbound
+ tests/appsec/iast/taint_sinks/test_vulnerability_detection.py:84:12: warning[possibly-unbound-attribute] Attribute `vulnerabilities_request_limit` on type `IASTEnvironment | None` is possibly unbound
- Found 6595 diagnostics
+ Found 6602 diagnostics

sympy (https://github.com/sympy/sympy)
+ sympy/polys/numberfields/galoisgroups.py:126:21: warning[possibly-unresolved-reference] Name `current_degree` used when possibly not defined
- Found 13401 diagnostics
+ Found 13402 diagnostics

scipy (https://github.com/scipy/scipy)
+ scipy/optimize/tests/test_bracket.py:318:9: error[unresolved-attribute] Unresolved attribute `count` on type `def f(x) -> Unknown`.
- Found 6420 diagnostics
+ Found 6421 diagnostics
No memory usage changes detected ✅

Copy link

codspeed-hq bot commented Jul 27, 2025

CodSpeed WallTime Performance Report

Merging #19579 will not alter performance

Comparing Glyphack:reachability-ambiguous-not-definitely-bound (c4ce73a) with main (18eaa65)

Summary

✅ 8 untouched benchmarks

Copy link

codspeed-hq bot commented Jul 27, 2025

CodSpeed Instrumentation Performance Report

Merging #19579 will not alter performance

Comparing Glyphack:reachability-ambiguous-not-definitely-bound (c4ce73a) with main (18eaa65)

Summary

✅ 42 untouched benchmarks

@sharkdp

This comment was marked as resolved.

@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch from d5dafc5 to 3430eea Compare July 29, 2025 15:17
@Glyphack

This comment was marked as resolved.

Copy link
Contributor

github-actions bot commented Jul 29, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch 4 times, most recently from d383152 to b05868f Compare August 4, 2025 18:37
@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch from b05868f to 9be1e82 Compare August 16, 2025 11:47
@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch from 9be1e82 to feca174 Compare August 18, 2025 22:31
@Glyphack

This comment was marked as resolved.

@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch from feca174 to 590ea0d Compare August 19, 2025 17:38
@Glyphack Glyphack force-pushed the reachability-ambiguous-not-definitely-bound branch from aa650e9 to a84b8ce Compare August 20, 2025 13:43
@Glyphack
Copy link
Contributor Author

Thank you for the feedback. I'm drafting the PR to apply the suggestion.

@Glyphack Glyphack marked this pull request as draft August 22, 2025 15:50
@sharkdp
Copy link
Contributor

sharkdp commented Aug 27, 2025

I'm looking into making the changes described above.

@sharkdp sharkdp force-pushed the reachability-ambiguous-not-definitely-bound branch from 4c1da8c to 2218db4 Compare August 27, 2025 14:31
@sharkdp sharkdp force-pushed the reachability-ambiguous-not-definitely-bound branch from 94d1897 to 71bd443 Compare August 27, 2025 15:15
Copy link
Contributor

github-actions bot commented Aug 27, 2025

ecosystem-analyzer results

Lint rule Added Removed Changed
unresolved-attribute 20 0 0
possibly-unbound-attribute 7 0 0
possibly-unresolved-reference 2 0 0
Total 29 0 0

sharkdp added a commit that referenced this pull request Aug 27, 2025
…tions (#20114)

## Summary

Properly preserve type qualifiers when accessing attributes on unions
and intersections. This is a prerequisite for
#19579.

Also fix a completely wrong implementation of
`map_with_boundness_and_qualifiers`. It now closely follows
`map_with_boundness` (just above).

## Test Plan

I thought about it, but didn't find any easy way to test this. This only
affected `Type::member`. Things like validation of attribute writes
(where type qualifiers like `ClassVar` and `Final` are important) were
already handling things correctly.
@sharkdp sharkdp force-pushed the reachability-ambiguous-not-definitely-bound branch from 966553c to a18a7ce Compare August 28, 2025 11:51
@sharkdp
Copy link
Contributor

sharkdp commented Aug 28, 2025

I tried various other solutions here, and non of them worked as well as what @Glyphack implemented in the first place. The main problem is that we can only build unions/intersections of types. We can't build unions/intersections of TypeQualifiers and boundness information, which requires us to merge them somehow when we operate on unions/intersections (for example, when we access an attribute on a union/intersection). With this representation, it is impossible to distinguish between a case where we have C | None (where an attribute access should definitely yield possibly-unbound, due to the None in the union), and something like C | Unknown (where we would like to silence the possibly-unbound-attribute diagnostic if the attribute is only possibly bound inside a method of C).

I also tried various completely different approaches of solving this problem, and none of them hit the right granularity. Either they are not able to solve the Toggle problem, or they are too big of a hammer, and lead to insufficient reachability analysis in real world use cases.

So I am returning this PR to a previous state (with a couple of smaller cleanups, renamings, and additional tests). I am still not very satisfied with the fact that we track boundness information in a TypeQualifier, but I'm out of ideas. And we would very much like to proceed with type-of-self.

@sharkdp sharkdp marked this pull request as ready for review August 28, 2025 12:02
Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you again, @Glyphack!

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thank you!

@sharkdp sharkdp merged commit d9aaacd into astral-sh:main Aug 28, 2025
38 checks passed
dcreager added a commit that referenced this pull request Aug 28, 2025
* main:
  Fix mdtest ignore python code blocks (#20139)
  [ty] add support for cyclic legacy generic protocols (#20125)
  [ty] add cycle detection for find_legacy_typevars (#20124)
  Use new diff rendering format in tests (#20101)
  [ty] Fix 'too many cycle iterations' for unions of literals (#20137)
  [ty] No boundness analysis for implicit instance attributes (#20128)
  Bump 0.12.11 (#20136)
  [ty] Benchmarks for problematic implicit instance attributes cases (#20133)
  [`pyflakes`] Fix `allowed-unused-imports` matching for top-level modules (`F401`) (#20115)
  Move GitLab output rendering to `ruff_db` (#20117)
  [ty] Evaluate reachability of non-definitely-bound to Ambiguous (#19579)
  [ty] Introduce a representation for the top/bottom materialization of an invariant generic (#20076)
  [`flake8-async`] Implement `blocking-http-call-httpx` (`ASYNC212`) (#20091)
  [ty] print diagnostics with fully qualified name to disambiguate some cases (#19850)
  [`ruff`] Preserve relative whitespace in multi-line expressions (`RUF033`) (#19647)
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
…tions (astral-sh#20114)

## Summary

Properly preserve type qualifiers when accessing attributes on unions
and intersections. This is a prerequisite for
astral-sh#19579.

Also fix a completely wrong implementation of
`map_with_boundness_and_qualifiers`. It now closely follows
`map_with_boundness` (just above).

## Test Plan

I thought about it, but didn't find any easy way to test this. This only
affected `Type::member`. Things like validation of attribute writes
(where type qualifiers like `ClassVar` and `Final` are important) were
already handling things correctly.
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
…al-sh#19579)

## Summary

closes astral-sh/ty#692

If the expression (or any child expressions) is not definitely bound the
reachability constraint evaluation is determined as ambiguous.

This fixes the infinite cycles panic in the following code:

```py
from typing import Literal

class Toggle:
    def __init__(self: "Toggle"):
        if not self.x:
            self.x: Literal[True] = True
```

Credit of this solution is for David.

## Test Plan

- Added a test case with too many cycle iterations panic.
- Previous tests.

---------

Co-authored-by: David Peter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecosystem-analyzer ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

too many cycle iterations in cyclic dependent attributes

3 participants