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.

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

Untested code in typing.py #105834

Closed
AlexWaygood opened this issue Jun 15, 2023 · 1 comment
Closed

Untested code in typing.py #105834

AlexWaygood opened this issue Jun 15, 2023 · 1 comment
Labels
tests Tests in the Lib/test dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@AlexWaygood
Copy link
Member

AlexWaygood commented Jun 15, 2023

Bug report

If you apply this diff to typing.py, all tests continue to pass:

diff --git a/Lib/typing.py b/Lib/typing.py
index 1dd9398344..98e19644a2 100644
--- a/Lib/typing.py
+++ b/Lib/typing.py
@@ -1931,6 +1931,7 @@ def _proto_hook(other):
                     if (isinstance(annotations, collections.abc.Mapping) and
                             attr in annotations and
                             issubclass(other, Generic) and getattr(other, '_is_protocol', False)):
+                        1/0
                         break

The break on line 1934 here is unreachable; thus, this whole block of code is pointless:

cpython/Lib/typing.py

Lines 1929 to 1934 in 3af2dc7

# ...or in annotations, if it is a sub-protocol.
annotations = getattr(base, '__annotations__', {})
if (isinstance(annotations, collections.abc.Mapping) and
attr in annotations and
issubclass(other, Generic) and getattr(other, '_is_protocol', False)):
break

I think we can say for sure that the break here is unreachable. This is the __subclasshook__ method that is monkey-patched onto all subclasses of typing.Protocol that do not define their own __subclasshook__ methods. This block of code in the __subclasshook__ method is inspecting the __annotations__ dictionary of other to see if it can find any protocol members in that dictionary. But we know that there can't be any protocol members in the __annotations__ dictionary, because if there were, that would make other a protocol with at least one non-callable member. If it's a protocol that has at least one non-callable member, the __subclasshook__ method is never called at all during isinstance() or issubclass() checks, because we raise TypeError in _ProtocolMeta.__subclasscheck__, short-circuiting the call to abc.ABCMeta.__subclasscheck__ that would call Protocol.__subclasshook__:

cpython/Lib/typing.py

Lines 1828 to 1831 in 3af2dc7

if not cls.__callable_proto_members_only__:
raise TypeError(
"Protocols with non-method members don't support issubclass()"
)

I believe that this block of code can therefore be safely deleted.

Linked PRs

@AlexWaygood
Copy link
Member Author

As has been pointed out in the PR discussion, this code is not in fact dead, just completely untested. (Thanks @JelleZijlstra!)

@AlexWaygood AlexWaygood changed the title Dead code in typing.py Untested code in typing.py Jun 16, 2023
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label Jun 16, 2023
AlexWaygood added a commit that referenced this issue Jun 16, 2023
…#105835)

Some parts of the implementation of `typing.Protocol` had poor test coverage
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 16, 2023
…tocols (pythonGH-105835)

Some parts of the implementation of `typing.Protocol` had poor test coverage
(cherry picked from commit 70c075c)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
AlexWaygood added a commit to AlexWaygood/cpython that referenced this issue Jun 16, 2023
…two protocols (python#105835)

Some parts of the implementation of `typing.Protocol` had poor test coverage
JelleZijlstra pushed a commit that referenced this issue Jun 16, 2023
…otocols (GH-105835) (#105859)

Some parts of the implementation of `typing.Protocol` had poor test coverage
(cherry picked from commit 70c075c)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
carljm added a commit to carljm/cpython that referenced this issue Jun 16, 2023
* main:
  pythongh-104799: PEP 695 backward compatibility for ast.unparse (python#105846)
  pythongh-105834: Add tests for calling `issubclass()` between two protocols (python#105835)
  CI: Remove docs build from Azure Pipelines (python#105823)
  pythongh-105844: Consistently use 'minor version' for X.Y versions (python#105851)
  Fix inaccuracies in "Assorted Topics" section of "Defining Extension Types" tutorial (python#104969)
  pythongh-105433: Add `pickle` tests for PEP695 (python#105443)
  bpo-44530: Document the change in MAKE_FUNCTION behavior (python#93189)
  pythonGH-103124: Multiline statement support for pdb (pythonGH-103125)
  pythonGH-105588: Add missing error checks to some obj2ast_* converters (pythonGH-105589)
gvanrossum pushed a commit to gvanrossum/cpython that referenced this issue Jun 18, 2023
…tocols (python#105835)

Some parts of the implementation of `typing.Protocol` had poor test coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant