From 49a46709a8d9791d0dec0ba9de07e9c14860acdd Mon Sep 17 00:00:00 2001 From: elazar Date: Tue, 26 Sep 2017 23:24:33 +0300 Subject: [PATCH 1/5] Cleanup check_reverse_op_method * Remove old comments, clean bail-out logic * Use fallback to find Instance when possible * Remove cast; move formatting logic to messages.py * remove unused method * rename variables to match check_overlapping_op_methods terminology * remove many unused imports --- mypy/checker.py | 82 ++++++++++++++++++++---------------------------- mypy/messages.py | 8 ++--- mypy/nodes.py | 6 ---- mypy/types.py | 3 ++ 4 files changed, 41 insertions(+), 58 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 12baa465c835..ab4f16e6d8da 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -3,7 +3,6 @@ import itertools import fnmatch from contextlib import contextmanager -import sys from typing import ( Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator @@ -11,23 +10,19 @@ 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, + Import, ImportFrom, ImportAll, ImportBase, + ARG_POS, 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 @@ -35,7 +30,7 @@ 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 @@ -811,51 +806,42 @@ 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) -> None: + def check_reverse_op_method(self, defn: FuncItem, + reverse_type: CallableType, reverse_name: str) -> 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 - # check_overlapping_op_methods(). - - 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 + # Plausibly the method could have too few arguments, which would result # in an error elsewhere. - 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 + if len(reverse_type.arg_types) != 2: + 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) + forward_name = nodes.normal_from_reverse_op[reverse_name] + forward_base = reverse_type.arg_types[1] + if isinstance(forward_base, (FunctionLike, TupleType, TypedDictType)): + forward_base = forward_base.fallback + if not (isinstance(forward_base, (Instance, UnionType)) + and forward_base.has_readable_member(forward_name)): + return + + 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, @@ -863,7 +849,7 @@ def check_overlapping_op_methods(self, 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. @@ -924,8 +910,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( diff --git a/mypy/messages.py b/mypy/messages.py index 2f3473b3e59d..c8773cccf804 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -915,12 +915,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( diff --git a/mypy/nodes.py b/mypy/nodes.py index 7f7ceb5fb8fe..2a8c7747332d 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -2058,15 +2058,9 @@ def __getitem__(self, name: str) -> 'SymbolTableNode': def __repr__(self) -> str: return '' % 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 diff --git a/mypy/types.py b/mypy/types.py index a53085a66a36..c102b3d30583 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -515,6 +515,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. From 40861f7d664358fd27ea636eb51702d8e175b3e7 Mon Sep 17 00:00:00 2001 From: Elazar Gershuni Date: Thu, 16 Nov 2017 15:32:12 +0200 Subject: [PATCH 2/5] Update checker.py --- mypy/checker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a75add7bf326..7b4994fed190 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -831,8 +831,8 @@ def check_reverse_op_method(self, defn: FuncItem, [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 reverse_name in ('__eq__', '__ne__'): From d355ff43ce5b540373cfc0b96191d7d77d521098 Mon Sep 17 00:00:00 2001 From: Elazar Gershuni Date: Fri, 17 Nov 2017 00:04:22 +0200 Subject: [PATCH 3/5] Assert len(reverse_type.arg_types) == 2 --- mypy/checker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 7b4994fed190..a3760cb21fc5 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -848,8 +848,7 @@ def check_reverse_op_method(self, defn: FuncItem, if ret_type.type.fullname() == 'builtins.object': return - if len(reverse_type.arg_types) != 2: - return + assert len(reverse_type.arg_types) == 2 forward_name = nodes.normal_from_reverse_op[reverse_name] forward_base = reverse_type.arg_types[1] From c51f1c7b1a1102bfcf11d09d1b2bafbc052b5400 Mon Sep 17 00:00:00 2001 From: Elazar Gershuni Date: Fri, 9 Feb 2018 16:52:53 +0200 Subject: [PATCH 4/5] add test for overlapping TupleType --- test-data/unit/check-classes.test | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index ba623eedc036..79cbfca42074 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -1751,6 +1751,16 @@ class Fraction(Real): [out] main:5: error: Signatures of "__radd__" of "Fraction" and "__add__" of "Real" are unsafely overlapping +[case testUnsafeOverlappingTuple] +from typing import Tuple + +class Real(Tuple[int, int]): + def __add__(self, other): return 1 +class Fraction(Real): + def __radd__(self, other: Real) -> Real: return other # E: Signatures of "__radd__" of "Fraction" and "__add__" of "Real" are unsafely overlapping + +[builtins fixtures/tuple.pyi] + [case testOverlappingNormalAndInplaceOperatorMethod] import typing class A: From 40bc07dd3c92e9960f0e346343e3e8c09267931d Mon Sep 17 00:00:00 2001 From: Elazar Gershuni Date: Tue, 27 Feb 2018 21:41:08 +0200 Subject: [PATCH 5/5] handle stararg self, TypeType and TypeVarType --- mypy/checker.py | 15 +++++++++++++-- test-data/unit/check-classes.test | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a6bccb824b5a..0e338bf0efa4 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -20,7 +20,7 @@ Context, Decorator, PrintStmt, BreakStmt, PassStmt, ContinueStmt, ComparisonExpr, StarExpr, EllipsisExpr, RefExpr, PromoteExpr, Import, ImportFrom, ImportAll, ImportBase, - ARG_POS, LITERAL_TYPE, MDEF, GDEF, + ARG_POS, ARG_STAR, LITERAL_TYPE, MDEF, GDEF, CONTRAVARIANT, COVARIANT, INVARIANT, ) from mypy import nodes @@ -897,13 +897,24 @@ def check_reverse_op_method(self, defn: FuncItem, if isinstance(ret_type, Instance): if ret_type.type.fullname() == 'builtins.object': return - + 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 diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 830da5441540..bdb0ee2df4c3 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -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