Skip to content

Cleanup check_reverse_op_method #4017

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

Merged
merged 7 commits into from
Mar 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 48 additions & 48 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,34 @@
import itertools
import fnmatch
from contextlib import contextmanager
import sys

from typing import (
Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator
)

from mypy.errors import Errors, report_internal_error
from mypy.nodes import (
SymbolTable, Statement, MypyFile, Var, Expression, Lvalue,
SymbolTable, Statement, MypyFile, Var, Expression, Lvalue, Node,
OverloadedFuncDef, FuncDef, FuncItem, FuncBase, TypeInfo,
ClassDef, GDEF, Block, AssignmentStmt, NameExpr, MemberExpr, IndexExpr,
ClassDef, Block, AssignmentStmt, NameExpr, MemberExpr, IndexExpr,
TupleExpr, ListExpr, ExpressionStmt, ReturnStmt, IfStmt,
WhileStmt, OperatorAssignmentStmt, WithStmt, AssertStmt,
RaiseStmt, TryStmt, ForStmt, DelStmt, CallExpr, IntExpr, StrExpr,
BytesExpr, UnicodeExpr, FloatExpr, OpExpr, UnaryExpr, CastExpr, RevealTypeExpr, SuperExpr,
TypeApplication, DictExpr, SliceExpr, LambdaExpr, TempNode, SymbolTableNode,
Context, ListComprehension, ConditionalExpr, GeneratorExpr,
Decorator, SetExpr, TypeVarExpr, NewTypeExpr, PrintStmt,
LITERAL_TYPE, BreakStmt, PassStmt, ContinueStmt, ComparisonExpr, StarExpr,
YieldFromExpr, NamedTupleExpr, TypedDictExpr, SetComprehension,
DictionaryComprehension, ComplexExpr, EllipsisExpr, TypeAliasExpr,
RefExpr, YieldExpr, BackquoteExpr, Import, ImportFrom, ImportAll, ImportBase,
AwaitExpr, PromoteExpr, Node, EnumCallExpr,
ARG_POS, MDEF,
CONTRAVARIANT, COVARIANT, INVARIANT)
UnicodeExpr, OpExpr, UnaryExpr, LambdaExpr, TempNode, SymbolTableNode,
Context, Decorator, PrintStmt, BreakStmt, PassStmt, ContinueStmt,
ComparisonExpr, StarExpr, EllipsisExpr, RefExpr, PromoteExpr,
Import, ImportFrom, ImportAll, ImportBase,
ARG_POS, ARG_STAR, LITERAL_TYPE, MDEF, GDEF,
CONTRAVARIANT, COVARIANT, INVARIANT,
)
from mypy import nodes
from mypy.literals import literal, literal_hash
from mypy.typeanal import has_any_from_unimported_type, check_for_explicit_any
from mypy.types import (
Type, AnyType, CallableType, FunctionLike, Overloaded, TupleType, TypedDictType,
Instance, NoneTyp, strip_type, TypeType, TypeOfAny,
UnionType, TypeVarId, TypeVarType, PartialType, DeletedType, UninhabitedType, TypeVarDef,
true_only, false_only, function_type, is_named_instance, union_items
true_only, false_only, function_type, is_named_instance, union_items,
)
from mypy.sametypes import is_same_type, is_same_types
from mypy.messages import MessageBuilder, make_inferred_type_note
Expand Down Expand Up @@ -869,9 +864,11 @@ def is_trivial_body(self, block: Block) -> bool:
(isinstance(stmt, ExpressionStmt) and
isinstance(stmt.expr, EllipsisExpr)))

def check_reverse_op_method(self, defn: FuncItem, typ: CallableType,
method: str, context: Context) -> None:
def check_reverse_op_method(self, defn: FuncItem,
reverse_type: CallableType, reverse_name: str,
context: Context) -> None:
"""Check a reverse operator method such as __radd__."""
# Decides whether it's worth calling check_overlapping_op_methods().

