Skip to content

Fix crashes on incorectly detected recursive aliases #18625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

ilevkivskyi
Copy link
Member

Fixes #18505
Fixes #16757

Fixing the crash is trivial, we simply give an error on something like type X = X, instead of crashing. But then I looked at what people actually want to do, they want to create an alias to something currently in scope (not a meaningless no-op alias). Right now we special-case classes/aliases to allow forward references (including recursive type aliases). However, I don't think we have any clear "scoping rules" for forward references. For example:

class C:
    Y = X
    class X: ...
class X: ...

where Y should point to, __main__.X or __main__.C.X? Moreover, before this PR forward references can take precedence over real references:

class X: ...
class C:
    Y = X  # this resolves to __main__.C.X
    class X: ...

After some thinking I found this is not something I can fix in a simple PR. So instead I do just two things here:

  • Fix the actual crashes (and other potential similar crashes).
  • Add minimal change to accommodate the typical use case.

@ilevkivskyi ilevkivskyi requested a review from JukkaL February 6, 2025 23:45
Copy link
Contributor

github-actions bot commented Feb 7, 2025

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Looks good. The logic is complicated, but users rely on this functionality so it's good that we support it.

@JukkaL JukkaL merged commit 75b5604 into python:master Feb 7, 2025
18 checks passed
@ilevkivskyi ilevkivskyi deleted the fix-class-name-resolution branch February 7, 2025 11:18
x612skm pushed a commit to x612skm/mypy-dev that referenced this pull request Feb 24, 2025
Fixes python#18505
Fixes python#16757

Fixing the crash is trivial, we simply give an error on something like
`type X = X`, instead of crashing. But then I looked at what people
actually want to do, they want to create an alias to something currently
in scope (not a meaningless no-op alias). Right now we special-case
classes/aliases to allow forward references (including recursive type
aliases). However, I don't think we have any clear "scoping rules" for
forward references. For example:
```python
class C:
    Y = X
    class X: ...
class X: ...
```
where `Y` should point to, `__main__.X` or `__main__.C.X`? Moreover,
before this PR forward references can take precedence over real
references:
```python
class X: ...
class C:
    Y = X  # this resolves to __main__.C.X
    class X: ...
```
After some thinking I found this is not something I can fix in a simple
PR. So instead I do just two things here:
* Fix the actual crashes (and other potential similar crashes).
* Add minimal change to accommodate the typical use case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mypy hangs on recursive type keyword in combination with an unresolved type name Infinite typechecking
2 participants