Skip to content

Commit

Permalink
Use __getattr__ to mark partial stub packages (python#5231)
Browse files Browse the repository at this point in the history
There is a problem with annotating large frameworks -- they are large. Therefore it is hard to produce good stubs in a single pass. A possible better workflow would be to allow indicating that a given (sub-)package is incomplete.

I propose to use `__getattr__` for the role of such indicator. A motivation is that currently adding a `__getattr__` to `package/__init__.pyi` already makes `from package import mod` work, but `import package.mod` still fails (plus simplicity of implementation). Here are the rules that I propose:
* One can declare a (sub-)package as incomplete by adding a `__getattr__` to its `__init__.pyi`
* If the return type of this function is `types.ModuleType` or `Any`, we assume that all imports from this (sub-)package succeed.
* Incomplete package can contain a complete subpackage:
```
# file a/__init__.pyi
from types import ModuleType
def __getattr__(attr: str) -> ModuleType: ...

# file a/b/__init__.pyi
# empty (i.e. complete package)

# file main.py
import a.d  # OK
import a.b.c  # Error module not found
```

Note: these rules apply only to stubs (i.e. `.pyi` files). I add several tests to illustrate this behaviour.
This PR shouldn't give any significant performance penalty because the added parsing/loading only happens when an error would be reported (for our internal workflow the penalty will be zero because of the flags we use).

This PR will allow gradually adding stub modules to a large framework package, without generating loads of false positives for user code.

Note: PEP 561 introduces the notion of a partial stub package, implemented in python#5227. I think however this is a bit different use case that I don't want to mix with this one for two reasons:
* Partial packages in PEP 561 are mainly focused on interaction between stubs and inline/runtime packages.
* The proposed feature may be also used in typeshed, not only for installed stub packages.
  • Loading branch information
ilevkivskyi authored Jun 25, 2018
1 parent 6d8d50c commit 4c3f800
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 7 deletions.
55 changes: 49 additions & 6 deletions mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1761,6 +1761,11 @@ def __init__(self,
caller_line: int = 0,
ancestor_for: 'Optional[State]' = None,
root_source: bool = False,
# If `temporary` is True, this State is being created to just
# quickly parse/load the tree, without an intention to further
# process it. With this flag, any changes to external state as well
# as error reporting should be avoided.
temporary: bool = False,
) -> None:
assert id or path or source is not None, "Neither id, path nor source given"
self.manager = manager
Expand All @@ -1782,9 +1787,10 @@ def __init__(self,
try:
path, follow_imports = find_module_and_diagnose(
manager, id, self.options, caller_state, caller_line,
ancestor_for, root_source)
ancestor_for, root_source, skip_diagnose=temporary)
except ModuleNotFound:
manager.missing_modules.add(id)
if not temporary:
manager.missing_modules.add(id)
raise
if follow_imports == 'silent':
self.ignore_all = True
Expand Down Expand Up @@ -2265,16 +2271,21 @@ def find_module_and_diagnose(manager: BuildManager,
caller_state: 'Optional[State]' = None,
caller_line: int = 0,
ancestor_for: 'Optional[State]' = None,
root_source: bool = False) -> Tuple[str, str]:
root_source: bool = False,
skip_diagnose: bool = False) -> Tuple[str, str]:
"""Find a module by name, respecting follow_imports and producing diagnostics.
If the module is not found, then the ModuleNotFound exception is raised.
Args:
id: module to find
options: the options for the module being loaded
caller_state: the state of the importing module, if applicable
caller_line: the line number of the import
ancestor_for: the child module this is an ancestor of, if applicable
root_source: whether this source was specified on the command line
skip_diagnose: skip any error diagnosis and reporting (but ModuleNotFound is
still raised if the module is missing)
The specified value of follow_imports for a module can be overridden
if the module is specified on the command line or if it is a stub,
Expand Down Expand Up @@ -2306,8 +2317,9 @@ def find_module_and_diagnose(manager: BuildManager,
and not options.follow_imports_for_stubs) # except when they aren't
or id == 'builtins'): # Builtins is always normal
follow_imports = 'normal'

if follow_imports == 'silent':
if skip_diagnose:
pass
elif follow_imports == 'silent':
# Still import it, but silence non-blocker errors.
manager.log("Silencing %s (%s)" % (path, id))
elif follow_imports == 'skip' or follow_imports == 'error':
Expand All @@ -2327,8 +2339,10 @@ def find_module_and_diagnose(manager: BuildManager,
# Could not find a module. Typically the reason is a
# misspelled module name, missing stub, module not in
# search path or the module has not been installed.
if skip_diagnose:
raise ModuleNotFound
if caller_state:
if not options.ignore_missing_imports:
if not (options.ignore_missing_imports or in_partial_package(id, manager)):
module_not_found(manager, caller_line, caller_state, id)
raise ModuleNotFound
else:
Expand All @@ -2338,6 +2352,35 @@ def find_module_and_diagnose(manager: BuildManager,
raise CompileError(["mypy: can't find module '%s'" % id])


def in_partial_package(id: str, manager: BuildManager) -> bool:
"""Check if a missing module can potentially be a part of a package.
This checks if there is any existing parent __init__.pyi stub that
defines a module-level __getattr__ (a.k.a. partial stub package).
"""
while '.' in id:
parent, _ = id.rsplit('.', 1)
if parent in manager.modules:
parent_mod = manager.modules[parent] # type: Optional[MypyFile]
else:
# Parent is not in build, try quickly if we can find it.
try:
parent_st = State(id=parent, path=None, source=None, manager=manager,
temporary=True)
except (ModuleNotFound, CompileError):
parent_mod = None
else:
parent_mod = parent_st.tree
if parent_mod is not None:
if parent_mod.is_partial_stub_package:
return True
else:
# Bail out soon, complete subpackage found
return False
id = parent
return False


def module_not_found(manager: BuildManager, line: int, caller_state: State,
target: str) -> None:
errors = manager.errors
Expand Down
6 changes: 6 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ class MypyFile(SymbolNode):
is_stub = False
# Is this loaded from the cache and thus missing the actual body of the file?
is_cache_skeleton = False
# Does this represent an __init__.pyi stub with a module __getattr__
# (i.e. a partial stub package), for such packages we suppress any missing
# module errors in addition to missing attribute errors.
is_partial_stub_package = False

def __init__(self,
defs: List[Statement],
Expand Down Expand Up @@ -252,6 +256,7 @@ def serialize(self) -> JsonDict:
'names': self.names.serialize(self._fullname),
'is_stub': self.is_stub,
'path': self.path,
'is_partial_stub_package': self.is_partial_stub_package,
}

@classmethod
Expand All @@ -263,6 +268,7 @@ def deserialize(cls, data: JsonDict) -> 'MypyFile':
tree.names = SymbolTable.deserialize(data['names'])
tree.is_stub = data['is_stub']
tree.path = data['path']
tree.is_partial_stub_package = data['is_partial_stub_package']
tree.is_cache_skeleton = True
return tree

Expand Down
13 changes: 12 additions & 1 deletion mypy/semanal_pass1.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
TryStmt, OverloadedFuncDef, Lvalue, Context, ImportedName, LDEF, GDEF, MDEF, UNBOUND_IMPORTED,
MODULE_REF, implicit_module_attrs
)
from mypy.types import Type, UnboundType, UnionType, AnyType, TypeOfAny, NoneTyp
from mypy.types import Type, UnboundType, UnionType, AnyType, TypeOfAny, NoneTyp, CallableType
from mypy.semanal import SemanticAnalyzerPass2, infer_reachability_of_if_statement
from mypy.semanal_shared import create_indirect_imported_name
from mypy.options import Options
Expand Down Expand Up @@ -154,6 +154,17 @@ def visit_func_def(self, func: FuncDef) -> None:
func.is_conditional = sem.block_depth[-1] > 0
func._fullname = sem.qualified_name(func.name())
at_module = sem.is_module_scope()
if (at_module and func.name() == '__getattr__' and
self.sem.cur_mod_node.is_package_init_file() and self.sem.cur_mod_node.is_stub):
if isinstance(func.type, CallableType):
ret = func.type.ret_type
if isinstance(ret, UnboundType) and not ret.args:
sym = self.sem.lookup_qualified(ret.name, func, suppress_errors=True)
# We only interpret a package as partial if the __getattr__ return type
# is either types.ModuleType of Any.
if sym and sym.node and sym.node.fullname() in ('types.ModuleType',
'typing.Any'):
self.sem.cur_mod_node.is_partial_stub_package = True
if at_module and func.name() in sem.globals:
# Already defined in this module.
original_sym = sem.globals[func.name()]
Expand Down
122 changes: 122 additions & 0 deletions test-data/unit/check-modules.test
Original file line number Diff line number Diff line change
Expand Up @@ -2173,6 +2173,128 @@ from c import x
x = str()
y = int()

[case testModuleGetattrInit1]
from a import b

x = b.f()
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[builtins fixtures/module.pyi]
[out]

[case testModuleGetattrInit2]
import a.b

x = a.b.f()
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[builtins fixtures/module.pyi]
[out]

[case testModuleGetattrInit3]
import a.b

x = a.b.f()
[file a/__init__.py]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[builtins fixtures/module.pyi]
[out]
main:1: error: Cannot find module named 'a.b'
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

[case testModuleGetattrInit4]
import a.b.c

x = a.b.c.f()
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[builtins fixtures/module.pyi]
[out]

[case testModuleGetattrInit5]
from a.b import f

x = f()
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[builtins fixtures/module.pyi]
[out]

[case testModuleGetattrInit5a]
from a.b import f

x = f()
[file a/__init__.pyi]
from types import ModuleType
def __getattr__(attr: str) -> ModuleType: ...
[builtins fixtures/module.pyi]
[out]


[case testModuleGetattrInit5b]
from a.b import f

x = f()
[file a/__init__.pyi]
def __getattr__(attr: str) -> int: ...
[builtins fixtures/module.pyi]
[out]
main:1: error: Cannot find module named 'a.b'
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

[case testModuleGetattrInit8]
import a.b.c.d

x = a.b.c.d.f()
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[file a/b/__init__.pyi]
# empty (i.e. complete subpackage)
[builtins fixtures/module.pyi]
[out]
main:1: error: Cannot find module named 'a.b.c'
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)
main:1: error: Cannot find module named 'a.b.c.d'

[case testModuleGetattrInit8a]
import a.b.c # Error
import a.d # OK
[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[file a/b/__init__.pyi]
# empty (i.e. complete subpackage)
[builtins fixtures/module.pyi]
[out]
main:1: error: Cannot find module named 'a.b.c'
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

[case testModuleGetattrInit10]
# flags: --config-file tmp/mypy.ini
import a.b.c # silenced
import a.b.d # error

[file a/__init__.pyi]
from typing import Any
def __getattr__(attr: str) -> Any: ...
[file a/b/__init__.pyi]
# empty (i.e. complete subpackage)

[file mypy.ini]
[[mypy]
[[mypy-a.b.c]
ignore_missing_imports = True
[builtins fixtures/module.pyi]
[out]
main:3: error: Cannot find module named 'a.b.d'
main:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

[case testIndirectFromImportWithinCycleUsedAsBaseClass-skip]
-- TODO: Fails because of missing ImportedName handling in mypy.typeanal
import a
Expand Down

0 comments on commit 4c3f800

Please sign in to comment.