From 5d31246e1bab9009417110aa25474bce25ba5d61 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 24 Feb 2022 13:22:42 +0000 Subject: [PATCH 01/19] Add two simple test cases --- test-data/unit/check-statements.test | 31 +++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 62d82f94a6c1..3fe117309205 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1555,7 +1555,6 @@ class LiteralReturn: return False [builtins fixtures/bool.pyi] - [case testWithStmtBoolExitReturnInStub] import stub @@ -1572,6 +1571,36 @@ class C3: def __exit__(self, x, y, z) -> Optional[bool]: pass [builtins fixtures/bool.pyi] +[case testWithStatementScopeSeparate] +class A: + def __enter__(self) -> A: pass + def __exit__(self, x, y, z) -> None: pass +class B: + def __enter__(self) -> B: pass + def __exit__(self, x, y, z) -> None: pass + +def f() -> None: + with A() as x: + reveal_type(x) # N: Revealed type is "__main__.A" + with B() as x: + reveal_type(x) # N: Revealed type is "__main__.B" + +[case testWithStatementScopeWidening] +class A: + def __enter__(self) -> A: pass + def __exit__(self, x, y, z) -> None: pass +class B: + def __enter__(self) -> B: pass + def __exit__(self, x, y, z) -> None: pass + +def f() -> None: + with A() as x: + reveal_type(x) # N: Revealed type is "__main__.A" + y = x # Use outside with makes the scope function-level + with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "__main__.A" + -- Chained assignment -- ------------------ From d87f624571a5c75b0e0c2a5142a575b2e09119fa Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 24 Feb 2022 14:28:03 +0000 Subject: [PATCH 02/19] WIP renaming in with statements --- mypy/build.py | 10 +- mypy/renaming.py | 141 +++++++++++++++++++++++++ test-data/unit/check-statements.test | 50 +++++---- test-data/unit/semanal-statements.test | 56 ++++++++++ 4 files changed, 235 insertions(+), 22 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 3ddbe40f1a90..898e50fc65e5 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -56,7 +56,7 @@ from mypy.fscache import FileSystemCache from mypy.metastore import MetadataStore, FilesystemMetadataStore, SqliteMetadataStore from mypy.typestate import TypeState, reset_global_state -from mypy.renaming import VariableRenameVisitor +from mypy.renaming import VariableRenameVisitor, VariableRenameVisitor2 from mypy.config_parser import parse_mypy_comments from mypy.freetree import free_tree from mypy.stubinfo import legacy_bundled_packages, is_legacy_bundled_package @@ -2119,9 +2119,11 @@ def semantic_analysis_pass1(self) -> None: analyzer.visit_file(self.tree, self.xpath, self.id, options) # TODO: Do this while constructing the AST? self.tree.names = SymbolTable() - if options.allow_redefinition: - # Perform renaming across the AST to allow variable redefinitions - self.tree.accept(VariableRenameVisitor()) + if not self.tree.is_stub: + if options.allow_redefinition: + # Perform renaming across the AST to allow variable redefinitions + self.tree.accept(VariableRenameVisitor()) + self.tree.accept(VariableRenameVisitor2()) def add_dependency(self, dep: str) -> None: if dep not in self.dependencies_set: diff --git a/mypy/renaming.py b/mypy/renaming.py index c200e94d58e7..c8a314ee954c 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -392,3 +392,144 @@ def record_assignment(self, name: str, can_be_redefined: bool) -> bool: else: # Assigns to an existing variable. return False + + +class VariableRenameVisitor2(TraverserVisitor): + def __init__(self) -> None: + self.bound_vars = set() + self.bad: List[Set[str]] = [] + self.refs: List[Dict[str, List[List[NameExpr]]]] = [] + + def visit_mypy_file(self, file_node: MypyFile) -> None: + """Rename variables within a file. + + This is the main entry point to this class. + """ + #self.clear() + with self.enter_scope(): + for d in file_node.defs: + d.accept(self) + + def visit_func_def(self, fdef: FuncDef) -> None: + self.reject_redefinition_of_vars_in_scope() + + with self.enter_scope(): + for arg in fdef.arguments: + self.record_bad(arg.variable.name) + + super().visit_func_def(fdef) + print('refs', self.refs) + print('bad', self.bad) + + #def visit_class_def(self, cdef: ClassDef) -> None: + # self.reject_redefinition_of_vars_in_scope() + # with self.enter_scope(): + # super().visit_class_def(cdef) + + def visit_with_stmt(self, stmt: WithStmt) -> None: + for expr in stmt.expr: + expr.accept(self) + names = [] + for target in stmt.target: + if target is not None: + assert isinstance(target, NameExpr) + name = target.name + d = self.refs[-1] + if name not in d: + d[name] = [] + d[name].append([]) + self.bound_vars.add(name) + names.append(name) + + #self.analyze_lvalue(target) + super().visit_with_stmt(stmt) + for name in names: + self.bound_vars.remove(name) + + #def visit_import(self, imp: Import) -> None: + # for id, as_id in imp.ids: + # self.record_bad(as_id or id) + + #def visit_import_from(self, imp: ImportFrom) -> None: + # for id, as_id in imp.names: + # self.record_bad(as_id or id, False) + + #def visit_match_stmt(self, s: MatchStmt) -> None: + # for i in range(len(s.patterns)): + # s.patterns[i].accept(self) + # guard = s.guards[i] + # if guard is not None: + # guard.accept(self) + # # We already entered a block, so visit this block's statements directly + # for stmt in s.bodies[i].body: + # stmt.accept(self) + + #def visit_capture_pattern(self, p: AsPattern) -> None: + # if p.name is not None: + # self.analyze_lvalue(p.name) + + def analyze_lvalue(self, lvalue: Lvalue, is_nested: bool = False) -> None: + if isinstance(lvalue, NameExpr): + name = lvalue.name + is_new = self.record_assignment(name, True) + if is_new: + self.handle_def(lvalue) + else: + self.handle_refine(lvalue) + if is_nested: + # This allows these to be redefined freely even if never read. Multiple + # assignment like "x, _ _ = y" defines dummy variables that are never read. + self.handle_ref(lvalue) + elif isinstance(lvalue, (ListExpr, TupleExpr)): + for item in lvalue.items: + self.analyze_lvalue(item, is_nested=True) + elif isinstance(lvalue, MemberExpr): + lvalue.expr.accept(self) + elif isinstance(lvalue, IndexExpr): + lvalue.base.accept(self) + lvalue.index.accept(self) + elif isinstance(lvalue, StarExpr): + # Propagate is_nested since in a typical use case like "x, *rest = ..." 'rest' may + # be freely reused. + self.analyze_lvalue(lvalue.expr, is_nested=is_nested) + + def visit_name_expr(self, expr: NameExpr) -> None: + name = expr.name + if name in self.bound_vars: + # record ref + self.refs[-1][name][-1].append(expr) + else: + self.record_bad(name) + + @contextmanager + def enter_scope(self) -> Iterator[None]: + self.bad.append(set()) + self.refs.append({}) + yield None + self.flush_refs() + self.bad.pop() + + def reject_redefinition_of_vars_in_scope(self) -> None: + # TODO + pass + + def record_bad(self, name: str) -> None: + self.bad[-1].add(name) + + def flush_refs(self) -> None: + for name, refs in self.refs[-1].items(): + if len(refs) <= 1 or name in self.bad[-1]: + continue + # At module top level, don't rename the final definition, + # as it may be publicly visible. + to_rename = refs[:-1] + for i, item in enumerate(to_rename): + self.rename_refs(item, i) + self.refs.pop() + + def rename_refs(self, names: List[NameExpr], index: int) -> None: + name = names[0].name + print('renaming', name) + new_name = name + "'" * (index + 1) + for expr in names: + expr.name = new_name diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 3fe117309205..0901b366bce6 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1571,35 +1571,49 @@ class C3: def __exit__(self, x, y, z) -> Optional[bool]: pass [builtins fixtures/bool.pyi] -[case testWithStatementScopeSeparate] -class A: - def __enter__(self) -> A: pass - def __exit__(self, x, y, z) -> None: pass -class B: - def __enter__(self) -> B: pass - def __exit__(self, x, y, z) -> None: pass +[case testWithStatementScope1] +from m import A, B -def f() -> None: +def f1() -> None: with A() as x: - reveal_type(x) # N: Revealed type is "__main__.A" + reveal_type(x) # N: Revealed type is "m.A" with B() as x: - reveal_type(x) # N: Revealed type is "__main__.B" + reveal_type(x) # N: Revealed type is "m.B" -[case testWithStatementScopeWidening] +def f2() -> None: + with A() as x: + reveal_type(x) # N: Revealed type is "m.A" + y = x # Use outside with makes the scope function-level + with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +[file m.pyi] class A: - def __enter__(self) -> A: pass - def __exit__(self, x, y, z) -> None: pass + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... class B: - def __enter__(self) -> B: pass - def __exit__(self, x, y, z) -> None: pass + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... -def f() -> None: +[case testWithStatementScope2] +from m import A, B + +def f2() -> None: with A() as x: - reveal_type(x) # N: Revealed type is "__main__.A" + reveal_type(x) # N: Revealed type is "m.A" y = x # Use outside with makes the scope function-level with B() as x: \ # E: Incompatible types in assignment (expression has type "B", variable has type "A") - reveal_type(x) # N: Revealed type is "__main__.A" + reveal_type(x) # N: Revealed type is "m.A" + +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... -- Chained assignment diff --git a/test-data/unit/semanal-statements.test b/test-data/unit/semanal-statements.test index 97dd5f048834..fdc5ca2bbbdd 100644 --- a/test-data/unit/semanal-statements.test +++ b/test-data/unit/semanal-statements.test @@ -1058,3 +1058,59 @@ MypyFile:1( AssignmentStmt:6( NameExpr(x [__main__.x]) StrExpr())) + +[case testSimpleWithRenaming] +with 0 as y: + z = y +with 1 as y: + y = 1 +[out] +MypyFile:1( + WithStmt:1( + Expr( + IntExpr(0)) + Target( + NameExpr(y'* [__main__.y'])) + Block:1( + AssignmentStmt:2( + NameExpr(z* [__main__.z]) + NameExpr(y' [__main__.y'])))) + WithStmt:3( + Expr( + IntExpr(1)) + Target( + NameExpr(y* [__main__.y])) + Block:3( + AssignmentStmt:4( + NameExpr(y [__main__.y]) + IntExpr(1))))) + +[case testSimpleWithRenamingFailure] +with 0 as y: + z = y +zz = y +with 1 as y: + y = 1 +[out] +MypyFile:1( + WithStmt:1( + Expr( + IntExpr(0)) + Target( + NameExpr(y* [__main__.y])) + Block:1( + AssignmentStmt:2( + NameExpr(z* [__main__.z]) + NameExpr(y [__main__.y])))) + AssignmentStmt:3( + NameExpr(zz* [__main__.zz]) + NameExpr(y [__main__.y])) + WithStmt:4( + Expr( + IntExpr(1)) + Target( + NameExpr(y [__main__.y])) + Block:4( + AssignmentStmt:5( + NameExpr(y [__main__.y]) + IntExpr(1))))) From 5509e632362984d359fd16802ade093201704192 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 24 Feb 2022 14:37:25 +0000 Subject: [PATCH 03/19] WIP handle functions --- mypy/renaming.py | 20 +++---- test-data/unit/check-statements.test | 84 ++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 10 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index c8a314ee954c..86d4f4d36777 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -510,21 +510,21 @@ def enter_scope(self) -> Iterator[None]: self.bad.pop() def reject_redefinition_of_vars_in_scope(self) -> None: - # TODO - pass + self.record_bad('*') def record_bad(self, name: str) -> None: self.bad[-1].add(name) def flush_refs(self) -> None: - for name, refs in self.refs[-1].items(): - if len(refs) <= 1 or name in self.bad[-1]: - continue - # At module top level, don't rename the final definition, - # as it may be publicly visible. - to_rename = refs[:-1] - for i, item in enumerate(to_rename): - self.rename_refs(item, i) + if '*' not in self.bad[-1]: + for name, refs in self.refs[-1].items(): + if len(refs) <= 1 or name in self.bad[-1]: + continue + # At module top level, don't rename the final definition, + # as it may be publicly visible. + to_rename = refs[:-1] + for i, item in enumerate(to_rename): + self.rename_refs(item, i) self.refs.pop() def rename_refs(self, names: List[NameExpr], index: int) -> None: diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 0901b366bce6..ec64b905ba28 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1615,6 +1615,90 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... +[case testWithStatementScope3] +from m import A, B + +with A() as x: + reveal_type(x) # N: Revealed type is "m.A" + +def f() -> None: + pass # Don't support function definition in the middle + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope4] +from m import A, B + +def f() -> None: + pass # function before with is unsupported + +with A() as x: + reveal_type(x) # N: Revealed type is "m.A" + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope5] +from m import A, B + +with A() as x: + reveal_type(x) # N: Revealed type is "m.A" + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +def f() -> None: + pass # function after with is unsupported + +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope6] +from m import A, B + +with A() as x: + def f() -> None: + pass # Function within with is unsupported + + reveal_type(x) # N: Revealed type is "m.A" + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + -- Chained assignment -- ------------------ From 93bdc1c1a73ca73a608147f81b312b5382bd3b25 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 11:43:39 +0000 Subject: [PATCH 04/19] WIP fix import statements --- mypy/renaming.py | 12 ++++---- test-data/unit/check-statements.test | 45 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index 86d4f4d36777..c29d9fcf9954 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -446,13 +446,13 @@ def visit_with_stmt(self, stmt: WithStmt) -> None: for name in names: self.bound_vars.remove(name) - #def visit_import(self, imp: Import) -> None: - # for id, as_id in imp.ids: - # self.record_bad(as_id or id) + def visit_import(self, imp: Import) -> None: + for id, as_id in imp.ids: + self.record_bad(as_id or id) - #def visit_import_from(self, imp: ImportFrom) -> None: - # for id, as_id in imp.names: - # self.record_bad(as_id or id, False) + def visit_import_from(self, imp: ImportFrom) -> None: + for id, as_id in imp.names: + self.record_bad(as_id or id) #def visit_match_stmt(self, s: MatchStmt) -> None: # for i in range(len(s.patterns)): diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index ec64b905ba28..5af15adc14a1 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1699,6 +1699,51 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... +[case testWithStatementScope7Imports] +from m import A, B, x + +with A() as x: \ + # E: Incompatible types in assignment (expression has type "A", variable has type "B") + reveal_type(x) # N: Revealed type is "m.B" + +with B() as x: + reveal_type(x) # N: Revealed type is "m.B" + +[file m.pyi] +from typing import Any + +x: B + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope8Imports] +from m import A, B +import m as x + +with A() as x: \ + # E: Incompatible types in assignment (expression has type "A", variable has type Module) + pass + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type Module) + pass + +[file m.pyi] +from typing import Any + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... +[builtins fixtures/module.pyi] + -- Chained assignment -- ------------------ From d3b942276a988a56f6b396219cc381afb6ae49ad Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 11:50:31 +0000 Subject: [PATCH 05/19] WIP support import * --- mypy/renaming.py | 5 ++++- test-data/unit/check-statements.test | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index c29d9fcf9954..d37bd8bb24be 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -5,7 +5,7 @@ from mypy.nodes import ( Block, AssignmentStmt, NameExpr, MypyFile, FuncDef, Lvalue, ListExpr, TupleExpr, WhileStmt, ForStmt, BreakStmt, ContinueStmt, TryStmt, WithStmt, MatchStmt, StarExpr, - ImportFrom, MemberExpr, IndexExpr, Import, ClassDef + ImportFrom, MemberExpr, IndexExpr, Import, ImportAll, ClassDef ) from mypy.patterns import AsPattern from mypy.traverser import TraverserVisitor @@ -454,6 +454,9 @@ def visit_import_from(self, imp: ImportFrom) -> None: for id, as_id in imp.names: self.record_bad(as_id or id) + def visit_import_all(self, imp: ImportAll) -> None: + self.record_bad('*') + #def visit_match_stmt(self, s: MatchStmt) -> None: # for i in range(len(s.patterns)): # s.patterns[i].accept(self) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 5af15adc14a1..b00aaadaf301 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1744,6 +1744,28 @@ class B: def __exit__(self, x, y, z) -> None: ... [builtins fixtures/module.pyi] +[case testWithStatementScope9ImportStar] +from m import A, B +from m import * + +with A() as x: + pass + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +[file m.pyi] +from typing import Any + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... +[builtins fixtures/module.pyi] + -- Chained assignment -- ------------------ From c81b14eb879cae17f24ee1d836f5802e4c836782 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 11:58:18 +0000 Subject: [PATCH 06/19] Fix nested with statements --- mypy/renaming.py | 30 ++++++++++++------- test-data/unit/check-statements.test | 43 +++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index d37bd8bb24be..4aa9f7636be2 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -396,7 +396,7 @@ def record_assignment(self, name: str, can_be_redefined: bool) -> bool: class VariableRenameVisitor2(TraverserVisitor): def __init__(self) -> None: - self.bound_vars = set() + self.bound_vars: List[str] = [] self.bad: List[Set[str]] = [] self.refs: List[Dict[str, List[List[NameExpr]]]] = [] @@ -429,22 +429,30 @@ def visit_func_def(self, fdef: FuncDef) -> None: def visit_with_stmt(self, stmt: WithStmt) -> None: for expr in stmt.expr: expr.accept(self) - names = [] + n = 0 for target in stmt.target: if target is not None: assert isinstance(target, NameExpr) name = target.name - d = self.refs[-1] - if name not in d: - d[name] = [] - d[name].append([]) - self.bound_vars.add(name) - names.append(name) + if name in self.bound_vars: + self.visit_name_expr(target) + else: + d = self.refs[-1] + if name not in d: + d[name] = [] + d[name].append([]) + self.bound_vars.append(name) + n += 1 #self.analyze_lvalue(target) - super().visit_with_stmt(stmt) - for name in names: - self.bound_vars.remove(name) + + for target in stmt.target: + if target: + target.accept(self) + stmt.body.accept(self) + + for _ in range(n): + self.bound_vars.pop() def visit_import(self, imp: Import) -> None: for id, as_id in imp.ids: diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index b00aaadaf301..3db5facf0d20 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1764,7 +1764,48 @@ class A: class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[builtins fixtures/module.pyi] + +[case testWithStatementScope10NestedWith] +from m import A, B + +with A() as x: + with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + reveal_type(x) # N: Revealed type is "m.A" + +with B() as x: + with A() as x: \ + # E: Incompatible types in assignment (expression has type "A", variable has type "B") + reveal_type(x) # N: Revealed type is "m.B" + +[file m.pyi] +from typing import Any + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope11NestedWith] +from m import A, B + +with A() as x: + with A() as y: + reveal_type(y) # N: Revealed type is "m.A" + with B() as y: + reveal_type(y) # N: Revealed type is "m.B" + +[file m.pyi] +from typing import Any + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... -- Chained assignment From 9c9f1458ab89a8a057374e4507f9ae4182a75ae5 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 13:29:12 +0000 Subject: [PATCH 07/19] Add test case --- test-data/unit/check-statements.test | 29 ++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 3db5facf0d20..c82481eadf91 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1800,6 +1800,35 @@ with A() as x: [file m.pyi] from typing import Any +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope12Outer] +from m import A, B + +x = A() # Outer scope should have no impact + +with A() as x: + pass + +def f() -> None: + with A() as x: + reveal_type(x) # N: Revealed type is "m.A" + with B() as x: + reveal_type(x) # N: Revealed type is "m.B" + +y = x + +with A() as x: + pass + +[file m.pyi] +from typing import Any + class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... From c60f4ef0171bc04700f4f110b6040f4af21c2f1d Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 13:32:17 +0000 Subject: [PATCH 08/19] Add test case --- test-data/unit/check-statements.test | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index c82481eadf91..2abb1d51033a 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1829,6 +1829,26 @@ with A() as x: [file m.pyi] from typing import Any +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope13] +from m import A, B + +with A() as x, B() as y: + reveal_type(x) # N: Revealed type is "m.A" + reveal_type(y) # N: Revealed type is "m.B" +with B() as x, A() as y: + reveal_type(x) # N: Revealed type is "m.B" + reveal_type(y) # N: Revealed type is "m.A" + +[file m.pyi] +from typing import Any + class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... From a615a196b432afc9c3c94dad7ff7dbeb24838fdc Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 13:43:12 +0000 Subject: [PATCH 09/19] Support multiple assignment --- mypy/renaming.py | 42 +++++++++------------------- test-data/unit/check-statements.test | 21 ++++++++++++++ 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index 4aa9f7636be2..d6cfd4212449 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -429,29 +429,16 @@ def visit_func_def(self, fdef: FuncDef) -> None: def visit_with_stmt(self, stmt: WithStmt) -> None: for expr in stmt.expr: expr.accept(self) - n = 0 + n = len(self.bound_vars) for target in stmt.target: if target is not None: - assert isinstance(target, NameExpr) - name = target.name - if name in self.bound_vars: - self.visit_name_expr(target) - else: - d = self.refs[-1] - if name not in d: - d[name] = [] - d[name].append([]) - self.bound_vars.append(name) - n += 1 - - #self.analyze_lvalue(target) - + self.analyze_lvalue(target) for target in stmt.target: if target: target.accept(self) stmt.body.accept(self) - for _ in range(n): + while len(self.bound_vars) > n: self.bound_vars.pop() def visit_import(self, imp: Import) -> None: @@ -479,30 +466,27 @@ def visit_import_all(self, imp: ImportAll) -> None: # if p.name is not None: # self.analyze_lvalue(p.name) - def analyze_lvalue(self, lvalue: Lvalue, is_nested: bool = False) -> None: + def analyze_lvalue(self, lvalue: Lvalue) -> None: if isinstance(lvalue, NameExpr): name = lvalue.name - is_new = self.record_assignment(name, True) - if is_new: - self.handle_def(lvalue) + if name in self.bound_vars: + self.visit_name_expr(lvalue) else: - self.handle_refine(lvalue) - if is_nested: - # This allows these to be redefined freely even if never read. Multiple - # assignment like "x, _ _ = y" defines dummy variables that are never read. - self.handle_ref(lvalue) + d = self.refs[-1] + if name not in d: + d[name] = [] + d[name].append([]) + self.bound_vars.append(name) elif isinstance(lvalue, (ListExpr, TupleExpr)): for item in lvalue.items: - self.analyze_lvalue(item, is_nested=True) + self.analyze_lvalue(item) elif isinstance(lvalue, MemberExpr): lvalue.expr.accept(self) elif isinstance(lvalue, IndexExpr): lvalue.base.accept(self) lvalue.index.accept(self) elif isinstance(lvalue, StarExpr): - # Propagate is_nested since in a typical use case like "x, *rest = ..." 'rest' may - # be freely reused. - self.analyze_lvalue(lvalue.expr, is_nested=is_nested) + self.analyze_lvalue(lvalue.expr) def visit_name_expr(self, expr: NameExpr) -> None: name = expr.name diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 2abb1d51033a..f1b9c907f3a4 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1856,6 +1856,27 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... +[case testWithStatementScope14] +from m import A, B + +with A() as (x, y): + reveal_type(x) # N: Revealed type is "m.A" + reveal_type(y) # N: Revealed type is "builtins.int" +with B() as [x, y]: + reveal_type(x) # N: Revealed type is "m.B" + reveal_type(y) # N: Revealed type is "builtins.str" + +[file m.pyi] +from typing import Any, Tuple + +class A: + def __enter__(self) -> Tuple[A, int]: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> Tuple[B, str]: ... + def __exit__(self, x, y, z) -> None: ... +[builtins fixtures/tuple.pyi] + -- Chained assignment -- ------------------ From 5b0a3d4110dc1f992498d8b628835e6ee2c49913 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 14:04:21 +0000 Subject: [PATCH 10/19] Add test case --- test-data/unit/check-statements.test | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index f1b9c907f3a4..cd99054d1c91 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1877,6 +1877,37 @@ class B: def __exit__(self, x, y, z) -> None: ... [builtins fixtures/tuple.pyi] +[case testWithStatementScope15] +from m import A, B, f + +with A() as x: + pass +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass +with B() as f(x).x: + pass + +with A() as y: + pass +with B() as y: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass +with B() as f(y)[0]: + pass + +[file m.pyi] +from typing import Any, Tuple + +def f(x): ... + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + -- Chained assignment -- ------------------ From b14b3bcfe1ddfc3e691c31d45b475c9a7affed4a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 14:08:33 +0000 Subject: [PATCH 11/19] Support classes --- mypy/renaming.py | 8 ++++---- test-data/unit/check-statements.test | 28 +++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index d6cfd4212449..cd26d9374f53 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -421,10 +421,10 @@ def visit_func_def(self, fdef: FuncDef) -> None: print('refs', self.refs) print('bad', self.bad) - #def visit_class_def(self, cdef: ClassDef) -> None: - # self.reject_redefinition_of_vars_in_scope() - # with self.enter_scope(): - # super().visit_class_def(cdef) + def visit_class_def(self, cdef: ClassDef) -> None: + self.reject_redefinition_of_vars_in_scope() + with self.enter_scope(): + super().visit_class_def(cdef) def visit_with_stmt(self, stmt: WithStmt) -> None: for expr in stmt.expr: diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index cd99054d1c91..e6418acc0b4b 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1897,10 +1897,36 @@ with B() as f(y)[0]: pass [file m.pyi] -from typing import Any, Tuple +from typing import Any def f(x): ... +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStatementScope16] +from m import A, B + +with A() as x: + pass + +class C: + with A() as y: + pass + with B() as y: + pass + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +[file m.pyi] +from typing import Any + class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... From cfc1017f5bdc05f4a74ce37d313b1db807931a1f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 14:33:37 +0000 Subject: [PATCH 12/19] Add match statement test case --- mypy/renaming.py | 14 ---------- test-data/unit/check-python310.test | 41 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index cd26d9374f53..f53869d0fd2c 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -452,20 +452,6 @@ def visit_import_from(self, imp: ImportFrom) -> None: def visit_import_all(self, imp: ImportAll) -> None: self.record_bad('*') - #def visit_match_stmt(self, s: MatchStmt) -> None: - # for i in range(len(s.patterns)): - # s.patterns[i].accept(self) - # guard = s.guards[i] - # if guard is not None: - # guard.accept(self) - # # We already entered a block, so visit this block's statements directly - # for stmt in s.bodies[i].body: - # stmt.accept(self) - - #def visit_capture_pattern(self, p: AsPattern) -> None: - # if p.name is not None: - # self.analyze_lvalue(p.name) - def analyze_lvalue(self, lvalue: Lvalue) -> None: if isinstance(lvalue, NameExpr): name = lvalue.name diff --git a/test-data/unit/check-python310.test b/test-data/unit/check-python310.test index 4c0324b9bec7..0d4f46d53924 100644 --- a/test-data/unit/check-python310.test +++ b/test-data/unit/check-python310.test @@ -1175,3 +1175,44 @@ def foo(value) -> int: # E: Missing return statement return 1 case 2: return 2 + +[case testWithStatementScopeAndMatchStatement] +from m import A, B + +with A() as x: + pass +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +with A() as y: + pass +with B() as y: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +with A() as z: + pass +with B() as z: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +with A() as zz: + pass +with B() as zz: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass + +match x: + case str(y) as z: + zz = y + +[file m.pyi] +from typing import Any + +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... From 3a8295991f3f7c03ba1306f1609d1ed91970aaa4 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 14:42:46 +0000 Subject: [PATCH 13/19] Cleanup --- mypy/renaming.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index f53869d0fd2c..a7d45f856970 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -405,21 +405,16 @@ def visit_mypy_file(self, file_node: MypyFile) -> None: This is the main entry point to this class. """ - #self.clear() with self.enter_scope(): for d in file_node.defs: d.accept(self) def visit_func_def(self, fdef: FuncDef) -> None: self.reject_redefinition_of_vars_in_scope() - with self.enter_scope(): for arg in fdef.arguments: self.record_bad(arg.variable.name) - super().visit_func_def(fdef) - print('refs', self.refs) - print('bad', self.bad) def visit_class_def(self, cdef: ClassDef) -> None: self.reject_redefinition_of_vars_in_scope() @@ -477,7 +472,7 @@ def analyze_lvalue(self, lvalue: Lvalue) -> None: def visit_name_expr(self, expr: NameExpr) -> None: name = expr.name if name in self.bound_vars: - # record ref + # Record reference so that it can be renamed later self.refs[-1][name][-1].append(expr) else: self.record_bad(name) @@ -501,7 +496,7 @@ def flush_refs(self) -> None: for name, refs in self.refs[-1].items(): if len(refs) <= 1 or name in self.bad[-1]: continue - # At module top level, don't rename the final definition, + # At module top level we must not rename the final definition, # as it may be publicly visible. to_rename = refs[:-1] for i, item in enumerate(to_rename): @@ -510,7 +505,6 @@ def flush_refs(self) -> None: def rename_refs(self, names: List[NameExpr], index: int) -> None: name = names[0].name - print('renaming', name) new_name = name + "'" * (index + 1) for expr in names: expr.name = new_name From b5e0e64241021633310c61de1fc4e8ac65145055 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 14:44:24 +0000 Subject: [PATCH 14/19] Refactor --- mypy/renaming.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index a7d45f856970..c0731f27b1d5 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -262,15 +262,9 @@ def flush_refs(self) -> None: # as it will be publicly visible outside the module. to_rename = refs[:-1] for i, item in enumerate(to_rename): - self.rename_refs(item, i) + rename_refs(item, i) self.refs.pop() - def rename_refs(self, names: List[NameExpr], index: int) -> None: - name = names[0].name - new_name = name + "'" * (index + 1) - for expr in names: - expr.name = new_name - # Helpers for determining which assignments define new variables def clear(self) -> None: @@ -500,11 +494,12 @@ def flush_refs(self) -> None: # as it may be publicly visible. to_rename = refs[:-1] for i, item in enumerate(to_rename): - self.rename_refs(item, i) + rename_refs(item, i) self.refs.pop() - def rename_refs(self, names: List[NameExpr], index: int) -> None: - name = names[0].name - new_name = name + "'" * (index + 1) - for expr in names: - expr.name = new_name + +def rename_refs(names: List[NameExpr], index: int) -> None: + name = names[0].name + new_name = name + "'" * (index + 1) + for expr in names: + expr.name = new_name From f7d627829d2bcd53b3bcb0357c719309a6ae88ec Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 16:11:47 +0000 Subject: [PATCH 15/19] Refactor --- mypy/build.py | 7 ++-- mypy/renaming.py | 86 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 26 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 898e50fc65e5..6513414dd298 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -56,7 +56,7 @@ from mypy.fscache import FileSystemCache from mypy.metastore import MetadataStore, FilesystemMetadataStore, SqliteMetadataStore from mypy.typestate import TypeState, reset_global_state -from mypy.renaming import VariableRenameVisitor, VariableRenameVisitor2 +from mypy.renaming import VariableRenameVisitor, LimitedVariableRenameVisitor from mypy.config_parser import parse_mypy_comments from mypy.freetree import free_tree from mypy.stubinfo import legacy_bundled_packages, is_legacy_bundled_package @@ -2120,10 +2120,11 @@ def semantic_analysis_pass1(self) -> None: # TODO: Do this while constructing the AST? self.tree.names = SymbolTable() if not self.tree.is_stub: + # Always perform some low-key variable renaming + self.tree.accept(LimitedVariableRenameVisitor()) if options.allow_redefinition: - # Perform renaming across the AST to allow variable redefinitions + # Perform more renaming across the AST to allow variable redefinitions self.tree.accept(VariableRenameVisitor()) - self.tree.accept(VariableRenameVisitor2()) def add_dependency(self, dep: str) -> None: if dep not in self.dependencies_set: diff --git a/mypy/renaming.py b/mypy/renaming.py index c0731f27b1d5..24f0bb52a572 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -388,10 +388,46 @@ def record_assignment(self, name: str, can_be_redefined: bool) -> bool: return False -class VariableRenameVisitor2(TraverserVisitor): +class LimitedVariableRenameVisitor(TraverserVisitor): + """Perform some limited variable renaming in with statements. + + This allows reusing a variable in multiple with statements with + different types. For example, the two instances of 'x' can have + incompatible types: + + with C() as x: + f(x) + with D() as x: + g(x) + + The above code gets renamed conceptually into this (not valid Python!): + + with C() as x': + f(x') + with D() as x: + g(x) + + If there's a reference to a variable defined in 'with' outside the + statement, or if there's any trickiness around variable visibility + (e.g. function definitions), we give up and won't perform renaming. + + The main use case is to allow binding both readable and writable + binary files into the same variable. These have different types: + + with open(fnam, 'rb') as f: ... + with open(fnam, 'wb') as f: ... + """ + def __init__(self) -> None: + # Short names of variables bound in with statements using "as" + # in a surrounding scope self.bound_vars: List[str] = [] - self.bad: List[Set[str]] = [] + # Names that can't be safely renamed, per scope ('*' means that + # no names can be renamed) + self.skipped: List[Set[str]] = [] + # References to variables that we may need to rename. List of + # scopes; each scope is a mapping from name to list of collections + # of names that refer to the same logical variable. self.refs: List[Dict[str, List[List[NameExpr]]]] = [] def visit_mypy_file(self, file_node: MypyFile) -> None: @@ -407,7 +443,7 @@ def visit_func_def(self, fdef: FuncDef) -> None: self.reject_redefinition_of_vars_in_scope() with self.enter_scope(): for arg in fdef.arguments: - self.record_bad(arg.variable.name) + self.record_skipped(arg.variable.name) super().visit_func_def(fdef) def visit_class_def(self, cdef: ClassDef) -> None: @@ -418,7 +454,7 @@ def visit_class_def(self, cdef: ClassDef) -> None: def visit_with_stmt(self, stmt: WithStmt) -> None: for expr in stmt.expr: expr.accept(self) - n = len(self.bound_vars) + old_len = len(self.bound_vars) for target in stmt.target: if target is not None: self.analyze_lvalue(target) @@ -427,30 +463,34 @@ def visit_with_stmt(self, stmt: WithStmt) -> None: target.accept(self) stmt.body.accept(self) - while len(self.bound_vars) > n: + while len(self.bound_vars) > old_len: self.bound_vars.pop() def visit_import(self, imp: Import) -> None: + # We don't support renaming imports for id, as_id in imp.ids: - self.record_bad(as_id or id) + self.record_skipped(as_id or id) def visit_import_from(self, imp: ImportFrom) -> None: + # We don't support renaming imports for id, as_id in imp.names: - self.record_bad(as_id or id) + self.record_skipped(as_id or id) def visit_import_all(self, imp: ImportAll) -> None: - self.record_bad('*') + # Give up, since we don't know all imported names yet + self.reject_redefinition_of_vars_in_scope() def analyze_lvalue(self, lvalue: Lvalue) -> None: if isinstance(lvalue, NameExpr): name = lvalue.name if name in self.bound_vars: + # Name bound in a surrounding with statement, so it can be renamed self.visit_name_expr(lvalue) else: - d = self.refs[-1] - if name not in d: - d[name] = [] - d[name].append([]) + var_info = self.refs[-1] + if name not in var_info: + var_info[name] = [] + var_info[name].append([]) self.bound_vars.append(name) elif isinstance(lvalue, (ListExpr, TupleExpr)): for item in lvalue.items: @@ -469,33 +509,33 @@ def visit_name_expr(self, expr: NameExpr) -> None: # Record reference so that it can be renamed later self.refs[-1][name][-1].append(expr) else: - self.record_bad(name) + self.record_skipped(name) @contextmanager def enter_scope(self) -> Iterator[None]: - self.bad.append(set()) + self.skipped.append(set()) self.refs.append({}) yield None self.flush_refs() - self.bad.pop() def reject_redefinition_of_vars_in_scope(self) -> None: - self.record_bad('*') + self.record_skipped('*') - def record_bad(self, name: str) -> None: - self.bad[-1].add(name) + def record_skipped(self, name: str) -> None: + self.skipped[-1].add(name) def flush_refs(self) -> None: - if '*' not in self.bad[-1]: - for name, refs in self.refs[-1].items(): - if len(refs) <= 1 or name in self.bad[-1]: + ref_dict = self.refs.pop() + skipped = self.skipped.pop() + if '*' not in skipped: + for name, refs in ref_dict.items(): + if len(refs) <= 1 or name in skipped: continue # At module top level we must not rename the final definition, - # as it may be publicly visible. + # as it may be publicly visible to_rename = refs[:-1] for i, item in enumerate(to_rename): rename_refs(item, i) - self.refs.pop() def rename_refs(names: List[NameExpr], index: int) -> None: From 2cff3489f5c0666957b9faf1bfc5a2bd1bc167f9 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 16:13:39 +0000 Subject: [PATCH 16/19] Minor refactoring --- mypy/renaming.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index 24f0bb52a572..f10f3b0abe60 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -466,20 +466,6 @@ def visit_with_stmt(self, stmt: WithStmt) -> None: while len(self.bound_vars) > old_len: self.bound_vars.pop() - def visit_import(self, imp: Import) -> None: - # We don't support renaming imports - for id, as_id in imp.ids: - self.record_skipped(as_id or id) - - def visit_import_from(self, imp: ImportFrom) -> None: - # We don't support renaming imports - for id, as_id in imp.names: - self.record_skipped(as_id or id) - - def visit_import_all(self, imp: ImportAll) -> None: - # Give up, since we don't know all imported names yet - self.reject_redefinition_of_vars_in_scope() - def analyze_lvalue(self, lvalue: Lvalue) -> None: if isinstance(lvalue, NameExpr): name = lvalue.name @@ -503,6 +489,20 @@ def analyze_lvalue(self, lvalue: Lvalue) -> None: elif isinstance(lvalue, StarExpr): self.analyze_lvalue(lvalue.expr) + def visit_import(self, imp: Import) -> None: + # We don't support renaming imports + for id, as_id in imp.ids: + self.record_skipped(as_id or id) + + def visit_import_from(self, imp: ImportFrom) -> None: + # We don't support renaming imports + for id, as_id in imp.names: + self.record_skipped(as_id or id) + + def visit_import_all(self, imp: ImportAll) -> None: + # Give up, since we don't know all imported names yet + self.reject_redefinition_of_vars_in_scope() + def visit_name_expr(self, expr: NameExpr) -> None: name = expr.name if name in self.bound_vars: From 8a5457b702edcf4a5e8230bbf19bd16a394de27b Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 16:24:53 +0000 Subject: [PATCH 17/19] Clean up tests --- test-data/unit/check-statements.test | 69 +++++++--------------------- 1 file changed, 16 insertions(+), 53 deletions(-) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index e6418acc0b4b..e29a46df94bd 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1571,7 +1571,7 @@ class C3: def __exit__(self, x, y, z) -> Optional[bool]: pass [builtins fixtures/bool.pyi] -[case testWithStatementScope1] +[case testWithStmtScopeBasics] from m import A, B def f1() -> None: @@ -1596,26 +1596,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope2] -from m import A, B - -def f2() -> None: - with A() as x: - reveal_type(x) # N: Revealed type is "m.A" - y = x # Use outside with makes the scope function-level - with B() as x: \ - # E: Incompatible types in assignment (expression has type "B", variable has type "A") - reveal_type(x) # N: Revealed type is "m.A" - -[file m.pyi] -class A: - def __enter__(self) -> A: ... - def __exit__(self, x, y, z) -> None: ... -class B: - def __enter__(self) -> B: ... - def __exit__(self, x, y, z) -> None: ... - -[case testWithStatementScope3] +[case testWithStmtScopeAndFuncDef] from m import A, B with A() as x: @@ -1636,7 +1617,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope4] +[case testWithStmtScopeAndFuncDef2] from m import A, B def f() -> None: @@ -1657,7 +1638,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope5] +[case testWithStmtScopeAndFuncDef3] from m import A, B with A() as x: @@ -1678,7 +1659,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope6] +[case testWithStmtScopeAndFuncDef4] from m import A, B with A() as x: @@ -1699,7 +1680,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope7Imports] +[case testWithStmtScopeAndImport1] from m import A, B, x with A() as x: \ @@ -1710,8 +1691,6 @@ with B() as x: reveal_type(x) # N: Revealed type is "m.B" [file m.pyi] -from typing import Any - x: B class A: @@ -1721,7 +1700,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope8Imports] +[case testWithStmtScopeAndImport2] from m import A, B import m as x @@ -1734,8 +1713,6 @@ with B() as x: \ pass [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1744,7 +1721,7 @@ class B: def __exit__(self, x, y, z) -> None: ... [builtins fixtures/module.pyi] -[case testWithStatementScope9ImportStar] +[case testWithStmtScopeAndImportStar] from m import A, B from m import * @@ -1756,8 +1733,6 @@ with B() as x: \ pass [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1765,7 +1740,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope10NestedWith] +[case testWithStmtScopeNestedWith1] from m import A, B with A() as x: @@ -1779,8 +1754,6 @@ with B() as x: reveal_type(x) # N: Revealed type is "m.B" [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1788,7 +1761,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope11NestedWith] +[case testWithStmtScopeNestedWith2] from m import A, B with A() as x: @@ -1798,8 +1771,6 @@ with A() as x: reveal_type(y) # N: Revealed type is "m.B" [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1807,7 +1778,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope12Outer] +[case testWithStmtScopeInnerAndOuterScopes] from m import A, B x = A() # Outer scope should have no impact @@ -1827,8 +1798,6 @@ with A() as x: pass [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1836,7 +1805,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope13] +[case testWithStmtScopeMultipleContextManagers] from m import A, B with A() as x, B() as y: @@ -1847,8 +1816,6 @@ with B() as x, A() as y: reveal_type(y) # N: Revealed type is "m.A" [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... @@ -1856,7 +1823,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope14] +[case testWithStmtScopeMultipleAssignment] from m import A, B with A() as (x, y): @@ -1867,7 +1834,7 @@ with B() as [x, y]: reveal_type(y) # N: Revealed type is "builtins.str" [file m.pyi] -from typing import Any, Tuple +from typing import Tuple class A: def __enter__(self) -> Tuple[A, int]: ... @@ -1877,7 +1844,7 @@ class B: def __exit__(self, x, y, z) -> None: ... [builtins fixtures/tuple.pyi] -[case testWithStatementScope15] +[case testWithStmtScopeComplexAssignments] from m import A, B, f with A() as x: @@ -1897,8 +1864,6 @@ with B() as f(y)[0]: pass [file m.pyi] -from typing import Any - def f(x): ... class A: @@ -1908,7 +1873,7 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... -[case testWithStatementScope16] +[case testWithStmtScopeAndClass] from m import A, B with A() as x: @@ -1925,8 +1890,6 @@ with B() as x: \ pass [file m.pyi] -from typing import Any - class A: def __enter__(self) -> A: ... def __exit__(self, x, y, z) -> None: ... From e2808748d46cc60727d0d7dafd1b4bd7d73da15a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 16:36:13 +0000 Subject: [PATCH 18/19] Fix nested scopes --- mypy/renaming.py | 4 ++- test-data/unit/check-statements.test | 38 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index f10f3b0abe60..72138c32b35e 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -507,7 +507,9 @@ def visit_name_expr(self, expr: NameExpr) -> None: name = expr.name if name in self.bound_vars: # Record reference so that it can be renamed later - self.refs[-1][name][-1].append(expr) + for scope in reversed(self.refs): + if name in scope: + scope[name][-1].append(expr) else: self.record_skipped(name) diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index e29a46df94bd..5819ace83603 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -1897,6 +1897,44 @@ class B: def __enter__(self) -> B: ... def __exit__(self, x, y, z) -> None: ... +[case testWithStmtScopeInnerScopeReference] +from m import A, B + +with A() as x: + def f() -> A: + return x + f() + +with B() as x: \ + # E: Incompatible types in assignment (expression has type "B", variable has type "A") + pass +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + +[case testWithStmtScopeAndLambda] +from m import A, B + +# This is technically not correct, since the lambda can outlive the with +# statement, but this behavior seems more intuitive. + +with A() as x: + lambda: reveal_type(x) # N: Revealed type is "m.A" + +with B() as x: + pass +[file m.pyi] +class A: + def __enter__(self) -> A: ... + def __exit__(self, x, y, z) -> None: ... +class B: + def __enter__(self) -> B: ... + def __exit__(self, x, y, z) -> None: ... + -- Chained assignment -- ------------------ From 93e95cb44379792291a8f7bfdd40e789369437c0 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 25 Feb 2022 16:38:32 +0000 Subject: [PATCH 19/19] Fix self check --- mypy/renaming.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/renaming.py b/mypy/renaming.py index 72138c32b35e..aa16d6dc8dd2 100644 --- a/mypy/renaming.py +++ b/mypy/renaming.py @@ -1,5 +1,5 @@ from contextlib import contextmanager -from typing import Dict, Iterator, List +from typing import Dict, Iterator, List, Set from typing_extensions import Final from mypy.nodes import (