Skip to content

Commit

Permalink
Improve error messages related to literal types (#6149)
Browse files Browse the repository at this point in the history
This pull request improves how we handle error messages with Literal types in
three different ways:

1. When the user tries constructing types like `Literal[3 + a]`, we now report
   an error message that says 'Invalid type: Literal[...] cannot contain
   arbitrary expressions'.

2. We no longer recommend using Literal[...] when doing `A = NewType('A', 4)`
   or `T = TypeVar('T', bound=4)`. This resolves 
   #5989.

   (The former suggestion is a bad one: you can't create a NewType of a
   Literal[...] type. The latter suggestion is a valid but stupid one: 
   `T = TypeVar('T', bound=Literal[4])` is basically the same thing as 
   `T = Literal[4]`.)

3. When the user tries using complex numbers inside Literals (e.g. 
  `Literal[3j]`), we now report an error message that says 'Parameter 1 
   of Literal[...] cannot be of type "complex"'. This is the same kind of 
   error message we previously used to report when the user tried using 
   floats inside of literals.

In order to accomplish bullet point 1, moved the "invalid type comment or
annotation" checks from the parsing layer to the semantic analysis layer.
This lets us customize which error message we report depending on whether
or not the invalid type appears in the context of a Literal[...] type.

In order to accomplish this, I repurposed RawLiteralType so it can represent
any arbitrary expression that does not convert directly into a type (and
renamed 'RawLiteralType' to 'RawExpressionType' to better reflect this new
usage). I also added an optional "note" field to that class: this lets the
parsing layer attach some extra context that would be difficult to obtain up
in the semantic analysis layer.

In order to accomplish bullet point 2, I modified the type analyzer so that
the caller can optionally suppress the error messages that would otherwise be
generated when a RawExpressionType appears outside of a Literal context.

Bullet point 3 only required a minor tweak to the parsing and error handling
code.
  • Loading branch information
Michael0x2a authored Jan 7, 2019
1 parent 80ca871 commit fd048ab
Show file tree
Hide file tree
Showing 15 changed files with 247 additions and 168 deletions.
27 changes: 15 additions & 12 deletions mypy/exprtotype.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

from mypy.nodes import (
Expression, NameExpr, MemberExpr, IndexExpr, TupleExpr, IntExpr, FloatExpr, UnaryExpr,
ListExpr, StrExpr, BytesExpr, UnicodeExpr, EllipsisExpr, CallExpr,
ComplexExpr, ListExpr, StrExpr, BytesExpr, UnicodeExpr, EllipsisExpr, CallExpr,
get_member_expr_fullname
)
from mypy.fastparse import parse_type_string
from mypy.types import (
Type, UnboundType, TypeList, EllipsisType, AnyType, Optional, CallableArgument, TypeOfAny,
RawLiteralType,
RawExpressionType,
)


Expand Down Expand Up @@ -39,9 +39,9 @@ def expr_to_unanalyzed_type(expr: Expression, _parent: Optional[Expression] = No
if isinstance(expr, NameExpr):
name = expr.name
if name == 'True':
return RawLiteralType(True, 'builtins.bool', line=expr.line, column=expr.column)
return RawExpressionType(True, 'builtins.bool', line=expr.line, column=expr.column)
elif name == 'False':
return RawLiteralType(False, 'builtins.bool', line=expr.line, column=expr.column)
return RawExpressionType(False, 'builtins.bool', line=expr.line, column=expr.column)
else:
return UnboundType(name, line=expr.line, column=expr.column)
elif isinstance(expr, MemberExpr):
Expand Down Expand Up @@ -122,17 +122,20 @@ def expr_to_unanalyzed_type(expr: Expression, _parent: Optional[Expression] = No
assume_str_is_unicode=True)
elif isinstance(expr, UnaryExpr):
typ = expr_to_unanalyzed_type(expr.expr)
if isinstance(typ, RawLiteralType) and isinstance(typ.value, int) and expr.op == '-':
typ.value *= -1
return typ
else:
raise TypeTranslationError()
if isinstance(typ, RawExpressionType):
if isinstance(typ.literal_value, int) and expr.op == '-':
typ.literal_value *= -1
return typ
raise TypeTranslationError()
elif isinstance(expr, IntExpr):
return RawLiteralType(expr.value, 'builtins.int', line=expr.line, column=expr.column)
return RawExpressionType(expr.value, 'builtins.int', line=expr.line, column=expr.column)
elif isinstance(expr, FloatExpr):
# Floats are not valid parameters for RawLiteralType, so we just
# Floats are not valid parameters for RawExpressionType , so we just
# pass in 'None' for now. We'll report the appropriate error at a later stage.
return RawLiteralType(None, 'builtins.float', line=expr.line, column=expr.column)
return RawExpressionType(None, 'builtins.float', line=expr.line, column=expr.column)
elif isinstance(expr, ComplexExpr):
# Same thing as above with complex numbers.
return RawExpressionType(None, 'builtins.complex', line=expr.line, column=expr.column)
elif isinstance(expr, EllipsisExpr):
return EllipsisType(expr.line)
else:
Expand Down
79 changes: 47 additions & 32 deletions mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
)
from mypy.types import (
Type, CallableType, AnyType, UnboundType, TupleType, TypeList, EllipsisType, CallableArgument,
TypeOfAny, Instance, RawLiteralType,
TypeOfAny, Instance, RawExpressionType,
)
from mypy import defaults
from mypy import messages
Expand Down Expand Up @@ -83,7 +83,6 @@
_dummy_fallback = Instance(MISSING_FALLBACK, [], -1) # type: Final

TYPE_COMMENT_SYNTAX_ERROR = 'syntax error in type comment' # type: Final
TYPE_COMMENT_AST_ERROR = 'invalid type comment or annotation' # type: Final


# Older versions of typing don't allow using overload outside stubs,
Expand Down Expand Up @@ -184,11 +183,11 @@ def parse_type_string(expr_string: str, expr_fallback_name: str,
node.original_str_fallback = expr_fallback_name
return node
else:
return RawLiteralType(expr_string, expr_fallback_name, line, column)
return RawExpressionType(expr_string, expr_fallback_name, line, column)
except (SyntaxError, ValueError):
# Note: the parser will raise a `ValueError` instead of a SyntaxError if
# the string happens to contain things like \x00.
return RawLiteralType(expr_string, expr_fallback_name, line, column)
return RawExpressionType(expr_string, expr_fallback_name, line, column)


def is_no_type_check_decorator(expr: ast3.expr) -> bool:
Expand Down Expand Up @@ -1069,6 +1068,24 @@ def __init__(self,
self.node_stack = [] # type: List[AST]
self.assume_str_is_unicode = assume_str_is_unicode

def invalid_type(self, node: AST, note: Optional[str] = None) -> RawExpressionType:
"""Constructs a type representing some expression that normally forms an invalid type.
For example, if we see a type hint that says "3 + 4", we would transform that
expression into a RawExpressionType.
The semantic analysis layer will report an "Invalid type" error when it
encounters this type, along with the given note if one is provided.
See RawExpressionType's docstring for more details on how it's used.
"""
return RawExpressionType(
None,
'typing.Any',
line=self.line,
column=getattr(node, 'col_offset', -1),
note=note,
)

@overload
def visit(self, node: ast3.expr) -> Type: ...

Expand All @@ -1086,8 +1103,7 @@ def visit(self, node: Optional[AST]) -> Optional[Type]: # noqa
if visitor is not None:
return visitor(node)
else:
self.fail(TYPE_COMMENT_AST_ERROR, self.line, getattr(node, 'col_offset', -1))
return AnyType(TypeOfAny.from_error)
return self.invalid_type(node)
finally:
self.node_stack.pop()

Expand Down Expand Up @@ -1124,12 +1140,10 @@ def visit_Call(self, e: Call) -> Type:
constructor = stringify_name(f)

if not isinstance(self.parent(), ast3.List):
self.fail(TYPE_COMMENT_AST_ERROR, self.line, e.col_offset)
note = None
if constructor:
self.note("Suggestion: use {}[...] instead of {}(...)".format(
constructor, constructor),
self.line, e.col_offset)
return AnyType(TypeOfAny.from_error)
note = "Suggestion: use {0}[...] instead of {0}(...)".format(constructor)
return self.invalid_type(e, note=note)
if not constructor:
self.fail("Expected arg constructor name", e.lineno, e.col_offset)

Expand Down Expand Up @@ -1183,7 +1197,7 @@ def visit_Name(self, n: Name) -> Type:

def visit_NameConstant(self, n: NameConstant) -> Type:
if isinstance(n.value, bool):
return RawLiteralType(n.value, 'builtins.bool', line=self.line)
return RawExpressionType(n.value, 'builtins.bool', line=self.line)
else:
return UnboundType(str(n.value), line=self.line)

Expand All @@ -1192,26 +1206,29 @@ def visit_UnaryOp(self, n: UnaryOp) -> Type:
# We support specifically Literal[-4] and nothing else.
# For example, Literal[+4] or Literal[~6] is not supported.
typ = self.visit(n.operand)
if isinstance(typ, RawLiteralType) and isinstance(n.op, USub):
if isinstance(typ.value, int):
typ.value *= -1
if isinstance(typ, RawExpressionType) and isinstance(n.op, USub):
if isinstance(typ.literal_value, int):
typ.literal_value *= -1
return typ
self.fail(TYPE_COMMENT_AST_ERROR, self.line, getattr(n, 'col_offset', -1))
return AnyType(TypeOfAny.from_error)
return self.invalid_type(n)

# Num(number n)
def visit_Num(self, n: Num) -> Type:
# Could be either float or int
numeric_value = n.n
if isinstance(numeric_value, int):
return RawLiteralType(numeric_value, 'builtins.int', line=self.line)
elif isinstance(numeric_value, float):
# Floats and other numbers are not valid parameters for RawLiteralType, so we just
# pass in 'None' for now. We'll report the appropriate error at a later stage.
return RawLiteralType(None, 'builtins.float', line=self.line)
if isinstance(n.n, int):
numeric_value = n.n
type_name = 'builtins.int'
else:
self.fail(TYPE_COMMENT_AST_ERROR, self.line, getattr(n, 'col_offset', -1))
return AnyType(TypeOfAny.from_error)
# Other kinds of numbers (floats, complex) are not valid parameters for
# RawExpressionType so we just pass in 'None' for now. We'll report the
# appropriate error at a later stage.
numeric_value = None
type_name = 'builtins.{}'.format(type(n.n).__name__)
return RawExpressionType(
numeric_value,
type_name,
line=self.line,
column=getattr(n, 'col_offset', -1),
)

# Str(string s)
def visit_Str(self, n: Str) -> Type:
Expand All @@ -1230,7 +1247,7 @@ def visit_Str(self, n: Str) -> Type:
# Bytes(bytes s)
def visit_Bytes(self, n: Bytes) -> Type:
contents = bytes_to_human_readable_repr(n.s)
return RawLiteralType(contents, 'builtins.bytes', self.line, column=n.col_offset)
return RawExpressionType(contents, 'builtins.bytes', self.line, column=n.col_offset)

# Subscript(expr value, slice slice, expr_context ctx)
def visit_Subscript(self, n: ast3.Subscript) -> Type:
Expand All @@ -1251,8 +1268,7 @@ def visit_Subscript(self, n: ast3.Subscript) -> Type:
return UnboundType(value.name, params, line=self.line,
empty_tuple_index=empty_tuple_index)
else:
self.fail(TYPE_COMMENT_AST_ERROR, self.line, getattr(n, 'col_offset', -1))
return AnyType(TypeOfAny.from_error)
return self.invalid_type(n)

def visit_Tuple(self, n: ast3.Tuple) -> Type:
return TupleType(self.translate_expr_list(n.elts), _dummy_fallback,
Expand All @@ -1265,8 +1281,7 @@ def visit_Attribute(self, n: Attribute) -> Type:
if isinstance(before_dot, UnboundType) and not before_dot.args:
return UnboundType("{}.{}".format(before_dot.name, n.attr), line=self.line)
else:
self.fail(TYPE_COMMENT_AST_ERROR, self.line, getattr(n, 'col_offset', -1))
return AnyType(TypeOfAny.from_error)
return self.invalid_type(n)

# Ellipsis
def visit_Ellipsis(self, n: ast3_Ellipsis) -> Type:
Expand Down
4 changes: 2 additions & 2 deletions mypy/indirection.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ def visit_tuple_type(self, t: types.TupleType) -> Set[str]:
def visit_typeddict_type(self, t: types.TypedDictType) -> Set[str]:
return self._visit(t.items.values()) | self._visit(t.fallback)

def visit_raw_literal_type(self, t: types.RawLiteralType) -> Set[str]:
assert False, "Unexpected RawLiteralType after semantic analysis phase"
def visit_raw_expression_type(self, t: types.RawExpressionType) -> Set[str]:
assert False, "Unexpected RawExpressionType after semantic analysis phase"

def visit_literal_type(self, t: types.LiteralType) -> Set[str]:
return self._visit(t.fallback)
Expand Down
1 change: 1 addition & 0 deletions mypy/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ def anal_type(self, t: Type, *,
tvar_scope: Optional[TypeVarScope] = None,
allow_tuple_literal: bool = False,
allow_unbound_tvars: bool = False,
report_invalid_types: bool = True,
third_pass: bool = False) -> Type:
"""Analyze an unbound type."""
raise NotImplementedError
Expand Down
17 changes: 14 additions & 3 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ def update_metaclass(self, defn: ClassDef) -> None:
return
defn.metaclass = metas.pop()

def expr_to_analyzed_type(self, expr: Expression) -> Type:
def expr_to_analyzed_type(self, expr: Expression, report_invalid_types: bool = True) -> Type:
if isinstance(expr, CallExpr):
expr.accept(self)
info = self.named_tuple_analyzer.check_namedtuple(expr, None, self.is_func_scope())
Expand All @@ -1295,7 +1295,7 @@ def expr_to_analyzed_type(self, expr: Expression) -> Type:
fallback = Instance(info, [])
return TupleType(info.tuple_type.items, fallback=fallback)
typ = expr_to_unanalyzed_type(expr)
return self.anal_type(typ)
return self.anal_type(typ, report_invalid_types=report_invalid_types)

def verify_base_classes(self, defn: ClassDef) -> bool:
info = defn.info
Expand Down Expand Up @@ -1686,6 +1686,7 @@ def type_analyzer(self, *,
tvar_scope: Optional[TypeVarScope] = None,
allow_tuple_literal: bool = False,
allow_unbound_tvars: bool = False,
report_invalid_types: bool = True,
third_pass: bool = False) -> TypeAnalyser:
if tvar_scope is None:
tvar_scope = self.tvar_scope
Expand All @@ -1696,6 +1697,7 @@ def type_analyzer(self, *,
self.is_typeshed_stub_file,
allow_unbound_tvars=allow_unbound_tvars,
allow_tuple_literal=allow_tuple_literal,
report_invalid_types=report_invalid_types,
allow_unnormalized=self.is_stub_file,
third_pass=third_pass)
tpan.in_dynamic_func = bool(self.function_stack and self.function_stack[-1].is_dynamic())
Expand All @@ -1706,10 +1708,12 @@ def anal_type(self, t: Type, *,
tvar_scope: Optional[TypeVarScope] = None,
allow_tuple_literal: bool = False,
allow_unbound_tvars: bool = False,
report_invalid_types: bool = True,
third_pass: bool = False) -> Type:
a = self.type_analyzer(tvar_scope=tvar_scope,
allow_unbound_tvars=allow_unbound_tvars,
allow_tuple_literal=allow_tuple_literal,
report_invalid_types=report_invalid_types,
third_pass=third_pass)
typ = t.accept(a)
self.add_type_alias_deps(a.aliases_used)
Expand Down Expand Up @@ -2394,7 +2398,14 @@ def process_typevar_parameters(self, args: List[Expression],
self.fail("TypeVar cannot have both values and an upper bound", context)
return None
try:
upper_bound = self.expr_to_analyzed_type(param_value)
# We want to use our custom error message below, so we suppress
# the default error message for invalid types here.
upper_bound = self.expr_to_analyzed_type(param_value,
report_invalid_types=False)
if isinstance(upper_bound, AnyType) and upper_bound.is_from_error:
self.fail("TypeVar 'bound' must be a type", param_value)
# Note: we do not return 'None' here -- we want to continue
# using the AnyType as the upper bound.
except TypeTranslationError:
self.fail("TypeVar 'bound' must be a type", param_value)
return None
Expand Down
6 changes: 4 additions & 2 deletions mypy/semanal_newtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,13 @@ def check_newtype_args(self, name: str, call: CallExpr, context: Context) -> Opt
self.fail(msg, context)
return None

old_type = self.api.anal_type(unanalyzed_type)
# 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)

# 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.type_of_any == TypeOfAny.from_error:
if isinstance(old_type, AnyType) and old_type.is_from_error:
self.fail(msg, context)
return None

Expand Down
1 change: 1 addition & 0 deletions mypy/semanal_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def anal_type(self, t: Type, *,
tvar_scope: Optional[TypeVarScope] = None,
allow_tuple_literal: bool = False,
allow_unbound_tvars: bool = False,
report_invalid_types: bool = True,
third_pass: bool = False) -> Type:
raise NotImplementedError

Expand Down
6 changes: 3 additions & 3 deletions mypy/server/astmerge.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
Type, SyntheticTypeVisitor, Instance, AnyType, NoneTyp, CallableType, DeletedType, PartialType,
TupleType, TypeType, TypeVarType, TypedDictType, UnboundType, UninhabitedType, UnionType,
Overloaded, TypeVarDef, TypeList, CallableArgument, EllipsisType, StarType, LiteralType,
RawLiteralType,
RawExpressionType,
)
from mypy.util import get_prefix, replace_object_state
from mypy.typestate import TypeState
Expand Down Expand Up @@ -331,7 +331,7 @@ class TypeReplaceVisitor(SyntheticTypeVisitor[None]):
"""Similar to NodeReplaceVisitor, but for type objects.
Note: this visitor may sometimes visit unanalyzed types
such as 'UnboundType' and 'RawLiteralType' For example, see
such as 'UnboundType' and 'RawExpressionType' For example, see
NodeReplaceVisitor.process_base_func.
"""

Expand Down Expand Up @@ -397,7 +397,7 @@ def visit_typeddict_type(self, typ: TypedDictType) -> None:
value_type.accept(self)
typ.fallback.accept(self)

def visit_raw_literal_type(self, t: RawLiteralType) -> None:
def visit_raw_expression_type(self, t: RawExpressionType) -> None:
pass

def visit_literal_type(self, typ: LiteralType) -> None:
Expand Down
6 changes: 3 additions & 3 deletions mypy/type_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

from mypy.types import (
Type, AnyType, CallableType, Overloaded, TupleType, TypedDictType, LiteralType,
RawLiteralType, Instance, NoneTyp, TypeType,
RawExpressionType, Instance, NoneTyp, TypeType,
UnionType, TypeVarType, PartialType, DeletedType, UninhabitedType, TypeVarDef,
UnboundType, ErasedType, ForwardRef, StarType, EllipsisType, TypeList, CallableArgument,
)
Expand Down Expand Up @@ -128,7 +128,7 @@ def visit_ellipsis_type(self, t: EllipsisType) -> T:
pass

@abstractmethod
def visit_raw_literal_type(self, t: RawLiteralType) -> T:
def visit_raw_expression_type(self, t: RawExpressionType) -> T:
pass


Expand Down Expand Up @@ -282,7 +282,7 @@ def visit_tuple_type(self, t: TupleType) -> T:
def visit_typeddict_type(self, t: TypedDictType) -> T:
return self.query_types(t.items.values())

def visit_raw_literal_type(self, t: RawLiteralType) -> T:
def visit_raw_expression_type(self, t: RawExpressionType) -> T:
return self.strategy([])

def visit_literal_type(self, t: LiteralType) -> T:
Expand Down
Loading

0 comments on commit fd048ab

Please sign in to comment.