Skip to content

Commit

Permalink
Minor updates to protocol semantics (#3996)
Browse files Browse the repository at this point in the history
This updates protocol semantic according to the discussion in python/typing#464:

    None should be considered a subtype of an empty protocol
    method = None rule should apply only to callable protocol members
    issublcass() is prohibited for protocols with non-method members.

Fixes #3906
Fixes #3938
Fixes #3939
  • Loading branch information
ilevkivskyi authored and gvanrossum committed Sep 29, 2017
1 parent e9d1ed7 commit b3ca169
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 12 deletions.
35 changes: 25 additions & 10 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from mypy import join
from mypy.meet import narrow_declared_type
from mypy.maptype import map_instance_to_supertype
from mypy.subtypes import is_subtype, is_equivalent, find_member
from mypy.subtypes import is_subtype, is_equivalent, find_member, non_method_protocol_members
from mypy import applytype
from mypy import erasetype
from mypy.checkmember import analyze_member_access, type_object_type, bind_self
Expand Down Expand Up @@ -264,22 +264,37 @@ def visit_call_expr(self, e: CallExpr, allow_none_return: bool = False) -> Type:
callee_type = self.apply_method_signature_hook(
e, callee_type, object_type, signature_hook)
ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname, object_type)
if (isinstance(e.callee, RefExpr) and len(e.args) == 2 and
e.callee.fullname in ('builtins.isinstance', 'builtins.issubclass')):
for expr in mypy.checker.flatten(e.args[1]):
tp = self.chk.type_map[expr]
if (isinstance(tp, CallableType) and tp.is_type_obj() and
tp.type_object().is_protocol and
not tp.type_object().runtime_protocol):
self.chk.fail('Only @runtime protocols can be used with'
' instance and class checks', e)
if isinstance(e.callee, RefExpr) and len(e.args) == 2:
if e.callee.fullname in ('builtins.isinstance', 'builtins.issubclass'):
self.check_runtime_protocol_test(e)
if e.callee.fullname == 'builtins.issubclass':
self.check_protocol_issubclass(e)
if isinstance(ret_type, UninhabitedType):
self.chk.binder.unreachable()
if not allow_none_return and isinstance(ret_type, NoneTyp):
self.chk.msg.does_not_return_value(callee_type, e)
return AnyType(TypeOfAny.from_error)
return ret_type

def check_runtime_protocol_test(self, e: CallExpr) -> None:
for expr in mypy.checker.flatten(e.args[1]):
tp = self.chk.type_map[expr]
if (isinstance(tp, CallableType) and tp.is_type_obj() and
tp.type_object().is_protocol and
not tp.type_object().runtime_protocol):
self.chk.fail('Only @runtime protocols can be used with'
' instance and class checks', e)

def check_protocol_issubclass(self, e: CallExpr) -> None:
for expr in mypy.checker.flatten(e.args[1]):
tp = self.chk.type_map[expr]
if (isinstance(tp, CallableType) and tp.is_type_obj() and
tp.type_object().is_protocol):
attr_members = non_method_protocol_members(tp.type_object())
if attr_members:
self.chk.msg.report_non_method_protocol(tp.type_object(),
attr_members, e)