# This used to check for some very obscure scenario. It now
# just decides whether it's worth calling
Expand All @@ -884,54 +881,57 @@ def check_reverse_op_method(self, defn: FuncItem, typ: CallableType,
[None, None],
AnyType(TypeOfAny.special_form),
self.named_type('builtins.function'))
if not is_subtype(typ, method_type):
self.msg.invalid_signature(typ, context)
if not is_subtype(reverse_type, method_type):
self.msg.invalid_signature(reverse_type, context)
return

if method in ('__eq__', '__ne__'):
if reverse_name in ('__eq__', '__ne__'):
# These are defined for all objects => can't cause trouble.
return

# With 'Any' or 'object' return type we are happy, since any possible
# return value is valid.
ret_type = typ.ret_type
ret_type = reverse_type.ret_type
if isinstance(ret_type, AnyType):
return
if isinstance(ret_type, Instance):
if ret_type.type.fullname() == 'builtins.object':
return

if len(typ.arg_types) == 2:
# TODO check self argument kind

# Check for the issue described above.
arg_type = typ.arg_types[1]
other_method = nodes.normal_from_reverse_op[method]
if isinstance(arg_type, Instance):
if not arg_type.type.has_readable_member(other_method):
return
elif isinstance(arg_type, AnyType):
return
elif isinstance(arg_type, UnionType):
if not arg_type.has_readable_member(other_method):
return
else:
return

typ2 = self.expr_checker.analyze_external_member_access(
other_method, arg_type, defn)
self.check_overlapping_op_methods(
typ, method, defn.info,
typ2, other_method, cast(Instance, arg_type),
defn)
if reverse_type.arg_kinds[0] == ARG_STAR:
reverse_type = reverse_type.copy_modified(arg_types=[reverse_type.arg_types[0]] * 2,
arg_kinds=[ARG_POS] * 2,
arg_names=[reverse_type.arg_names[0], "_"])
assert len(reverse_type.arg_types) == 2

forward_name = nodes.normal_from_reverse_op[reverse_name]
forward_inst = reverse_type.arg_types[1]
if isinstance(forward_inst, TypeVarType):
forward_inst = forward_inst.upper_bound
if isinstance(forward_inst, (FunctionLike, TupleType, TypedDictType)):
forward_inst = forward_inst.fallback
if isinstance(forward_inst, TypeType):
item = forward_inst.item
if isinstance(item, Instance):
opt_meta = item.type.metaclass_type
if opt_meta is not None:
forward_inst = opt_meta
if not (isinstance(forward_inst, (Instance, UnionType))
and forward_inst.has_readable_member(forward_name)):
return
forward_base = reverse_type.arg_types[1]
forward_type = self.expr_checker.analyze_external_member_access(forward_name, forward_base,
context=defn)
self.check_overlapping_op_methods(reverse_type, reverse_name, defn.info,
forward_type, forward_name, forward_base,
context=defn)

