Skip to content

[ty] Preserve checks for recursive descriptor setter types - #29148

Open
charliermarsh wants to merge 1 commit into
charlie/codex-tuple-promotionfrom
charlie/codex-descriptor-dependencies
Open

charliermarsh wants to merge 1 commit into
charlie/codex-tuple-promotionfrom
charlie/codex-descriptor-dependencies

Conversation

@charliermarsh

@charliermarsh charliermarsh commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Summary

This PR fixes a stack overflow when checking a protocol's writable member whose descriptor setter accepts a recursive type. For example, checking the assignment below previously aborted the process instead of reporting an invalid assignment:

from __future__ import annotations

from typing import Protocol

class Recursive[T](Protocol):
    next: Recursive[list[T]]

class Descriptor:
    def __init__(self, getter: object) -> None: ...
    def __get__(self, instance: object, owner: type | None = None) -> int:
        return 1

    def __set__[U](self, instance: object, value: Recursive[int]) -> None: ...

class HasValue(Protocol):
    @Descriptor
    def value(self) -> int: ...

def update(obj: HasValue) -> None:
    obj.value = 1  # Invalid: the setter requires Recursive[int].

The setter's unused U triggers a search to determine whether the accepted value type depends on that type parameter. The existing search follows next through Recursive[int], Recursive[list[int]], Recursive[list[list[int]]], and so on. Its recursion guard remembers exact types, so these changing specializations never repeat and the search overflows the stack. Removing U avoids the search and the crash.

We replace that general type search with a dependency check that recognizes repeated definitions and inspects the arguments of recursive references without repeatedly expanding their bodies. Here, it establishes that the setter accepts Recursive[int] independently of U, so we can report the invalid assignment normally. The same fix covers recursive TypedDicts and type aliases, while retaining checks for assignments and protocol compatibility.

@charliermarsh charliermarsh added bug An issue describing something that isn't working, or a PR that fixes a bug ty The ty type checker labels Oct 6, 2026
@charliermarsh
charliermarsh added this pull request to stack #29150 October 6, 2026 20:05
@astral-sh-bot

astral-sh-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 98.24%. The percentage of expected errors that received a diagnostic held steady at 98.24%. The number of fully passing files held steady at 134/146.

@astral-sh-bot

astral-sh-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot

astral-sh-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

ecosystem-analyzer results

No diagnostic changes detected ✅

Large timing changes:

Project Old Time New Time Change
django-test-migrations 0.10s 0.05s -53%

Flaky changes detected. This PR summary excludes flaky changes; see the HTML report for details.

Full report with detailed diff (timing results)

@charliermarsh
charliermarsh marked this pull request as ready for review October 6, 2026 20:57
@charliermarsh
charliermarsh requested a review from a team as a code owner October 6, 2026 20:57
@charliermarsh
charliermarsh removed this pull request from stack #29150 October 6, 2026 23:50
@charliermarsh
charliermarsh added this pull request to stack #29156 October 6, 2026 23:50
Inspect dependencies on the setter signature explicitly, including recursive aliases, protocols, and TypedDicts. Recursive specializations that are independent of the setter parameters retain their writable-member checks.

Co-authored-by: Ibraheem Ahmed <ibraheem@ibraheem.ca>
@charliermarsh
charliermarsh force-pushed the charlie/codex-descriptor-dependencies branch from edcaf05 to e55268b Compare October 7, 2026 02:00

This branch was successfully deployed

1 active deployment
automations — e55268b9 Deployed Oct 7, 2026 by charliermarsh via security-review / security review #90921
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue describing something that isn't working, or a PR that fixes a bug ty The ty type checker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant