Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add basic support for enum literals #6668

Merged
merged 4 commits into from
Apr 12, 2019
Merged

Add basic support for enum literals #6668

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fa1aa8
Add basic support for enum literals
Michael0x2a Apr 12, 2019
da79b01
Respond to code review
Michael0x2a Apr 12, 2019
52f3d55
Add test case for using function in enums
Michael0x2a Apr 12, 2019
0f08e69
Respond to 2nd code review
Michael0x2a Apr 12, 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
4 changes: 2 additions & 2 deletions mypy/newsemanal/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3989,10 +3989,10 @@ def lookup_fully_qualified(self, name: str) -> SymbolTableNode:
module namespace is ignored.
"""
parts = name.split('.')
n = self.modules[parts[0]]
n = self.modules[parts[0]] # type: Union[MypyFile, TypeInfo]
for i in range(1, len(parts) - 1):
next_sym = n.names[parts[i]]
assert isinstance(next_sym.node, MypyFile)
assert isinstance(next_sym.node, (MypyFile, TypeInfo))
Copy link
Member

Choose a reason for hiding this comment

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

I think there is some misunderstanding/bug somewhere. This function should never be used for things like local lookup. It should be only used to construct a known type like builtins.str. For local lookup one needs to use lookup_qualified(). Why do you need this modification?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made this change in order to get this example working:

from enum import Enum
from typing_extensions import Literal

class Outer:
    class Inner(Enum):
        FOO = 1
        BAR = 2

x: Literal[Outer.Inner.FOO]
reveal_type(x)

Basically, the literal-handling code in typeanal receives back the string "module.Outer.Inner". We then call typeanal.named_type_with_normalized_str which in turn calls typeanal.named_type which in turn calls semanal.lookup_fully_qualified. That function then ends up crashing because "Outer" is a TypeInfo, not a MypyFile.

FWIW I was under the impression that:

  1. lookup_qualified is for looking up fully-qualified types, and has a bunch of error handling for when the thing we're trying to look up doesn't exist.
  2. lookup_fully_qualified is a simpler version of lookup_qualified, except without error handling.
  3. lookup is for looking up some unqualified name across all namespaces (local and global).

I also noticed that lookup_qualified handles both MypyFile and TypeInfo, and so figured that the fact that lookup_fully_qualified doesn't was an oversight.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I was under the impression that

This is a wrong impression. Which is normal, we have more than dozen lookup functions in the code-base (likely caused by the fact that many people who are not familiar with mypy contributed some reinvented bicycles, which is again normal). Largely, they can be separated in two classes:

  • Local lookup: what given name or member expression points to using Python name resolution
  • Global lookup: find a symbol using its "full address" used to construct some standard known types.

The named_type() and friends are from the second group. While the first group is roughly two methods lookup() and lookup_qualified().

Essentially what is going on here looks like a hack (and TBH I don't like it). You already have the TypeInfo needed to construct the literal type, but instead you get its full name and pass it to RawExpressionType that later passes the name to find the same TypeInfo again.

Initially, string name was passed to RawLiteralExpression because it is was constructed at very early stage. Here however, it looks unnecessary. Why can't we just immediately return LiteralType there with a fallback instance constructed from a known TypeInfo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The named_type() and friends are from the second group. While the first group is roughly two methods lookup() and lookup_qualified().

Hmm, ok. In that case, what's the correct way of looking up a nested type via a global lookup?

It seems all of the "global lookup" functions assume that all types are defined within modules only, which I'm not so sure is correct.

Why can't we just immediately return LiteralType there with a fallback instance constructed from a known TypeInfo?

Sure, we can do that. Is it safe to just construct new whenever we want Instances though? E.g. there isn't a need to register them in some global registry?

Copy link
Member

Choose a reason for hiding this comment

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

It seems all of the "global lookup" functions assume that all types are defined within modules only, which I'm not so sure is correct.

It is not correct, but this is kind of optimization. After the refactoring we will have two methods, one fast for builtin things like builtins.str, and another one that can be used in plugins etc., with full semantics.

E.g. there isn't a need to register them in some global registry?

What kind of registry do you mean? I think one can just call Instance(info, args, line, column) or am I missing something?.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What kind of registry do you mean? I think one can just call Instance(info, args, line, column) or am I missing something?.

Honestly, I have no idea. Instances and TypeInfos are stuffed with so much functionality that I always get a little worried that touching either of them or creating new ones will break something or interfere with something related to incremental or fine-grained mode in some weird way.

n = next_sym.node
return n.names[parts[-1]]

Expand Down
38 changes: 32 additions & 6 deletions mypy/newsemanal/typeanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ def __init__(self,
# Names of type aliases encountered while analysing a type will be collected here.
self.aliases_used = set() # type: Set[str]

def visit_unbound_type(self, t: UnboundType) -> Type:
typ = self.visit_unbound_type_nonoptional(t)
def visit_unbound_type(self, t: UnboundType, defining_literal: bool = False) -> Type:
typ = self.visit_unbound_type_nonoptional(t, defining_literal)
if t.optional:
# We don't need to worry about double-wrapping Optionals or
# wrapping Anys: Union simplification will take care of that.
return make_optional_type(typ)
return typ

def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type:
def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) -> Type:
sym = self.lookup_qualified(t.name, t, suppress_errors=self.third_pass)
if sym is not None:
node = sym.node
Expand Down Expand Up @@ -217,7 +217,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type:
elif isinstance(node, TypeInfo):
return self.analyze_type_with_type_info(node, t.args, t)
else:
return self.analyze_unbound_type_without_type_info(t, sym)
return self.analyze_unbound_type_without_type_info(t, sym, defining_literal)
else: # sym is None
if self.third_pass:
self.fail('Invalid type "{}"'.format(t.name), t)
Expand Down Expand Up @@ -348,7 +348,8 @@ def analyze_type_with_type_info(self, info: TypeInfo, args: List[Type], ctx: Con
fallback=instance)
return instance

def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTableNode) -> Type:
def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTableNode,
defining_literal: bool) -> Type:
"""Figure out what an unbound type that doesn't refer to a TypeInfo node means.

