Skip to content

New semantic analyzer: Fix (de)serialization of broken named tuples #6421

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

Merged
merged 6 commits into from
Feb 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1794,8 +1794,13 @@ def patch_dependency_parents(self) -> None:
semantic analyzer will perform this patch for us when processing stale
SCCs.
"""
Analyzer = Union[SemanticAnalyzerPass2, NewSemanticAnalyzer] # noqa
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this change? Is is related to the rest of this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed because otherwise incremental tests crash with the new analyzer (I added couple incremental tests) with something like 'BuildManager' doesn't have attribute 'semantic_analyzer'.

if self.manager.options.new_semantic_analyzer:
analyzer = self.manager.new_semantic_analyzer # type: Analyzer
else:
analyzer = self.manager.semantic_analyzer
for dep in self.dependencies:
self.manager.semantic_analyzer.add_submodules_to_parent_modules(dep, True)
analyzer.add_submodules_to_parent_modules(dep, True)

def fix_suppressed_dependencies(self, graph: Graph) -> None:
"""Corrects whether dependencies are considered stale in silent mode.
Expand Down
22 changes: 22 additions & 0 deletions mypy/newsemanal/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3955,6 +3955,28 @@ def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
module_hidden=module_hidden)
return self.add_symbol_table_node(name, symbol, context)

def add_symbol_skip_local(self, name: str, node: SymbolNode) -> None:
"""Same as above, but skipping the local namespace.

This doesn't check for previous definition and is only used
for serialization of method-level classes.

Classes defined within methods can be exposed through an
attribute type, but method-level symbol tables aren't serialized.
This method can be used to add such classes to an enclosing,
serialized symbol table.
"""
# TODO: currently this is only used by named tuples. Use this method
# also by typed dicts and normal classes, see issue #6422.
if self.type is not None:
names = self.type.names
kind = MDEF
else:
names = self.globals
kind = GDEF
symbol = SymbolTableNode(kind, node)
names[name] = symbol

def current_symbol_table(self) -> SymbolTable:
if self.is_func_scope():
assert self.locals[-1] is not None
Expand Down
39 changes: 33 additions & 6 deletions mypy/newsemanal/semanal_namedtuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,18 @@ def check_namedtuple(self,
return True, info
name = cast(Union[StrExpr, BytesExpr, UnicodeExpr], call.args[0]).value
if name != var_name or is_func_scope:
# Give it a unique name derived from the line number.
# There are three special cases where need to give it a unique name derived
# from the line number:
# * There is a name mismatch with l.h.s., therefore we need to disambiguate
# situations like:
# A = NamedTuple('Same', [('x', int)])
# B = NamedTuple('Same', [('y', str)])
# * This is a base class expression, since it often matches the class name:
# class NT(NamedTuple('NT', [...])):
# ...
# * This is a local (function or method level) named tuple, since
# two methods of a class can define a named tuple with the same name,
# and they will be stored in the same namespace (see below).
name += '@' + str(call.line)
if len(defaults) > 0:
default_items = {
Expand All @@ -189,15 +200,31 @@ def check_namedtuple(self,
else:
default_items = {}
info = self.build_namedtuple_typeinfo(name, items, types, default_items)
# Store it as a global just in case it would remain anonymous.
# (Or in the nearest class if there is one.)
self.store_namedtuple_info(info, var_name or name, call, is_typed)
# If var_name is not None (i.e. this is not a base class expression), we always
# store the generated TypeInfo under var_name in the current scope, so that
# other definitions can use it.
if var_name:
self.store_namedtuple_info(info, var_name, call, is_typed)
# There are three cases where we need to store the generated TypeInfo
# second time (for the purpose of serialization):
# * If there is a name mismatch like One = NamedTuple('Other', [...])
# we also store the info under name 'Other@lineno', this is needed
# because classes are (de)serialized using their actual fullname, not
# the name of l.h.s.
# * If this is a method level named tuple. It can leak from the method
# via assignment to self attribute and therefore needs to be serialized
# (local namespaces are not serialized).
# * If it is a base class expression. It was not stored above, since
# there is no var_name (but it still needs to be serialized
# since it is in MRO of some class).
if name != var_name or is_func_scope:
# NOTE: we skip local namespaces since they are not serialized.
self.api.add_symbol_skip_local(name, info)
return True, info

def store_namedtuple_info(self, info: TypeInfo, name: str,
call: CallExpr, is_typed: bool) -> None:
stnode = SymbolTableNode(GDEF, info)
self.api.add_symbol_table_node(name, stnode)
self.api.add_symbol(name, info, call)
call.analyzed = NamedTupleExpr(info, is_typed=is_typed)
call.analyzed.set_line(call.line, call.column)

Expand Down
21 changes: 19 additions & 2 deletions mypy/newsemanal/semanal_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
from mypy_extensions import trait

from mypy.nodes import (
Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF
Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF,
SymbolNode
)
from mypy.util import correct_relative_import
from mypy.types import Type, FunctionLike, Instance
Expand Down Expand Up @@ -122,7 +123,23 @@ def schedule_patch(self, priority: int, fn: Callable[[], None]) -> None:

@abstractmethod
def add_symbol_table_node(self, name: str, stnode: SymbolTableNode) -> bool:
"""Add node to global symbol table (or to nearest class if there is one)."""
"""Add node to the current symbol table."""
raise NotImplementedError

@abstractmethod
def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
module_public: bool = True, module_hidden: bool = False) -> bool:
"""Add symbol to the current symbol table."""
raise NotImplementedError

@abstractmethod
def add_symbol_skip_local(self, name: str, node: SymbolNode) -> None:
"""Add symbol to the current symbol table, skipping locals.

This is used to store symbol nodes in a symbol table that
is going to be serialized (local namespaces are not serialized).
See implementation docstring for more details.
"""
raise NotImplementedError

@abstractmethod
Expand Down
37 changes: 37 additions & 0 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -4931,3 +4931,40 @@ import pack.mod
import a
[out]
[out2]

[case testNewAnalyzerIncrementalBrokenNamedTuple]
# flags: --new-semantic-analyzer
import a
[file a.py]
from b import NT
x: NT
[file a.py.2]
from b import NT
x: NT
reveal_type(x)
[file b.py]
from typing import NamedTuple
NT = NamedTuple('BadName', [('x', int)])
[out]
[out2]
tmp/a.py:3: error: Revealed type is 'Tuple[builtins.int, fallback=b.BadName@2]'

[case testNewAnalyzerIncrementalMethodNamedTuple]
# flags: --new-semantic-analyzer
import a
[file a.py]
from b import C
x: C
[file a.py.2]
from b import C
x: C
reveal_type(x.h)
[file b.py]
from typing import NamedTuple
class C:
def __init__(self) -> None:
self.h: Hidden
Hidden = NamedTuple('Hidden', [('x', int)])
[out]
[out2]
tmp/a.py:3: error: Revealed type is 'Tuple[builtins.int, fallback=b.C.Hidden@5]'