From 2ce9115cf08ef2767c8f9ea3e2596cff596120a6 Mon Sep 17 00:00:00 2001 From: Vlad Starostin Date: Tue, 6 Feb 2018 17:11:02 +0300 Subject: [PATCH 1/3] Fix --warn-return-any for NotImplemented --- .gitignore | 1 + mypy/checker.py | 10 ++++- mypy/sharedparse.py | 47 +++++++++++++++++++++- test-data/unit/check-warnings.test | 15 +++++++ test-data/unit/fixtures/notimplemented.pyi | 13 ++++++ 5 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 test-data/unit/fixtures/notimplemented.pyi diff --git a/.gitignore b/.gitignore index 98cbe9c3e6be1..02e39c0c3bcdd 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ docs/build/ .cache .runtest_log.json dmypy.json +.pytest_cache/ # Packages *.egg diff --git a/mypy/checker.py b/mypy/checker.py index 8e4af18c17c3f..23046ae6638b8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -59,6 +59,7 @@ from mypy.meet import is_overlapping_types from mypy.options import Options from mypy.plugin import Plugin, CheckerPluginInterface +from mypy.sharedparse import BINARY_MAGIC_METHODS from mypy import experiments @@ -2180,7 +2181,10 @@ def check_return_stmt(self, s: ReturnStmt) -> None: # (Unless you asked to be warned in that case, and the # function is not declared to return Any) if (self.options.warn_return_any and not self.current_node_deferred and - not is_proper_subtype(AnyType(TypeOfAny.special_form), return_type)): + not is_proper_subtype(AnyType(TypeOfAny.special_form), + return_type) and + not (defn.name() in BINARY_MAGIC_METHODS and + is_literal_not_implemented(s.expr))): self.msg.incorrectly_returning_any(return_type, s) return @@ -3232,6 +3236,10 @@ def remove_optional(typ: Type) -> Type: return typ +def is_literal_not_implemented(n: Expression) -> bool: + return isinstance(n, NameExpr) and n.fullname == 'builtins.NotImplemented' + + def builtin_item_type(tp: Type) -> Optional[Type]: """Get the item type of a builtin container. diff --git a/mypy/sharedparse.py b/mypy/sharedparse.py index 1b3e5a3a94604..91015b6d693fa 100644 --- a/mypy/sharedparse.py +++ b/mypy/sharedparse.py @@ -16,7 +16,6 @@ "__delitem__", "__divmod__", "__div__", - "__divmod__", "__enter__", "__exit__", "__eq__", @@ -92,6 +91,52 @@ MAGIC_METHODS_POS_ARGS_ONLY = MAGIC_METHODS - MAGIC_METHODS_ALLOWING_KWARGS +BINARY_MAGIC_METHODS = { + "__add__", + "__and__", + "__cmp__", + "__divmod__", + "__div__", + "__eq__", + "__floordiv__", + "__ge__", + "__gt__", + "__iadd__", + "__iand__", + "__idiv__", + "__ifloordiv__", + "__ilshift__", + "__imod__", + "__imul__", + "__ior__", + "__ipow__", + "__irshift__", + "__isub__", + "__ixor__", + "__le__", + "__lshift__", + "__lt__", + "__mod__", + "__mul__", + "__or__", + "__pow__", + "__radd__", + "__rand__", + "__rdiv__", + "__rfloordiv__", + "__rlshift__", + "__rmod__", + "__rmul__", + "__ror__", + "__rpow__", + "__rrshift__", + "__rshift__", + "__rsub__", + "__rxor__", + "__sub__", + "__xor__", +} + def special_function_elide_names(name: str) -> bool: return name in MAGIC_METHODS_POS_ARGS_ONLY diff --git a/test-data/unit/check-warnings.test b/test-data/unit/check-warnings.test index 4acbfe69df80f..d812ed83efa8c 100644 --- a/test-data/unit/check-warnings.test +++ b/test-data/unit/check-warnings.test @@ -143,6 +143,21 @@ def f() -> int: return g() [out] main:4: warning: Returning Any from function declared to return "int" +[case testReturnAnyForNotImplementedInBinaryMagicMethods] +# flags: --warn-return-any +class A: + def __eq__(self, other: object) -> bool: return NotImplemented +[builtins fixtures/notimplemented.pyi] +[out] + +[case testReturnAnyForNotImplementedInNormalMethods] +# flags: --warn-return-any +class A: + def some(self) -> bool: return NotImplemented +[builtins fixtures/notimplemented.pyi] +[out] +main:3: warning: Returning Any from function declared to return "bool" + [case testReturnAnyFromTypedFunctionWithSpecificFormatting] # flags: --warn-return-any from typing import Any, Tuple diff --git a/test-data/unit/fixtures/notimplemented.pyi b/test-data/unit/fixtures/notimplemented.pyi new file mode 100644 index 0000000000000..e619a6c5ad851 --- /dev/null +++ b/test-data/unit/fixtures/notimplemented.pyi @@ -0,0 +1,13 @@ +# builtins stub used in NotImplemented related cases. +from typing import Any, cast + + +class object: + def __init__(self) -> None: pass + +class type: pass +class function: pass +class bool: pass +class int: pass +class str: pass +NotImplemented = cast(Any, None) From 3e0b67b785f1a6aefc6e66f020c508e7ae76b310 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 6 Feb 2018 13:37:27 -0800 Subject: [PATCH 2/3] Undo add of .pytest_cache/ to .gitignore --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 02e39c0c3bcdd..98cbe9c3e6be1 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,6 @@ docs/build/ .cache .runtest_log.json dmypy.json -.pytest_cache/ # Packages *.egg From a8ac19839e107c495781dc607c91f0c137e6811a Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 6 Feb 2018 13:40:08 -0800 Subject: [PATCH 3/3] Reformat a long condition It's okay to move 'and' to the start of the next line. --- mypy/checker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 23046ae6638b8..109fc24043bdc 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2180,10 +2180,10 @@ def check_return_stmt(self, s: ReturnStmt) -> None: if isinstance(typ, AnyType): # (Unless you asked to be warned in that case, and the # function is not declared to return Any) - if (self.options.warn_return_any and not self.current_node_deferred and - not is_proper_subtype(AnyType(TypeOfAny.special_form), - return_type) and - not (defn.name() in BINARY_MAGIC_METHODS and + if (self.options.warn_return_any + and not self.current_node_deferred + and not is_proper_subtype(AnyType(TypeOfAny.special_form), return_type) + and not (defn.name() in BINARY_MAGIC_METHODS and is_literal_not_implemented(s.expr))): self.msg.incorrectly_returning_any(return_type, s) return