Skip to content

New semantic analyzer: Make assignment analysis logic more straightforward #6527

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 27 commits into from
Mar 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b6fd5d9
Work towards better assignment analysis
Feb 15, 2019
5c1e97c
Merge remote-tracking branch 'upstream/master' into polish-aliases-new
ilevkivskyi Mar 6, 2019
6a80f22
Minor fixes after merge
ilevkivskyi Mar 6, 2019
5f9cfe4
Merge remote-tracking branch 'upstream/master' into polish-aliases-new
Mar 7, 2019
88dec62
Update logic and fix tests
Mar 7, 2019
e27727f
Make all assignment types follow the same pattern
Mar 7, 2019
9a6e3c3
Replace temporary Var hacks with a simpler and better hack
Mar 7, 2019
7ba8c8f
Some cleanups and comments/docstrings
Mar 7, 2019
746a424
Add one more test
Mar 7, 2019
1e1804b
Some more cleanups
Mar 8, 2019
555a209
Typo and comment
Mar 8, 2019
4378bd7
Add enum symbols exactly once
Mar 8, 2019
259eb37
Update NewType to mimic the logic in ClassDef
Mar 8, 2019
2e6e8e8
Minor cleanups, self-check, and lint
Mar 8, 2019
b3321dc
Re-enable previously mistakenly skipped file
Mar 8, 2019
91e7118
Merge remote-tracking branch 'upstream/master' into polish-aliases-new
Mar 8, 2019
ca9bf7c
Update order of errors in one more test
Mar 8, 2019
a9b746e
Whitespace
Mar 8, 2019
e4f4003
Update a (way outdated) docstring
Mar 8, 2019
98c8da8
Enable (and fix) newtype test file
Mar 9, 2019
469ad27
Always store info if it is a NewType (there is no way back to Var)
Mar 9, 2019
4a80d0c
One more tricky corner case
Mar 9, 2019
7c25d3d
Fix self-check
Mar 11, 2019
adc5c8a
Merge remote-tracking branch 'upstream/master' into polish-aliases-new
Mar 14, 2019
9e6ad03
Ban placeholder alias targets (they can be only nested, like in base …
Mar 15, 2019
58c88b5
Address CR
Mar 15, 2019
bf330d5
Address CR
Mar 15, 2019
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
9 changes: 8 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1715,6 +1715,11 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
with self.enter_final_context(s.is_final_def):
self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)

if s.is_alias_def:
# We do this mostly for compatibility with old semantic analyzer.
# TODO: should we get rid of this?
self.store_type(s.lvalues[-1], self.expr_checker.accept(s.rvalue))

if (s.type is not None and
self.options.disallow_any_unimported and
has_any_from_unimported_type(s.type)):
Expand Down Expand Up @@ -1824,7 +1829,9 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
self.msg.concrete_only_assign(lvalue_type, rvalue)
return
if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType):
self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)
# Don't use type binder for definitions of special forms, like named tuples.
if not (isinstance(lvalue, NameExpr) and lvalue.is_special_form):
self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)

elif index_lvalue:
self.check_indexed_assignment(index_lvalue, rvalue, lvalue)
Expand Down
383 changes: 275 additions & 108 deletions mypy/newsemanal/semanal.py

Large diffs are not rendered by default.

43 changes: 25 additions & 18 deletions mypy/newsemanal/semanal_enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,22 @@ def __init__(self, options: Options, api: SemanticAnalyzerInterface) -> None:
self.options = options
self.api = api

def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> None:
"""Check if s defines an Enum; if yes, store the definition in symbol table."""
def process_enum_call(self, s: AssignmentStmt, is_func_scope: bool) -> bool:
"""Check if s defines an Enum; if yes, store the definition in symbol table.

Return True if this looks like an Enum definition (but maybe with errors),
otherwise return False.
"""
if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], NameExpr):
return
return False
lvalue = s.lvalues[0]
name = lvalue.name
enum_call = self.check_enum_call(s.rvalue, name, is_func_scope)
if enum_call is None:
return
return False
# Yes, it's a valid Enum definition. Add it to the symbol table.
node = self.api.lookup(name, s)
if node:
node.kind = GDEF # TODO locally defined Enum
node.node = enum_call
self.api.add_symbol(name, enum_call, s)
return True

