Skip to content

Commit

Permalink
Fix spurious name undefined error in class body within import cycle (#…
Browse files Browse the repository at this point in the history
…10498)

This could sometimes happen with protobuf stubs. The issue is a quite
tricky one, since it only happens if files in an import cycle are
checked in a specific order. The order may depend on the order files
are given as arguments to mypy.

The problem was with code like this, if `Foo` and `Base` are defined
in different files within an import cycle:

```
# m.py
from m2 import Base

class Foo(Base):
    x: Bar  # <<-- Unexpected error: "Bar" undefined
    class Bar: ...
```

Due to the import cycle, `Base` could be a placeholder node when
semantically analyzing `m` for the first time. This caused another
pass over `m`. On the second pass `Bar` was reported as undefined,
because of an incorrect namespace completeness check. We were checking
the completeness of the *module-level* namespace, when we should have
looked at the completeness of the *class* namespace.

If `Base` was ready during the first pass, the example worked as
expected, since neither the module nor the class namespace was
complete.

Errors about undefined things are only supposed to be generated when
the target namespace is complete (i.e., all names are included in
the symbol table, possibly as placholders).

This fixes the issue by keeping track of whether a class body is being
processed for the first time. During the first time the namespace is
being built, so it's incomplete.

This may not work in some very tricky scenarios where we need to
process the body of a class more than twice, but these cases are
probably very rare, so this fix should get us most of the way there.
  • Loading branch information
JukkaL authored May 19, 2021
1 parent 02e016f commit 7f2377e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 3 deletions.
16 changes: 15 additions & 1 deletion mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ def __init__(self,
self.imports = set()
self.type = None
self.type_stack = []
# Are the namespaces of classes being processed complete?
self.incomplete_type_stack = [] # type: List[bool]
self.tvar_scope = TypeVarLikeScope()
self.function_stack = []
self.block_depth = [0]
Expand Down Expand Up @@ -526,6 +528,7 @@ def file_context(self,
self.num_incomplete_refs = 0

if active_type:
self.incomplete_type_stack.append(False)
scope.enter_class(active_type)
self.enter_class(active_type.defn.info)
for tvar in active_type.defn.type_vars:
Expand All @@ -537,6 +540,7 @@ def file_context(self,
scope.leave()
self.leave_class()
self.type = None
self.incomplete_type_stack.pop()
scope.leave()
del self.options

Expand Down Expand Up @@ -1047,8 +1051,10 @@ def check_decorated_function_is_method(self, decorator: str,

def visit_class_def(self, defn: ClassDef) -> None:
self.statement = defn
self.incomplete_type_stack.append(not defn.info)
with self.tvar_scope_frame(self.tvar_scope.class_frame()):
self.analyze_class(defn)
self.incomplete_type_stack.pop()

def analyze_class(self, defn: ClassDef) -> None:
fullname = self.qualified_name(defn.name)
Expand Down Expand Up @@ -4749,7 +4755,15 @@ def check_no_global(self,
self.name_already_defined(name, ctx, self.globals[name])

def name_not_defined(self, name: str, ctx: Context, namespace: Optional[str] = None) -> None:
if self.is_incomplete_namespace(namespace or self.cur_mod_id):
incomplete = self.is_incomplete_namespace(namespace or self.cur_mod_id)
if (namespace is None
and self.type
and not self.is_func_scope()
and self.incomplete_type_stack[-1]
and not self.final_iteration):
# We are processing a class body for the first time, so it is incomplete.
incomplete = True
if incomplete:
# Target namespace is incomplete, so it's possible that the name will be defined
# later on. Defer current target.
self.record_incomplete_ref()
Expand Down
2 changes: 1 addition & 1 deletion mypy/semanal_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ def semantic_analyze_target(target: str,
Return tuple with these items:
- list of deferred targets
- was some definition incomplete
- was some definition incomplete (need to run another pass)
- were any new names were defined (or placeholders replaced)
"""
state.manager.processed_targets.append(target)
Expand Down
6 changes: 5 additions & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -3799,7 +3799,7 @@ int.__eq__(int)
int.__eq__(3, 4)
[builtins fixtures/args.pyi]
[out]
main:33: error: Too few arguments for "__eq__" of "int"
main:33: error: Too few arguments for "__eq__" of "int"
main:33: error: Unsupported operand types for == ("int" and "Type[int]")

[case testMroSetAfterError]
Expand Down Expand Up @@ -6817,3 +6817,7 @@ class A(metaclass=ABCMeta):
@final
class B(A): # E: Final class __main__.B has abstract attributes "foo"
pass

[case testUndefinedBaseclassInNestedClass]
class C:
class C1(XX): pass # E: Name "XX" is not defined
39 changes: 39 additions & 0 deletions test-data/unit/check-modules.test
Original file line number Diff line number Diff line change
Expand Up @@ -2943,3 +2943,42 @@ def f(x: str) -> None: ...
[file mypy.ini]
\[mypy]
mypy_path = tmp/xx

[case testImportCycleSpecialCase2]
import m

[file m.pyi]
from f import F
class M: pass

[file f.pyi]
from m import M

from typing import Generic, TypeVar

T = TypeVar("T")

class W(Generic[T]): ...

class F(M):
A = W[int]
x: C
class C(W[F.A]): ...

[case testImportCycleSpecialCase3]
import f

[file m.pyi]
from f import F
class M: pass

[file f.pyi]
from m import M

from typing import Generic, TypeVar

T = TypeVar("T")

class F(M):
x: C
class C: ...

0 comments on commit 7f2377e

Please sign in to comment.