This is something unusual. We try our best to find out what it is.
Expand All @@ -373,6 +374,27 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl
if self.allow_unbound_tvars and unbound_tvar and not self.third_pass:
return t

# Option 3:
# Enum value. Note: Enum values are not real types, so we return
# RawExpressionType only when this function is being called by
# one of the Literal[...] handlers -- when `defining_literal` is True.
#
# It's unsafe to return RawExpressionType in any other case, since
# the type would leak out of the semantic analysis phase.
if isinstance(sym.node, Var) and sym.node.info and sym.node.info.is_enum:
short_name = sym.node.name()
base_enum_name = sym.node.info.fullname()
if not defining_literal:
msg = "Invalid type: try using Literal[{}] instead?".format(name)
self.fail(msg, t)
return AnyType(TypeOfAny.from_error)
return RawExpressionType(
literal_value=short_name,
base_type_name=base_enum_name,
line=t.line,
column=t.column,
)

# None of the above options worked, we give up.
self.fail('Invalid type "{}"'.format(name), t)

Expand Down Expand Up @@ -631,7 +653,11 @@ def analyze_literal_param(self, idx: int, arg: Type, ctx: Context) -> Optional[L
# If arg is an UnboundType that was *not* originally defined as
# a string, try expanding it in case it's a type alias or something.
if isinstance(arg, UnboundType):
arg = self.anal_type(arg)
self.nesting_level += 1
try:
arg = self.visit_unbound_type(arg, defining_literal=True)
finally:
self.nesting_level -= 1

# Literal[...] cannot contain Any. Give up and add an error message
# (if we haven't already).
Expand Down
6 changes: 3 additions & 3 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
from mypy.typeanal import (
TypeAnalyser, analyze_type_alias, no_subscript_builtin_alias,
TypeVariableQuery, TypeVarList, remove_dups, has_any_from_unimported_type,
check_for_explicit_any
check_for_explicit_any, expand_type_alias
Michael0x2a marked this conversation as resolved.
Show resolved Hide resolved
)
from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError
from mypy.sametypes import is_same_type
Expand Down Expand Up @@ -3590,10 +3590,10 @@ def lookup_fully_qualified(self, name: str) -> SymbolTableNode:
module namespace is ignored.
"""
parts = name.split('.')
n = self.modules[parts[0]]
n = self.modules[parts[0]] # type: Union[MypyFile, TypeInfo]
for i in range(1, len(parts) - 1):
next_sym = n.names[parts[i]]
assert isinstance(next_sym.node, MypyFile)
assert isinstance(next_sym.node, (MypyFile, TypeInfo))
Michael0x2a marked this conversation as resolved.
Show resolved Hide resolved
n = next_sym.node
return n.names[parts[-1]]

Expand Down
47 changes: 40 additions & 7 deletions mypy/typeanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ def __init__(self,
# Names of type aliases encountered while analysing a type will be collected here.
self.aliases_used = set() # type: Set[str]

def visit_unbound_type(self, t: UnboundType) -> Type:
typ = self.visit_unbound_type_nonoptional(t)
def visit_unbound_type(self, t: UnboundType, defining_literal: bool = False) -> Type:
typ = self.visit_unbound_type_nonoptional(t, defining_literal)
if t.optional:
# We don't need to worry about double-wrapping Optionals or
# wrapping Anys: Union simplification will take care of that.
return make_optional_type(typ)
return typ

def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type:
def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) -> Type:
sym = self.lookup(t.name, t, suppress_errors=self.third_pass)
if '.' in t.name:
# Handle indirect references to imported names.
Expand Down Expand Up @@ -245,11 +245,14 @@ def visit_unbound_type_nonoptional(self, t: UnboundType) -> Type:
all_vars = node.alias_tvars
target = node.target
an_args = self.anal_array(t.args)
return expand_type_alias(target, all_vars, an_args, self.fail, node.no_args, t)
out = expand_type_alias(target, all_vars, an_args, self.fail, node.no_args, t)
if 'RED' in str(t):
print('...')
Michael0x2a marked this conversation as resolved.
Show resolved Hide resolved
return out
Michael0x2a marked this conversation as resolved.
Show resolved Hide resolved
elif isinstance(node, TypeInfo):
return self.analyze_unbound_type_with_type_info(t, node)
else:
return self.analyze_unbound_type_without_type_info(t, sym)
return self.analyze_unbound_type_without_type_info(t, sym, defining_literal)
else: # sym is None
if self.third_pass:
self.fail('Invalid type "{}"'.format(t.name), t)
Expand Down Expand Up @@ -368,7 +371,8 @@ def analyze_unbound_type_with_type_info(self, t: UnboundType, info: TypeInfo) ->
fallback=instance)
return instance

def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTableNode) -> Type:
def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTableNode,
defining_literal: bool) -> Type:
"""Figure out what an unbound type that doesn't refer to a TypeInfo node means.

This is something unusual. We try our best to find out what it is.
Expand All @@ -377,6 +381,7 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl
if name is None:
assert sym.node is not None
name = sym.node.name()

# Option 1:
# Something with an Any type -- make it an alias for Any in a type
# context. This is slightly problematic as it allows using the type 'Any'
Expand All @@ -385,14 +390,37 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl
if isinstance(sym.node, Var) and isinstance(sym.node.type, AnyType):
return AnyType(TypeOfAny.from_unimported_type,
missing_import_name=sym.node.type.missing_import_name)

# Option 2:
# Unbound type variable. Currently these may be still valid,
# for example when defining a generic type alias.
unbound_tvar = (isinstance(sym.node, TypeVarExpr) and
(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

# Option 3:
# Enum value. Note: Enum values are not real types, so we return
# RawExpressionType only when this function is being called by
# one of the Literal[...] handlers -- when `defining_literal` is True.
#
# It's unsafe to return RawExpressionType in any other case, since
# the type would leak out of the semantic analysis phase.
if isinstance(sym.node, Var) and sym.node.info and sym.node.info.is_enum:
short_name = sym.node.name()
base_enum_name = sym.node.info.fullname()
if not defining_literal:
msg = "Invalid type: try using Literal[{}] instead?".format(name)
self.fail(msg, t)
return AnyType(TypeOfAny.from_error)
return RawExpressionType(
literal_value=short_name,
base_type_name=base_enum_name,
line=t.line,
column=t.column,
)

# Option 4:
# If it is not something clearly bad (like a known function, variable,
# type variable, or module), and it is still not too late, we try deferring
# this type using a forward reference wrapper. It will be revisited in
Expand All @@ -410,6 +438,7 @@ def analyze_unbound_type_without_type_info(self, t: UnboundType, sym: SymbolTabl
self.fail('Unsupported forward reference to "{}"'.format(t.name), t)
return AnyType(TypeOfAny.from_error)
return ForwardRef(t)

# None of the above options worked, we give up.
self.fail('Invalid type "{}"'.format(name), t)
if self.third_pass and isinstance(sym.node, TypeVarExpr):
Expand Down Expand Up @@ -657,7 +686,11 @@ def analyze_literal_param(self, idx: int, arg: Type, ctx: Context) -> Optional[L
# If arg is an UnboundType that was *not* originally defined as
# a string, try expanding it in case it's a type alias or something.
if isinstance(arg, UnboundType):
arg = self.anal_type(arg)
self.nesting_level += 1
try:
arg = self.visit_unbound_type(arg, defining_literal=True)
finally:
self.nesting_level -= 1

# Literal[...] cannot contain Any. Give up and add an error message
# (if we haven't already).
Expand Down
13 changes: 12 additions & 1 deletion mypy/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,6 +1443,9 @@ class LiteralType(Type):

For example, 'Literal[42]' is represented as
'LiteralType(value=42, fallback=instance_of_int)'

As another example, `Literal[Color.RED]` (where Color is an enum) is
represented as `LiteralType(value="RED", fallback=instance_of_color)'.
"""
__slots__ = ('value', 'fallback')

Expand All @@ -1464,15 +1467,23 @@ def __eq__(self, other: object) -> bool:
else:
return NotImplemented

def is_fallback_enum(self) -> bool:
return self.fallback.type.is_enum

def value_repr(self) -> str:
"""Returns the string representation of the underlying type.

This function is almost equivalent to running `repr(self.value)`,
except it includes some additional logic to correctly handle cases
where the value is a string, byte string, or a unicode string.
where the value is a string, byte string, a unicode string, or an enum.
"""
raw = repr(self.value)
fallback_name = self.fallback.type.fullname()

# If this is backed by an enum,
if self.is_fallback_enum():
return '{}.{}'.format(fallback_name, self.value)

if fallback_name == 'builtins.bytes':
# Note: 'builtins.bytes' only appears in Python 3, so we want to
# explicitly prefix with a "b"
Expand Down
Loading