agronholm / typeguard

Run-time type checker for Python
Other
1.56k stars 114 forks source link

Extend `TYPE_CHECKING` handling to classes and assignments #456

Open bersbersbers opened 7 months ago

bersbersbers commented 7 months ago

Things to check first

Typeguard version

4.2.1

Python version

3.12.3

What happened?

typechecked code tries to use a class that is hidden behind TYPE_CHECKING and should not be required at runtime.

How can we reproduce the bug?

bug.py

# python bug.py && mypy bug.py && pyright bug.py

from __future__ import annotations

from typing import TYPE_CHECKING, TypedDict

# from typeguard import typechecked

if TYPE_CHECKING:

    class Args(TypedDict):
        x: int
        y: int

# @typechecked
def bug() -> None:
    args: Args = {"x": 1, "y": 1}
    print(args)

bug()

python bug.py && mypy bug.py && pyright bug.py

{'x': 1, 'y': 1}
Success: no issues found in 1 source file
0 errors, 0 warnings, 0 informations 

Now add the @typechecked decorator, and run python bug.py again:

Traceback (most recent call last):
  File "C:\Code\project\bug.py", line 22, in <module>
    bug()
  File "C:\Code\project\bug.py", line 18, in bug
    args: Args = {"x": 1, "y": 1}
          ^^^^
NameError: name 'Args' is not defined. Did you mean: 'args'?

One workaround is to add bug2.py:

from typing import TypedDict

class Args(TypedDict):
    x: int
    y: int

and modify bug.py to

# python bug.py && mypy bug.py && pyright bug.py

from __future__ import annotations

from typing import TYPE_CHECKING

from typeguard import typechecked

if TYPE_CHECKING:
    from bug2 import Args

@typechecked
def bug() -> None:
    args: Args = {"x": 1, "y": 1}
    print(args)

bug()

But that feels kind of unnecessary, I think.

jolaf commented 2 months ago

Here's what looks like one more example of the same issue, out in the wild:

test.py:

from typeguard import install_import_hook
with install_import_hook(('A', 'urllib3',)):
    import A

A.py:

from urllib3.poolmanager import PoolManager

PoolManager().connection_from_url('https://google.com')

print("OK")
$ python3 A.py
OK

$ python3 test.py
/usr/lib/python3/dist-packages/urllib3/poolmanager.py:147: TypeHintWarning: Cannot resolve forward reference 'ssl.TLSVersion | None'
  return key_class(**context)
/usr/lib/python3/dist-packages/urllib3/poolmanager.py:147: TypeHintWarning: Cannot resolve forward reference 'ssl.SSLContext | None'
  return key_class(**context)
/usr/lib/python3/dist-packages/urllib3/poolmanager.py:330: TypeHintWarning: Cannot resolve forward reference 'ssl.TLSVersion | None'
  def connection_from_pool_key(
/usr/lib/python3/dist-packages/urllib3/poolmanager.py:330: TypeHintWarning: Cannot resolve forward reference 'ssl.SSLContext | None'
  def connection_from_pool_key(
Traceback (most recent call last):
  File "/.../test.py", line 3, in <module>
    import A
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "/home/.../.local/lib/python3.12/site-packages/typeguard/_importhook.py", line 98, in exec_module
    super().exec_module(module)
  File "/.../A.py", line 3, in <module>
    PoolManager().connection_from_url('https://google.com')
  File "/usr/lib/python3/dist-packages/urllib3/poolmanager.py", line 370, in connection_from_url
    return self.connection_from_host(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/urllib3/poolmanager.py", line 303, in connection_from_host
    return self.connection_from_context(request_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/urllib3/poolmanager.py", line 328, in connection_from_context
    return self.connection_from_pool_key(pool_key, request_context=request_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/urllib3/poolmanager.py", line 351, in connection_from_pool_key
    pool = self._new_pool(scheme, host, port, request_context=request_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/urllib3/poolmanager.py", line 265, in _new_pool
    return pool_cls(host, port, **request_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 1001, in __init__
    ssl_minimum_version: ssl.TLSVersion | None = None,
                         ^^^
NameError: name 'ssl' is not defined. Did you forget to import 'ssl'?

$ python --version
Python 3.12.3

$ pip list | grep typeguard
typeguard                          4.3.0

$ pip list | grep urllib3
urllib3                            2.0.7
agronholm commented 3 weeks ago

I think that a more comprehensive change is required, so that we don't try to check against any name not present in the current scope. It's better to have false negatives than NameErrors.