Skip to content

Implement ClassVar semantics (fixes #2771) #2873

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

Merged
merged 24 commits into from
Mar 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d6b370d
Implement ClassVar semantics (fixes #2771)
miedzinski Feb 15, 2017
950b383
Fix type annotations
miedzinski Feb 15, 2017
cdfdbbc
Remove ClassVarType, use Var.is_classvar flag
miedzinski Feb 16, 2017
01a7a69
Add test case against TypeVar
miedzinski Feb 16, 2017
d52b4e3
Fix if statement in check_classvar_definition
miedzinski Feb 17, 2017
15e1e85
Change error message about ClassVar arg count
miedzinski Feb 17, 2017
781245f
Add more tests
miedzinski Feb 17, 2017
cd61a5d
Remove duplicate test case
miedzinski Feb 17, 2017
9c29911
Move ClassVar override checks to TypeChecker
miedzinski Feb 17, 2017
b618718
Prohibit ClassVar inside other types or in signatures
miedzinski Feb 17, 2017
cb6786f
Prohibit defining ClassVars on self
miedzinski Feb 17, 2017
a427d61
Add more tests
miedzinski Feb 17, 2017
35e2c68
Fix analysing implicit tuple types
miedzinski Feb 18, 2017
e121158
Add more meaningful error message on attribute override
miedzinski Feb 18, 2017
5e2aafc
Add better error message on assignments
miedzinski Feb 18, 2017
137215e
Add more precise errors on ClassVar definitions
miedzinski Feb 18, 2017
5dd131a
Minor style improvements
miedzinski Feb 18, 2017
30a4185
Prohibit ClassVars in for and with statements
miedzinski Feb 18, 2017
bcce34d
Rename check_classvar_definition to is_classvar
miedzinski Feb 18, 2017
7ddca4b
Don't consider types in signatures as nested
miedzinski Feb 19, 2017
92b3532
Prohibit generic class variables
miedzinski Feb 22, 2017
20b1abe
Prohibit access to generic instance variables via class
miedzinski Feb 28, 2017
3362e65
Merge branch 'master' into classvar
miedzinski Mar 9, 2017
5430b32
Add incremental checking test
miedzinski Mar 9, 2017
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
25 changes: 25 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,15 @@ def check_compatibility_all_supers(self, lvalue: NameExpr, lvalue_type: Type,
lvalue.kind == MDEF and
len(lvalue_node.info.bases) > 0):

for base in lvalue_node.info.mro[1:]:
tnode = base.names.get(lvalue_node.name())
if tnode is not None:
if not self.check_compatibility_classvar_super(lvalue_node,
base,
tnode.node):
# Show only one error per variable
break

for base in lvalue_node.info.mro[1:]:
# Only check __slots__ against the 'object'
# If a base class defines a Tuple of 3 elements, a child of
Expand Down Expand Up @@ -1278,6 +1287,22 @@ def lvalue_type_from_base(self, expr_node: Var,

return None, None

def check_compatibility_classvar_super(self, node: Var,
base: TypeInfo, base_node: Node) -> bool:
if not isinstance(base_node, Var):
return True
if node.is_classvar and not base_node.is_classvar:
self.fail('Cannot override instance variable '
'(previously declared on base class "%s") '
'with class variable' % base.name(), node)
return False
elif not node.is_classvar and base_node.is_classvar:
self.fail('Cannot override class variable '
'(previously declared on base class "%s") '
'with instance variable' % base.name(), node)
return False
return True

def check_assignment_to_multiple_lvalues(self, lvalues: List[Lvalue], rvalue: Expression,
context: Context,
infer_lvalue_type: bool = True) -> None:
Expand Down
6 changes: 5 additions & 1 deletion mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from mypy.types import (
Type, Instance, AnyType, TupleType, TypedDictType, CallableType, FunctionLike, TypeVarDef,
Overloaded, TypeVarType, UnionType, PartialType,
DeletedType, NoneTyp, TypeType, function_type
DeletedType, NoneTyp, TypeType, function_type, get_type_vars,
)
from mypy.nodes import (
TypeInfo, FuncBase, Var, FuncDef, SymbolNode, Context, MypyFile, TypeVarExpr,
Expand Down Expand Up @@ -270,6 +270,8 @@ def analyze_var(name: str, var: Var, itype: Instance, info: TypeInfo, node: Cont
if is_lvalue and var.is_property and not var.is_settable_property:
# TODO allow setting attributes in subclass (although it is probably an error)
msg.read_only_property(name, info, node)
if is_lvalue and var.is_classvar:
msg.cant_assign_to_classvar(name, node)
if var.is_initialized_in_class and isinstance(t, FunctionLike) and not t.is_type_obj():
if is_lvalue:
if var.is_property:
Expand Down Expand Up @@ -394,6 +396,8 @@ def analyze_class_attribute_access(itype: Instance,
if t:
if isinstance(t, PartialType):
return handle_partial_attribute_type(t, is_lvalue, msg, node.node)
if not is_method and (isinstance(t, TypeVarType) or get_type_vars(t)):
msg.fail(messages.GENERIC_INSTANCE_VAR_CLASS_ACCESS, context)
is_classmethod = is_decorated and cast(Decorator, node.node).func.is_class
return add_class_tvars(t, itype, is_classmethod, builtin_type, original_type)
elif isinstance(node.node, Var):
Expand Down
4 changes: 4 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
MALFORMED_ASSERT = 'Assertion is always true, perhaps remove parentheses?'
NON_BOOLEAN_IN_CONDITIONAL = 'Condition must be a boolean'
DUPLICATE_TYPE_SIGNATURES = 'Function has duplicate type signatures'
GENERIC_INSTANCE_VAR_CLASS_ACCESS = 'Access to generic instance variables via class is ambiguous'

ARG_CONSTRUCTOR_NAMES = {
ARG_POS: "Arg",
Expand Down Expand Up @@ -814,6 +815,9 @@ def base_class_definitions_incompatible(self, name: str, base1: TypeInfo,
def cant_assign_to_method(self, context: Context) -> None:
self.fail(CANNOT_ASSIGN_TO_METHOD, context)

def cant_assign_to_classvar(self, name: str, context: Context) -> None:
self.fail('Cannot assign to class variable "%s" via instance' % name, context)

def read_only_property(self, name: str, type: TypeInfo,
context: Context) -> None:
self.fail('Property "{}" defined in "{}" is read-only'.format(
Expand Down
4 changes: 3 additions & 1 deletion mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,13 +642,15 @@ class Var(SymbolNode):
is_classmethod = False
is_property = False
is_settable_property = False
is_classvar = False
# Set to true when this variable refers to a module we were unable to
# parse for some reason (eg a silenced module)
is_suppressed_import = False

FLAGS = [
'is_self', 'is_ready', 'is_initialized_in_class', 'is_staticmethod',
'is_classmethod', 'is_property', 'is_settable_property', 'is_suppressed_import'
'is_classmethod', 'is_property', 'is_settable_property', 'is_suppressed_import',
'is_classvar'
]

def __init__(self, name: str, type: 'mypy.types.Type' = None) -> None:
Expand Down
62 changes: 48 additions & 14 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ def analyze_function(self, defn: FuncItem) -> None:
if defn.type:
# Signature must be analyzed in the surrounding scope so that
# class-level imported names and type variables are in scope.
self.check_classvar_in_signature(defn.type)
defn.type = self.anal_type(defn.type)
self.check_function_signature(defn)
if isinstance(defn, FuncDef):
Expand Down Expand Up @@ -524,6 +525,20 @@ def analyze_function(self, defn: FuncItem) -> None:
self.leave()
self.function_stack.pop()

def check_classvar_in_signature(self, typ: Type) -> None:
t = None # type: Type
if isinstance(typ, Overloaded):
for t in typ.items():
self.check_classvar_in_signature(t)
return
if not isinstance(typ, CallableType):
return
for t in typ.arg_types + [typ.ret_type]:
if self.is_classvar(t):
self.fail_invalid_classvar(t)
# Show only one error per signature
break

def add_func_type_variables_to_symbol_table(
self, defn: FuncItem) -> List[SymbolTableNode]:
nodes = [] # type: List[SymbolTableNode]
Expand Down Expand Up @@ -1345,30 +1360,19 @@ def visit_block_maybe(self, b: Block) -> None:
def anal_type(self, t: Type, allow_tuple_literal: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

Is this change really necessary? I am asking for two reasons:

  • This looks unrelated to class variables
  • The transformation you made is actually not equivalent, this code returns more precise type on failure (TupleType with corresponding number of Anys as items, instead of just plain AnyType() in typeanal.py code)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. I've restored old behaviour, although in TypeAnalyser. This is needed if we want to fail on

class A:
    x, y = None, None  # type: ClassVar, ClassVar

Since I've removed the test for this we could allow this, but these variables wouldn't get is_classvar flag. I would keep it as is.

aliasing: bool = False) -> Type:
if t:
if allow_tuple_literal:
# Types such as (t1, t2, ...) only allowed in assignment statements. They'll
# generate errors elsewhere, and Tuple[t1, t2, ...] must be used instead.
if isinstance(t, TupleType):
# Unlike TypeAnalyser, also allow implicit tuple types (without Tuple[...]).
star_count = sum(1 for item in t.items if isinstance(item, StarType))
if star_count > 1:
self.fail('At most one star type allowed in a tuple', t)
return TupleType([AnyType() for _ in t.items],
self.builtin_type('builtins.tuple'), t.line)
items = [self.anal_type(item, True)
for item in t.items]
return TupleType(items, self.builtin_type('builtins.tuple'), t.line)
a = TypeAnalyser(self.lookup_qualified,
self.lookup_fully_qualified,
self.fail,
aliasing=aliasing)
aliasing=aliasing,
allow_tuple_literal=allow_tuple_literal)
return t.accept(a)
else:
return None

def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
for lval in s.lvalues:
self.analyze_lvalue(lval, explicit_type=s.type is not None)
self.check_classvar(s)
s.rvalue.accept(self)
if s.type:
allow_tuple_literal = isinstance(s.lvalues[-1], (TupleExpr, ListExpr))
Expand Down Expand Up @@ -2194,6 +2198,32 @@ def build_typeddict_typeinfo(self, name: str, items: List[str],

return info

def check_classvar(self, s: AssignmentStmt) -> None:
lvalue = s.lvalues[0]
if len(s.lvalues) != 1 or not isinstance(lvalue, RefExpr):
return
if not self.is_classvar(s.type):
return
if self.is_class_scope() and isinstance(lvalue, NameExpr):
node = lvalue.node
if isinstance(node, Var):
node.is_classvar = True
elif not isinstance(lvalue, MemberExpr) or self.is_self_member_ref(lvalue):
# In case of member access, report error only when assigning to self
# Other kinds of member assignments should be already reported
self.fail_invalid_classvar(lvalue)

def is_classvar(self, typ: Type) -> bool:
if not isinstance(typ, UnboundType):
return False
sym = self.lookup_qualified(typ.name, typ)
if not sym or not sym.node:
return False
return sym.node.fullname() == 'typing.ClassVar'

def fail_invalid_classvar(self, context: Context) -> None:
self.fail('ClassVar can only be used for assignments in class body', context)

def visit_decorator(self, dec: Decorator) -> None:
for d in dec.decorators:
d.accept(self)
Expand Down Expand Up @@ -2295,6 +2325,8 @@ def visit_for_stmt(self, s: ForStmt) -> None:
# Bind index variables and check if they define new names.
self.analyze_lvalue(s.index, explicit_type=s.index_type is not None)
if s.index_type:
if self.is_classvar(s.index_type):
self.fail_invalid_classvar(s.index)
allow_tuple_literal = isinstance(s.index, (TupleExpr, ListExpr))
s.index_type = self.anal_type(s.index_type, allow_tuple_literal)
self.store_declared_types(s.index, s.index_type)
Expand Down Expand Up @@ -2370,6 +2402,8 @@ def visit_with_stmt(self, s: WithStmt) -> None:
# Since we have a target, pop the next type from types
if types:
t = types.pop(0)
if self.is_classvar(t):
self.fail_invalid_classvar(n)
allow_tuple_literal = isinstance(n, (TupleExpr, ListExpr))
t = self.anal_type(t, allow_tuple_literal)
new_types.append(t)
Expand Down
1 change: 1 addition & 0 deletions mypy/test/testcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
'check-varargs.test',
'check-newsyntax.test',
'check-underscores.test',
'check-classvar.test',
]

files.extend(fast_parser_files)
Expand Down
1 change: 1 addition & 0 deletions mypy/test/testsemanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
'semanal-abstractclasses.test',
'semanal-namedtuple.test',
'semanal-typeddict.test',
'semanal-classvar.test',
'semanal-python2.test']


Expand Down
71 changes: 52 additions & 19 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
"""Semantic analysis of types"""

from collections import OrderedDict
from typing import Callable, cast, List, Optional
from typing import Callable, List, Optional, Set

from mypy.types import (
Type, UnboundType, TypeVarType, TupleType, TypedDictType, UnionType, Instance,
Type, UnboundType, TypeVarType, TupleType, TypedDictType, UnionType, Instance, TypeVarId,
AnyType, CallableType, Void, NoneTyp, DeletedType, TypeList, TypeVarDef, TypeVisitor,
StarType, PartialType, EllipsisType, UninhabitedType, TypeType, get_typ_args, set_typ_args,
get_type_vars,
)
from mypy.nodes import (
BOUND_TVAR, UNBOUND_TVAR, TYPE_ALIAS, UNBOUND_IMPORTED,
Expand Down Expand Up @@ -81,11 +82,15 @@ def __init__(self,
lookup_func: Callable[[str, Context], SymbolTableNode],
lookup_fqn_func: Callable[[str], SymbolTableNode],
fail_func: Callable[[str, Context], None], *,
aliasing: bool = False) -> None:
aliasing: bool = False,
allow_tuple_literal: bool = False) -> None:
self.lookup = lookup_func
self.lookup_fqn_func = lookup_fqn_func
self.fail = fail_func
self.aliasing = aliasing
self.allow_tuple_literal = allow_tuple_literal
# Positive if we are analyzing arguments of another (outer) type
self.nesting_level = 0

def visit_unbound_type(self, t: UnboundType) -> Type:
if t.optional:
Expand Down Expand Up @@ -120,7 +125,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
return self.builtin_type('builtins.tuple')
if len(t.args) == 2 and isinstance(t.args[1], EllipsisType):
# Tuple[T, ...] (uniform, variable-length tuple)
instance = self.builtin_type('builtins.tuple', [t.args[0].accept(self)])
instance = self.builtin_type('builtins.tuple', [self.anal_type(t.args[0])])
instance.line = t.line
return instance
return self.tuple_type(self.anal_array(t.args))
Expand All @@ -132,22 +137,34 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
if len(t.args) != 1:
self.fail('Optional[...] must have exactly one type argument', t)
return AnyType()
items = self.anal_array(t.args)
item = self.anal_type(t.args[0])
if experiments.STRICT_OPTIONAL:
return UnionType.make_simplified_union([items[0], NoneTyp()])
return UnionType.make_simplified_union([item, NoneTyp()])
else:
# Without strict Optional checking Optional[t] is just an alias for t.
return items[0]
return item
elif fullname == 'typing.Callable':
return self.analyze_callable_type(t)
elif fullname == 'typing.Type':
if len(t.args) == 0:
return TypeType(AnyType(), line=t.line)
if len(t.args) != 1:
self.fail('Type[...] must have exactly one type argument', t)
items = self.anal_array(t.args)
item = items[0]
item = self.anal_type(t.args[0])
return TypeType(item, line=t.line)
elif fullname == 'typing.ClassVar':
if self.nesting_level > 0:
self.fail('Invalid type: ClassVar nested inside other type', t)
if len(t.args) == 0:
return AnyType(line=t.line)
if len(t.args) != 1:
self.fail('ClassVar[...] must have at most one type argument', t)
return AnyType()
item = self.anal_type(t.args[0])
if isinstance(item, TypeVarType) or get_type_vars(item):
self.fail('Invalid type: ClassVar cannot be generic', t)
return AnyType()
return item
elif fullname == 'mypy_extensions.NoReturn':
return UninhabitedType(is_noreturn=True)
elif sym.kind == TYPE_ALIAS:
Expand Down Expand Up @@ -290,31 +307,38 @@ def visit_type_var(self, t: TypeVarType) -> Type:
return t

def visit_callable_type(self, t: CallableType) -> Type:
return t.copy_modified(arg_types=self.anal_array(t.arg_types),
ret_type=t.ret_type.accept(self),
return t.copy_modified(arg_types=self.anal_array(t.arg_types, nested=False),
ret_type=self.anal_type(t.ret_type, nested=False),
fallback=t.fallback or self.builtin_type('builtins.function'),
variables=self.anal_var_defs(t.variables))

def visit_tuple_type(self, t: TupleType) -> Type:
if t.implicit:
# Types such as (t1, t2, ...) only allowed in assignment statements. They'll
# generate errors elsewhere, and Tuple[t1, t2, ...] must be used instead.
if t.implicit and not self.allow_tuple_literal:
self.fail('Invalid tuple literal type', t)
return AnyType()
star_count = sum(1 for item in t.items if isinstance(item, StarType))
if star_count > 1:
self.fail('At most one star type allowed in a tuple', t)
return AnyType()
if t.implicit:
return TupleType([AnyType() for _ in t.items],
self.builtin_type('builtins.tuple'),
t.line)
else:
return AnyType()
fallback = t.fallback if t.fallback else self.builtin_type('builtins.tuple', [AnyType()])
return TupleType(self.anal_array(t.items), fallback, t.line)

def visit_typeddict_type(self, t: TypedDictType) -> Type:
items = OrderedDict([
(item_name, item_type.accept(self))
(item_name, self.anal_type(item_type))
for (item_name, item_type) in t.items.items()
])
return TypedDictType(items, t.fallback)

def visit_star_type(self, t: StarType) -> Type:
return StarType(t.type.accept(self), t.line)
return StarType(self.anal_type(t.type), t.line)

def visit_union_type(self, t: UnionType) -> Type:
return UnionType(self.anal_array(t.items), t.line)
Expand All @@ -327,7 +351,7 @@ def visit_ellipsis_type(self, t: EllipsisType) -> Type:
return AnyType()

def visit_type_type(self, t: TypeType) -> Type:
return TypeType(t.item.accept(self), line=t.line)
return TypeType(self.anal_type(t.item), line=t.line)

def analyze_callable_type(self, t: UnboundType) -> Type:
fallback = self.builtin_type('builtins.function')
Expand All @@ -340,7 +364,7 @@ def analyze_callable_type(self, t: UnboundType) -> Type:
fallback=fallback,
is_ellipsis_args=True)
elif len(t.args) == 2:
ret_type = t.args[1].accept(self)
ret_type = self.anal_type(t.args[1])
if isinstance(t.args[0], TypeList):
# Callable[[ARG, ...], RET] (ordinary callable type)
args = t.args[0].items
Expand All @@ -364,12 +388,21 @@ def analyze_callable_type(self, t: UnboundType) -> Type:
self.fail('Invalid function type', t)
return AnyType()

def anal_array(self, a: List[Type]) -> List[Type]:
def anal_array(self, a: List[Type], nested: bool = True) -> List[Type]:
res = [] # type: List[Type]
for t in a:
res.append(t.accept(self))
res.append(self.anal_type(t, nested))
return res

def anal_type(self, t: Type, nested: bool = True) -> Type:
if nested:
self.nesting_level += 1
try:
return t.accept(self)
finally:
if nested:
self.nesting_level -= 1

def anal_var_defs(self, var_defs: List[TypeVarDef]) -> List[TypeVarDef]:
a = [] # type: List[TypeVarDef]
for vd in var_defs:
Expand Down
Loading