Skip to content

Commit

Permalink
New semantic analyzer: fix crash on broken overload at class scope (#…
Browse files Browse the repository at this point in the history
…7181)

Fixes #7044

The crash is caused by the fact that `.type` for broken overloads can be `None`. There is another example where we deal with this in `analyze_ref_expr()`. The crash doesn't happen on the old analyzer because it doesn't add the broken overload to the symbol table.

I solve the crash by generating a dummy `Overloaded()` type in this case. An alternative solution would be to re-analyze a broken overload as independent (re-)definitions, but I think this is more error-prone to keep an overload in AST, but a bunch of functions in symbol table.

Using this opportunity I also remove a redundant `add_symbol()` call (we always add the overload before even processing it).

Note that the behavior of the new analyzer is still different from the old one, but IMO it is more consistent in some sense.
  • Loading branch information
ilevkivskyi committed Jul 9, 2019
1 parent dc58be0 commit 438a3e9
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 5 deletions.
2 changes: 2 additions & 0 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type:
elif isinstance(node, OverloadedFuncDef) and node.type is not None:
# node.type is None when there are multiple definitions of a function
# and it's decorated by something that is not typing.overload
# TODO: use a dummy Overloaded instead of AnyType in this case
# like we do in mypy.types.function_type()?
result = node.type
elif isinstance(node, TypeInfo):
# Reference to a type object.
Expand Down
2 changes: 0 additions & 2 deletions mypy/newsemanal/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,6 @@ def analyze_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
self.process_final_in_overload(defn)
self.process_static_or_class_method_in_overload(defn)

self.add_symbol(defn.name(), defn, defn)

def analyze_overload_sigs_and_impl(
self,
defn: OverloadedFuncDef) -> Tuple[List[CallableType],
Expand Down
18 changes: 15 additions & 3 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2177,9 +2177,21 @@ def function_type(func: mypy.nodes.FuncBase, fallback: Instance) -> FunctionLike
return func.type
else:
# Implicit type signature with dynamic types.
# Overloaded functions always have a signature, so func must be an ordinary function.
assert isinstance(func, mypy.nodes.FuncItem), str(func)
return callable_type(func, fallback)
if isinstance(func, mypy.nodes.FuncItem):
return callable_type(func, fallback)
else:
# Broken overloads can have self.type set to None.
# TODO: should we instead always set the type in semantic analyzer?
assert isinstance(func, mypy.nodes.OverloadedFuncDef)
any_type = AnyType(TypeOfAny.from_error)
dummy = CallableType([any_type, any_type],
[ARG_STAR, ARG_STAR2],
[None, None], any_type,
fallback,
line=func.line, is_ellipsis_args=True)
# Return an Overloaded, because some callers may expect that
# an OverloadedFuncDef has an Overloaded type.
return Overloaded([dummy])


def callable_type(fdef: mypy.nodes.FuncItem, fallback: Instance,
Expand Down
19 changes: 19 additions & 0 deletions test-data/unit/check-newsemanal.test
Original file line number Diff line number Diff line change
Expand Up @@ -3113,3 +3113,22 @@ class A: pass
def f() -> None: pass

[targets m, m.f, __main__]

[case testNewAnalyzerNoCrashOnCustomProperty]
# flags: --ignore-missing-imports
from unimported import custom

class User:
first_name: str

@custom
def name(self) -> str:
return self.first_name

@name.setter # type: ignore
def name(self, value: str) -> None:
self.first_name = value

def __init__(self, name: str) -> None:
self.name = name # E: Cannot assign to a method \
# E: Incompatible types in assignment (expression has type "str", variable has type overloaded function)

0 comments on commit 438a3e9

Please sign in to comment.