Skip to content
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

Final names and attributes #5522

Merged
merged 58 commits into from
Sep 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
c6125f1
Start adding tests
Aug 19, 2018
8f2bee6
Add some flags
Aug 19, 2018
61707ab
Add semanal logic for (not really) variables
Aug 20, 2018
75ccad3
Add checks for re-assignment; self and other fixes; work on tests
Aug 20, 2018
84ac046
Tests for invalid declarations
Aug 20, 2018
395cbd5
Move the logic for MemberExpr to checkmember.py; more tests
Aug 20, 2018
67c76bf
Implement special access on class objects
Aug 20, 2018
f6b173e
Fix class re-assignment; more tests
Aug 20, 2018
5053902
Create new Var with bare Final on self; search all bases for final; t…
Aug 20, 2018
8a4ed37
Multiple inheritance; fix lint
Aug 20, 2018
c6a2d99
Start writing docs
Aug 20, 2018
0ca2770
Final classes and methods (+ some tests)
Aug 21, 2018
aa3ab4b
Minor fixes; more overriding tests
Aug 21, 2018
ad42674
An overload fix; more tests
Aug 21, 2018
a106d08
Tweak error messages; move error logic to messages.py
Aug 21, 2018
37c4bb6
Add protocol tests
Aug 21, 2018
b6531d3
Add some comments and docstrings; some reorg
Aug 21, 2018
8366d3f
Add more docs
Aug 21, 2018
312cbc7
Minor fixes in docs
Aug 21, 2018
ec15a63
Some fine-grained stuff; add diff tests
Aug 22, 2018
c8a8d9b
Clarify constant re-export; add few tests
Aug 22, 2018
157a936
Add more incremental tests (coarse and fine)
Aug 22, 2018
eb945c9
Tweak some targets in fine-grained tests
Aug 22, 2018
58e4243
Only keep basic tests in coarse-grained
Aug 22, 2018
8602c61
Store final value for simple literals
Aug 22, 2018
7e05f7d
Prohibit augmented assignments and final within loops
Aug 23, 2018
fa7f8d5
Address CR
ilevkivskyi Aug 26, 2018
8d2973c
Fix alias on newer Python versions
ilevkivskyi Aug 26, 2018
430660e
Merge branch 'master' into final
ilevkivskyi Aug 29, 2018
11a157e
Move final docs to a separate file
Aug 31, 2018
b5dca3b
Add section header
Aug 31, 2018
1912fd8
Fix the rest of comments about docs
Aug 31, 2018
235b28f
Prohibit overriding writeable with final
Aug 31, 2018
86e883a
Fix some tests
ilevkivskyi Sep 1, 2018
aa8b436
Allow deferred final definition in __init__
ilevkivskyi Sep 1, 2018
95da7ce
Fix self-check
ilevkivskyi Sep 1, 2018
b50f3de
Update new Var final flags
ilevkivskyi Sep 1, 2018
2f74d87
Simplify/shorten some error messages
ilevkivskyi Sep 1, 2018
f9f1182
Merge remote-tracking branch 'upstream/master' into final
ilevkivskyi Sep 1, 2018
0364868
Further simplify error messages; use one-line errors instead of notes
ilevkivskyi Sep 1, 2018
632def5
Merge remote-tracking branch 'upstream/master' into final
ilevkivskyi Sep 1, 2018
cc8ce09
Merge branch 'master' into final
ilevkivskyi Sep 3, 2018
35bf8cf
Merge branch 'master' into final
ilevkivskyi Sep 5, 2018
53720e3
Remove ClassVar discussion
ilevkivskyi Sep 6, 2018
953119d
Update docs
ilevkivskyi Sep 6, 2018
ed009ee
Minor fixes in code
ilevkivskyi Sep 7, 2018
b8c67aa
Stricter rules for final in overloads; update tests
ilevkivskyi Sep 7, 2018
c9abd9d
Minor fixes
ilevkivskyi Sep 7, 2018
41b996f
Address some CR
ilevkivskyi Sep 11, 2018
7a85285
More CR; update tests
ilevkivskyi Sep 11, 2018
b37f35f
Prohibit bare final without r.h.s.
ilevkivskyi Sep 11, 2018
f521723
Merge remote-tracking branch 'upstream/master' into final
ilevkivskyi Sep 11, 2018
66fdbc8
Minor fixes
ilevkivskyi Sep 11, 2018
a76da46
Kill the doc part
ilevkivskyi Sep 11, 2018
aefd17c
Add test case for delayed definition in a subclass
Sep 11, 2018
309e1a5
Show less errors; add a test
Sep 11, 2018
98270fc
Fix self-check
Sep 11, 2018
8b52fda
Fix grammar
Sep 11, 2018
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
160 changes: 157 additions & 3 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ def __init__(self, errors: Errors, modules: Dict[str, MypyFile], options: Option
# If True, process function definitions. If False, don't. This is used
# for processing module top levels in fine-grained incremental mode.
self.recurse_into_functions = True
# This internal flag is used to track whether we a currently type-checking
# a final declaration (assignment), so that some errors should be suppressed.
# Should not be set manually, use get_final_context/enter_final_context instead.
# NOTE: we use the context manager to avoid "threading" an additional `is_final_def`
# argument through various `checker` and `checkmember` functions.
self._is_final_def = False

def reset(self) -> None:
"""Cleanup stale state that might be left over from a typechecking run.
Expand Down Expand Up @@ -1254,6 +1260,17 @@ def check_method_or_accessor_override_for_base(self, defn: Union[FuncBase, Decor
"""Check if method definition is compatible with a base class."""
if base:
name = defn.name()
base_attr = base.names.get(name)
if base_attr:
# First, check if we override a final (always an error, even with Any types).
if (isinstance(base_attr.node, (Var, FuncBase, Decorator))
and base_attr.node.is_final):
self.msg.cant_override_final(name, base.name(), defn)
# Second, final can't override anything writeable independently of types.
if defn.is_final:
self.check_no_writable(name, base_attr.node, defn)

# Check the type of override.
if name not in ('__init__', '__new__', '__init_subclass__'):
# Check method override
# (__init__, __new__, __init_subclass__ are special).
Expand All @@ -1280,6 +1297,7 @@ def check_method_override_for_base_with_name(
context = defn
else:
context = defn.func

# Construct the type of the overriding method.
if isinstance(defn, FuncBase):
typ = self.function_type(defn) # type: Type
Expand Down Expand Up @@ -1453,6 +1471,9 @@ def visit_class_def(self, defn: ClassDef) -> None:
typ = defn.info
if typ.is_protocol and typ.defn.type_vars:
self.check_protocol_variance(defn)
for base in typ.mro[1:]:
if base.is_final:
self.fail('Cannot inherit from final class "{}"'.format(base.name()), defn)
with self.tscope.class_scope(defn.info), self.enter_partial_types(is_class=True):
old_binder = self.binder
self.binder = ConditionalTypeBinder()
Expand Down Expand Up @@ -1564,6 +1585,12 @@ def check_compatibility(self, name: str, base1: TypeInfo,
if second_type is None:
self.msg.cannot_determine_type_in_base(name, base2.name(), ctx)
ok = True
# Final attributes can never be overridden, but can override
# non-final read-only attributes.
if isinstance(second.node, (Var, FuncBase, Decorator)) and second.node.is_final:
self.msg.cant_override_final(name, base2.name(), ctx)
if isinstance(first.node, (Var, FuncBase, Decorator)) and first.node.is_final:
self.check_no_writable(name, second.node, ctx)
# __slots__ is special and the type can vary across class hierarchy.
if name == '__slots__':
ok = True
Expand Down Expand Up @@ -1611,7 +1638,8 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:

Handle all kinds of assignment statements (simple, indexed, multiple).
"""
self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)
with self.enter_final_context(s.is_final_def):
self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)

