Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Crash when (Mutable)Sequence is imported from collections.abc in typeshed/stdlib/builtins.pyi #11860

Closed
AlexWaygood opened this issue Dec 28, 2021 · 2 comments · Fixed by #14088
Closed

Crash when (Mutable)Sequence is imported from collections.abc in typeshed/stdlib/builtins.pyi #11860

AlexWaygood opened this issue Dec 28, 2021 · 2 comments · Fixed by #14088
Labels
affects-typeshed Anything that blocks a typeshed change crash semantic-analyzer Problems that happen during semantic analysis topic-pep-585 PEP 585 (builtin generics)

Comments

@AlexWaygood
Copy link
Member

Crash Report

Mypy crashes if one or both of Sequence and MutableSequence is imported from collections.abc instead of typing in the typeshed stub for builtins.

Traceback

stdlib\builtins.pyi:1506: error: Unexpected "..."
C:\Users\Alex\Desktop\Code dump\typeshed\stdlib\collections\__init__.pyi: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 0.930
Traceback (most recent call last):
  File "C:\Program Files\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\Alex\Desktop\Code dump\typeshed\.venv3\Scripts\mypy.exe\__main__.py", line 7, in <module>
    sys.exit(console_entry())
  File "mypy\build.py", line 1972, in wrap_context
  File "mypy\semanal_main.py", line 326, in semantic_analyze_target
  File "mypy\semanal.py", line 405, in refresh_partial
  File "mypy\semanal.py", line 414, in refresh_top_level
  File "mypy\semanal.py", line 444, in add_implicit_module_attrs
AssertionError:
C:\Users\Alex\Desktop\Code dump\typeshed\stdlib\collections\__init__.pyi: : note: use --pdb to drop into pdb

To Reproduce

  1. Clone typeshed
  2. Change one or both of Sequence and MutableSequence in typeshed/stdlib/builtins.pyi so that they are imported from collections.abc rather than typing.
  3. cd into the typeshed repo.
  4. Run mypy stdlib/functools.py --show-traceback (the file you run mypy on is irrelevant).

Using the typeshed CI, I have reproduced this issue:

  • On MacOS, Windows and Ubuntu
  • On Python versions 3.6-3.10 inclusive.
  • Importing just Sequence from collections.abc; importing just MutableSequence from collections.abc; and importing both Sequence and MutableSequence from collections.abc.

Mypy version used
0.930
@AlexWaygood
Copy link
Member Author

Cc. @JukkaL, @sobolevn, as requested in python/typeshed#4820 🙂

This was referenced Jan 18, 2022
Merged
Closed
Closed
@AlexWaygood AlexWaygood mentioned this issue Feb 11, 2022
Merged
@AlexWaygood
Copy link
Member Author

@JelleZijlstra, could you add the "topic-pep-585" label to this? :)

@JelleZijlstra JelleZijlstra added the topic-pep-585 PEP 585 (builtin generics) label Mar 20, 2022
@AlexWaygood AlexWaygood added semantic-analyzer Problems that happen during semantic analysis affects-typeshed Anything that blocks a typeshed change labels Mar 26, 2022
@AlexWaygood AlexWaygood mentioned this issue Apr 18, 2022
Merged
Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Nov 14, 2022
@Michael0x2a
Adjust SCC setup to enable earlier collections.abc import
e88bd88 
Fixes python#11860 (?)

Typeshed is currently unable to import Sequence, MutableSequence,
or ByteString from collections.abc within builtins.pyi. It seems
this is because:

1. In order to analyze `collections.abc`, we first need to analyze
   `collections`.

2. Since `collections` is a package containing an `__init__.pyi`
   file, the `add_implicit_module_attrs` function will try adding
   the `__path__` variable to the symboltable.

3. The `__path__` variable has type `builtins.str`. But str is a
   subclass of Sequence, which we have not analyzed yet since we're
   still in the middle of analyzing `collections` and `collections.abc`.

This diff tries repairing this by:

1. Adding `_collections_abc` and `collections.abc` to the set of
   special-cased core modules we deliberately process early.

2. Modifying `add_implicit_module_attrs` so it does the same trick
   we do for the `__doc__` symbol and fall back to using an UnboundType
   if `builtins.str` is not defined yet.

To be 100% honest, I'm not really sold on this PR for a few reasons:

- I was able to test these changes manually, but wasn't sure how to
  write tests for them.
- We have 3-4 subtly different lists of "core modules" scattered
  throughout mypy. For example, see `CORE_BUILTIN_MODULES` in
  mypy/build.py or try grepping for the string `"typing"` in the mypy
  dir. Arguably, we should defer landing this PR until we've had a
  chance to consolidate these lists and confirm there are no additional
  places where we need to special-case `_collections_abc`,
  `collections`, and `collections.abc`.
