-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Support type aliases in fine-grained incremental mode #4525
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
Changes from 31 commits
3eed2a5
3e895d6
8d42ca7
c0a6ed7
54289f0
0e365e8
8d88890
e43821f
2b25e91
d5306f2
fa6d426
d9b397e
d7adb15
3a4b12b
afa4967
d19455a
3984464
224d02d
108a40d
b3a0488
ee1fdc9
811be9c
f31e6af
edabb05
b3db8e0
7b2a13c
8b666bb
d1e0d07
2803764
9ad014c
6e1cc7a
3b9b19e
e5ab3eb
4832e37
b08aef8
35f4380
a027399
8f3cb32
e8e01c9
b259da4
1fb320c
6817ba5
b8893f0
ef7f3b9
86b1bfd
0a4a23e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,15 @@ | |
|
||
import os | ||
from abc import abstractmethod | ||
from collections import OrderedDict | ||
from collections import OrderedDict, defaultdict | ||
from typing import ( | ||
Any, TypeVar, List, Tuple, cast, Set, Dict, Union, Optional, Callable, Sequence, | ||
Any, TypeVar, List, Tuple, cast, Set, Dict, Union, Optional, Callable, Sequence | ||
) | ||
|
||
MYPY = False | ||
if MYPY: | ||
from typing import DefaultDict | ||
|
||
import mypy.strconv | ||
from mypy.util import short_type | ||
from mypy.visitor import NodeVisitor, StatementVisitor, ExpressionVisitor | ||
|
@@ -194,6 +198,8 @@ class MypyFile(SymbolNode): | |
path = '' | ||
# Top-level definitions and statements | ||
defs = None # type: List[Statement] | ||
# Type alias dependencies as mapping from target to set of alias full names | ||
alias_deps = None # type: DefaultDict[str, Set[str]] | ||
# Is there a UTF-8 BOM at the start? | ||
is_bom = False | ||
names = None # type: SymbolTable | ||
|
@@ -215,6 +221,7 @@ def __init__(self, | |
self.line = 1 # Dummy line number | ||
self.imports = imports | ||
self.is_bom = is_bom | ||
self.alias_deps = defaultdict(set) | ||
if ignored_lines: | ||
self.ignored_lines = ignored_lines | ||
else: | ||
|
@@ -2334,6 +2341,11 @@ class SymbolTableNode: | |
normalized = False # type: bool | ||
# Was this defined by assignment to self attribute? | ||
implicit = False # type: bool | ||
# Is this node refers to other node via node aliasing? | ||
# (This is currently used for simple aliases like `A = int` instead of .type_override) | ||
is_aliasing = False # type: bool | ||
# This includes full names of aliases used in this alias. | ||
alias_depends_on = None # type: Set[str] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this actually is necessary. Even without this, the target that contains the alias definition should depend on these aliases, right? And change in the alias should be detected by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally this shouldn't be necessary. But IIRC correctly some tests failed without this. I added a TODO about this in |
||
|
||
def __init__(self, | ||
kind: int, | ||
|
@@ -2352,6 +2364,7 @@ def __init__(self, | |
self.alias_tvars = alias_tvars | ||
self.implicit = implicit | ||
self.module_hidden = module_hidden | ||
self.alias_depends_on = set() | ||
|
||
@property | ||
def fullname(self) -> Optional[str]: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
"""Track current scope to easily calculate the corresponding fine-grained target. | ||
|
||
This is currently only used by mypy.semanal and mypy.server.deps. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment can easily become stale. What about rewording as "TODO: Use everywhere where we track targets, including in mypy.errors". |
||
""" | ||
|
||
from typing import List, Optional | ||
|
||
from mypy.nodes import TypeInfo, FuncItem | ||
|
||
|
||
class Scope: | ||
"""Track which target we are processing at any given time.""" | ||
|
||
def __init__(self) -> None: | ||
self.module = None # type: Optional[str] | ||
self.classes = [] # type: List[TypeInfo] | ||
self.function = None # type: Optional[FuncItem] | ||
# Number of nested scopes ignored (that don't get their own separate targets) | ||
self.ignored = 0 | ||
|
||
def current_module_id(self) -> str: | ||
assert self.module | ||
return self.module | ||
|
||
def current_target(self) -> str: | ||
"""Return the current target (non-class; for a class return enclosing module).""" | ||
assert self.module | ||
target = self.module | ||
if self.function: | ||
if self.classes: | ||
target += '.' + '.'.join(c.name() for c in self.classes) | ||
target += '.' + self.function.name() | ||
return target | ||
|
||
def current_full_target(self) -> str: | ||
"""Return the current target (may be a class).""" | ||
assert self.module | ||
target = self.module | ||
if self.classes: | ||
target += '.' + '.'.join(c.name() for c in self.classes) | ||
if self.function: | ||
target += '.' + self.function.name() | ||
return target | ||
|
||
def enter_file(self, prefix: str) -> None: | ||
self.module = prefix | ||
self.classes = [] | ||
self.function = None | ||
self.ignored = 0 | ||
|
||
def enter_function(self, fdef: FuncItem) -> None: | ||
if not self.function: | ||
self.function = fdef | ||
else: | ||
# Nested functions are part of the topmost function target. | ||
self.ignored += 1 | ||
|
||
def enter_class(self, info: TypeInfo) -> None: | ||
"""Enter a class target scope.""" | ||
if not self.function: | ||
self.classes.append(info) | ||
else: | ||
# Classes within functions are part of the enclosing function target. | ||
self.ignored += 1 | ||
|
||
def leave(self) -> None: | ||
"""Leave the innermost scope (can be any kind of scope).""" | ||
if self.ignored: | ||
# Leave a scope that's included in the enclosing target. | ||
self.ignored -= 1 | ||
elif self.function: | ||
# Function is always the innermost target. | ||
self.function = None | ||
elif self.classes: | ||
# Leave the innermost class. | ||
self.classes.pop() | ||
else: | ||
# Leave module. | ||
assert self.module | ||
self.module = None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,7 @@ | |
from mypy import join | ||
from mypy.util import get_prefix, correct_relative_import | ||
from mypy.semanal_shared import PRIORITY_FALLBACKS | ||
from mypy.scope import Scope | ||
|
||
|
||
T = TypeVar('T') | ||
|
@@ -255,6 +256,7 @@ def __init__(self, | |
# 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 | ||
self.scope = Scope() | ||
|
||
def visit_file(self, file_node: MypyFile, fnam: str, options: Options, | ||
patches: List[Tuple[int, Callable[[], None]]]) -> None: | ||
|
@@ -287,8 +289,10 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, | |
v.is_ready = True | ||
|
||
defs = file_node.defs | ||
self.scope.enter_file(file_node.fullname()) | ||
for d in defs: | ||
self.accept(d) | ||
self.scope.leave() | ||
|
||
if self.cur_mod_id == 'builtins': | ||
remove_imported_names_from_symtable(self.globals, 'builtins') | ||
|
@@ -305,11 +309,13 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, | |
|
||
def refresh_partial(self, node: Union[MypyFile, FuncItem, OverloadedFuncDef]) -> None: | ||
"""Refresh a stale target in fine-grained incremental mode.""" | ||
self.scope.enter_file(self.cur_mod_id) | ||
if isinstance(node, MypyFile): | ||
self.refresh_top_level(node) | ||
else: | ||
self.recurse_into_functions = True | ||
self.accept(node) | ||
self.scope.leave() | ||
|
||
def refresh_top_level(self, file_node: MypyFile) -> None: | ||
"""Reanalyze a stale module top-level in fine-grained incremental mode.""" | ||
|
@@ -591,15 +597,19 @@ def analyze_property_with_multi_part_definition(self, defn: OverloadedFuncDef) - | |
|
||
def analyze_function(self, defn: FuncItem) -> None: | ||
is_method = self.is_class_scope() | ||
self.scope.enter_function(defn) | ||
with self.tvar_scope_frame(self.tvar_scope.method_frame()): | ||
if defn.type: | ||
self.check_classvar_in_signature(defn.type) | ||
assert isinstance(defn.type, CallableType) | ||
# Signature must be analyzed in the surrounding scope so that | ||
# class-level imported names and type variables are in scope. | ||
defn.type = self.type_analyzer().visit_callable_type(defn.type, nested=False) | ||
analyzer = self.type_analyzer() | ||
defn.type = analyzer.visit_callable_type(defn.type, nested=False) | ||
self.add_type_alias_deps(analyzer.aliases_used) | ||
self.check_function_signature(defn) | ||
if isinstance(defn, FuncDef): | ||
assert isinstance(defn.type, CallableType) | ||
defn.type = set_callable_name(defn.type, defn) | ||
for arg in defn.arguments: | ||
if arg.initializer: | ||
|
@@ -633,6 +643,7 @@ def analyze_function(self, defn: FuncItem) -> None: | |
|
||
self.leave() | ||
self.function_stack.pop() | ||
self.scope.leave() | ||
|
||
def check_classvar_in_signature(self, typ: Type) -> None: | ||
if isinstance(typ, Overloaded): | ||
|
@@ -660,10 +671,12 @@ def check_function_signature(self, fdef: FuncItem) -> None: | |
self.fail('Type signature has too many arguments', fdef, blocker=True) | ||
|
||
def visit_class_def(self, defn: ClassDef) -> None: | ||
self.scope.enter_class(defn.info) | ||
with self.analyze_class_body(defn) as should_continue: | ||
if should_continue: | ||
# Analyze class body. | ||
defn.defs.accept(self) | ||
self.scope.leave() | ||
|
||
@contextmanager | ||
def analyze_class_body(self, defn: ClassDef) -> Iterator[bool]: | ||
|
@@ -1679,7 +1692,24 @@ def anal_type(self, t: Type, *, | |
aliasing=aliasing, | ||
allow_tuple_literal=allow_tuple_literal, | ||
third_pass=third_pass) | ||
return t.accept(a) | ||
typ = t.accept(a) | ||
self.add_type_alias_deps(a.aliases_used) | ||
return typ | ||
|
||
def add_type_alias_deps(self, aliases_used: Iterable[str], | ||
target: Optional[str] = None) -> None: | ||
"""Add full names of type aliases on which the current node depends. | ||
|
||
This is used by fine-grained incremental mode to re-check the corresponding nodes. | ||
If `target` is None, then the target node used will be the current scope. | ||
""" | ||
if not aliases_used: | ||
# A basic optimization to avoid adding targets with no dependencies to | ||
# the `alias_deps` dict. | ||
return | ||
if target is None: | ||
target = self.scope.current_full_target() | ||
self.cur_mod_node.alias_deps[target].update(aliases_used) | ||
|
||
def visit_assignment_stmt(self, s: AssignmentStmt) -> None: | ||
for lval in s.lvalues: | ||
|
@@ -1755,10 +1785,16 @@ def alias_fallback(self, tp: Type) -> Instance: | |
return Instance(fb_info, []) | ||
|
||
def analyze_alias(self, rvalue: Expression, | ||
warn_bound_tvar: bool = False) -> Tuple[Optional[Type], List[str]]: | ||
warn_bound_tvar: bool = False) -> Tuple[Optional[Type], List[str], | ||
Set[str], List[str]]: | ||
"""Check if 'rvalue' represents a valid type allowed for aliasing | ||
(e.g. not a type variable). If yes, return the corresponding type and a list of | ||
qualified type variable names for generic aliases. | ||
(e.g. not a type variable). If yes, return the corresponding type, a list of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: Use an empty line after the first line in a multi-line docstring. |
||
qualified type variable names for generic aliases, a set of names the alias depends on, | ||
and a list of type variables if the alias is generic. | ||
An schematic example for the dependencies: | ||
A = int | ||
B = str | ||
analyze_alias(Dict[A, B])[2] == {'__main__.A', '__main__.B'} | ||
""" | ||
dynamic = bool(self.function_stack and self.function_stack[-1].is_dynamic()) | ||
global_scope = not self.type and not self.function_stack | ||
|
@@ -1775,12 +1811,17 @@ def analyze_alias(self, rvalue: Expression, | |
in_dynamic_func=dynamic, | ||
global_scope=global_scope, | ||
warn_bound_tvar=warn_bound_tvar) | ||
typ = None # type: Optional[Type] | ||
if res: | ||
alias_tvars = [name for (name, _) in | ||
res.accept(TypeVariableQuery(self.lookup_qualified, self.tvar_scope))] | ||
typ, depends_on = res | ||
found_type_vars = typ.accept(TypeVariableQuery(self.lookup_qualified, self.tvar_scope)) | ||
alias_tvars = [name for (name, node) in found_type_vars] | ||
qualified_tvars = [node.fullname() for (name, node) in found_type_vars] | ||
else: | ||
alias_tvars = [] | ||
return res, alias_tvars | ||
depends_on = set() | ||
qualified_tvars = [] | ||
return typ, alias_tvars, depends_on, qualified_tvars | ||
|
||
def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: | ||
"""Check if assignment creates a type alias and set it up as needed. | ||
|
@@ -1809,11 +1850,19 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: | |
# annotations (see the second rule). | ||
return | ||
rvalue = s.rvalue | ||
res, alias_tvars = self.analyze_alias(rvalue, warn_bound_tvar=True) | ||
res, alias_tvars, depends_on, qualified_tvars = self.analyze_alias(rvalue, | ||
warn_bound_tvar=True) | ||
if not res: | ||
return | ||
node = self.lookup(lvalue.name, lvalue) | ||
assert node is not None | ||
node.alias_depends_on = depends_on.copy() | ||
if lvalue.fullname is not None: | ||
# To avoid extra attributes on SymbolTableNode we add the fullname | ||
# of alias to what it depends on. | ||
node.alias_depends_on.add(lvalue.fullname) | ||
self.add_type_alias_deps(depends_on) | ||
self.add_type_alias_deps(qualified_tvars, target='<%s>' % lvalue.fullname) | ||
if not lvalue.is_inferred_def: | ||
# Type aliases can't be re-defined. | ||
if node and (node.kind == TYPE_ALIAS or isinstance(node.node, TypeInfo)): | ||
|
@@ -1830,6 +1879,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None: | |
# For simple (on-generic) aliases we use aliasing TypeInfo's | ||
# to allow using them in runtime context where it makes sense. | ||
node.node = res.type | ||
node.is_aliasing = True | ||
if isinstance(rvalue, RefExpr): | ||
sym = self.lookup_type_node(rvalue) | ||
if sym: | ||
|
@@ -3439,12 +3489,15 @@ def visit_index_expr(self, expr: IndexExpr) -> None: | |
elif isinstance(expr.base, RefExpr) and expr.base.kind == TYPE_ALIAS: | ||
# Special form -- subscripting a generic type alias. | ||
# Perform the type substitution and create a new alias. | ||
res, alias_tvars = self.analyze_alias(expr) | ||
res, alias_tvars, depends_on, _ = self.analyze_alias(expr) | ||
assert res is not None, "Failed analyzing already defined alias" | ||
expr.analyzed = TypeAliasExpr(res, alias_tvars, fallback=self.alias_fallback(res), | ||
in_runtime=True) | ||
expr.analyzed.line = expr.line | ||
expr.analyzed.column = expr.column | ||
# We also store fine-grained dependencies to correctly re-process nodes | ||
# with situations like `L = LongGeneric; x = L[int]()`. | ||
self.add_type_alias_deps(depends_on) | ||
elif refers_to_class_or_function(expr.base): | ||
# Special form -- type application. | ||
# Translate index to an unanalyzed type. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add docstring for
alias_depends_on
.There are starting to be quite a few alias-related attributes in symbol table nodes. Maybe it's time to add a new AST node for type aliases soon after this PR has been merged so that we can remove several of these attributes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will take care of it as part of #4082