Skip to content

Commit

Permalink
Fix crash on star unpacking to underscore (#14624)
Browse files Browse the repository at this point in the history
Fixes #14250

This one is interesting. It looks like most likely this was caused by my
PR #7127 that fixed other crash.
After looking a bit more, `StarType` is something old, and should never
by used. At least I didn't find `visit_star_type()` in any of the type
visitors. Actually mypy already uses `assert False`, if we get to a
non-special-cased star expression.

Btw, I noticed that `pythoneval` test with empty expected output passes
in case of a crash (at least on my machine), so I fix this too.
  • Loading branch information
ilevkivskyi authored Feb 8, 2023
1 parent f6a8037 commit 891e035
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 79 deletions.
8 changes: 3 additions & 5 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@
Overloaded,
PartialType,
ProperType,
StarType,
TupleType,
Type,
TypeAliasType,
Expand Down Expand Up @@ -3288,7 +3287,7 @@ def check_assignment_to_multiple_lvalues(
last_idx: int | None = None
for idx_rval, rval in enumerate(rvalue.items):
if isinstance(rval, StarExpr):
typs = get_proper_type(self.expr_checker.visit_star_expr(rval).type)
typs = get_proper_type(self.expr_checker.accept(rval.expr))
if isinstance(typs, TupleType):
rvalues.extend([TempNode(typ) for typ in typs.items])
elif self.type_is_iterable(typs) and isinstance(typs, Instance):
Expand All @@ -3311,7 +3310,7 @@ def check_assignment_to_multiple_lvalues(
iterable_end: int | None = None
for i, rval in enumerate(rvalues):
if isinstance(rval, StarExpr):
typs = get_proper_type(self.expr_checker.visit_star_expr(rval).type)
typs = get_proper_type(self.expr_checker.accept(rval.expr))
if self.type_is_iterable(typs) and isinstance(typs, Instance):
if iterable_start is None:
iterable_start = i
Expand Down Expand Up @@ -3674,8 +3673,7 @@ def check_lvalue(self, lvalue: Lvalue) -> tuple[Type | None, IndexExpr | None, V
]
lvalue_type = TupleType(types, self.named_type("builtins.tuple"))
elif isinstance(lvalue, StarExpr):
typ, _, _ = self.check_lvalue(lvalue.expr)
lvalue_type = StarType(typ) if typ else None
lvalue_type, _, _ = self.check_lvalue(lvalue.expr)
else:
lvalue_type = self.expr_checker.accept(lvalue)

Expand Down
6 changes: 3 additions & 3 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@
ParamSpecType,
PartialType,
ProperType,
StarType,
TupleType,
Type,
TypeAliasType,
Expand Down Expand Up @@ -5160,8 +5159,9 @@ def visit_typeddict_expr(self, e: TypedDictExpr) -> Type:
def visit__promote_expr(self, e: PromoteExpr) -> Type:
return e.type

def visit_star_expr(self, e: StarExpr) -> StarType:
return StarType(self.accept(e.expr))
def visit_star_expr(self, e: StarExpr) -> Type:
# TODO: should this ever be called (see e.g. mypyc visitor)?
return self.accept(e.expr)

def object_type(self) -> Instance:
"""Return instance type 'object'."""
Expand Down
8 changes: 1 addition & 7 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@
ParamSpecType,
PlaceholderType,
ProperType,
StarType,
TrivialSyntheticTypeTranslator,
TupleType,
Type,
Expand Down Expand Up @@ -3873,8 +3872,6 @@ def check_lvalue_validity(self, node: Expression | SymbolNode | None, ctx: Conte
self.fail(message_registry.CANNOT_ASSIGN_TO_TYPE, ctx)

def store_declared_types(self, lvalue: Lvalue, typ: Type) -> None:
if isinstance(typ, StarType) and not isinstance(lvalue, StarExpr):
self.fail("Star type only allowed for starred expressions", lvalue)
if isinstance(lvalue, RefExpr):
lvalue.is_inferred_def = False
if isinstance(lvalue.node, Var):
Expand Down Expand Up @@ -3902,10 +3899,7 @@ def store_declared_types(self, lvalue: Lvalue, typ: Type) -> None:
self.fail("Tuple type expected for multiple variables", lvalue)
elif isinstance(lvalue, StarExpr):
# Historical behavior for the old parser
if isinstance(typ, StarType):
self.store_declared_types(lvalue.expr, typ.type)
else:
self.store_declared_types(lvalue.expr, typ)
self.store_declared_types(lvalue.expr, typ)
else:
# This has been flagged elsewhere as an error, so just ignore here.
pass
Expand Down
4 changes: 0 additions & 4 deletions mypy/server/astmerge.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
PartialType,
PlaceholderType,
RawExpressionType,
StarType,
SyntheticTypeVisitor,
TupleType,
Type,
Expand Down Expand Up @@ -519,9 +518,6 @@ def visit_callable_argument(self, typ: CallableArgument) -> None:
def visit_ellipsis_type(self, typ: EllipsisType) -> None:
pass

def visit_star_type(self, typ: StarType) -> None:
typ.type.accept(self)

def visit_uninhabited_type(self, typ: UninhabitedType) -> None:
pass

Expand Down
4 changes: 4 additions & 0 deletions mypy/test/testpythoneval.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ def test_python_evaluation(testcase: DataDrivenTestCase, cache_dir: str) -> None
# Normalize paths so that the output is the same on Windows and Linux/macOS.
line = line.replace(test_temp_dir + os.sep, test_temp_dir + "/")
output.append(line.rstrip("\r\n"))
if returncode > 1 and not testcase.output:
# Either api.run() doesn't work well in case of a crash, or pytest interferes with it.
# Tweak output to prevent tests with empty expected output to pass in case of a crash.
output.append("!!! Mypy crashed !!!")
if returncode == 0 and not output:
# Execute the program.
proc = subprocess.run(
Expand Down
14 changes: 2 additions & 12 deletions mypy/type_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
PartialType,
PlaceholderType,
RawExpressionType,
StarType,
TupleType,
Type,
TypeAliasType,
Expand Down Expand Up @@ -153,11 +152,8 @@ def visit_unpack_type(self, t: UnpackType) -> T:
class SyntheticTypeVisitor(TypeVisitor[T]):
"""A TypeVisitor that also knows how to visit synthetic AST constructs.
Not just real types."""

@abstractmethod
def visit_star_type(self, t: StarType) -> T:
pass
Not just real types.
"""

@abstractmethod
def visit_type_list(self, t: TypeList) -> T:
Expand Down Expand Up @@ -386,9 +382,6 @@ def visit_raw_expression_type(self, t: RawExpressionType) -> T:
def visit_literal_type(self, t: LiteralType) -> T:
return self.strategy([])

def visit_star_type(self, t: StarType) -> T:
return t.type.accept(self)

def visit_union_type(self, t: UnionType) -> T:
return self.query_types(t.items)

Expand Down Expand Up @@ -529,9 +522,6 @@ def visit_raw_expression_type(self, t: RawExpressionType) -> bool:
def visit_literal_type(self, t: LiteralType) -> bool:
return self.default

def visit_star_type(self, t: StarType) -> bool:
return t.type.accept(self)

def visit_union_type(self, t: UnionType) -> bool:
return self.query_types(t.items)

Expand Down
16 changes: 1 addition & 15 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
PlaceholderType,
RawExpressionType,
RequiredType,
StarType,
SyntheticTypeVisitor,
TrivialSyntheticTypeTranslator,
TupleType,
Expand Down Expand Up @@ -1031,17 +1030,7 @@ def visit_tuple_type(self, t: TupleType) -> Type:
code=codes.SYNTAX,
)
return AnyType(TypeOfAny.from_error)
star_count = sum(1 for item in t.items if isinstance(item, StarType))
if star_count > 1:
self.fail("At most one star type allowed in a tuple", t)
if t.implicit:
return TupleType(
[AnyType(TypeOfAny.from_error) for _ in t.items],
self.named_type("builtins.tuple"),
t.line,
)
else:
return AnyType(TypeOfAny.from_error)

any_type = AnyType(TypeOfAny.special_form)
# If the fallback isn't filled in yet, its type will be the falsey FakeInfo
fallback = (
Expand Down Expand Up @@ -1093,9 +1082,6 @@ def visit_raw_expression_type(self, t: RawExpressionType) -> Type:
def visit_literal_type(self, t: LiteralType) -> Type:
return t

def visit_star_type(self, t: StarType) -> Type:
return StarType(self.anal_type(t.type), t.line)

def visit_union_type(self, t: UnionType) -> Type:
if (
t.uses_pep604_syntax is True
Expand Down
29 changes: 0 additions & 29 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2592,28 +2592,6 @@ def is_singleton_type(self) -> bool:
return self.is_enum_literal() or isinstance(self.value, bool)


class StarType(ProperType):
"""The star type *type_parameter.
This is not a real type but a syntactic AST construct.
"""

__slots__ = ("type",)

type: Type

def __init__(self, type: Type, line: int = -1, column: int = -1) -> None:
super().__init__(line, column)
self.type = type

def accept(self, visitor: TypeVisitor[T]) -> T:
assert isinstance(visitor, SyntheticTypeVisitor)
return cast(T, visitor.visit_star_type(self))

def serialize(self) -> JsonDict:
assert False, "Synthetic types don't serialize"


class UnionType(ProperType):
"""The union type Union[T1, ..., Tn] (at least one type argument)."""

Expand Down Expand Up @@ -3185,10 +3163,6 @@ def visit_raw_expression_type(self, t: RawExpressionType) -> str:
def visit_literal_type(self, t: LiteralType) -> str:
return f"Literal[{t.value_repr()}]"

def visit_star_type(self, t: StarType) -> str:
s = t.type.accept(self)
return f"*{s}"

def visit_union_type(self, t: UnionType) -> str:
s = self.list_str(t.items)
return f"Union[{s}]"
Expand Down Expand Up @@ -3245,9 +3219,6 @@ def visit_ellipsis_type(self, t: EllipsisType) -> Type:
def visit_raw_expression_type(self, t: RawExpressionType) -> Type:
return t

def visit_star_type(self, t: StarType) -> Type:
return t

def visit_type_list(self, t: TypeList) -> Type:
return t

Expand Down
4 changes: 0 additions & 4 deletions mypy/typetraverser.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
PartialType,
PlaceholderType,
RawExpressionType,
StarType,
SyntheticTypeVisitor,
TupleType,
Type,
Expand Down Expand Up @@ -115,9 +114,6 @@ def visit_unbound_type(self, t: UnboundType) -> None:
def visit_type_list(self, t: TypeList) -> None:
self.traverse_types(t.items)

def visit_star_type(self, t: StarType) -> None:
t.type.accept(self)

def visit_ellipsis_type(self, t: EllipsisType) -> None:
pass

Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/pythoneval.test
Original file line number Diff line number Diff line change
Expand Up @@ -1901,3 +1901,26 @@ a[x]
_testNativeIntTypes.py:14: error: Unsupported operand types for + ("i64" and "i32")
_testNativeIntTypes.py:15: note: Revealed type is "mypy_extensions.i64"
_testNativeIntTypes.py:16: note: Revealed type is "mypy_extensions.i64"

[case testStarUnpackNestedUnderscore]
from typing import Tuple, Dict, List

def crash() -> None:
d: Dict[int, Tuple[str, int, str]] = {}
k, (v1, *_) = next(iter(d.items()))

def test1() -> None:
vs: List[str]
d: Dict[int, Tuple[str, int, int]] = {}
k, (v1, *vs) = next(iter(d.items()))
reveal_type(vs)

def test2() -> None:
d: Dict[int, Tuple[str, int, str]] = {}
k, (v1, *vs) = next(iter(d.items()))
reveal_type(vs)
[out]
_testStarUnpackNestedUnderscore.py:10: error: List item 0 has incompatible type "int"; expected "str"
_testStarUnpackNestedUnderscore.py:10: error: List item 1 has incompatible type "int"; expected "str"
_testStarUnpackNestedUnderscore.py:11: note: Revealed type is "builtins.list[builtins.str]"
_testStarUnpackNestedUnderscore.py:16: note: Revealed type is "builtins.list[builtins.object]"

0 comments on commit 891e035

Please sign in to comment.