- PEP 585 attempted to declare that we should one day remove entries like
  Sequence from `typing` module, but this realistically doesn't seem
  ever achievable given that (a) it would break backwards compat and
  (b) there doesn't seem to be any incentives for users to proactively
  switch. In that case, is there any pressing reason to change typeshed?

Regardless, this is a crash and my goal atm is to de-crash mypy, so
I'm throwing this over the wall.
@Michael0x2a Michael0x2a mentioned this issue Nov 14, 2022
Merged
JelleZijlstra pushed a commit that referenced this issue Jan 26, 2023
@Michael0x2a
Adjust SCC setup to enable earlier collections.abc import in typeshed (…
6442b02 
…#14088)

Fixes #11860 (?)

Typeshed is currently unable to import Sequence, MutableSequence, or
ByteString from collections.abc within builtins.pyi. It seems this is
because:

1. In order to analyze `collections.abc`, we first need to analyze
`collections`.

2. Since `collections` is a package containing an `__init__.pyi` file,
the `add_implicit_module_attrs` function will try adding the `__path__`
variable to the symboltable.

3. The `__path__` variable has type `builtins.str`. But str is a
subclass of Sequence, which we have not analyzed yet since we're still
in the middle of analyzing `collections` and `collections.abc`.

This diff tries repairing this by:

1. Adding `_collections_abc` and `collections.abc` to the set of
special-cased core modules we deliberately process early.

2. Modifying `add_implicit_module_attrs` so it does the same trick we do
for the `__doc__` symbol and fall back to using an UnboundType if
`builtins.str` is not defined yet.

To be 100% honest, I'm not really sold on this PR for a few reasons:

- I was able to test these changes manually, but wasn't sure how to
write tests for them.
- We have 3-4 subtly different lists of "core modules" scattered
throughout mypy. For example, see `CORE_BUILTIN_MODULES` in
mypy/build.py or try grepping for the string `"typing"` in the mypy dir.
Arguably, we should defer landing this PR until we've had a chance to
consolidate these lists and confirm there are no additional places where
we need to special-case `_collections_abc`, `collections`, and
`collections.abc`.
- PEP 585 attempted to declare that we should one day remove entries
like Sequence from `typing` module, but this realistically doesn't seem
ever achievable given that (a) it would break backwards compat and (b)
there doesn't seem to be any incentives for users to proactively switch.
In that case, is there any pressing reason to change typeshed?

Regardless, this is a crash and my goal atm is to de-crash mypy, so I'm
throwing this over the wall.
AlexWaygood pushed a commit to AlexWaygood/mypy that referenced this issue Jan 26, 2023
@Michael0x2a @AlexWaygood
Adjust SCC setup to enable earlier collections.abc import in typeshed (…
3740e98 
…python#14088)

Fixes python#11860 (?)

Typeshed is currently unable to import Sequence, MutableSequence, or
ByteString from collections.abc within builtins.pyi. It seems this is
because:

1. In order to analyze `collections.abc`, we first need to analyze
`collections`.

2. Since `collections` is a package containing an `__init__.pyi` file,
the `add_implicit_module_attrs` function will try adding the `__path__`
variable to the symboltable.

3. The `__path__` variable has type `builtins.str`. But str is a
subclass of Sequence, which we have not analyzed yet since we're still
in the middle of analyzing `collections` and `collections.abc`.

This diff tries repairing this by:

1. Adding `_collections_abc` and `collections.abc` to the set of
special-cased core modules we deliberately process early.

2. Modifying `add_implicit_module_attrs` so it does the same trick we do
for the `__doc__` symbol and fall back to using an UnboundType if
`builtins.str` is not defined yet.

To be 100% honest, I'm not really sold on this PR for a few reasons:

- I was able to test these changes manually, but wasn't sure how to
write tests for them.
- We have 3-4 subtly different lists of "core modules" scattered
throughout mypy. For example, see `CORE_BUILTIN_MODULES` in
mypy/build.py or try grepping for the string `"typing"` in the mypy dir.
Arguably, we should defer landing this PR until we've had a chance to
consolidate these lists and confirm there are no additional places where
we need to special-case `_collections_abc`, `collections`, and
`collections.abc`.
- PEP 585 attempted to declare that we should one day remove entries
like Sequence from `typing` module, but this realistically doesn't seem
ever achievable given that (a) it would break backwards compat and (b)
there doesn't seem to be any incentives for users to proactively switch.
In that case, is there any pressing reason to change typeshed?

Regardless, this is a crash and my goal atm is to de-crash mypy, so I'm
throwing this over the wall.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects-typeshed Anything that blocks a typeshed change crash semantic-analyzer Problems that happen during semantic analysis topic-pep-585 PEP 585 (builtin generics)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adjust SCC setup to enable earlier collections.abc import in typeshed Michael0x2a/mypy
2 participants
@JelleZijlstra @AlexWaygood