def check_typeddict_call(self, callee: TypedDictType,
arg_kinds: List[int],
arg_names: Sequence[Optional[str]],
Expand Down
2 changes: 2 additions & 0 deletions mypy/meet.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ def narrow_declared_type(declared: Type, narrowed: Type) -> Type:
return narrowed
elif isinstance(declared, (Instance, TupleType)):
return meet_types(declared, narrowed)
elif isinstance(declared, TypeType) and isinstance(narrowed, TypeType):
return TypeType.make_normalized(narrow_declared_type(declared.item, narrowed.item))
return narrowed


Expand Down
9 changes: 9 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,15 @@ def concrete_only_call(self, typ: Type, context: Context) -> None:
self.fail("Only concrete class can be given where {} is expected"
.format(self.format(typ)), context)

def report_non_method_protocol(self, tp: TypeInfo, members: List[str],
context: Context) -> None:
self.fail("Only protocols that don't have non-method members can be"
" used with issubclass()", context)
if len(members) < 3:
attrs = ', '.join(members)
self.note('Protocol "{}" has non-method member(s): {}'
.format(tp.name(), attrs), context)

def note_call(self, subtype: Type, call: Type, context: Context) -> None:
self.note('"{}.__call__" has type {}'.format(self.format_bare(subtype),
self.format(call, verbosity=1)), context)
Expand Down
21 changes: 19 additions & 2 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ def visit_any(self, left: AnyType) -> bool:
def visit_none_type(self, left: NoneTyp) -> bool:
if experiments.STRICT_OPTIONAL:
return (isinstance(self.right, NoneTyp) or
is_named_instance(self.right, 'builtins.object'))
is_named_instance(self.right, 'builtins.object') or
isinstance(self.right, Instance) and self.right.type.is_protocol and
not self.right.type.protocol_members)
else:
return True

Expand Down Expand Up @@ -386,7 +388,7 @@ def f(self) -> A: ...
is_compat = is_proper_subtype(subtype, supertype)
if not is_compat:
return False
if isinstance(subtype, NoneTyp) and member.startswith('__') and member.endswith('__'):
if isinstance(subtype, NoneTyp) and isinstance(supertype, CallableType):
# We want __hash__ = None idiom to work even without --strict-optional
return False
subflags = get_member_flags(member, left.type)
Expand Down Expand Up @@ -516,6 +518,21 @@ def find_node_type(node: Union[Var, FuncBase], itype: Instance, subtype: Type) -
return typ


def non_method_protocol_members(tp: TypeInfo) -> List[str]:
"""Find all non-callable members of a protocol."""

assert tp.is_protocol
result = [] # type: List[str]
anytype = AnyType(TypeOfAny.special_form)
instance = Instance(tp, [anytype] * len(tp.defn.type_vars))

for member in tp.protocol_members:
typ = find_member(member, instance, instance)
if not isinstance(typ, CallableType):
result.append(member)
return result


def is_callable_subtype(left: CallableType, right: CallableType,
ignore_return: bool = False,
ignore_pos_arg_names: bool = False,
Expand Down
61 changes: 61 additions & 0 deletions test-data/unit/check-protocols.test
Original file line number Diff line number Diff line change
Expand Up @@ -2118,3 +2118,64 @@ main:10: note: def other(self, *args: Any, hint: Optional[str] = ..., **
main:10: note: Got:
main:10: note: def other(self) -> int

[case testObjectAllowedInProtocolBases]
from typing import Protocol
class P(Protocol, object):
pass
[out]

[case testNoneSubtypeOfEmptyProtocol]
from typing import Protocol
class P(Protocol):
pass

x: P = None
[out]

[case testNoneSubtypeOfAllProtocolsWithoutStrictOptional]
from typing import Protocol
class P(Protocol):
attr: int
def meth(self, arg: str) -> str:
pass

x: P = None
[out]

[case testNoneSubtypeOfEmptyProtocolStrict]
# flags: --strict-optional
from typing import Protocol
class P(Protocol):
pass
x: P = None

class PBad(Protocol):
x: int
y: PBad = None # E: Incompatible types in assignment (expression has type "None", variable has type "PBad")
[out]

[case testOnlyMethodProtocolUsableWithIsSubclass]
from typing import Protocol, runtime, Union, Type
@runtime
class P(Protocol):
def meth(self) -> int:
pass
@runtime
class PBad(Protocol):
x: str

class C:
x: str
def meth(self) -> int:
pass
class E: pass

cls: Type[Union[C, E]]
issubclass(cls, PBad) # E: Only protocols that don't have non-method members can be used with issubclass() \
# N: Protocol "PBad" has non-method member(s): x
if issubclass(cls, P):
reveal_type(cls) # E: Revealed type is 'Type[__main__.C]'
else:
reveal_type(cls) # E: Revealed type is 'Type[__main__.E]'
[builtins fixtures/isinstance.pyi]
[out]

0 comments on commit b3ca169

Please sign in to comment.