Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
20 changes: 20 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,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 @@ -1257,6 +1266,17 @@ 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 (isinstance(base_node, Var) and
((node.is_classvar and not base_node.is_classvar) or
(not node.is_classvar and base_node.is_classvar))):
self.fail('Invalid class attribute definition '
'(previously declared on base class "%s")' % 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
2 changes: 2 additions & 0 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,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(node)
Copy link
Member

Choose a reason for hiding this comment

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

This looks surprisingly simple, but maybe more corner cases will be necessary for more tests, see below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but I think it works. There are tests with subclasses, callables, Type type and everything passes.

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
4 changes: 4 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
CANNOT_ACCESS_INIT = 'Cannot access "__init__" directly'
CANNOT_ASSIGN_TO_METHOD = 'Cannot assign to a method'
CANNOT_ASSIGN_TO_TYPE = 'Cannot assign to a type'
CANNOT_ASSIGN_TO_CLASSVAR = 'Illegal assignment to class variable'
INCONSISTENT_ABSTRACT_OVERLOAD = \
'Overloaded method has both abstract and non-abstract variants'
READ_ONLY_PROPERTY_OVERRIDES_READ_WRITE = \
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, context: Context) -> None:
self.fail(CANNOT_ASSIGN_TO_CLASSVAR, 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 @@ -637,13 +637,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
40 changes: 26 additions & 14 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1306,30 +1306,19 @@ def visit_block_maybe(self, b: Block) -> None:
def anal_type(self, t: Type, allow_tuple_literal: bool = False,
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)
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.

s.rvalue.accept(self)
if s.type:
allow_tuple_literal = isinstance(s.lvalues[-1], (TupleExpr, ListExpr))
Expand Down Expand Up @@ -2159,6 +2148,29 @@ 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, (NameExpr, MemberExpr)):
Copy link
Member

Choose a reason for hiding this comment

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

You can just use RefExpr instead of (NameExpr, MemberExpr)

return
if not self.check_classvar_definition(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 definition', lvalue)

def check_classvar_definition(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 visit_decorator(self, dec: Decorator) -> None:
for d in dec.decorators:
d.accept(self)
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
41 changes: 32 additions & 9 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,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 +124,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_nested(t.args[0])])
instance.line = t.line
return instance
return self.tuple_type(self.anal_array(t.args))
Expand Down Expand Up @@ -148,6 +152,16 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
items = self.anal_array(t.args)
item = items[0]
return TypeType(item, line=t.line)
elif fullname == 'typing.ClassVar':
if self.nesting_level > 0:
self.fail('Invalid ClassVar definition', t)
if len(t.args) == 0:
return AnyType(line=t.line)
if len(t.args) != 1:
Copy link
Member

Choose a reason for hiding this comment

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

I think a bare ClassVar should be also allowed meaning ClassVar[Any]

self.fail('ClassVar[...] must have at most one type argument', t)
return AnyType()
items = self.anal_array(t.args)
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be cleaner if you just return self.anal_nested(t.args[0])

return items[0]
elif fullname == 'mypy_extensions.NoReturn':
return UninhabitedType(is_noreturn=True)
elif sym.kind == TYPE_ALIAS:
Expand Down Expand Up @@ -291,12 +305,14 @@ def visit_type_var(self, t: TypeVarType) -> Type:

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),
ret_type=self.anal_nested(t.ret_type),
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))
Expand All @@ -308,13 +324,13 @@ def visit_tuple_type(self, t: TupleType) -> Type:

def visit_typeddict_type(self, t: TypedDictType) -> Type:
items = OrderedDict([
(item_name, item_type.accept(self))
(item_name, self.anal_nested(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_nested(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 +343,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_nested(t.item), line=t.line)

def analyze_callable_type(self, t: UnboundType) -> Type:
fallback = self.builtin_type('builtins.function')
Expand All @@ -340,7 +356,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_nested(t.args[1])
if isinstance(t.args[0], TypeList):
# Callable[[ARG, ...], RET] (ordinary callable type)
args = t.args[0].items
Expand All @@ -367,9 +383,16 @@ def analyze_callable_type(self, t: UnboundType) -> Type:
def anal_array(self, a: List[Type]) -> List[Type]:
res = [] # type: List[Type]
for t in a:
res.append(t.accept(self))
res.append(self.anal_nested(t))
return res

def anal_nested(self, t: Type) -> Type:
self.nesting_level += 1
try:
return t.accept(self)
finally:
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