From 62b58754b13f522a52cebfeb65ad102db3da7010 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Thu, 17 Feb 2022 11:57:39 +0000 Subject: [PATCH 1/9] stubtest: error if a dunder method is missing from a stub --- mypy/stubtest.py | 66 ++++++++++++++++++++++++++++++++++++--- mypy/test/teststubtest.py | 16 +++++++--- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index e67992ace5f8..aa8cbcba1ae3 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -25,7 +25,7 @@ from mypy import nodes from mypy.config_parser import parse_config_file from mypy.options import Options -from mypy.util import FancyFormatter, bytes_to_human_readable_repr, is_dunder, SPECIAL_DUNDERS +from mypy.util import FancyFormatter, bytes_to_human_readable_repr, is_dunder class Missing: @@ -243,6 +243,56 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: ) +IGNORED_DUNDERS = frozenset({ + # Very special attributes + "__weakref__", + "__slots__", + "__dict__", + "__text_signature__", + "__match_args__", + # Pickle methods + "__setstate__", + "__getstate__", + "__getnewargs__", + "__getinitargs__", + "__reduce_ex__", + # typing implementation details + "__parameters__", + "__origin__", + "__args__", + "__orig_bases__", + "__mro_entries__", + "__forward_is_class__", + "__forward_module__", + "__final__", + # isinstance/issubclass hooks that type-checkers don't usually care about + "__instancecheck__", + "__subclasshook__", + "__subclasscheck__", + # Dataclasses implementation details + "__dataclass_fields__", + "__dataclass_params__", + # ctypes weirdness + "__ctype_be__", + "__ctype_le__", + "__ctypes_from_outparam__", + # Two float methods only used internally for CPython test suite, not for public use + "__set_format__", + "__getformat__", + # These two are basically useless for type checkers + "__hash__", + "__getattr__", + "__abstractmethods__", # For some reason, mypy doesn't infer that classes with ABCMeta as the metaclass have this inherited + "__doc__", # Can only ever be str | None, who cares? + "__del__", # Only ever called when an object is being deleted, who cares? + "__new_member__", # If an enum class defines `__new__`, the method is renamed to be `__new_member__` +}) + + +def is_private(name: str) -> bool: + return name.startswith("_") and not is_dunder(name) + + @verify.register(nodes.TypeInfo) def verify_typeinfo( stub: nodes.TypeInfo, runtime: MaybeMissing[Type[Any]], object_path: List[str] @@ -274,11 +324,9 @@ class SubClass(runtime): # type: ignore # Check everything already defined in the stub to_check = set(stub.names) - # There's a reasonable case to be made that we should always check all dunders, but it's - # currently quite noisy. We could turn this into a denylist instead of an allowlist. to_check.update( # cast to workaround mypyc complaints - m for m in cast(Any, vars)(runtime) if not m.startswith("_") or m in SPECIAL_DUNDERS + m for m in cast(Any, vars)(runtime) if not is_private(m) and m not in IGNORED_DUNDERS ) for entry in sorted(to_check): @@ -713,7 +761,15 @@ def verify_funcitem( def verify_none( stub: Missing, runtime: MaybeMissing[Any], object_path: List[str] ) -> Iterator[Error]: - yield Error(object_path, "is not present in stub", stub, runtime) + # Do not error for an object missing from the stub + # If the runtime object is a types.WrapperDescriptorType + # and has a non-special dunder name. + # The vast majority of these are false positives. + if not ( + isinstance(runtime, types.WrapperDescriptorType) + and is_dunder(runtime.__name__, exclude_special=True) + ): + yield Error(object_path, "is not present in stub", stub, runtime) @verify.register(nodes.Var) diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 2852299548ed..0d3da5130d6a 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -37,6 +37,7 @@ def use_tmp_dir() -> Iterator[None]: VT = TypeVar('VT') class object: + __module__: str def __init__(self) -> None: pass class type: ... @@ -678,7 +679,7 @@ def test_non_public_2(self) -> Iterator[Case]: yield Case(stub="g: int", runtime="def g(): ...", error="g") @collect_cases - def test_special_dunders(self) -> Iterator[Case]: + def test_dunders(self) -> Iterator[Case]: yield Case( stub="class A:\n def __init__(self, a: int, b: int) -> None: ...", runtime="class A:\n def __init__(self, a, bx): pass", @@ -689,21 +690,26 @@ def test_special_dunders(self) -> Iterator[Case]: runtime="class B:\n def __call__(self, c, dx): pass", error="B.__call__", ) + yield Case( + stub="class C:\n def __or__(self, other: C) -> C: ...", + runtime="class C: ...", + error="C.__or__", + ) if sys.version_info >= (3, 6): yield Case( stub=( - "class C:\n" + "class D:\n" " def __init_subclass__(\n" " cls, e: int = ..., **kwargs: int\n" " ) -> None: ...\n" ), - runtime="class C:\n def __init_subclass__(cls, e=1, **kwargs): pass", + runtime="class D:\n def __init_subclass__(cls, e=1, **kwargs): pass", error=None, ) if sys.version_info >= (3, 9): yield Case( - stub="class D:\n def __class_getitem__(cls, type: type) -> type: ...", - runtime="class D:\n def __class_getitem__(cls, type): ...", + stub="class E:\n def __class_getitem__(cls, type: type) -> type: ...", + runtime="class E:\n def __class_getitem__(cls, type): ...", error=None, ) From 8324e2fd2d16e1b5e8eaa49227ce3467d62c301d Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Thu, 17 Feb 2022 12:31:27 +0000 Subject: [PATCH 2/9] flake8 --- mypy/stubtest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index aa8cbcba1ae3..189e3751e160 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -282,10 +282,11 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: # These two are basically useless for type checkers "__hash__", "__getattr__", - "__abstractmethods__", # For some reason, mypy doesn't infer that classes with ABCMeta as the metaclass have this inherited + # For some reason, mypy doesn't infer classes with metaclass=ABCMeta inherit this attribute + "__abstractmethods__", "__doc__", # Can only ever be str | None, who cares? "__del__", # Only ever called when an object is being deleted, who cares? - "__new_member__", # If an enum class defines `__new__`, the method is renamed to be `__new_member__` + "__new_member__", # If an enum defines __new__, the method is renamed as __new_member__ }) From ce0cc14083b87c26861433ec02495341def24a34 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Thu, 17 Feb 2022 12:36:46 +0000 Subject: [PATCH 3/9] py36 fixes (ew) --- mypy/stubtest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 189e3751e160..ac02af5d52d9 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -763,12 +763,12 @@ def verify_none( stub: Missing, runtime: MaybeMissing[Any], object_path: List[str] ) -> Iterator[Error]: # Do not error for an object missing from the stub - # If the runtime object is a types.WrapperDescriptorType + # If the runtime object is a types.WrapperDescriptorType object # and has a non-special dunder name. # The vast majority of these are false positives. if not ( - isinstance(runtime, types.WrapperDescriptorType) - and is_dunder(runtime.__name__, exclude_special=True) + isinstance(runtime, type(object.__init__)) + and is_dunder(getattr(runtime, "__name__", ""), exclude_special=True) ): yield Error(object_path, "is not present in stub", stub, runtime) From 4425a3b7b68962597be9c9b8d7266f781574b9e7 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Thu, 17 Feb 2022 15:09:36 +0000 Subject: [PATCH 4/9] Make tests actually test what I want them to test --- mypy/stubtest.py | 1 + mypy/test/teststubtest.py | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index ac02af5d52d9..b72c12added7 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -256,6 +256,7 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: "__getnewargs__", "__getinitargs__", "__reduce_ex__", + "__reduce__", # typing implementation details "__parameters__", "__origin__", diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 0d3da5130d6a..9964cd20cadf 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -658,6 +658,16 @@ def h(x: str): ... yield Case( stub="from mystery import A, B as B, C as D # type: ignore", runtime="", error="B" ) + yield Case( + stub="class Y: ...", + runtime="__all__ += ['Y']\nclass Y:\n def __or__(self, other): return self|other", + error="Y.__or__" + ) + yield Case( + stub="class Z: ...", + runtime="__all__ += ['Z']\nclass Z:\n def __reduce__(self): return (Z,)", + error=None + ) @collect_cases def test_missing_no_runtime_all(self) -> Iterator[Case]: @@ -690,26 +700,21 @@ def test_dunders(self) -> Iterator[Case]: runtime="class B:\n def __call__(self, c, dx): pass", error="B.__call__", ) - yield Case( - stub="class C:\n def __or__(self, other: C) -> C: ...", - runtime="class C: ...", - error="C.__or__", - ) if sys.version_info >= (3, 6): yield Case( stub=( - "class D:\n" + "class C:\n" " def __init_subclass__(\n" " cls, e: int = ..., **kwargs: int\n" " ) -> None: ...\n" ), - runtime="class D:\n def __init_subclass__(cls, e=1, **kwargs): pass", + runtime="class C:\n def __init_subclass__(cls, e=1, **kwargs): pass", error=None, ) if sys.version_info >= (3, 9): yield Case( - stub="class E:\n def __class_getitem__(cls, type: type) -> type: ...", - runtime="class E:\n def __class_getitem__(cls, type): ...", + stub="class D:\n def __class_getitem__(cls, type: type) -> type: ...", + runtime="class D:\n def __class_getitem__(cls, type): ...", error=None, ) From ba25ed0b222738e5f923ad08c8256d82c01470b3 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Fri, 18 Feb 2022 14:52:54 +0000 Subject: [PATCH 5/9] Improve py36 workaround, add comment re `__match_args__` --- mypy/stubtest.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index b72c12added7..e56d387e4cfb 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -49,6 +49,22 @@ def _style(message: str, **kwargs: Any) -> str: return _formatter.style(message, **kwargs) +if sys.version_info >= (3, 7): + def is_dunder_slot_wrapper(obj: object) -> bool: + return ( + isinstance(obj, types.WrapperDescriptorType) + and is_dunder(obj.__name__, exclude_special=True) + ) + +else: + # types.WrapperDescriptorType was added in 3.7 + def is_dunder_slot_wrapper(obj: object) -> bool: + return ( + isinstance(obj, type(object.__init__)) + and is_dunder(getattr(obj, "__name__", ""), exclude_special=True) + ) + + class Error: def __init__( self, @@ -249,7 +265,6 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: "__slots__", "__dict__", "__text_signature__", - "__match_args__", # Pickle methods "__setstate__", "__getstate__", @@ -285,6 +300,9 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: "__getattr__", # For some reason, mypy doesn't infer classes with metaclass=ABCMeta inherit this attribute "__abstractmethods__", + # Ideally we'd include __match_args__ in stubs, + # but this currently has issues + "__match_args__", "__doc__", # Can only ever be str | None, who cares? "__del__", # Only ever called when an object is being deleted, who cares? "__new_member__", # If an enum defines __new__, the method is renamed as __new_member__ @@ -767,10 +785,7 @@ def verify_none( # If the runtime object is a types.WrapperDescriptorType object # and has a non-special dunder name. # The vast majority of these are false positives. - if not ( - isinstance(runtime, type(object.__init__)) - and is_dunder(getattr(runtime, "__name__", ""), exclude_special=True) - ): + if not is_dunder_slot_wrapper(runtime): yield Error(object_path, "is not present in stub", stub, runtime) From 749b2c3adebf1aec52828f46f69e987c0288a8d3 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Sat, 19 Feb 2022 22:40:02 +0000 Subject: [PATCH 6/9] Remove 5 methods from the ignorelist --- mypy/stubtest.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 7fac3eb9d83e..be9afa636a17 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -277,9 +277,6 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: "__origin__", "__args__", "__orig_bases__", - "__mro_entries__", - "__forward_is_class__", - "__forward_module__", "__final__", # isinstance/issubclass hooks that type-checkers don't usually care about "__instancecheck__", @@ -292,9 +289,6 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: "__ctype_be__", "__ctype_le__", "__ctypes_from_outparam__", - # Two float methods only used internally for CPython test suite, not for public use - "__set_format__", - "__getformat__", # These two are basically useless for type checkers "__hash__", "__getattr__", From cfb4c3a0a9ad8b27cad40a36935d7355a3bb1f50 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Sat, 19 Feb 2022 22:52:15 +0000 Subject: [PATCH 7/9] Get rid of 3rd arg in `getattr` call --- mypy/stubtest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index be9afa636a17..331f9c578b23 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -61,7 +61,7 @@ def is_dunder_slot_wrapper(obj: object) -> bool: def is_dunder_slot_wrapper(obj: object) -> bool: return ( isinstance(obj, type(object.__init__)) - and is_dunder(getattr(obj, "__name__", ""), exclude_special=True) + and is_dunder(getattr(obj, "__name__"), exclude_special=True) ) From a71b1b3c8acb6ea435d9c3b35a7e75ffe1b033bf Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Sat, 19 Feb 2022 22:59:25 +0000 Subject: [PATCH 8/9] Move logic from `verify_none` into `verify_typeinfo` --- mypy/stubtest.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 331f9c578b23..39c1e483133d 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -354,9 +354,14 @@ class SubClass(runtime): # type: ignore except Exception: # Catch all exceptions in case the runtime raises an unexpected exception # from __getattr__ or similar. - pass - else: - yield from verify(stub_to_verify, runtime_attr, object_path + [entry]) + continue + # Do not error for an object missing from the stub + # If the runtime object is a types.WrapperDescriptorType object + # and has a non-special dunder name. + # The vast majority of these are false positives. + if isinstance(stub_to_verify, Missing) and is_dunder_slot_wrapper(runtime_attr): + continue + yield from verify(stub_to_verify, runtime_attr, object_path + [entry]) def _verify_static_class_methods( @@ -808,12 +813,7 @@ def verify_funcitem( def verify_none( stub: Missing, runtime: MaybeMissing[Any], object_path: List[str] ) -> Iterator[Error]: - # Do not error for an object missing from the stub - # If the runtime object is a types.WrapperDescriptorType object - # and has a non-special dunder name. - # The vast majority of these are false positives. - if not is_dunder_slot_wrapper(runtime): - yield Error(object_path, "is not present in stub", stub, runtime) + yield Error(object_path, "is not present in stub", stub, runtime) @verify.register(nodes.Var) From 1720eaaf69b55cfe79de41be314c255536290ae6 Mon Sep 17 00:00:00 2001 From: Alex Waygood <Alex.Waygood@Gmail.com> Date: Sat, 19 Feb 2022 23:13:17 +0000 Subject: [PATCH 9/9] Death to the `is_dunder_slot_wrapper` helper function --- mypy/stubtest.py | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 39c1e483133d..dacd31dbcf84 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -49,22 +49,6 @@ def _style(message: str, **kwargs: Any) -> str: return _formatter.style(message, **kwargs) -if sys.version_info >= (3, 7): - def is_dunder_slot_wrapper(obj: object) -> bool: - return ( - isinstance(obj, types.WrapperDescriptorType) - and is_dunder(obj.__name__, exclude_special=True) - ) - -else: - # types.WrapperDescriptorType was added in 3.7 - def is_dunder_slot_wrapper(obj: object) -> bool: - return ( - isinstance(obj, type(object.__init__)) - and is_dunder(getattr(obj, "__name__"), exclude_special=True) - ) - - class Error: def __init__( self, @@ -303,6 +287,12 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool: }) +if sys.version_info >= (3, 7): + _WrapperDescriptorType = types.WrapperDescriptorType +else: + _WrapperDescriptorType = type(object.__init__) + + def is_private(name: str) -> bool: return name.startswith("_") and not is_dunder(name) @@ -359,9 +349,12 @@ class SubClass(runtime): # type: ignore # If the runtime object is a types.WrapperDescriptorType object # and has a non-special dunder name. # The vast majority of these are false positives. - if isinstance(stub_to_verify, Missing) and is_dunder_slot_wrapper(runtime_attr): - continue - yield from verify(stub_to_verify, runtime_attr, object_path + [entry]) + if not ( + isinstance(stub_to_verify, Missing) + and isinstance(runtime_attr, _WrapperDescriptorType) + and is_dunder(mangled_entry, exclude_special=True) + ): + yield from verify(stub_to_verify, runtime_attr, object_path + [entry]) def _verify_static_class_methods(