Skip to content

stubtest: error if module level dunder is missing, housekeeping #12217

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

Merged
merged 4 commits into from
Feb 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 88 additions & 52 deletions mypy/stubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ def get_description(self, concise: bool = False) -> str:
return "".join(output)


# ====================
# Core logic
# ====================


def test_module(module_name: str) -> Iterator[Error]:
"""Tests a given module's stub against introspecting it at runtime.

Expand Down Expand Up @@ -204,7 +209,7 @@ def verify_mypyfile(
to_check = set(
m
for m, o in stub.names.items()
if not o.module_hidden and (not m.startswith("_") or hasattr(runtime, m))
if not o.module_hidden and (not is_probably_private(m) or hasattr(runtime, m))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning something very similar, which was why I factored it out into a helper function even though it was only used once in my patch 😆

)

def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
Expand All @@ -220,15 +225,15 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
else [
m
for m in dir(runtime)
if not m.startswith("_")
if not is_probably_private(m)
# Ensure that the object's module is `runtime`, since in the absence of __all__ we
# don't have a good way to detect re-exports at runtime.
and _belongs_to_runtime(runtime, m)
]
)
# Check all things declared in module's __all__, falling back to our best guess
to_check.update(runtime_public_contents)
to_check.difference_update({"__file__", "__doc__", "__name__", "__builtins__", "__package__"})
to_check.difference_update(IGNORED_MODULE_DUNDERS)

for entry in sorted(to_check):
stub_entry = stub.names[entry].node if entry in stub.names else MISSING
Expand All @@ -243,60 +248,12 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
)


IGNORED_DUNDERS = frozenset({
# Very special attributes
"__weakref__",
"__slots__",
"__dict__",
"__text_signature__",
# Pickle methods
"__setstate__",
"__getstate__",
"__getnewargs__",
"__getinitargs__",
"__reduce_ex__",
"__reduce__",
# typing implementation details
"__parameters__",
"__origin__",
"__args__",
"__orig_bases__",
"__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__",
# These two are basically useless for type checkers
"__hash__",
"__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__
})


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)


@verify.register(nodes.TypeInfo)
def verify_typeinfo(
stub: nodes.TypeInfo, runtime: MaybeMissing[Type[Any]], object_path: List[str]
Expand Down Expand Up @@ -330,7 +287,9 @@ class SubClass(runtime): # type: ignore
to_check = set(stub.names)
to_check.update(
# cast to workaround mypyc complaints
m for m in cast(Any, vars)(runtime) if not is_private(m) and m not in IGNORED_DUNDERS
m
for m in cast(Any, vars)(runtime)
if not is_probably_private(m) and m not in ALLOW_MISSING_CLASS_DUNDERS
)

for entry in sorted(to_check):
Expand Down Expand Up @@ -1009,6 +968,78 @@ def verify_typealias(
)


# ====================
# Helpers
# ====================


IGNORED_MODULE_DUNDERS = frozenset(
{
"__file__",
"__doc__",
"__name__",
"__builtins__",
"__package__",
"__cached__",
"__loader__",
"__spec__",
"__path__", # mypy adds __path__ to packages, but C packages don't have it
"__getattr__", # resulting behaviour might be typed explicitly
# TODO: remove the following from this list
"__author__",
"__version__",
"__copyright__",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"__copyright__",
"__copyright__",
"__about__",

Another one I noticed in my experiments recently!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see you consider heap.__about__ a true positive?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could just put __about__: str in the stub. Doesn't do any harm.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite educational :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning here is that it's only clearly a true negative if the Python runtime is what's creating the attribute. Everything else you might conceivably want to have stubbed. I'm especially eager to remove __version__ from this list, which is widely used in third party stubs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that makes sense!

}
)

ALLOW_MISSING_CLASS_DUNDERS = frozenset(
{
# Special attributes
"__dict__",
"__text_signature__",
"__weakref__",
"__del__", # Only ever called when an object is being deleted, who cares?
# These two are basically useless for type checkers
"__hash__",
"__getattr__", # resulting behaviour might be typed explicitly
# isinstance/issubclass hooks that type-checkers don't usually care about
"__instancecheck__",
"__subclasshook__",
"__subclasscheck__",
# Pickle methods
"__setstate__",
"__getstate__",
"__getnewargs__",
"__getinitargs__",
"__reduce_ex__",
"__reduce__",
# ctypes weirdness
"__ctype_be__",
"__ctype_le__",
"__ctypes_from_outparam__",
# mypy limitations
"__abstractmethods__", # Classes with metaclass=ABCMeta inherit this attribute
"__new_member__", # If an enum defines __new__, the method is renamed as __new_member__
"__dataclass_fields__", # Generated by dataclasses
"__dataclass_params__", # Generated by dataclasses
"__doc__", # mypy's semanal for namedtuples assumes this is str, not Optional[str]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should be a pretty easy fix

# typing implementation details, consider removing some of these:
"__parameters__",
"__origin__",
"__args__",
"__orig_bases__",
"__final__",
# Consider removing these:
"__match_args__",
"__slots__",
}
)


def is_probably_private(name: str) -> bool:
return name.startswith("_") and not is_dunder(name)


def is_probably_a_function(runtime: Any) -> bool:
return (
isinstance(runtime, (types.FunctionType, types.BuiltinFunctionType))
Expand Down Expand Up @@ -1151,6 +1182,11 @@ def anytype() -> mypy.types.AnyType:
return mypy.types.LiteralType(value=value, fallback=fallback)


# ====================
# Build and entrypoint
# ====================


_all_stubs: Dict[str, nodes.MypyFile] = {}


Expand Down
10 changes: 8 additions & 2 deletions mypy/test/teststubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ def test(*args: Any, **kwargs: Any) -> None:
for c in cases:
if c.error is None:
continue
expected_error = "{}.{}".format(TEST_MODULE_NAME, c.error)
expected_error = c.error
if expected_error == "":
expected_error = TEST_MODULE_NAME
elif not expected_error.startswith(f"{TEST_MODULE_NAME}."):
expected_error = f"{TEST_MODULE_NAME}.{expected_error}"
assert expected_error not in expected_errors, (
"collect_cases merges cases into a single stubtest invocation; we already "
"expect an error for {}".format(expected_error)
Expand Down Expand Up @@ -730,7 +734,9 @@ def test_missing_no_runtime_all(self) -> Iterator[Case]:

@collect_cases
def test_non_public_1(self) -> Iterator[Case]:
yield Case(stub="__all__: list[str]", runtime="", error=None) # dummy case
yield Case(
stub="__all__: list[str]", runtime="", error="test_module.__all__"
) # dummy case
yield Case(stub="_f: int", runtime="def _f(): ...", error="_f")

@collect_cases
Expand Down