if (s.type is not None and
self.options.disallow_any_unimported and
Expand All @@ -1632,7 +1660,13 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
self.expr_checker.accept(s.rvalue)
rvalue = self.temp_node(self.type_map[s.rvalue], s)
for lv in s.lvalues[:-1]:
self.check_assignment(lv, rvalue, s.type is None)
with self.enter_final_context(s.is_final_def):
self.check_assignment(lv, rvalue, s.type is None)

self.check_final(s)
if (s.is_final_def and s.type and not has_no_typevars(s.type)
and self.scope.active_class() is not None):
self.fail("Final name declared in class body cannot depend on type variables", s)

def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type: bool = True,
new_syntax: bool = False) -> None:
Expand Down Expand Up @@ -1742,6 +1776,12 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, lvalue_type: Optional[
# Show only one error per variable
break

if not self.check_compatibility_final_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 @@ -1852,6 +1892,11 @@ def lvalue_type_from_base(self, expr_node: Var,
# value, not the Callable
if base_node.is_property:
base_type = base_type.ret_type
if isinstance(base_type, FunctionLike) and isinstance(base_node,
OverloadedFuncDef):
# Same for properties with setter
if base_node.is_property:
base_type = base_type.items()[0].ret_type

return base_type, base_node

Expand All @@ -1873,6 +1918,109 @@ def check_compatibility_classvar_super(self, node: Var,
return False
return True

def check_compatibility_final_super(self, node: Var,
base: TypeInfo, base_node: Optional[Node]) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs a docstring. Explain what this checks and the return value. It looks like there's subtlety about what this checks which would be good to call out -- it looks like this only checks for some subsets of errors, as other cases are handled elsewhere.

"""Check if an assignment overrides a final attribute in a base class.

This only checks situations where either a node in base class is not a variable
but a final method, or where override is explicitly declared as final.
In these cases we give a more detailed error message. In addition, we check that
a final variable doesn't override writeable attribute, which is not safe.

Other situations are checked in `check_final()`.
"""
if not isinstance(base_node, (Var, FuncBase, Decorator)):
return True
if base_node.is_final and (node.is_final or not isinstance(base_node, Var)):
# Give this error only for explicit override attempt with `Final`, or
# if we are overriding a final method with variable.
# Other override attempts will be flagged as assignment to constant
# in `check_final()`.
self.msg.cant_override_final(node.name(), base.name(), node)
return False
if node.is_final:
self.check_no_writable(node.name(), base_node, node)
return True

def check_no_writable(self, name: str, base_node: Optional[Node], ctx: Context) -> None:
"""Check that a final variable doesn't override writeable attribute.

This is done to prevent situations like this:
class C:
attr = 1
class D(C):
attr: Final = 2

x: C = D()
x.attr = 3 # Oops!
"""
if isinstance(base_node, Var):
ok = False
elif isinstance(base_node, OverloadedFuncDef) and base_node.is_property:
first_item = cast(Decorator, base_node.items[0])
ok = not first_item.var.is_settable_property
else:
ok = True
if not ok:
self.msg.final_cant_override_writable(name, ctx)

def get_final_context(self) -> bool:
"""Check whether we a currently checking a final declaration."""
return self._is_final_def

@contextmanager
def enter_final_context(self, is_final_def: bool) -> Iterator[None]:
"""Store whether the current checked assignment is a final declaration."""
old_ctx = self._is_final_def
self._is_final_def = is_final_def
try:
yield
finally:
self._is_final_def = old_ctx

def check_final(self, s: Union[AssignmentStmt, OperatorAssignmentStmt]) -> None:
"""Check if this assignment does not assign to a final attribute.

This function performs the check only for name assignments at module
and class scope. The assignments to `obj.attr` and `Cls.attr` are checked
in checkmember.py.
"""
if isinstance(s, AssignmentStmt):
lvs = self.flatten_lvalues(s.lvalues)
else:
lvs = [s.lvalue]
is_final_decl = s.is_final_def if isinstance(s, AssignmentStmt) else False
if is_final_decl and self.scope.active_class():
lv = lvs[0]
assert isinstance(lv, RefExpr)
assert isinstance(lv.node, Var)
if (lv.node.final_unset_in_class and not lv.node.final_set_in_init and
not self.is_stub and # It is OK to skip initializer in stub files.
# Avoid extra error messages, if there is no type in Final[...],
# then we already reported the error about missing r.h.s.
isinstance(s, AssignmentStmt) and s.type is not None):
self.msg.final_without_value(s)
for lv in lvs:
if isinstance(lv, RefExpr) and isinstance(lv.node, Var):
name = lv.node.name()
cls = self.scope.active_class()
if cls is not None:
# Theses additional checks exist to give more error messages
# even if the final attribute was overridden with a new symbol
# (which is itself an error)...
for base in cls.mro[1:]:
sym = base.names.get(name)
# We only give this error if base node is variable,
# overriding final method will be caught in
# `check_compatibility_final_super()`.
if sym and isinstance(sym.node, Var):
if sym.node.is_final and not is_final_decl:
self.msg.cant_assign_to_final(name, sym.node.info is None, s)
# ...but only once
break
if lv.node.is_final and not is_final_decl:
self.msg.cant_assign_to_final(name, lv.node.info is None, s)

def check_assignment_to_multiple_lvalues(self, lvalues: List[Lvalue], rvalue: Expression,
context: Context,
infer_lvalue_type: bool = True) -> None:
Expand Down Expand Up @@ -2520,7 +2668,12 @@ def visit_while_stmt(self, s: WhileStmt) -> None:
def visit_operator_assignment_stmt(self,
s: OperatorAssignmentStmt) -> None:
"""Type check an operator assignment statement, e.g. x += 1."""
lvalue_type = self.expr_checker.accept(s.lvalue)
if isinstance(s.lvalue, MemberExpr):
# Special case, some additional errors may be given for
# assignments to read-only or final attributes.
lvalue_type = self.expr_checker.visit_member_expr(s.lvalue, True)
else:
lvalue_type = self.expr_checker.accept(s.lvalue)
inplace, method = infer_operator_assignment_method(lvalue_type, s.op)
if inplace:
# There is __ifoo__, treat as x = x.__ifoo__(y)
Expand All @@ -2534,6 +2687,7 @@ def visit_operator_assignment_stmt(self,
expr.set_line(s)
self.check_assignment(lvalue=s.lvalue, rvalue=expr,
infer_lvalue_type=True, new_syntax=False)
self.check_final(s)

def visit_assert_stmt(self, s: AssertStmt) -> None:
self.expr_checker.accept(s.expr)
Expand Down
4 changes: 2 additions & 2 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -1609,10 +1609,10 @@ def apply_generic_arguments(self, callable: CallableType, types: Sequence[Option
"""Simple wrapper around mypy.applytype.apply_generic_arguments."""
return applytype.apply_generic_arguments(callable, types, self.msg, context)

def visit_member_expr(self, e: MemberExpr) -> Type:
def visit_member_expr(self, e: MemberExpr, is_lvalue: bool = False) -> Type:
"""Visit member expression (of form e.id)."""
self.chk.module_refs.update(extract_refexpr_names(e))
result = self.analyze_ordinary_member_access(e, False)
result = self.analyze_ordinary_member_access(e, is_lvalue)
return self.narrow_type_from_binder(e, result)

def analyze_ordinary_member_access(self, e: MemberExpr,
Expand Down
25 changes: 25 additions & 0 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ def analyze_member_var_access(name: str, itype: Instance, info: TypeInfo,

if isinstance(v, Var):
implicit = info[name].implicit

# An assignment to final attribute is always an error,
# independently of types.
if is_lvalue and not chk.get_final_context():
check_final_member(name, info, msg, node)

return analyze_var(name, v, itype, info, node, is_lvalue, msg,
original_type, builtin_type, not_ready_callback,
chk=chk, implicit=implicit)
Expand Down Expand Up @@ -304,6 +310,14 @@ def analyze_member_var_access(name: str, itype: Instance, info: TypeInfo,
return msg.has_no_attr(original_type, itype, name, node)


def check_final_member(name: str, info: TypeInfo, msg: MessageBuilder, ctx: Context) -> None:
"""Give an error if the name being assigned was declared as final."""
for base in info.mro:
sym = base.names.get(name)
if sym and isinstance(sym.node, (Var, FuncBase, Decorator)) and sym.node.is_final:
msg.cant_assign_to_final(name, attr_assign=True, ctx=ctx)


def analyze_descriptor_access(instance_type: Type, descriptor_type: Type,
builtin_type: Callable[[str], Instance],
msg: MessageBuilder,
Expand Down Expand Up @@ -535,6 +549,17 @@ def analyze_class_attribute_access(itype: Instance,
if isinstance(node.node, TypeInfo):
msg.fail(messages.CANNOT_ASSIGN_TO_TYPE, context)

# If a final attribute was declared on `self` in `__init__`, then it
# can't be accessed on the class object.
if node.implicit and isinstance(node.node, Var) and node.node.is_final:
msg.fail('Cannot access final instance '
'attribute "{}" on class object'.format(node.node.name()), context)

# An assignment to final attribute on class object is also always an error,
# independently of types.
if is_lvalue and not chk.get_final_context():
check_final_member(name, itype.type, msg, context)

if itype.type.is_enum and not (is_lvalue or is_decorated or is_method):
return itype

Expand Down
21 changes: 21 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,27 @@ def cant_assign_to_method(self, context: Context) -> None:
def cant_assign_to_classvar(self, name: str, context: Context) -> None:
self.fail('Cannot assign to class variable "%s" via instance' % name, context)

def final_cant_override_writable(self, name: str, ctx: Context) -> None:
self.fail('Cannot override writable attribute "{}" with a final one'.format(name), ctx)

def cant_override_final(self, name: str, base_name: str, ctx: Context) -> None:
self.fail('Cannot override final attribute "{}"'
' (previously declared in base class "{}")'.format(name, base_name), ctx)

def cant_assign_to_final(self, name: str, attr_assign: bool, ctx: Context) -> None:
ilevkivskyi marked this conversation as resolved.
Show resolved Hide resolved
"""Warn about a prohibited assignment to a final attribute.

Pass `attr_assign=True` if the assignment assigns to an attribute.
"""
kind = "attribute" if attr_assign else "name"
self.fail('Cannot assign to final {} "{}"'.format(kind, name), ctx)

def protocol_members_cant_be_final(self, ctx: Context) -> None:
self.fail("Protocol member cannot be final", ctx)

def final_without_value(self, ctx: Context) -> None:
self.fail("Final name must be initialized with a value", ctx)

def read_only_property(self, name: str, type: TypeInfo,
context: Context) -> None:
self.fail('Property "{}" defined in "{}" is read-only'.format(
Expand Down
Loading