Skip to content

Commit

Permalink
New analyzer: fix issues with order-dependent results (#7158)
Browse files Browse the repository at this point in the history
When we add a placeholder, we should do it through `mark_incomplete`,
since the name needs to added to the set of missing names. Otherwise
a later definition can take precedence over an earlier definition,
if the first definition initially creates a placeholder.

Fixes #7157 and probably another issue related to type aliases, which
was exposed by the fix.
  • Loading branch information
JukkaL committed Jul 5, 2019
1 parent e1f113a commit cc3b54b
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 24 deletions.
30 changes: 18 additions & 12 deletions mypy/newsemanal/semanal.py
Expand Up @@ -203,7 +203,10 @@ class NewSemanticAnalyzer(NodeVisitor[None],
# Note that missing names are per module, _not_ per namespace. This means that e.g.
# a missing name at global scope will block adding same name at a class scope.
# This should not affect correctness and is purely a performance issue,
# since it can cause unnecessary deferrals.
# since it can cause unnecessary deferrals. These are represented as
# PlaceholderNodes in the symbol table. We use this to ensure that the first
# definition takes precedence even if it's incomplete.
#
# Note that a star import adds a special name '*' to the set, this blocks
# adding _any_ names in the current file.
missing_names = None # type: Set[str]
Expand Down Expand Up @@ -1702,7 +1705,9 @@ def process_imported_symbol(self,
if self.final_iteration:
self.report_missing_module_attribute(module_id, id, imported_id, context)
return
self.record_incomplete_ref()
else:
# This might become a type.
self.mark_incomplete(imported_id, node.node, becomes_typeinfo=True)
existing_symbol = self.globals.get(imported_id)
if (existing_symbol and not isinstance(existing_symbol.node, PlaceholderNode) and
not isinstance(node.node, PlaceholderNode)):
Expand Down Expand Up @@ -2408,11 +2413,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
if self.found_incomplete_ref(tag) or has_placeholder(res):
# Since we have got here, we know this must be a type alias (incomplete refs
# may appear in nested positions), therefore use becomes_typeinfo=True.
placeholder = PlaceholderNode(self.qualified_name(lvalue.name),
rvalue,
s.line,
becomes_typeinfo=True)
self.add_symbol(lvalue.name, placeholder, s)
self.mark_incomplete(lvalue.name, rvalue, becomes_typeinfo=True)
return True
self.add_type_alias_deps(depends_on)
# In addition to the aliases used, we add deps on unbound
Expand Down Expand Up @@ -4139,6 +4140,9 @@ def add_symbol_table_node(self,
# need to do anything.
old = existing.node
new = symbol.node
if isinstance(new, PlaceholderNode):
# We don't know whether this is okay. Let's wait until the next iteration.
return False
if not is_same_symbol(old, new):
if isinstance(new, (FuncDef, Decorator, OverloadedFuncDef, TypeInfo)):
self.add_redefinition(names, name, symbol)
Expand Down Expand Up @@ -4306,7 +4310,7 @@ def mark_incomplete(self, name: str, node: Node,
self.defer(node)
if name == '*':
self.incomplete = True
elif name not in self.current_symbol_table() and not self.is_global_or_nonlocal(name):
elif not self.is_global_or_nonlocal(name):
fullname = self.qualified_name(name)
assert self.statement
placeholder = PlaceholderNode(fullname, node, self.statement.line,
Expand Down Expand Up @@ -4809,10 +4813,12 @@ def is_valid_replacement(old: SymbolTableNode, new: SymbolTableNode) -> bool:
2. Placeholder that isn't known to become type replaced with a
placeholder that can become a type
"""
return (isinstance(old.node, PlaceholderNode)
and (not isinstance(new.node, PlaceholderNode)
or (not old.node.becomes_typeinfo
and new.node.becomes_typeinfo)))
if isinstance(old.node, PlaceholderNode):
if isinstance(new.node, PlaceholderNode):
return not old.node.becomes_typeinfo and new.node.becomes_typeinfo
else:
return True
return False


def is_same_symbol(a: Optional[SymbolNode], b: Optional[SymbolNode]) -> bool:
Expand Down
15 changes: 11 additions & 4 deletions test-data/unit/check-incremental.test
Expand Up @@ -4497,29 +4497,36 @@ tmp/a.py:3: note: Revealed type is 'builtins.list[builtins.list[builtins.list[An
[case testRecursiveAliasImported2]
# flags: --new-semantic-analyzer
import a

[file a.py]
import lib
x: int

[file a.py.2]
import lib
x: lib.A
reveal_type(x)

[file lib.pyi]
from typing import List
from other import B
MYPY = False
if MYPY: # Force processing order
from other import B
A = List[B] # type: ignore

[file other.pyi]
from typing import List
from lib import A
B = List[A]

[builtins fixtures/list.pyi]
[out]
tmp/other.pyi:2: error: Module 'lib' has no attribute 'A'
tmp/other.pyi:3: error: Cannot resolve name "B" (possible cyclic definition)
tmp/lib.pyi:2: error: Module 'other' has no attribute 'B'
[out2]
tmp/other.pyi:2: error: Module 'lib' has no attribute 'A'
tmp/other.pyi:3: error: Cannot resolve name "B" (possible cyclic definition)
tmp/lib.pyi:2: error: Module 'other' has no attribute 'B'
tmp/a.py:3: note: Revealed type is 'builtins.list[Any]'
tmp/a.py:3: note: Revealed type is 'builtins.list[builtins.list[Any]]'

[case testRecursiveNamedTupleTypedDict]
# https://github.com/python/mypy/issues/7125
Expand Down
93 changes: 85 additions & 8 deletions test-data/unit/check-newsemanal.test
Expand Up @@ -272,7 +272,33 @@ reveal_type(x) # N: Revealed type is 'TypedDict('__main__.T2', {'x': builtins.st
y: T4
reveal_type(y) # N: Revealed type is 'TypedDict('__main__.T4', {'x': builtins.str, 'y': __main__.A})'

[case testNewAnalyzerRedefinitionAndDeferral]
[case testNewAnalyzerRedefinitionAndDeferral1a]
import a

[file a.py]
MYPY = False
if MYPY:
from b import x as y
x = 0

def y(): pass # E: Name 'y' already defined on line 4
reveal_type(y) # N: Revealed type is 'builtins.int'

y2 = y
class y2: pass # E: Name 'y2' already defined on line 9
reveal_type(y2) # N: Revealed type is 'builtins.int'

y3, y4 = y, y
if MYPY: # Tweak processing order
from b import f as y3 # E: Incompatible import of "y3" (imported name has type "Callable[[], Any]", local name has type "int")
reveal_type(y3) # N: Revealed type is 'builtins.int'

[file b.py]
from a import x

def f(): pass

[case testNewAnalyzerRedefinitionAndDeferral1b]
import a

[file a.py]
Expand All @@ -291,11 +317,26 @@ from b import f as y3 # E: Incompatible import of "y3" (imported name has type "
reveal_type(y3) # N: Revealed type is 'builtins.int'

[file b.py]
from a import x
MYPY = False
if MYPY: # Tweak processing order
from a import x

def f(): pass

[case testNewAnalyzerRedefinitionAndDeferral2]
[case testNewAnalyzerRedefinitionAndDeferral2a]
import a

[file a.py]
MYPY = False
if MYPY: # Tweak processing order
from b import C as C2
class C: pass
class C2: pass # E: Name 'C2' already defined on line 4

[file b.py]
from a import C

[case testNewAnalyzerRedefinitionAndDeferral2b]
import a

[file a.py]
Expand All @@ -304,7 +345,9 @@ class C: pass

class C2: pass # E: Name 'C2' already defined on line 2
[file b.py]
from a import C
MYPY = False
if MYPY: # Tweak processing order
from a import C

[case testNewAnalyzerRedefinitionAndDeferral3]
import a
Expand All @@ -320,7 +363,7 @@ reveal_type(b) # N: Revealed type is 'Any'
[file b.py]
from a import f

[case testNewAnalyzerImportStarForwardRef]
[case testNewAnalyzerImportStarForwardRef1]
import a

[file a.py]
Expand All @@ -331,6 +374,25 @@ from b import *

class A: pass # E: Name 'A' already defined (possibly by an import)

[file b.py]
class A: pass
MYPY = False
if MYPY: # Tweak processing order
from a import x

[case testNewAnalyzerImportStarForwardRef2]
import a

[file a.py]
x: A
reveal_type(x) # N: Revealed type is 'b.A'

MYPY = False
if MYPY: # Tweak processing order
from b import *

class A: pass # E: Name 'A' already defined (possibly by an import)

[file b.py]
class A: pass
from a import x
Expand Down Expand Up @@ -1005,19 +1067,34 @@ import a
class C(B): ...
class B: ...

[case testNewAnalyzerImportOverExistingInCycleStar]
[case testNewAnalyzerImportOverExistingInCycleStar1]
import a
[file a.py]
C = 1
from b import * # E: Name 'C' already defined on line 1 \
# E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")
MYPY = False
if MYPY: # Tweak processing ordre
from b import * # E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")

[file b.py]
import a

class C(B): ...
class B: ...

[case testNewAnalyzerImportOverExistingInCycleStar2]
import a
[file a.py]
C = 1
from b import * # E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")

[file b.py]
MYPY = False
if MYPY: # Tweak processing order
import a

class C(B): ...
class B: ...

[case testNewAnalyzerIncompleteFixture]
from typing import Tuple

Expand Down

0 comments on commit cc3b54b

Please sign in to comment.