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.

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

Treat methods with empty bodies in Protocols as abstract #12118

Merged
merged 11 commits into from
Aug 2, 2022
52 changes: 5 additions & 47 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@
CONTRAVARIANT,
COVARIANT,
GDEF,
IMPLICITLY_ABSTRACT,
INVARIANT,
IS_ABSTRACT,
LDEF,
LITERAL_TYPE,
MDEF,
Expand Down Expand Up @@ -115,7 +117,6 @@
ReturnStmt,
StarExpr,
Statement,
StrExpr,
SymbolTable,
SymbolTableNode,
TempNode,
Expand All @@ -134,7 +135,7 @@
from mypy.plugin import CheckerPluginInterface, Plugin
from mypy.sametypes import is_same_type
from mypy.scope import Scope
from mypy.semanal import refers_to_fullname, set_callable_name
from mypy.semanal import is_trivial_body, refers_to_fullname, set_callable_name
from mypy.semanal_enum import ENUM_BASES, ENUM_SPECIAL_PROPS
from mypy.sharedparse import BINARY_MAGIC_METHODS
from mypy.state import state
Expand Down Expand Up @@ -618,7 +619,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
for fdef in defn.items:
assert isinstance(fdef, Decorator)
self.check_func_item(fdef.func, name=fdef.func.name)
if fdef.func.is_abstract:
if fdef.func.abstract_status in (IS_ABSTRACT, IMPLICITLY_ABSTRACT):
num_abstract += 1
if num_abstract not in (0, len(defn.items)):
self.fail(message_registry.INCONSISTENT_ABSTRACT_OVERLOAD, defn)
Expand Down Expand Up @@ -1171,7 +1172,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
item.arguments[i].variable.type = arg_type

# Type check initialization expressions.
body_is_trivial = self.is_trivial_body(defn.body)
body_is_trivial = is_trivial_body(defn.body)
self.check_default_args(item, body_is_trivial)

# Type check body in a new scope.
Expand Down Expand Up @@ -1339,49 +1340,6 @@ def check___new___signature(self, fdef: FuncDef, typ: CallableType) -> None:
"but must return a subtype of",
)

def is_trivial_body(self, block: Block) -> bool:
"""Returns 'true' if the given body is "trivial" -- if it contains just a "pass",
"..." (ellipsis), or "raise NotImplementedError()". A trivial body may also
start with a statement containing just a string (e.g. a docstring).

Note: functions that raise other kinds of exceptions do not count as
"trivial". We use this function to help us determine when it's ok to
relax certain checks on body, but functions that raise arbitrary exceptions
are more likely to do non-trivial work. For example:

def halt(self, reason: str = ...) -> NoReturn:
raise MyCustomError("Fatal error: " + reason, self.line, self.context)

A function that raises just NotImplementedError is much less likely to be
this complex.
"""
body = block.body

# Skip a docstring
if body and isinstance(body[0], ExpressionStmt) and isinstance(body[0].expr, StrExpr):
body = block.body[1:]

if len(body) == 0:
# There's only a docstring (or no body at all).
return True
elif len(body) > 1:
return False

stmt = body[0]

if isinstance(stmt, RaiseStmt):
expr = stmt.expr
if expr is None:
return False
if isinstance(expr, CallExpr):
expr = expr.callee

return isinstance(expr, NameExpr) and expr.fullname == "builtins.NotImplementedError"

return isinstance(stmt, PassStmt) or (
isinstance(stmt, ExpressionStmt) and isinstance(stmt.expr, EllipsisExpr)
)