def check_enum_call(self,
node: Expression,
Expand Down Expand Up @@ -62,18 +64,19 @@ class A(enum.Enum):
items, values, ok = self.parse_enum_call_args(call, fullname.split('.')[-1])
if not ok:
# Error. Construct dummy return value.
return self.build_enum_call_typeinfo(var_name, [], fullname)
name = cast(Union[StrExpr, UnicodeExpr], call.args[0]).value
if name != var_name or is_func_scope:
# Give it a unique name derived from the line number.
name += '@' + str(call.line)
info = self.build_enum_call_typeinfo(name, items, fullname)
# Store it as a global just in case it would remain anonymous.
# (Or in the nearest class if there is one.)
stnode = SymbolTableNode(GDEF, info)
self.api.add_symbol_table_node(name, stnode)
info = self.build_enum_call_typeinfo(var_name, [], fullname)
else:
name = cast(Union[StrExpr, UnicodeExpr], call.args[0]).value
if name != var_name or is_func_scope:
# Give it a unique name derived from the line number.
name += '@' + str(call.line)
info = self.build_enum_call_typeinfo(name, items, fullname)
# Store generated TypeInfo under both names, see semanal_namedtuple for more details.
if name != var_name or is_func_scope:
self.api.add_symbol_skip_local(name, info)
call.analyzed = EnumCallExpr(info, items, values)
call.analyzed.set_line(call.line, call.column)
info.line = node.line
return info

def build_enum_call_typeinfo(self, name: str, items: List[str], fullname: str) -> TypeInfo:
Expand All @@ -93,6 +96,10 @@ def build_enum_call_typeinfo(self, name: str, items: List[str], fullname: str) -
def parse_enum_call_args(self, call: CallExpr,
class_name: str) -> Tuple[List[str],
List[Optional[Expression]], bool]:
"""Parse arguments of an Enum call.

Return a tuple of fields, values, was there an error.
"""
args = call.args
if len(args) < 2:
return self.fail_enum_call_arg("Too few arguments for %s()" % class_name, call)
Expand Down
106 changes: 75 additions & 31 deletions mypy/newsemanal/semanal_newtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@

from typing import Tuple, Optional

from mypy.types import Type, Instance, CallableType, NoneTyp, TupleType, AnyType, TypeOfAny
from mypy.types import (
Type, Instance, CallableType, NoneTyp, TupleType, AnyType, PlaceholderType,
TypeOfAny
)
from mypy.nodes import (
AssignmentStmt, NewTypeExpr, CallExpr, NameExpr, RefExpr, Context, StrExpr, BytesExpr,
UnicodeExpr, Block, FuncDef, Argument, TypeInfo, Var, SymbolTableNode, GDEF, MDEF, ARG_POS
UnicodeExpr, Block, FuncDef, Argument, TypeInfo, Var, SymbolTableNode, MDEF, ARG_POS,
PlaceholderNode
)
from mypy.newsemanal.semanal_shared import SemanticAnalyzerInterface
from mypy.options import Options
Expand All @@ -26,17 +30,36 @@ def __init__(self,
self.api = api
self.msg = msg

def process_newtype_declaration(self, s: AssignmentStmt) -> None:
"""Check if s declares a NewType; if yes, store it in symbol table."""
# Extract and check all information from newtype declaration
def process_newtype_declaration(self, s: AssignmentStmt) -> bool:
"""Check if s declares a NewType; if yes, store it in symbol table.

Return True if it's a NewType declaration. The current target may be
deferred as a side effect if the base type is not ready, even if
the return value is True.

The logic in this function mostly copies the logic for visit_class_def()
with a single (non-Generic) base.
"""
name, call = self.analyze_newtype_declaration(s)
if name is None or call is None:
return

old_type = self.check_newtype_args(name, call, s)
call.analyzed = NewTypeExpr(name, old_type, line=call.line)
return False
# OK, now we know this is a NewType. But the base type may be not ready yet,
# add placeholder as we do for ClassDef.

fullname = self.api.qualified_name(name)
if (not call.analyzed or
isinstance(call.analyzed, NewTypeExpr) and not call.analyzed.info):
# Start from labeling this as a future class, as we do for normal ClassDefs.
self.api.add_symbol(name, PlaceholderNode(fullname, s, becomes_typeinfo=True), s)

old_type, should_defer = self.check_newtype_args(name, call, s)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It wasn't immediately clear to me what old_type refers to. Maybe rename to base_type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately base_type is already taken in this file.

if not call.analyzed:
call.analyzed = NewTypeExpr(name, old_type, line=call.line)
if old_type is None:
return
if should_defer:
# Base type is not ready.
self.api.defer()
return True

# Create the corresponding class definition if the aliased type is subtypeable
if isinstance(old_type, TupleType):
Expand All @@ -48,9 +71,14 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> None:
self.fail("NewType cannot be used with protocol classes", s)
newtype_class_info = self.build_newtype_typeinfo(name, old_type, old_type)
else:
message = "Argument 2 to NewType(...) must be subclassable (got {})"
self.fail(message.format(self.msg.format(old_type)), s)
return
if old_type is not None:
message = "Argument 2 to NewType(...) must be subclassable (got {})"
self.fail(message.format(self.msg.format(old_type)), s)
# Otherwise the error was already reported.
old_type = AnyType(TypeOfAny.from_error)
object_type = self.api.named_type('__builtins__.object')
newtype_class_info = self.build_newtype_typeinfo(name, old_type, object_type)
newtype_class_info.fallback_to_any = True

check_for_explicit_any(old_type, self.options, self.api.is_typeshed_stub_file, self.msg,
context=s)
Expand All @@ -59,13 +87,16 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> None:
self.msg.unimported_type_becomes_any("Argument 2 to NewType(...)", old_type, s)

# If so, add it to the symbol table.
node = self.api.lookup(name, s)
if node is None:
self.fail("Could not find {} in current namespace".format(name), s)
return
# TODO: why does NewType work in local scopes despite always being of kind GDEF?
node.kind = GDEF
call.analyzed.info = node.node = newtype_class_info
assert isinstance(call.analyzed, NewTypeExpr)
# As we do for normal classes, create the TypeInfo only once, then just
# update base classes on next iterations (to get rid of placeholders there).
if not call.analyzed.info:
call.analyzed.info = newtype_class_info
else:
call.analyzed.info.bases = newtype_class_info.bases
self.api.add_symbol(name, call.analyzed.info, s)
newtype_class_info.line = s.line
return True

def analyze_newtype_declaration(self,
s: AssignmentStmt) -> Tuple[Optional[str], Optional[CallExpr]]:
Expand All @@ -76,13 +107,18 @@ def analyze_newtype_declaration(self,
and isinstance(s.rvalue, CallExpr)
and isinstance(s.rvalue.callee, RefExpr)
and s.rvalue.callee.fullname == 'typing.NewType'):
lvalue = s.lvalues[0]
name = s.lvalues[0].name
if not lvalue.is_inferred_def:
if s.type:
self.fail("Cannot declare the type of a NewType declaration", s)
else:
self.fail("Cannot redefine '%s' as a NewType" % name, s)

if s.type:
self.fail("Cannot declare the type of a NewType declaration", s)

names = self.api.current_symbol_table()
existing = names.get(name)
# Give a better error message than generic "Name already defined",
# like the old semantic analyzer does.
if (existing and
not isinstance(existing.node, PlaceholderNode) and not s.rvalue.analyzed):
self.fail("Cannot redefine '%s' as a NewType" % name, s)

# This dummy NewTypeExpr marks the call as sufficiently analyzed; it will be
# overwritten later with a fully complete NewTypeExpr if there are no other
Expand All @@ -91,12 +127,17 @@ def analyze_newtype_declaration(self,

return name, call

def check_newtype_args(self, name: str, call: CallExpr, context: Context) -> Optional[Type]:
def check_newtype_args(self, name: str, call: CallExpr,
context: Context) -> Tuple[Optional[Type], bool]:
"""Ananlyze base type in NewType call.

Return a tuple (type, should defer).
"""
has_failed = False
args, arg_kinds = call.args, call.arg_kinds
if len(args) != 2 or arg_kinds[0] != ARG_POS or arg_kinds[1] != ARG_POS:
self.fail("NewType(...) expects exactly two positional arguments", context)
return None
return None, False

# Check first argument
if not isinstance(args[0], (StrExpr, BytesExpr, UnicodeExpr)):
Expand All @@ -113,19 +154,22 @@ def check_newtype_args(self, name: str, call: CallExpr, context: Context) -> Opt
unanalyzed_type = expr_to_unanalyzed_type(args[1])
except TypeTranslationError:
self.fail(msg, context)
return None
return None, False

# We want to use our custom error message (see above), so we suppress
# the default error message for invalid types here.
old_type = self.api.anal_type(unanalyzed_type, report_invalid_types=False)
should_defer = False
if old_type is None or isinstance(old_type, PlaceholderType):
should_defer = True

# The caller of this function assumes that if we return a Type, it's always
# a valid one. So, we translate AnyTypes created from errors into None.
if isinstance(old_type, AnyType) and old_type.is_from_error:
self.fail(msg, context)
return None
return None, False

return None if has_failed else old_type
return None if has_failed else old_type, should_defer

def build_newtype_typeinfo(self, name: str, old_type: Type, base_type: Instance) -> TypeInfo:
info = self.api.basic_new_typeinfo(name, base_type)
Expand Down
10 changes: 9 additions & 1 deletion mypy/newsemanal/semanal_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from mypy.nodes import (
Context, SymbolTableNode, MypyFile, ImportedName, FuncDef, Node, TypeInfo, Expression, GDEF,
SymbolNode
SymbolNode, SymbolTable
)
from mypy.util import correct_relative_import
from mypy.types import Type, FunctionLike, Instance, TupleType
Expand Down Expand Up @@ -123,6 +123,14 @@ def add_symbol_table_node(self, name: str, stnode: SymbolTableNode) -> bool:
"""Add node to the current symbol table."""
raise NotImplementedError

@abstractmethod
def current_symbol_table(self) -> SymbolTable:
"""Get currently active symbol table.

May be module, class, or local namespace.
"""
raise NotImplementedError

@abstractmethod
def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
module_public: bool = True, module_hidden: bool = False) -> bool:
Expand Down
52 changes: 3 additions & 49 deletions mypy/newsemanal/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,48 +75,6 @@ def analyze_type_alias(node: Expression,
full names of type aliases it depends on (directly or indirectly).
Return None otherwise. 'node' must have been semantically analyzed.
"""
# Quickly return None if the expression doesn't look like a type. Note
# that we don't support straight string literals as type aliases
# (only string literals within index expressions).
if isinstance(node, RefExpr):
# Note that this misses the case where someone tried to use a
# class-referenced type variable as a type alias. It's easier to catch
# that one in checkmember.py
if isinstance(node.node, TypeVarExpr):
api.fail('Type variable "{}" is invalid as target for type alias'.format(
node.fullname), node)
return None
if not (isinstance(node.node, TypeInfo) or
node.fullname in ('typing.Any', 'typing.Tuple', 'typing.Callable') or
isinstance(node.node, TypeAlias)):
return None
elif isinstance(node, IndexExpr):
base = node.base
if isinstance(base, RefExpr):
if not (isinstance(base.node, TypeInfo) or
base.fullname in type_constructors or
isinstance(base.node, TypeAlias)):
return None
# Enums can't be generic, and without this check we may incorrectly interpret indexing
# an Enum class as creating a type alias.
if isinstance(base.node, TypeInfo) and base.node.is_enum:
return None
else:
return None
elif isinstance(node, CallExpr):
if (isinstance(node.callee, NameExpr) and len(node.args) == 1 and
isinstance(node.args[0], NameExpr)):
call = api.lookup_qualified(node.callee.name, node.callee)
arg = api.lookup_qualified(node.args[0].name, node.args[0])
if (call is not None and call.node and call.node.fullname() == 'builtins.type' and
arg is not None and arg.node and arg.node.fullname() == 'builtins.None'):
return NoneTyp(), set()
return None
return None
else:
return None

# It's a type alias (though it may be an invalid one).
try:
type = expr_to_unanalyzed_type(node)
except TypeTranslationError:
Expand Down Expand Up @@ -414,14 +372,10 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl
(not self.tvar_scope or self.tvar_scope.get_binding(sym) is None))
if self.allow_unbound_tvars and unbound_tvar and not self.third_pass:
return t

# None of the above options worked, we give up.
# NOTE: 'final_iteration' is iteration when we hit the maximum number of iterations limit.
if unbound_tvar or self.api.final_iteration:
# TODO: This is problematic, since we will have to wait until the maximum number
# of iterations to report an invalid type.
self.fail('Invalid type "{}"'.format(name), t)
else:
self.api.defer()
self.fail('Invalid type "{}"'.format(name), t)

if self.third_pass and isinstance(sym.node, TypeVarExpr):
self.note_func("Forward references to type variables are prohibited", t)
return AnyType(TypeOfAny.from_error)
Expand Down
4 changes: 3 additions & 1 deletion mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1418,11 +1418,13 @@ class NameExpr(RefExpr):
This refers to a local name, global name or a module.
"""

__slots__ = ('name',)
__slots__ = ('name', 'is_special_form')

def __init__(self, name: str) -> None:
super().__init__()
self.name = name # Name referred to (may be qualified)
# Is this a l.h.s. of a special form assignment like typed dict or type variable?
self.is_special_form = False

def accept(self, visitor: ExpressionVisitor[T]) -> T:
return visitor.visit_name_expr(self)
Expand Down
4 changes: 3 additions & 1 deletion mypy/strconv.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,9 @@ def visit_star_expr(self, o: 'mypy.nodes.StarExpr') -> str:
return self.dump([o.expr], o)

def visit_name_expr(self, o: 'mypy.nodes.NameExpr') -> str:
pretty = self.pretty_name(o.name, o.kind, o.fullname, o.is_inferred_def, o.node)
pretty = self.pretty_name(o.name, o.kind, o.fullname,
o.is_inferred_def or o.is_special_form,
o.node)
if isinstance(o.node, mypy.nodes.Var) and o.node.is_final:
pretty += ' = {}'.format(o.node.final_value)
return short_type(o) + '(' + pretty + ')'
Expand Down
2 changes: 0 additions & 2 deletions mypy/test/hacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
'check-incremental.test',
'check-literal.test',
'check-modules.test',
'check-newtype.test',
'check-overloading.test',
'check-python2.test',
'check-semanal-error.test',
'check-statements.test',
'check-unions.test',
'check-unreachable-code.test',
Expand Down
Loading