def check_overlapping_op_methods(self,
reverse_type: CallableType,
reverse_name: str,
reverse_class: TypeInfo,
forward_type: Type,
forward_name: str,
forward_base: Instance,
forward_base: Type,
context: Context) -> None:
"""Check for overlapping method and reverse method signatures.

Expand Down Expand Up @@ -992,8 +992,8 @@ def check_overlapping_op_methods(self,
if is_unsafe_overlapping_signatures(forward_tweaked,
reverse_tweaked):
self.msg.operator_method_signatures_overlap(
reverse_class.name(), reverse_name,
forward_base.type.name(), forward_name, context)
reverse_class, reverse_name,
forward_base, forward_name, context)
elif isinstance(forward_item, Overloaded):
for item in forward_item.items():
self.check_overlapping_op_methods(
Expand Down
8 changes: 4 additions & 4 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -932,12 +932,12 @@ def overloaded_signatures_ret_specific(self, index1: int, context: Context) -> N
'of signature {}'.format(index1), context)

def operator_method_signatures_overlap(
self, reverse_class: str, reverse_method: str, forward_class: str,
self, reverse_class: TypeInfo, reverse_method: str, forward_class: Type,
forward_method: str, context: Context) -> None:
self.fail('Signatures of "{}" of "{}" and "{}" of "{}" '
self.fail('Signatures of "{}" of "{}" and "{}" of {} '
'are unsafely overlapping'.format(
reverse_method, reverse_class,
forward_method, forward_class),
reverse_method, reverse_class.name(),
forward_method, self.format(forward_class)),
context)

def forward_operator_not_callable(
Expand Down
6 changes: 0 additions & 6 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2074,15 +2074,9 @@ def __getitem__(self, name: str) -> 'SymbolTableNode':
def __repr__(self) -> str:
return '<TypeInfo %s>' % self.fullname()

# IDEA: Refactor the has* methods to be more consistent and document
# them.

def has_readable_member(self, name: str) -> bool:
return self.get(name) is not None

def has_method(self, name: str) -> bool:
return self.get_method(name) is not None

def get_method(self, name: str) -> Optional[FuncBase]:
if self.mro is None: # Might be because of a previous error.
return None
Expand Down
3 changes: 3 additions & 0 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,9 @@ def deserialize(cls, data: Union[JsonDict, str]) -> 'Instance':
def copy_modified(self, *, args: List[Type]) -> 'Instance':
return Instance(self.type, args, self.line, self.column, self.erased)

def has_readable_member(self, name: str) -> bool:
return self.type.has_readable_member(name)


class TypeVarType(Type):
"""A type variable type.
Expand Down
24 changes: 23 additions & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -1705,6 +1705,28 @@ class A:
def __add__(self, x: str) -> int: pass
def __radd__(self, x: 'A') -> str: pass

[case testReverseOperatorStar]
class B:
def __radd__(*self) -> int: pass
def __rsub__(*self: 'B') -> int: pass

[case testReverseOperatorTypeVar]
from typing import TypeVar
T = TypeVar("T", bound='Real')
class Real:
def __add__(self, other) -> str: ...
class Fraction(Real):
def __radd__(self, other: T) -> T: ... # E: Signatures of "__radd__" of "Fraction" and "__add__" of "T" are unsafely overlapping

[case testReverseOperatorTypeType]
from typing import TypeVar, Type
class Real(type):
def __add__(self, other) -> str: ...
class Fraction(Real):
def __radd__(self, other: Type['A']) -> Real: ... # E: Signatures of "__radd__" of "Fraction" and "__add__" of "Type[A]" are unsafely overlapping

class A(metaclass=Real): pass

[case testAbstractReverseOperatorMethod]
import typing
from abc import abstractmethod
Expand Down Expand Up @@ -1751,7 +1773,7 @@ tmp/foo.pyi:6: error: Signatures of "__radd__" of "B" and "__add__" of "X" are u
[case testUnsafeOverlappingWithLineNo]
from typing import TypeVar
class Real:
def __add__(self, other): ...
def __add__(self, other) -> str: ...
class Fraction(Real):
def __radd__(self, other: Real) -> Real: ...
[out]
Expand Down
8 changes: 8 additions & 0 deletions test-data/unit/check-namedtuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,14 @@ my_eval(A([B(1), B(2)])) # OK
[builtins fixtures/isinstancelist.pyi]
[out]

[case testUnsafeOverlappingNamedTuple]
from typing import NamedTuple

class Real(NamedTuple):
def __sub__(self, other) -> str: return ""
class Fraction(Real):
def __rsub__(self, other: Real) -> Real: return other # E: Signatures of "__rsub__" of "Fraction" and "__sub__" of "Real" are unsafely overlapping

[case testForwardReferenceInNamedTuple]
from typing import NamedTuple

Expand Down