From 69b31445280d7c495fa0268b24fc558bcbe74505 Mon Sep 17 00:00:00 2001 From: Chad Dombrova Date: Wed, 29 Nov 2023 00:14:38 -0500 Subject: [PATCH] Fix stubgen regressions with pybind11 and mypy 1.7 (#16504) This addresses several regressions identified in https://github.com/python/mypy/issues/16486 The primary regression from https://github.com/python/mypy/pull/15770 is that pybind11 properties with docstrings were erroneously assigned `typeshed. Incomplete`. The reason for the regression is that as of the introduction of the `--include-docstring` feature (https://github.com/python/mypy/pull/13284, not my PR, ftr), `./misc/test-stubgenc.sh` began always reporting success. That has been fixed. It was also pointed out that `--include-docstring` does not work for C-extensions. This was not actually a regression as it turns out this feature was never implemented for C-extensions (though the tests suggested it had been), but luckily my efforts to unify the pure-python and C-extension code-paths made fixing this super easy (barely an inconvenience)! So that is working now. I added back the extended list of `typing` objects that generate implicit imports for the inspection-based stub generator. I originally removed these because I encountered an issue generating stubs for `PySide2` (and another internal library) where there was an object with the same name as one of the `typing` objects and the auto-import created broken stubs. I felt somewhat justified in this decision as there was a straightforward solution -- e.g. use `list` or `typing.List` instead of `List`. That said, I recognize that the problem that I encountered is more niche than the general desire to add import statements for typing objects, so I've changed the behavior back for now, with the intention to eventually add a flag to control this behavior. --- misc/test-stubgenc.sh | 2 +- mypy/stubdoc.py | 3 +- mypy/stubgen.py | 1 + mypy/stubgenc.py | 52 +++++++++++++++++-- mypy/stubutil.py | 18 +++---- test-data/pybind11_mypy_demo/src/main.cpp | 11 +++- .../pybind11_mypy_demo/__init__.pyi | 1 + .../pybind11_mypy_demo/basics.pyi | 32 ++++++------ .../stubgen/pybind11_mypy_demo/basics.pyi | 3 +- 9 files changed, 88 insertions(+), 35 deletions(-) diff --git a/misc/test-stubgenc.sh b/misc/test-stubgenc.sh index 7713e1b04e43..5cb5140eba76 100755 --- a/misc/test-stubgenc.sh +++ b/misc/test-stubgenc.sh @@ -24,7 +24,7 @@ function stubgenc_test() { # Compare generated stubs to expected ones if ! git diff --exit-code "$STUBGEN_OUTPUT_FOLDER"; then - EXIT=$? + EXIT=1 fi } diff --git a/mypy/stubdoc.py b/mypy/stubdoc.py index 126ac44e142e..86ff6e2bb540 100644 --- a/mypy/stubdoc.py +++ b/mypy/stubdoc.py @@ -383,7 +383,8 @@ def infer_ret_type_sig_from_docstring(docstr: str, name: str) -> str | None: def infer_ret_type_sig_from_anon_docstring(docstr: str) -> str | None: """Convert signature in form of "(self: TestClass, arg0) -> int" to their return type.""" - return infer_ret_type_sig_from_docstring("stub" + docstr.strip(), "stub") + lines = ["stub" + line.strip() for line in docstr.splitlines() if line.strip().startswith("(")] + return infer_ret_type_sig_from_docstring("".join(lines), "stub") def parse_signature(sig: str) -> tuple[str, list[str], list[str]] | None: diff --git a/mypy/stubgen.py b/mypy/stubgen.py index fff6ab058459..23b5fde9dff2 100755 --- a/mypy/stubgen.py +++ b/mypy/stubgen.py @@ -1700,6 +1700,7 @@ def generate_stubs(options: Options) -> None: doc_dir=options.doc_dir, include_private=options.include_private, export_less=options.export_less, + include_docstrings=options.include_docstrings, ) num_modules = len(all_modules) if not options.quiet and num_modules > 0: diff --git a/mypy/stubgenc.py b/mypy/stubgenc.py index 0ad79a4265b3..39288197f477 100755 --- a/mypy/stubgenc.py +++ b/mypy/stubgenc.py @@ -126,10 +126,12 @@ def get_property_type(self, default_type: str | None, ctx: FunctionContext) -> s """Infer property type from docstring or docstring signature.""" if ctx.docstring is not None: inferred = infer_ret_type_sig_from_anon_docstring(ctx.docstring) - if not inferred: - inferred = infer_ret_type_sig_from_docstring(ctx.docstring, ctx.name) - if not inferred: - inferred = infer_prop_type_from_docstring(ctx.docstring) + if inferred: + return inferred + inferred = infer_ret_type_sig_from_docstring(ctx.docstring, ctx.name) + if inferred: + return inferred + inferred = infer_prop_type_from_docstring(ctx.docstring) return inferred else: return None @@ -237,6 +239,26 @@ def __init__( self.resort_members = self.is_c_module super().__init__(_all_, include_private, export_less, include_docstrings) self.module_name = module_name + if self.is_c_module: + # Add additional implicit imports. + # C-extensions are given more lattitude since they do not import the typing module. + self.known_imports.update( + { + "typing": [ + "Any", + "Callable", + "ClassVar", + "Dict", + "Iterable", + "Iterator", + "List", + "NamedTuple", + "Optional", + "Tuple", + "Union", + ] + } + ) def get_default_function_sig(self, func: object, ctx: FunctionContext) -> FunctionSig: argspec = None @@ -590,9 +612,29 @@ def generate_function_stub( if inferred[0].args and inferred[0].args[0].name == "cls": decorators.append("@classmethod") + if docstring: + docstring = self._indent_docstring(docstring) output.extend(self.format_func_def(inferred, decorators=decorators, docstring=docstring)) self._fix_iter(ctx, inferred, output) + def _indent_docstring(self, docstring: str) -> str: + """Fix indentation of docstring extracted from pybind11 or other binding generators.""" + lines = docstring.splitlines(keepends=True) + indent = self._indent + " " + if len(lines) > 1: + if not all(line.startswith(indent) or not line.strip() for line in lines): + # if the docstring is not indented, then indent all but the first line + for i, line in enumerate(lines[1:]): + if line.strip(): + lines[i + 1] = indent + line + # if there's a trailing newline, add a final line to visually indent the quoted docstring + if lines[-1].endswith("\n"): + if len(lines) > 1: + lines.append(indent) + else: + lines[-1] = lines[-1][:-1] + return "".join(lines) + def _fix_iter( self, ctx: FunctionContext, inferred: list[FunctionSig], output: list[str] ) -> None: @@ -640,7 +682,7 @@ def generate_property_stub( if fget: alt_docstr = getattr(fget, "__doc__", None) if alt_docstr and docstring: - docstring += alt_docstr + docstring += "\n" + alt_docstr elif alt_docstr: docstring = alt_docstr diff --git a/mypy/stubutil.py b/mypy/stubutil.py index 5ec240087145..b8d601ed3c6b 100644 --- a/mypy/stubutil.py +++ b/mypy/stubutil.py @@ -576,6 +576,14 @@ def __init__( self.sig_generators = self.get_sig_generators() # populated by visit_mypy_file self.module_name: str = "" + # These are "soft" imports for objects which might appear in annotations but not have + # a corresponding import statement. + self.known_imports = { + "_typeshed": ["Incomplete"], + "typing": ["Any", "TypeVar", "NamedTuple"], + "collections.abc": ["Generator"], + "typing_extensions": ["TypedDict", "ParamSpec", "TypeVarTuple"], + } def get_sig_generators(self) -> list[SignatureGenerator]: return [] @@ -667,15 +675,7 @@ def set_defined_names(self, defined_names: set[str]) -> None: for name in self._all_ or (): self.import_tracker.reexport(name) - # These are "soft" imports for objects which might appear in annotations but not have - # a corresponding import statement. - known_imports = { - "_typeshed": ["Incomplete"], - "typing": ["Any", "TypeVar", "NamedTuple"], - "collections.abc": ["Generator"], - "typing_extensions": ["TypedDict", "ParamSpec", "TypeVarTuple"], - } - for pkg, imports in known_imports.items(): + for pkg, imports in self.known_imports.items(): for t in imports: # require=False means that the import won't be added unless require_name() is called # for the object during generation. diff --git a/test-data/pybind11_mypy_demo/src/main.cpp b/test-data/pybind11_mypy_demo/src/main.cpp index 00e5b2f4e871..192a90cf8e30 100644 --- a/test-data/pybind11_mypy_demo/src/main.cpp +++ b/test-data/pybind11_mypy_demo/src/main.cpp @@ -44,6 +44,7 @@ #include #include +#include namespace py = pybind11; @@ -102,6 +103,11 @@ struct Point { return distance_to(other.x, other.y); } + std::vector as_vector() + { + return std::vector{x, y}; + } + double x, y; }; @@ -134,14 +140,15 @@ void bind_basics(py::module& basics) { .def(py::init(), py::arg("x"), py::arg("y")) .def("distance_to", py::overload_cast(&Point::distance_to, py::const_), py::arg("x"), py::arg("y")) .def("distance_to", py::overload_cast(&Point::distance_to, py::const_), py::arg("other")) - .def_readwrite("x", &Point::x) + .def("as_list", &Point::as_vector) + .def_readwrite("x", &Point::x, "some docstring") .def_property("y", [](Point& self){ return self.y; }, [](Point& self, double value){ self.y = value; } ) .def_property_readonly("length", &Point::length) .def_property_readonly_static("x_axis", [](py::object cls){return Point::x_axis;}) - .def_property_readonly_static("y_axis", [](py::object cls){return Point::y_axis;}) + .def_property_readonly_static("y_axis", [](py::object cls){return Point::y_axis;}, "another docstring") .def_readwrite_static("length_unit", &Point::length_unit) .def_property_static("angle_unit", [](py::object& /*cls*/){ return Point::angle_unit; }, diff --git a/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/__init__.pyi b/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/__init__.pyi index e69de29bb2d1..0cb252f00259 100644 --- a/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/__init__.pyi +++ b/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/__init__.pyi @@ -0,0 +1 @@ +from . import basics as basics diff --git a/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/basics.pyi b/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/basics.pyi index 676d7f6d3f15..b761291e11f3 100644 --- a/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/basics.pyi +++ b/test-data/pybind11_mypy_demo/stubgen-include-docs/pybind11_mypy_demo/basics.pyi @@ -1,7 +1,7 @@ -from typing import ClassVar +from typing import ClassVar, List, overload -from typing import overload PI: float +__version__: str class Point: class AngleUnit: @@ -13,8 +13,6 @@ class Point: """__init__(self: pybind11_mypy_demo.basics.Point.AngleUnit, value: int) -> None""" def __eq__(self, other: object) -> bool: """__eq__(self: object, other: object) -> bool""" - def __getstate__(self) -> int: - """__getstate__(self: object) -> int""" def __hash__(self) -> int: """__hash__(self: object) -> int""" def __index__(self) -> int: @@ -23,8 +21,6 @@ class Point: """__int__(self: pybind11_mypy_demo.basics.Point.AngleUnit) -> int""" def __ne__(self, other: object) -> bool: """__ne__(self: object, other: object) -> bool""" - def __setstate__(self, state: int) -> None: - """__setstate__(self: pybind11_mypy_demo.basics.Point.AngleUnit, state: int) -> None""" @property def name(self) -> str: ... @property @@ -40,8 +36,6 @@ class Point: """__init__(self: pybind11_mypy_demo.basics.Point.LengthUnit, value: int) -> None""" def __eq__(self, other: object) -> bool: """__eq__(self: object, other: object) -> bool""" - def __getstate__(self) -> int: - """__getstate__(self: object) -> int""" def __hash__(self) -> int: """__hash__(self: object) -> int""" def __index__(self) -> int: @@ -50,8 +44,6 @@ class Point: """__int__(self: pybind11_mypy_demo.basics.Point.LengthUnit) -> int""" def __ne__(self, other: object) -> bool: """__ne__(self: object, other: object) -> bool""" - def __setstate__(self, state: int) -> None: - """__setstate__(self: pybind11_mypy_demo.basics.Point.LengthUnit, state: int) -> None""" @property def name(self) -> str: ... @property @@ -70,7 +62,8 @@ class Point: 1. __init__(self: pybind11_mypy_demo.basics.Point) -> None - 2. __init__(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> None""" + 2. __init__(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> None + """ @overload def __init__(self, x: float, y: float) -> None: """__init__(*args, **kwargs) @@ -78,7 +71,10 @@ class Point: 1. __init__(self: pybind11_mypy_demo.basics.Point) -> None - 2. __init__(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> None""" + 2. __init__(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> None + """ + def as_list(self) -> List[float]: + """as_list(self: pybind11_mypy_demo.basics.Point) -> List[float]""" @overload def distance_to(self, x: float, y: float) -> float: """distance_to(*args, **kwargs) @@ -86,7 +82,8 @@ class Point: 1. distance_to(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> float - 2. distance_to(self: pybind11_mypy_demo.basics.Point, other: pybind11_mypy_demo.basics.Point) -> float""" + 2. distance_to(self: pybind11_mypy_demo.basics.Point, other: pybind11_mypy_demo.basics.Point) -> float + """ @overload def distance_to(self, other: Point) -> float: """distance_to(*args, **kwargs) @@ -94,19 +91,22 @@ class Point: 1. distance_to(self: pybind11_mypy_demo.basics.Point, x: float, y: float) -> float - 2. distance_to(self: pybind11_mypy_demo.basics.Point, other: pybind11_mypy_demo.basics.Point) -> float""" + 2. distance_to(self: pybind11_mypy_demo.basics.Point, other: pybind11_mypy_demo.basics.Point) -> float + """ @property def length(self) -> float: ... def answer() -> int: '''answer() -> int - answer docstring, with end quote"''' + answer docstring, with end quote" + ''' def midpoint(left: float, right: float) -> float: """midpoint(left: float, right: float) -> float""" def sum(arg0: int, arg1: int) -> int: '''sum(arg0: int, arg1: int) -> int - multiline docstring test, edge case quotes """\'\'\'''' + multiline docstring test, edge case quotes """\'\'\' + ''' def weighted_midpoint(left: float, right: float, alpha: float = ...) -> float: """weighted_midpoint(left: float, right: float, alpha: float = 0.5) -> float""" diff --git a/test-data/pybind11_mypy_demo/stubgen/pybind11_mypy_demo/basics.pyi b/test-data/pybind11_mypy_demo/stubgen/pybind11_mypy_demo/basics.pyi index 6527f5733eaf..6f164a03edcc 100644 --- a/test-data/pybind11_mypy_demo/stubgen/pybind11_mypy_demo/basics.pyi +++ b/test-data/pybind11_mypy_demo/stubgen/pybind11_mypy_demo/basics.pyi @@ -1,4 +1,4 @@ -from typing import ClassVar, overload +from typing import ClassVar, List, overload PI: float __version__: str @@ -47,6 +47,7 @@ class Point: def __init__(self) -> None: ... @overload def __init__(self, x: float, y: float) -> None: ... + def as_list(self) -> List[float]: ... @overload def distance_to(self, x: float, y: float) -> float: ... @overload