Skip to content

Commit

Permalink
Fixed bad type checking logic when a union element is shadowed
Browse files Browse the repository at this point in the history
Fixes #394. Fixes #395.
  • Loading branch information
agronholm committed Sep 9, 2023
1 parent 2fc1dd5 commit 4d1768c
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 36 deletions.
5 changes: 5 additions & 0 deletions docs/versionhistory.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ This library adheres to
encountering an annotation with a subscript containing an ignored type (imported
within an ``if TYPE_CHECKING:`` block)
(`#397 <https://github.com/agronholm/typeguard/issues/397>`_)
- Fixed type checking not being skipped when the target is a union (PEP 604 or
``typing.Union``) where one of the elements is an ignored type (shadowed by an
argument, variable assignment or an ``if TYPE_CHECKING`` import)
(`#394 <https://github.com/agronholm/typeguard/issues/394>`_,
`#395 <https://github.com/agronholm/typeguard/issues/395>`_)

**4.1.3** (2023-08-27)

Expand Down
19 changes: 9 additions & 10 deletions src/typeguard/_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,11 @@ def visit_BinOp(self, node: BinOp) -> Any:
self.generic_visit(node)

if isinstance(node.op, BitOr):
# If either branch of the BinOp has been transformed to `None`
# then the `ast.generic_visit` will eliminate that branch completely.
# If this happens, treat the BinOp as just the other branch.
if not hasattr(node, "left") and not hasattr(node, "right"):
# If either branch of the BinOp has been transformed to `None`, it means
# that a type in the union was ignored, so the entire annotation should e
# ignored
if not hasattr(node, "left") or not hasattr(node, "right"):
return None
elif not hasattr(node, "left"):
return node.right
elif not hasattr(node, "right"):
return node.left

# Return Any if either side is Any
if self._memo.name_matches(node.left, *anytype_names):
Expand Down Expand Up @@ -439,8 +435,11 @@ def visit_Subscript(self, node: Subscript) -> Any:
# If this is a Union and any of the items is Any, erase the entire
# annotation
if self._memo.name_matches(node.value, "typing.Union") and any(
isinstance(item, expr)
and self._memo.name_matches(item, *anytype_names)
item is None
or (
isinstance(item, expr)
and self._memo.name_matches(item, *anytype_names)
)
for item in items
):
return None
Expand Down
109 changes: 83 additions & 26 deletions tests/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ def foo(x: Optional[Hashable]) -> Optional[Hashable]:
).strip()
)

def test_union(self) -> None:
def test_subscript_within_union(self) -> None:
# Regression test for #397
node = parse(
dedent(
Expand Down Expand Up @@ -1042,6 +1042,34 @@ def foo(x: Union[Iterable[Hashable], str]) -> None:
).strip()
)

def test_pep604_union(self) -> None:
node = parse(
dedent(
"""
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import Hashable
def foo(x: Hashable | str) -> None:
pass
"""
)
)
TypeguardTransformer().visit(node)
assert (
unparse(node)
== dedent(
"""
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import Hashable
def foo(x: Hashable | str) -> None:
pass
"""
).strip()
)


class TestAssign:
def test_annotated_assign(self) -> None:
Expand Down Expand Up @@ -1600,35 +1628,64 @@ def foo(x: str) -> None:
)


def test_union_annotation_with_or_operator() -> None:
node = parse(
dedent(
"""
from __future__ import annotations
class TestTypeShadowedByArgument:
def test_typing_union(self) -> None:
# Regression test for #394
node = parse(
dedent(
"""
from __future__ import annotations
from typing import Union
class A:
...
class A:
...
def foo(A: A | None) -> None:
pass
"""
def foo(A: Union[A, None]) -> None:
pass
"""
)
)
)
TypeguardTransformer(["foo"]).visit(node)
assert (
unparse(node)
== dedent(
"""
from __future__ import annotations
TypeguardTransformer(["foo"]).visit(node)
assert (
unparse(node)
== dedent(
"""
from __future__ import annotations
from typing import Union
def foo(A: A | None) -> None:
from typeguard import TypeCheckMemo
from typeguard._functions import check_argument_types
memo = TypeCheckMemo(globals(), locals())
check_argument_types('foo', {'A': (A, None)}, memo)
"""
).strip()
)
def foo(A: Union[A, None]) -> None:
pass
"""
).strip()
)

def test_pep604_union(self) -> None:
# Regression test for #395
node = parse(
dedent(
"""
from __future__ import annotations
class A:
...
def foo(A: A | None) -> None:
pass
"""
)
)
TypeguardTransformer(["foo"]).visit(node)
assert (
unparse(node)
== dedent(
"""
from __future__ import annotations
def foo(A: A | None) -> None:
pass
"""
).strip()
)


def test_dont_parse_annotated_2nd_arg() -> None:
Expand Down

0 comments on commit 4d1768c

Please sign in to comment.