def check_reverse_op_method(
self, defn: FuncItem, reverse_type: CallableType, reverse_name: str, context: Context
) -> None:
Expand Down
52 changes: 43 additions & 9 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
ARG_POS,
ARG_STAR,
ARG_STAR2,
IMPLICITLY_ABSTRACT,
LITERAL_TYPE,
REVEAL_TYPE,
ArgKind,
Expand Down Expand Up @@ -96,6 +97,7 @@
)
from mypy.sametypes import is_same_type
from mypy.semanal_enum import ENUM_BASES
from mypy.state import state
from mypy.subtypes import is_equivalent, is_subtype, non_method_protocol_members
from mypy.traverser import has_await_expression
from mypy.typeanal import (
Expand Down Expand Up @@ -1236,24 +1238,32 @@ def check_callable_call(

if (
callee.is_type_obj()
and callee.type_object().is_abstract
and callee.type_object().is_protocol
# Exception for Type[...]
and not callee.from_type_type
and not callee.type_object().fallback_to_any
):
type = callee.type_object()
self.msg.cannot_instantiate_abstract_class(
callee.type_object().name, type.abstract_attributes, context
self.chk.fail(
message_registry.CANNOT_INSTANTIATE_PROTOCOL.format(callee.type_object().name),
context,
)
elif (
callee.is_type_obj()
and callee.type_object().is_protocol
and callee.type_object().is_abstract
# Exception for Type[...]
and not callee.from_type_type
and not callee.type_object().fallback_to_any
):
self.chk.fail(
message_registry.CANNOT_INSTANTIATE_PROTOCOL.format(callee.type_object().name),
context,
type = callee.type_object()
# Determine whether the implicitly abstract attributes are functions with
# None-compatible return types.
abstract_attributes: Dict[str, bool] = {}
for attr_name, abstract_status in type.abstract_attributes:
if abstract_status == IMPLICITLY_ABSTRACT:
abstract_attributes[attr_name] = self.can_return_none(type, attr_name)
else:
abstract_attributes[attr_name] = False
self.msg.cannot_instantiate_abstract_class(
callee.type_object().name, abstract_attributes, context
)

formal_to_actual = map_actuals_to_formals(
Expand Down Expand Up @@ -1335,6 +1345,30 @@ def check_callable_call(
callee = callee.copy_modified(ret_type=new_ret_type)
return callee.ret_type, callee

def can_return_none(self, type: TypeInfo, attr_name: str) -> bool:
"""Is the given attribute a method with a None-compatible return type?

Overloads are only checked if there is an implementation.
"""
if not state.strict_optional:
# If strict-optional is not set, is_subtype(NoneType(), T) is always True.
# So, we cannot do anything useful here in that case.
return False
for base in type.mro:
symnode = base.names.get(attr_name)
if symnode is None:
continue
node = symnode.node
if isinstance(node, OverloadedFuncDef):
node = node.impl
if isinstance(node, Decorator):
node = node.func
if isinstance(node, FuncDef):
if node.type is not None:
assert isinstance(node.type, CallableType)
return is_subtype(NoneType(), node.type.ret_type)
return False

def analyze_type_type_callee(self, item: ProperType, context: Context) -> Type:
"""Analyze the callee X in X(...) where X is Type[item].

Expand Down
20 changes: 19 additions & 1 deletion mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ def incompatible_conditional_function_def(self, defn: FuncDef) -> None:
self.fail("All conditional function variants must have identical " "signatures", defn)

def cannot_instantiate_abstract_class(
self, class_name: str, abstract_attributes: List[str], context: Context
self, class_name: str, abstract_attributes: Dict[str, bool], context: Context
) -> None:
attrs = format_string_list([f'"{a}"' for a in abstract_attributes])
self.fail(
Expand All @@ -1330,6 +1330,24 @@ def cannot_instantiate_abstract_class(
context,
code=codes.ABSTRACT,
)
attrs_with_none = [
f'"{a}"'
for a, implicit_and_can_return_none in abstract_attributes.items()
if implicit_and_can_return_none
]
if not attrs_with_none:
return
if len(attrs_with_none) == 1:
note = (
"The following method was marked implicitly abstract because it has an empty "
"function body: {}. If it is not meant to be abstract, explicitly return None."
)
else:
note = (
"The following methods were marked implicitly abstract because they have empty "
"function bodies: {}. If they are not meant to be abstract, explicitly return None."
)
self.note(note.format(format_string_list(attrs_with_none)), context, code=codes.ABSTRACT)

def base_class_definitions_incompatible(
self, name: str, base1: TypeInfo, base2: TypeInfo, context: Context
Expand Down
21 changes: 16 additions & 5 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,14 @@ def is_dynamic(self) -> bool:
return self.type is None


FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional", "is_abstract"]
FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional"]

# Abstract status of a function
NOT_ABSTRACT: Final = 0
# Explicitly abstract (with @abstractmethod or overload without implementation)
IS_ABSTRACT: Final = 1
# Implicitly abstract: used for functions with trivial bodies defined in Protocols
IMPLICITLY_ABSTRACT: Final = 2
tmke8 marked this conversation as resolved.
Show resolved Hide resolved


class FuncDef(FuncItem, SymbolNode, Statement):
Expand All @@ -771,7 +778,7 @@ class FuncDef(FuncItem, SymbolNode, Statement):
"_name",
"is_decorated",
"is_conditional",
"is_abstract",
"abstract_status",
"original_def",
"deco_line",
)
Expand All @@ -788,7 +795,7 @@ def __init__(
self._name = name
self.is_decorated = False
self.is_conditional = False # Defined conditionally (within block)?
self.is_abstract = False
self.abstract_status = NOT_ABSTRACT
self.is_final = False
# Original conditional definition
self.original_def: Union[None, FuncDef, Var, Decorator] = None
Expand Down Expand Up @@ -817,6 +824,7 @@ def serialize(self) -> JsonDict:
"arg_kinds": [int(x.value) for x in self.arg_kinds],
"type": None if self.type is None else self.type.serialize(),
"flags": get_flags(self, FUNCDEF_FLAGS),
"abstract_status": self.abstract_status,
# TODO: Do we need expanded, original_def?
}

Expand All @@ -839,6 +847,7 @@ def deserialize(cls, data: JsonDict) -> "FuncDef":
# NOTE: ret.info is set in the fixup phase.
ret.arg_names = data["arg_names"]
ret.arg_kinds = [ArgKind(x) for x in data["arg_kinds"]]
ret.abstract_status = data["abstract_status"]
# Leave these uninitialized so that future uses will trigger an error
del ret.arguments
del ret.max_pos
Expand Down Expand Up @@ -2674,7 +2683,9 @@ class is generic then it will be a type constructor of higher kind.
is_abstract: bool # Does the class have any abstract attributes?
is_protocol: bool # Is this a protocol class?
runtime_protocol: bool # Does this protocol support isinstance checks?
abstract_attributes: List[str]
# List of names of abstract attributes together with their abstract status.
# The abstract status must be one of `NOT_ABSTRACT`, `IS_ABSTRACT`, `IMPLICITLY_ABSTRACT`.
abstract_attributes: List[Tuple[str, int]]
tmke8 marked this conversation as resolved.
Show resolved Hide resolved
deletable_attributes: List[str] # Used by mypyc only
# Does this type have concrete `__slots__` defined?
# If class does not have `__slots__` defined then it is `None`,
Expand Down Expand Up @@ -3034,7 +3045,7 @@ def deserialize(cls, data: JsonDict) -> "TypeInfo":
ti = TypeInfo(names, defn, module_name)
ti._fullname = data["fullname"]
# TODO: Is there a reason to reconstruct ti.subtypes?
ti.abstract_attributes = data["abstract_attributes"]
ti.abstract_attributes = [(attr[0], attr[1]) for attr in data["abstract_attributes"]]
ti.type_vars = data["type_vars"]
ti.has_param_spec_type = data["has_param_spec_type"]
ti.bases = [mypy.types.Instance.deserialize(b) for b in data["bases"]]
Expand Down
Loading