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

Fine-grained: Don't infer partial types from multiple targets #4553

Merged
merged 13 commits into from
Feb 20, 2018
100 changes: 86 additions & 14 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@
('is_upper_bound', bool), # False => precise type
])

# Keeps track of partial types in a single scope. In fine-grained incremental
# mode partial types initially defined at the top level cannot be completed in
# a function, and we use the 'is_function' attribute to enforce this.
PartialTypeScope = NamedTuple('PartialTypeScope', [('map', Dict[Var, Context]),
('is_function', bool)])


class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
"""Mypy type checker.
Expand Down Expand Up @@ -136,7 +142,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
# Flags; true for dynamically typed functions
dynamic_funcs = None # type: List[bool]
# Stack of collections of variables with partial types
partial_types = None # type: List[Dict[Var, Context]]
partial_types = None # type: List[PartialTypeScope]
# Vars for which partial type errors are already reported
# (to avoid logically duplicate errors with different error context).
partial_reported = None # type: Set[Var]
Expand Down Expand Up @@ -632,7 +638,7 @@ def check_func_item(self, defn: FuncItem,
self.dynamic_funcs.append(defn.is_dynamic() and not type_override)

with self.errors.enter_function(fdef.name()) if fdef else nothing():
with self.enter_partial_types():
with self.enter_partial_types(is_function=True):
typ = self.function_type(defn)
if type_override:
typ = type_override.copy_modified(line=typ.line, column=typ.column)
Expand Down Expand Up @@ -1244,7 +1250,7 @@ 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)
with self.errors.enter_type(defn.name), self.enter_partial_types():
with self.errors.enter_type(defn.name), self.enter_partial_types(is_class=True):
old_binder = self.binder
self.binder = ConditionalTypeBinder()
with self.binder.top_frame_context():
Expand Down Expand Up @@ -1487,7 +1493,7 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
lvalue_type.item.type.is_protocol)):
self.msg.concrete_only_assign(lvalue_type, rvalue)
return
if rvalue_type and infer_lvalue_type:
if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType):
self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)
elif index_lvalue:
self.check_indexed_assignment(index_lvalue, rvalue, lvalue)
Expand Down Expand Up @@ -1996,7 +2002,7 @@ def infer_partial_type(self, name: Var, lvalue: Lvalue, init_type: Type) -> bool
else:
return False
self.set_inferred_type(name, lvalue, partial_type)
self.partial_types[-1][name] = lvalue
self.partial_types[-1].map[name] = lvalue
return True

def set_inferred_type(self, var: Var, lvalue: Lvalue, type: Type) -> None:
Expand Down Expand Up @@ -3111,33 +3117,99 @@ def lookup_qualified(self, name: str) -> SymbolTableNode:
raise KeyError(msg.format(last, name))

@contextmanager
def enter_partial_types(self) -> Iterator[None]:
def enter_partial_types(self, *, is_function: bool = False,
is_class: bool = False) -> Iterator[None]:
"""Enter a new scope for collecting partial types.

Also report errors for variables which still have partial
Also report errors for (some) variables which still have partial
types, i.e. we couldn't infer a complete type.
"""
self.partial_types.append({})
self.partial_types.append(PartialTypeScope({}, is_function))
yield

partial_types = self.partial_types.pop()
partial_types, _ = self.partial_types.pop()
if not self.current_node_deferred:
for var, context in partial_types.items():
if isinstance(var.type, PartialType) and var.type.type is None:
# None partial type: assume variable is intended to have type None
# If we require local partial types, there are a few exceptions where
# we fall back to inferring just "None" as the type from a None initaliazer:
#
# 1. If all happens within a single function this is acceptable, since only
# the topmost function is a separate target in fine-grained incremental mode.
# We primarily want to avoid "splitting" partial types across targets.
#
# 2. A None initializer in the class body if the attribute is defined in a base
# class is fine, since the attribute is already defined and it's currently okay
# to vary the type of an attribute covariantly. The None type will still be
# checked for compatibility with base classes elsewhere. Without this exception
# mypy could require an annotation for an attribute that already has been
# declared in a base class, which would be bad.
allow_none = (not self.options.local_partial_types
or is_function
or (is_class and self.is_defined_in_base_class(var)))
if (allow_none
and isinstance(var.type, PartialType)
and var.type.type is None):
var.type = NoneTyp()
else:
if var not in self.partial_reported:
self.msg.need_annotation_for_var(var, context)
self.partial_reported.add(var)
# Give the variable an 'Any' type to avoid generating multiple errors
# from a single missing annotation.
var.type = AnyType(TypeOfAny.from_error)

def is_defined_in_base_class(self, var: Var) -> bool:
if var.info is not None:
for base in var.info.mro[1:]:
if base.get(var.name()) is not None:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we also check that it is not None in base class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I'm not sure what you mean by this. Can you give an example where this would make a difference?

Copy link
Member

Choose a reason for hiding this comment

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

I mean add ... and not isinstance(base.get(var.name()), NoneTyp)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

base.get() returns Optional[SymbolTableNode] so that exact check doesn't make sense. If you meant guarding agains a definition like x = None # type: None in a base class, I don't agree -- it should be treated as a valid definition. If you still don't agree, can you give me a test case where this would make a difference?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sorry, you are right, this is perfectly valid.

return True
if var.info.fallback_to_any:
return True
return False

def find_partial_types(self, var: Var) -> Optional[Dict[Var, Context]]:
for partial_types in reversed(self.partial_types):
if var in partial_types:
return partial_types
"""Look for an active partial type scope containing variable.

