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

stubgen: include __all__ in output #16356

Merged
merged 5 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Unreleased

...
Stubgen will now include `__all__` in its output if it is in the input file (PR [16356](https://github.com/python/mypy/pull/16356)).

#### Other Notable Changes and Fixes
...
Expand Down
18 changes: 14 additions & 4 deletions mypy/stubutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,20 @@ def get_imports(self) -> str:

def output(self) -> str:
"""Return the text for the stub."""
imports = self.get_imports()
if imports and self._output:
imports += "\n"
return imports + "".join(self._output)
pieces: list[str] = []
if imports := self.get_imports():
pieces.append(imports)
if dunder_all := self.get_dunder_all():
pieces.append(dunder_all)
if self._output:
pieces.append("".join(self._output))
return "\n".join(pieces)

def get_dunder_all(self) -> str:
"""Return the __all__ list for the stub."""
if self._all_:
return f"__all__ = {self._all_!r}\n"
return ""

def add(self, string: str) -> None:
"""Add text to generated stub."""
Expand Down
38 changes: 38 additions & 0 deletions test-data/unit/stubgen.test
Original file line number Diff line number Diff line change
Expand Up @@ -587,20 +587,26 @@ __all__ = [] + ['f']
def f(): ...
def g(): ...
[out]
__all__ = ['f']

def f() -> None: ...

[case testOmitDefsNotInAll_semanal]
__all__ = ['f']
def f(): ...
def g(): ...
[out]
__all__ = ['f']

def f() -> None: ...

[case testOmitDefsNotInAll_inspect]
__all__ = [] + ['f']
def f(): ...
def g(): ...
[out]
__all__ = ['f']

def f(): ...

[case testVarDefsNotInAll_import]
Expand All @@ -610,6 +616,8 @@ x = 1
y = 1
def g(): ...
[out]
__all__ = ['f', 'g']

def f() -> None: ...
def g() -> None: ...

Expand All @@ -620,6 +628,8 @@ x = 1
y = 1
def g(): ...
[out]
__all__ = ['f', 'g']

def f(): ...
def g(): ...

Expand All @@ -628,6 +638,8 @@ __all__ = [] + ['f']
def f(): ...
class A: ...
[out]
__all__ = ['f']

def f() -> None: ...

class A: ...
Expand All @@ -637,6 +649,8 @@ __all__ = [] + ['f']
def f(): ...
class A: ...
[out]
__all__ = ['f']

def f(): ...

class A: ...
Expand All @@ -647,6 +661,8 @@ class A:
x = 1
def f(self): ...
[out]
__all__ = ['A']

class A:
x: int
def f(self) -> None: ...
Expand Down Expand Up @@ -684,6 +700,8 @@ x = 1
[out]
from re import match as match, sub as sub

__all__ = ['match', 'sub', 'x']

x: int

[case testExportModule_import]
Expand All @@ -694,6 +712,8 @@ y = 2
[out]
import re as re

__all__ = ['re', 'x']

x: int

[case testExportModule2_import]
Expand All @@ -704,6 +724,8 @@ y = 2
[out]
import re as re

__all__ = ['re', 'x']

x: int

[case testExportModuleAs_import]
Expand All @@ -714,6 +736,8 @@ y = 2
[out]
import re as rex

__all__ = ['rex', 'x']

x: int

[case testExportModuleInPackage_import]
Expand All @@ -722,13 +746,17 @@ __all__ = ['p']
[out]
import urllib.parse as p

__all__ = ['p']

[case testExportPackageOfAModule_import]
import urllib.parse
__all__ = ['urllib']

[out]
import urllib as urllib

__all__ = ['urllib']
Comment on lines 751 to +758
Copy link
Member

@AlexWaygood AlexWaygood Oct 28, 2023

Choose a reason for hiding this comment

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

kinda cool that stubgen has sophisticated enough inference to do this. Didn't realise that it had this capability!


[case testRelativeImportAll]
from .x import *
[out]
Expand All @@ -741,6 +769,8 @@ x = 1
class C:
def g(self): ...
[out]
__all__ = ['f', 'x', 'C', 'g']

def f() -> None: ...

x: int
Expand All @@ -758,6 +788,8 @@ x = 1
class C:
def g(self): ...
[out]
__all__ = ['f', 'x', 'C', 'g']
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Maybe it's problematic if stubgen adds __all__ to the generated stub, even though a symbol in __all__ at runtime (in this case, g) isn't actually defined at runtime. On the other hand, maybe it's outside of stubgen's purview to worry about such things; maybe it should just copy the runtime __all__ faithfully into the stub even when __all__ is weird at runtime, as you're doing now.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's probably better to omit the undefined names so the generated stub won't create errors for type checkers, so I pushed that change. Could go either way though.

Copy link
Member

Choose a reason for hiding this comment

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

(For future readers of this thread: we decided to go back on this; see the discussion in #16356 (comment) for why)


def f(): ...

x: int
Expand Down Expand Up @@ -2343,6 +2375,8 @@ else:
[out]
import cookielib as cookielib

__all__ = ['cookielib']

[case testCannotCalculateMRO_semanal]
class X: pass

Expand Down Expand Up @@ -2788,6 +2822,8 @@ class A: pass
# p/__init__.pyi
from p.a import A

__all__ = ['a']

a: A
# p/a.pyi
class A: ...
Expand Down Expand Up @@ -2963,6 +2999,8 @@ __version__ = ''
[out]
from m import __version__ as __version__
Copy link
Member

@AlexWaygood AlexWaygood Oct 28, 2023

Choose a reason for hiding this comment

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

This was a problem before your PR, but I'm not sure what the justification is for not importing __about__ and __author__ in the generated stub here, given that they're included in __all__ at runtime

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, they're in IGNORED_DUNDERS in stubutil.py. It generally makes sense to exclude these names as they're not useful in stubs, but we shouldn't ignore them if they're in __all__.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed a change so that IGNORED_DUNDERS aren't ignored if they're in __all__.


__all__ = ['__about__', '__author__', '__version__']

[case testAttrsClass_semanal]
import attrs

Expand Down
Loading