From 133ba62fa60a24446d78b24174f88140f3744ba6 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 28 Jun 2019 19:51:08 +0300 Subject: [PATCH 1/3] Fix variables in __all__ being flagged by --no-implicit-reexport The natural expectation is that when a variable is explicitly added to a module's `__all__`, the module explicitly means to export it. Previously, this was not the case, and a `from [..] import [..] as [..]` reexport was required by `--no-implicit-export` even if the variable is listed in `__all__`. Fixes https://github.com/python/mypy/issues/7042 --- docs/source/command_line.rst | 7 +++++-- docs/source/config_file.rst | 7 +++++-- mypy/semanal.py | 10 ++++++++-- test-data/unit/check-flags.test | 13 +++++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index a9b1dda6832f..cd304f3a67da 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -429,8 +429,8 @@ of the above sections. ``--no-implicit-reexport`` By default, imported values to a module are treated as exported and mypy allows other modules to import them. This flag changes the behavior to not re-export unless - the item is imported using from-as. Note this is always treated as enabled for - stub files. For example: + the item is imported using from-as or is included in ``__all__``. Note this is + always treated as enabled for stub files. For example: .. code-block:: python @@ -438,6 +438,9 @@ of the above sections. from foo import bar # This will re-export it as bar and allow other modules to import it from foo import bar as bar + # This will also re-export bar + from foo import bar + __all__ = ['bar'] ``--strict-equality`` diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index d01fa4893644..94df11682a84 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -310,8 +310,8 @@ Miscellaneous strictness flags ``implicit_reexport`` (bool, default True) By default, imported values to a module are treated as exported and mypy allows other modules to import them. When false, mypy will not re-export unless - the item is imported using from-as. Note that mypy treats stub files as if this - is always disabled. For example: + the item is imported using from-as or is included in ``__all__``. Note that mypy + treats stub files as if this is always disabled. For example: .. code-block:: python @@ -319,6 +319,9 @@ Miscellaneous strictness flags from foo import bar # This will re-export it as bar and allow other modules to import it from foo import bar as bar + # This will also re-export bar + from foo import bar + __all__ = ['bar'] ``strict_equality`` (bool, default False) Prohibit equality checks, identity checks, and container checks between diff --git a/mypy/semanal.py b/mypy/semanal.py index 2664d40ddb2f..16f5aa79b2d7 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -463,10 +463,16 @@ def add_builtin_aliases(self, tree: MypyFile) -> None: del tree.names[name] def adjust_public_exports(self) -> None: - """Make variables not in __all__ not be public""" + """Adjust the module visibility of globals due to __all__.""" if '__all__' in self.globals: for name, g in self.globals.items(): - if name not in self.all_exports: + # Being included in __all__ explicitly exports and makes public. + if name in self.all_exports: + g.module_public = True + g.module_hidden = False + # But when __all__ is defined, and a symbol is not included in it, + # it cannot be public. + else: g.module_public = False @contextmanager diff --git a/test-data/unit/check-flags.test b/test-data/unit/check-flags.test index 7e8cf53c8c89..7b7bbb992977 100644 --- a/test-data/unit/check-flags.test +++ b/test-data/unit/check-flags.test @@ -1145,6 +1145,19 @@ from other_module_1 import a [out] main:2: error: Module 'other_module_2' has no attribute 'a' +[case testNoImplicitReexportRespectsAll] +# flags: --no-implicit-reexport +from other_module_2 import a +from other_module_2 import b +[file other_module_1.py] +a = 5 +b = 6 +[file other_module_2.py] +from other_module_1 import a, b +__all__ = ('b',) +[out] +main:2: error: Module 'other_module_2' has no attribute 'a' + [case testNoImplicitReexportMypyIni] # flags: --config-file tmp/mypy.ini from other_module_2 import a From d24a9af56d0b7cda8f914fe8df2ca59fd7d75786 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 17 Aug 2019 14:50:44 +0300 Subject: [PATCH 2/3] Don't reexport star-imported symbols when implicit reexports are disabled Currently, in regular files, symbols imported into module A with `from B import *` are considered explicitly exported from module B as well, i.e. they are considered part of module's B public API. This behavior makes sense in stub files as a shorthand (and is specified in PEP 484), but for regular files I think it is better to be explicit: add the symbols to `__all__`. Further discussion: https://github.com/python/mypy/issues/7042#issuecomment-509005322 --- mypy/semanal.py | 10 +++++++++- test-data/unit/check-flags.test | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 16f5aa79b2d7..a63750c674e6 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1869,7 +1869,15 @@ def visit_import_all(self, i: ImportAll) -> None: if self.process_import_over_existing_name( name, existing_symbol, node, i): continue - self.add_imported_symbol(name, node, i) + # In stub files, `from x import *` always reexports the symbols. + # In regular files, only if implicit reexports are enabled. + module_public = ( + self.is_stub_file + or self.options.implicit_reexport + ) + self.add_imported_symbol(name, node, i, + module_public=module_public, + module_hidden=not module_public) else: # Don't add any dummy symbols for 'from x import *' if 'x' is unknown. pass diff --git a/test-data/unit/check-flags.test b/test-data/unit/check-flags.test index 7b7bbb992977..f67ffa48372c 100644 --- a/test-data/unit/check-flags.test +++ b/test-data/unit/check-flags.test @@ -1158,6 +1158,29 @@ __all__ = ('b',) [out] main:2: error: Module 'other_module_2' has no attribute 'a' +[case testNoImplicitReexportStarConsideredImplicit] +# flags: --no-implicit-reexport +from other_module_2 import a +[file other_module_1.py] +a = 5 +[file other_module_2.py] +from other_module_1 import * +[out] +main:2: error: Module 'other_module_2' has no attribute 'a' + +[case testNoImplicitReexportStarCanBeReexportedWithAll] +# flags: --no-implicit-reexport +from other_module_2 import a +from other_module_2 import b +[file other_module_1.py] +a = 5 +b = 6 +[file other_module_2.py] +from other_module_1 import * +__all__ = ('b',) +[out] +main:2: error: Module 'other_module_2' has no attribute 'a' + [case testNoImplicitReexportMypyIni] # flags: --config-file tmp/mypy.ini from other_module_2 import a From 267a850d2fe0e53d0e86e5e45b2ddde05d548f1b Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 19 Aug 2019 15:45:10 +0100 Subject: [PATCH 3/3] Remove unnecessary wrap --- mypy/semanal.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index a63750c674e6..ea22b4243ca2 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1871,10 +1871,7 @@ def visit_import_all(self, i: ImportAll) -> None: continue # In stub files, `from x import *` always reexports the symbols. # In regular files, only if implicit reexports are enabled. - module_public = ( - self.is_stub_file - or self.options.implicit_reexport - ) + module_public = self.is_stub_file or self.options.implicit_reexport self.add_imported_symbol(name, node, i, module_public=module_public, module_hidden=not module_public)