A scope is active if assignments in the current context can refine a partial
type originally defined in the scope. This is affected by the local_partial_types
configuration option.
"""
in_scope, partial_types = self.find_partial_types_in_all_scopes(var)
if in_scope:
return partial_types
return None

def find_partial_types_in_all_scopes(self, var: Var) -> Tuple[bool,
Optional[Dict[Var, Context]]]:
"""Look for partial type scope containing variable.

Return tuple (is the scope active, scope).
"""
active = self.partial_types
inactive = [] # type: List[PartialTypeScope]
if self.options.local_partial_types:
# All scopes within the outermost function are active. Scopes out of
# the outermost function are inactive to allow local reasoning (important
# for fine-grained incremental mode).
for i, t in enumerate(self.partial_types):
if t.is_function:
active = self.partial_types[i:]
inactive = self.partial_types[:i]
break
else:
# Not within a function -- only the innermost scope is in scope.
active = self.partial_types[-1:]
inactive = self.partial_types[:-1]
# First look within in-scope partial types.
for scope in reversed(active):
if var in scope.map:
return True, scope.map
# Then for out-of-scope partial types.
for scope in reversed(inactive):
if var in scope.map:
return False, scope.map
return False, None

def temp_node(self, t: Type, context: Optional[Context] = None) -> TempNode:
"""Create a temporary node with the given, fixed type."""
temp = TempNode(t)
Expand Down
12 changes: 8 additions & 4 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,20 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type:
# Variable reference.
result = self.analyze_var_ref(node, e)
if isinstance(result, PartialType):
if result.type is None:
in_scope, partial_types = self.chk.find_partial_types_in_all_scopes(node)
if result.type is None and in_scope:
# 'None' partial type. It has a well-defined type. In an lvalue context
# we want to preserve the knowledge of it being a partial type.
if not lvalue:
result = NoneTyp()
else:
partial_types = self.chk.find_partial_types(node)
if partial_types is not None and not self.chk.current_node_deferred:
context = partial_types[node]
self.msg.need_annotation_for_var(node, context)
if in_scope:
context = partial_types[node]
self.msg.need_annotation_for_var(node, context)
else:
# Defer the node -- we might get a better type in the outer scope
self.chk.handle_cannot_determine_type(node.name(), e)
result = AnyType(TypeOfAny.special_form)
elif isinstance(node, FuncDef):
# Reference to a global function.
Expand Down
5 changes: 4 additions & 1 deletion mypy/dmypy_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ def __init__(self, flags: List[str]) -> None:
options.cache_fine_grained = True # set this so that cache options match
else:
options.cache_dir = os.devnull
# Fine-grained incremental doesn't support general partial types
# (details in https://github.com/python/mypy/issues/4492)
options.local_partial_types = True

def serve(self) -> None:
"""Serve requests, synchronously (no thread or fork)."""
Expand Down Expand Up @@ -180,7 +183,7 @@ def cmd_stop(self) -> Dict[str, object]:
"""Stop daemon."""
return {}

last_sources = None
last_sources = None # type: List[mypy.build.BuildSource]

def cmd_check(self, files: Sequence[str]) -> Dict[str, object]:
"""Check a list of files."""
Expand Down
3 changes: 3 additions & 0 deletions mypy/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,9 @@ def add_invertible_flag(flag: str,
parser.add_argument('--dump-graph', action='store_true', help=argparse.SUPPRESS)
# --semantic-analysis-only does exactly that.
parser.add_argument('--semantic-analysis-only', action='store_true', help=argparse.SUPPRESS)
# --local-partial-types disallows partial types spanning module top level and a function
# (implicitly defined in fine-grained incremental mode)
parser.add_argument('--local-partial-types', action='store_true', help=argparse.SUPPRESS)
# deprecated options
parser.add_argument('--disallow-any', dest='special-opts:disallow_any',
help=argparse.SUPPRESS)
Expand Down
2 changes: 2 additions & 0 deletions mypy/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def __init__(self) -> None:
self.show_column_numbers = False # type: bool
self.dump_graph = False
self.dump_deps = False
# If True, partial types can't span a module top level and a function
self.local_partial_types = False

def __eq__(self, other: object) -> bool:
return self.__class__ == other.__class__ and self.__dict__ == other.__dict__
Expand Down
1 change: 1 addition & 0 deletions mypy/test/testdmypy.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def run_case_once(self, testcase: DataDrivenTestCase, incremental_step: int) ->
if 'fine-grained' in testcase.file:
server_options.append('--experimental')
options.fine_grained_incremental = True
options.local_partial_types = True
self.server = dmypy_server.Server(server_options) # TODO: Fix ugly API
self.server.options = options

Expand Down
1 change: 1 addition & 0 deletions mypy/test/testfinegrained.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def build(self,
options.fine_grained_incremental = not build_cache
options.use_fine_grained_cache = enable_cache and not build_cache
options.cache_fine_grained = enable_cache
options.local_partial_types = True

main_path = os.path.join(test_temp_dir, 'main')
with open(main_path, 'w') as f:
Expand Down
18 changes: 18 additions & 0 deletions test-data/unit/check-dmypy-fine-grained.test
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,21 @@ tmp/a.py:1: error: "int" not callable
[delete nonexistent_stub.pyi.2]
[out1]
[out2]

[case testPartialNoneTypeFineGrainedIncremental]
# cmd: mypy -m a b
[file a.py]
import b
b.y
x = None
def f() -> None:
global x
x = ''
[file b.py]
y = 0
[file b.py.2]
y = ''
[out1]
tmp/a.py:3: error: Need type annotation for 'x'
[out2]
tmp/a.py:3: error: Need type annotation for 'x'
Loading