Skip to content

Commit

Permalink
Fix dep generation of super() and class names (#4513)
Browse files Browse the repository at this point in the history
Class names need to generate dependencies to __init__ even when not
directly being called, since they could be passed elsewhere and
called. We generate spurious dependencies in cases such as classes
being referenced in isinstance checks and expect statements, but
that's not a big deal.

Handling super() deps is bundled in with this because the __init__ fix
broke a super() using test that previously worked somewhat by accident.
  • Loading branch information
msullivan authored Jan 26, 2018
1 parent d8b884f commit 8ec7046
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 15 deletions.
36 changes: 21 additions & 15 deletions mypy/server/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class 'mod.Cls'. This can also refer to an attribute inherited from a
ImportFrom, CallExpr, CastExpr, TypeVarExpr, TypeApplication, IndexExpr, UnaryExpr, OpExpr,
ComparisonExpr, GeneratorExpr, DictionaryComprehension, StarExpr, PrintStmt, ForStmt, WithStmt,
TupleExpr, ListExpr, OperatorAssignmentStmt, DelStmt, YieldFromExpr, Decorator, Block,
TypeInfo, FuncBase, OverloadedFuncDef, RefExpr, Var, NamedTupleExpr, LDEF, MDEF, GDEF,
TypeInfo, FuncBase, OverloadedFuncDef, RefExpr, SuperExpr, Var, NamedTupleExpr,
LDEF, MDEF, GDEF,
op_methods, reverse_op_methods, ops_with_inplace_method, unary_op_methods
)
from mypy.traverser import TraverserVisitor
Expand Down Expand Up @@ -154,7 +155,6 @@ def __init__(self,
# protocols
# metaclasses
# type aliases
# super()
# functional enum
# type variable with value restriction

Expand Down Expand Up @@ -368,9 +368,18 @@ def visit_del_stmt(self, o: DelStmt) -> None:

# Expressions

# TODO
# dependency on __init__ (e.g. ClassName())
# super()
def process_global_ref_expr(self, o: RefExpr) -> None:
if o.fullname is not None:
self.add_dependency(make_trigger(o.fullname))

# If this is a reference to a type, generate a dependency to its
# constructor.
# TODO: avoid generating spurious dependencies for isinstancce checks,
# except statements, class attribute reference, etc, if perf problem.
typ = self.type_map.get(o)
if isinstance(typ, FunctionLike) and typ.is_type_obj():
class_name = typ.type_object().fullname()
self.add_dependency(make_trigger(class_name + '.__init__'))

def visit_name_expr(self, o: NameExpr) -> None:
if o.kind == LDEF:
Expand All @@ -381,17 +390,13 @@ def visit_name_expr(self, o: NameExpr) -> None:
# Direct reference to member is only possible in the scope that
# defined the name, so no dependency is required.
return
if o.fullname is not None:
trigger = make_trigger(o.fullname)
self.add_dependency(trigger)
self.process_global_ref_expr(o)

def visit_member_expr(self, e: MemberExpr) -> None:
super().visit_member_expr(e)
if e.kind is not None:
# Reference to a module attribute
if e.fullname is not None:
trigger = make_trigger(e.fullname)
self.add_dependency(trigger)
self.process_global_ref_expr(e)
else:
# Reference to a non-module attribute
if e.expr not in self.type_map:
Expand All @@ -401,12 +406,13 @@ def visit_member_expr(self, e: MemberExpr) -> None:
typ = self.type_map[e.expr]
self.add_attribute_dependency(typ, e.name)

def visit_super_expr(self, e: SuperExpr) -> None:
super().visit_super_expr(e)
if e.info is not None:
self.add_dependency(make_trigger(e.info.fullname() + '.' + e.name))

def visit_call_expr(self, e: CallExpr) -> None:
super().visit_call_expr(e)
callee_type = self.type_map.get(e.callee)
if isinstance(callee_type, FunctionLike) and callee_type.is_type_obj():
class_name = callee_type.type_object().fullname()
self.add_dependency(make_trigger(class_name + '.__init__'))

def visit_cast_expr(self, e: CastExpr) -> None:
super().visit_cast_expr(e)
Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/deps-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,35 @@ class D:
<m.D.g> -> m.D.g
<m.D> -> m.D
<m.f> -> m.f

[case testClassSuper]
class C:
def __init__(self, x: int) -> None: pass
def foo(self) -> None: pass

class D(C):
def __init__(self, x: int) -> None:
super().__init__(x)
super().foo()
[out]
<m.C.__init__> -> <m.D.__init__>, m.D.__init__
<m.C.foo> -> <m.D.foo>
<m.C> -> m, m.C, m.D
<m.D.__init__> -> m.D.__init__
<m.D.foo> -> m.D.__init__
<m.D> -> m.D

[case testClassMissingInit]
class C:
def __init__(self, x: int) -> None: pass

class D(C):
pass

def foo() -> None:
D(6)
[out]
<m.C.__init__> -> <m.D.__init__>
<m.C> -> m, m.C, m.D
<m.D.__init__> -> m.foo
<m.D> -> m.D, m.foo
6 changes: 6 additions & 0 deletions test-data/unit/deps-statements.test
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ def g() -> None:
f3()
[builtins fixtures/exception.pyi]
[out]
-- The dependency on the ctor is basically spurious but not a problem
<m.A.__init__> -> m.g
<m.A> -> m.A, m.g
<m.B.__init__> -> m.g
<m.B.f> -> m.g
<m.B> -> m.B, m.g
<m.f1> -> m.g
Expand All @@ -195,7 +198,10 @@ def g() -> None:
f2()
[builtins fixtures/exception.pyi]
[out]
-- The dependency on the ctor is basically spurious but not a problem
<m.A.__init__> -> m.g
<m.A> -> m.A, m.g
<m.B.__init__> -> m.g
<m.B> -> m.B, m.g
<m.f1> -> m.g
<m.f2> -> m.g
Expand Down
4 changes: 4 additions & 0 deletions test-data/unit/deps.test
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ def f(x: object) -> None:
x.g()
[builtins fixtures/isinstancelist.pyi]
[out]
-- The dependency on the ctor is basically spurious but not a problem
<m.A.__init__> -> m.f
<m.A.g> -> m.f
<m.A> -> m.A, m.f

Expand All @@ -353,6 +355,8 @@ def f(x: A) -> None:
[builtins fixtures/isinstancelist.pyi]
[out]
<m.A> -> <m.f>, m.A, m.f
-- The dependency on the ctor is basically spurious but not a problem
<m.B.__init__> -> m.f
<m.B> -> m.B, m.f

[case testAttributeWithClassType1]
Expand Down
51 changes: 51 additions & 0 deletions test-data/unit/fine-grained.test
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,42 @@ class A:
==
main:4: error: Too few arguments for "A"

[case testConstructorSignatureChanged2]
from typing import Callable
import m

def use(x: Callable[[], m.A]) -> None:
x()
def f() -> None:
use(m.A)
[file m.py]
class A:
def __init__(self) -> None: pass
[file m.py.2]
class A:
def __init__(self, x: int) -> None: pass
[out]
==
-- This is a bad error message
main:7: error: Argument 1 to "use" has incompatible type "Type[A]"; expected "Callable[[], A]"

[case testConstructorSignatureChanged3]
from a import C
class D(C):
def g(self) -> None:
super().__init__()
D()
[file a.py]
class C:
def __init__(self) -> None: pass
[file a.py.2]
class C:
def __init__(self, x: int) -> None: pass
[out]
==
main:4: error: Too few arguments for "__init__" of "C"
main:5: error: Too few arguments for "D"

[case testConstructorAdded]
import m

Expand Down Expand Up @@ -700,6 +736,21 @@ class B(A): pass
==
main:4: error: Too few arguments for "B"

[case testSuperField]
from a import C
class D(C):
def g(self) -> int:
return super().x
[file a.py]
class C:
def __init__(self) -> None: self.x = 12
[file a.py.2]
class C:
def __init__(self) -> None: self.x = 'ar'
[out]
==
main:4: error: Incompatible return value type (got "str", expected "int")

[case testImportFrom]
from m import f

Expand Down

0 comments on commit 8ec7046

Please sign in to comment.