From a99a7e6ea964935bc3e614e72d943ba216357af8 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 19:48:16 +0100 Subject: [PATCH 01/13] Re-apply previous PR --- docs/source/class_basics.rst | 19 + docs/source/error_code_list.rst | 20 + docs/source/protocols.rst | 13 +- mypy/checker.py | 36 +- mypy/checkmember.py | 26 ++ mypy/errorcodes.py | 3 + mypy/main.py | 3 + mypy/messages.py | 8 + mypy/nodes.py | 6 +- mypy/options.py | 2 + mypy/semanal.py | 10 + mypy/server/astdiff.py | 8 + mypy/test/testcheck.py | 2 + mypy/test/testcmdline.py | 2 + mypy/test/testdeps.py | 1 + mypy/test/testdiff.py | 1 + mypy/test/testfinegrained.py | 2 + mypy/test/testmerge.py | 1 + mypy/test/testpythoneval.py | 1 + mypy/test/testtypegen.py | 1 + mypy/visitor.py | 3 +- mypy_bootstrap.ini | 5 + mypy_self_check.ini | 5 + mypyc/test-data/commandline.test | 2 +- mypyc/test/test_run.py | 1 + mypyc/test/testutil.py | 1 + test-data/unit/check-abstract.test | 587 ++++++++++++++++++++++++++- test-data/unit/check-errorcodes.test | 13 + test-data/unit/diff.test | 13 + test-data/unit/fine-grained.test | 43 ++ 30 files changed, 807 insertions(+), 31 deletions(-) diff --git a/docs/source/class_basics.rst b/docs/source/class_basics.rst index 1eaba59a10c2..3c769f9c9802 100644 --- a/docs/source/class_basics.rst +++ b/docs/source/class_basics.rst @@ -308,6 +308,25 @@ however: in this case, but any attempt to construct an instance will be flagged as an error. +Mypy allows you to omit the body for an abstract method, but if you do so, +it is unsafe to call such method via ``super()``. For example: + +.. code-block:: python + from abc import abstractmethod + class Base: + @abstractmethod + def foo(self) -> int: pass + @abstractmethod + def bar(self) -> int: + return 0 + class Sub(Base): + def foo(self) -> int: + return super().foo() + 1 # error: Call to abstract method "foo" of "Base" + # with trivial body via super() is unsafe + @abstractmethod + def bar(self) -> int: + return super().bar() + 1 # This is OK however. + A class can inherit any number of classes, both abstract and concrete. As with normal overrides, a dynamically typed method can override or implement a statically typed method defined in any base diff --git a/docs/source/error_code_list.rst b/docs/source/error_code_list.rst index 5c1f0bedb980..2859f8cfce9f 100644 --- a/docs/source/error_code_list.rst +++ b/docs/source/error_code_list.rst @@ -564,6 +564,26 @@ Example: # Error: Cannot instantiate abstract class "Thing" with abstract attribute "save" [abstract] t = Thing() +Check that call to an abstract method via super is valid [safe-super] +--------------------------------------------------------------------- + +Abstract methods often don't have any default implementation, i.e. their +bodies are just empty. Calling such methods in subclasses via ``super()`` +will cause runtime errors, so mypy prevents you from doing so: + +.. code-block:: python + from abc import abstractmethod + class Base: + @abstractmethod + def foo(self) -> int: ... + class Sub(Base): + def foo(self) -> int: + return super().foo() + 1 # error: Call to abstract method "foo" of "Base" with + # trivial body via super() is unsafe [safe-super] + Sub().foo() # This will crash at runtime. +Mypy considers the following as trivial bodies: a ``pass`` statement, a literal +ellipsis ``...``, a docstring, and a ``raise NotImplementedError`` statement. + Check the target of NewType [valid-newtype] ------------------------------------------- diff --git a/docs/source/protocols.rst b/docs/source/protocols.rst index 48530310c8cb..4fe2b8aca975 100644 --- a/docs/source/protocols.rst +++ b/docs/source/protocols.rst @@ -373,7 +373,18 @@ protocols. If you explicitly subclass these protocols you can inherit these default implementations. Explicitly including a protocol as a base class is also a way of documenting that your class implements a particular protocol, and it forces mypy to verify that your class -implementation is actually compatible with the protocol. +implementation is actually compatible with the protocol. In particular, +omitting a value for an attribute or a method body will make it implicitly +abstract: + +.. code-block:: python + class SomeProto(Protocol): + attr: int # Note, no right hand side + def method(self) -> str: ... # Literal ... here + class ExplicitSubclass(SomeProto): + pass + ExplicitSubclass() # error: Cannot instantiate abstract class 'ExplicitSubclass' + # with abstract attributes 'attr' and 'method' Recursive protocols ******************* diff --git a/mypy/checker.py b/mypy/checker.py index de98fa0fa179..f0582a2d9d2e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -70,6 +70,7 @@ LDEF, LITERAL_TYPE, MDEF, + NOT_ABSTRACT, AssertStmt, AssignmentExpr, AssignmentStmt, @@ -620,7 +621,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: self.visit_decorator(cast(Decorator, defn.items[0])) for fdef in defn.items: assert isinstance(fdef, Decorator) - self.check_func_item(fdef.func, name=fdef.func.name) + self.check_func_item(fdef.func, name=fdef.func.name, allow_empty=True) if fdef.func.abstract_status in (IS_ABSTRACT, IMPLICITLY_ABSTRACT): num_abstract += 1 if num_abstract not in (0, len(defn.items)): @@ -986,7 +987,11 @@ def _visit_func_def(self, defn: FuncDef) -> None: ) def check_func_item( - self, defn: FuncItem, type_override: CallableType | None = None, name: str | None = None + self, + defn: FuncItem, + type_override: CallableType | None = None, + name: str | None = None, + allow_empty: bool = False, ) -> None: """Type check a function. @@ -1000,7 +1005,7 @@ def check_func_item( typ = type_override.copy_modified(line=typ.line, column=typ.column) if isinstance(typ, CallableType): with self.enter_attribute_inference_context(): - self.check_func_def(defn, typ, name) + self.check_func_def(defn, typ, name, allow_empty) else: raise RuntimeError("Not supported") @@ -1017,7 +1022,9 @@ def enter_attribute_inference_context(self) -> Iterator[None]: yield None self.inferred_attribute_types = old_types - def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> None: + def check_func_def( + self, defn: FuncItem, typ: CallableType, name: str | None, allow_empty: bool = False + ) -> None: """Type check a function definition.""" # Expand type variables with value restrictions to ordinary types. expanded = self.expand_typevars(defn, typ) @@ -1189,7 +1196,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> self.accept(item.body) unreachable = self.binder.is_unreachable() - if not unreachable and not body_is_trivial: + if not unreachable: if defn.is_generator or is_named_instance( self.return_types[-1], "typing.AwaitableGenerator" ): @@ -1202,9 +1209,22 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> return_type = self.return_types[-1] return_type = get_proper_type(return_type) + allow_empty = allow_empty or self.options.allow_empty_bodies + + show_error = ( + not body_is_trivial + or + # Allow empty bodies for abstract methods, overloads, in tests and stubs. + not allow_empty + and not (isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT) + and not self.is_stub + ) + if self.options.warn_no_return: - if not self.current_node_deferred and not isinstance( - return_type, (NoneType, AnyType) + if ( + not self.current_node_deferred + and not isinstance(return_type, (NoneType, AnyType)) + and show_error ): # Control flow fell off the end of a function that was # declared to return a non-None type and is not @@ -1214,7 +1234,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn) else: self.fail(message_registry.MISSING_RETURN_STATEMENT, defn) - else: + elif show_error: # similar to code in check_return_stmt self.check_subtype( subtype_label="implicitly returns", diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 89199cf8f553..ad8e95814b98 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -296,6 +296,24 @@ def analyze_instance_member_access( # Look up the member. First look up the method dictionary. method = info.get_method(name) if method and not isinstance(method, Decorator): + unsafe_super = False + if mx.is_super: + if isinstance(method, FuncDef) and method.is_trivial_body: + unsafe_super = True + impl = method + elif isinstance(method, OverloadedFuncDef): + if method.impl: + impl = method.impl if isinstance(method.impl, FuncDef) else method.impl.func + unsafe_super = impl.is_trivial_body + if unsafe_super: + ret_type = ( + impl.type.ret_type + if isinstance(impl.type, CallableType) + else AnyType(TypeOfAny.unannotated) + ) + if not subtypes.is_subtype(NoneType(), ret_type): + mx.msg.unsafe_super(method.name, method.info.name, mx.context) + if method.is_property: assert isinstance(method, OverloadedFuncDef) first_item = cast(Decorator, method.items[0]) @@ -449,6 +467,14 @@ def analyze_member_var_access( if isinstance(vv, Decorator): # The associated Var node of a decorator contains the type. v = vv.var + if mx.is_super and vv.func.is_trivial_body: + ret_type = ( + vv.func.type.ret_type + if isinstance(vv.func.type, CallableType) + else AnyType(TypeOfAny.unannotated) + ) + if not subtypes.is_subtype(NoneType(), ret_type): + mx.msg.unsafe_super(vv.func.name, vv.func.info.name, mx.context) if isinstance(vv, TypeInfo): # If the associated variable is a TypeInfo synthesize a Var node for diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 511808fa7888..48d5f9c4e27c 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -96,6 +96,9 @@ def __str__(self) -> str: UNUSED_COROUTINE: Final = ErrorCode( "unused-coroutine", "Ensure that all coroutines are used", "General" ) +SAFE_SUPER: Final = ErrorCode( + "safe-super", "Warn about calls to abstract methods with empty/trivial bodies", "General" +) # These error codes aren't enabled by default. NO_UNTYPED_DEF: Final[ErrorCode] = ErrorCode( diff --git a/mypy/main.py b/mypy/main.py index dcae77f24f8a..eaa85289e848 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -999,6 +999,9 @@ def add_invertible_flag( "the contents of SHADOW_FILE instead.", ) add_invertible_flag("--fast-exit", default=True, help=argparse.SUPPRESS, group=internals_group) + add_invertible_flag( + "--allow-empty-bodies", default=False, help=argparse.SUPPRESS, group=internals_group + ) report_group = parser.add_argument_group( title="Report generation", description="Generate a report in the specified format." diff --git a/mypy/messages.py b/mypy/messages.py index 6e7aa164ac91..fa5d3569b966 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1231,6 +1231,14 @@ def first_argument_for_super_must_be_type(self, actual: Type, context: Context) code=codes.ARG_TYPE, ) + def unsafe_super(self, method: str, cls: str, ctx: Context) -> None: + self.fail( + 'Call to abstract method "{}" of "{}" with trivial body' + " via super() is unsafe".format(method, cls), + ctx, + code=codes.SAFE_SUPER, + ) + def too_few_string_formatting_arguments(self, context: Context) -> None: self.fail("Not enough arguments for format string", context, code=codes.STRING_FORMATTING) diff --git a/mypy/nodes.py b/mypy/nodes.py index fd0228e2a254..6b7f25e7c6bc 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -758,7 +758,7 @@ def is_dynamic(self) -> bool: return self.type is None -FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional"] +FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional", "is_trivial_body"] # Abstract status of a function NOT_ABSTRACT: Final = 0 @@ -781,6 +781,7 @@ class FuncDef(FuncItem, SymbolNode, Statement): "abstract_status", "original_def", "deco_line", + "is_trivial_body", ) # Note that all __init__ args must have default values @@ -796,6 +797,9 @@ def __init__( self.is_decorated = False self.is_conditional = False # Defined conditionally (within block)? self.abstract_status = NOT_ABSTRACT + # Is this an abstract method with trivial body? + # Such methods can't be called via super(). + self.is_trivial_body = False self.is_final = False # Original conditional definition self.original_def: None | FuncDef | Var | Decorator = None diff --git a/mypy/options.py b/mypy/options.py index b129303c304c..db70ae0e13f3 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -296,6 +296,8 @@ def __init__(self) -> None: self.fast_exit = True # fast path for finding modules from source set self.fast_module_lookup = False + # Allow empty function bodies even if it is not safe, used for testing only. + self.allow_empty_bodies = False # Used to transform source code before parsing if not None # TODO: Make the type precise (AnyStr -> AnyStr) self.transform_source: Callable[[Any], Any] | None = None diff --git a/mypy/semanal.py b/mypy/semanal.py index acd962c674ee..1b05ced4c8fd 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -77,6 +77,7 @@ IS_ABSTRACT, LDEF, MDEF, + NOT_ABSTRACT, REVEAL_LOCALS, REVEAL_TYPE, RUNTIME_PROTOCOL_DECOS, @@ -861,6 +862,12 @@ def analyze_func_def(self, defn: FuncDef) -> None: and is_trivial_body(defn.body) ): defn.abstract_status = IMPLICITLY_ABSTRACT + if ( + is_trivial_body(defn.body) + and not self.is_stub_file + and defn.abstract_status != NOT_ABSTRACT + ): + defn.is_trivial_body = True if ( defn.is_coroutine @@ -1038,6 +1045,8 @@ def process_overload_impl(self, defn: OverloadedFuncDef) -> None: assert self.type is not None if self.type.is_protocol: impl.abstract_status = IMPLICITLY_ABSTRACT + if impl.abstract_status != NOT_ABSTRACT: + impl.is_trivial_body = True def analyze_overload_sigs_and_impl( self, defn: OverloadedFuncDef @@ -1125,6 +1134,7 @@ def handle_missing_overload_implementation(self, defn: OverloadedFuncDef) -> Non else: item.abstract_status = IS_ABSTRACT else: + # TODO: also allow omitting an implementation for abstract methods in ABCs? self.fail( "An overloaded function outside a stub file must have an implementation", defn, diff --git a/mypy/server/astdiff.py b/mypy/server/astdiff.py index 815e2ca281eb..41a79db480c9 100644 --- a/mypy/server/astdiff.py +++ b/mypy/server/astdiff.py @@ -60,6 +60,7 @@ class level -- these are handled at attribute level (say, 'mod.Cls.method' UNBOUND_IMPORTED, Decorator, FuncBase, + FuncDef, FuncItem, MypyFile, OverloadedFuncDef, @@ -217,6 +218,12 @@ def snapshot_definition(node: SymbolNode | None, common: tuple[object, ...]) -> signature = snapshot_type(node.type) else: signature = snapshot_untyped_signature(node) + impl: FuncDef | None = None + if isinstance(node, FuncDef): + impl = node + elif isinstance(node, OverloadedFuncDef) and node.impl: + impl = node.impl.func if isinstance(node.impl, Decorator) else node.impl + is_trivial_body = impl.is_trivial_body if impl else False return ( "Func", common, @@ -225,6 +232,7 @@ def snapshot_definition(node: SymbolNode | None, common: tuple[object, ...]) -> node.is_class, node.is_static, signature, + is_trivial_body, ) elif isinstance(node, Var): return ("Var", common, snapshot_optional_type(node.type), node.is_final) diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py index f62c5a8fe0f7..aee98004a8bb 100644 --- a/mypy/test/testcheck.py +++ b/mypy/test/testcheck.py @@ -120,6 +120,8 @@ def run_case_once( options.show_column_numbers = True if "errorcodes" in testcase.file: options.show_error_codes = True + if "abstract" not in testcase.file: + options.allow_empty_bodies = not testcase.name.endswith("_no_empty") if incremental_step and options.incremental: # Don't overwrite # flags: --no-incremental in incremental test cases diff --git a/mypy/test/testcmdline.py b/mypy/test/testcmdline.py index 684b082021de..6605385e3228 100644 --- a/mypy/test/testcmdline.py +++ b/mypy/test/testcmdline.py @@ -57,6 +57,8 @@ def test_python_cmdline(testcase: DataDrivenTestCase, step: int) -> None: args.append("--show-traceback") if "--error-summary" not in args: args.append("--no-error-summary") + if "--disallow-empty-bodies" not in args: + args.append("--allow-empty-bodies") # Type check the program. fixed = [python3_path, "-m", "mypy"] env = os.environ.copy() diff --git a/mypy/test/testdeps.py b/mypy/test/testdeps.py index 3a2bfa4d9c63..ae1c613f7563 100644 --- a/mypy/test/testdeps.py +++ b/mypy/test/testdeps.py @@ -33,6 +33,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: options.cache_dir = os.devnull options.export_types = True options.preserve_asts = True + options.allow_empty_bodies = True messages, files, type_map = self.build(src, options) a = messages if files is None or type_map is None: diff --git a/mypy/test/testdiff.py b/mypy/test/testdiff.py index 4ef82720fdcb..5e2e0bc2ca5a 100644 --- a/mypy/test/testdiff.py +++ b/mypy/test/testdiff.py @@ -54,6 +54,7 @@ def build(self, source: str, options: Options) -> tuple[list[str], dict[str, Myp options.show_traceback = True options.cache_dir = os.devnull options.python_version = PYTHON3_VERSION + options.allow_empty_bodies = True try: result = build.build( sources=[BuildSource("main", None, source)], diff --git a/mypy/test/testfinegrained.py b/mypy/test/testfinegrained.py index bd5628799c8b..e797b4b7a35b 100644 --- a/mypy/test/testfinegrained.py +++ b/mypy/test/testfinegrained.py @@ -152,6 +152,8 @@ def get_options(self, source: str, testcase: DataDrivenTestCase, build_cache: bo options.cache_fine_grained = self.use_cache options.local_partial_types = True options.enable_incomplete_features = True + # Treat empty bodies safely for these test cases. + options.allow_empty_bodies = not testcase.name.endswith("_no_empty") if re.search("flags:.*--follow-imports", source) is None: # Override the default for follow_imports options.follow_imports = "error" diff --git a/mypy/test/testmerge.py b/mypy/test/testmerge.py index 32586623640d..595aba49d8b7 100644 --- a/mypy/test/testmerge.py +++ b/mypy/test/testmerge.py @@ -113,6 +113,7 @@ def build(self, source: str, testcase: DataDrivenTestCase) -> BuildResult | None options.use_builtins_fixtures = True options.export_types = True options.show_traceback = True + options.allow_empty_bodies = True main_path = os.path.join(test_temp_dir, "main") with open(main_path, "w", encoding="utf8") as f: f.write(source) diff --git a/mypy/test/testpythoneval.py b/mypy/test/testpythoneval.py index a5eaea769515..5888159e50d5 100644 --- a/mypy/test/testpythoneval.py +++ b/mypy/test/testpythoneval.py @@ -52,6 +52,7 @@ def test_python_evaluation(testcase: DataDrivenTestCase, cache_dir: str) -> None "--no-strict-optional", "--no-silence-site-packages", "--no-error-summary", + "--allow-empty-bodies", ] interpreter = python3_path mypy_cmdline.append(f"--python-version={'.'.join(map(str, PYTHON3_VERSION))}") diff --git a/mypy/test/testtypegen.py b/mypy/test/testtypegen.py index 634649c973e5..db155a337980 100644 --- a/mypy/test/testtypegen.py +++ b/mypy/test/testtypegen.py @@ -34,6 +34,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: options.show_traceback = True options.export_types = True options.preserve_asts = True + options.allow_empty_bodies = True result = build.build( sources=[BuildSource("main", None, src)], options=options, diff --git a/mypy/visitor.py b/mypy/visitor.py index 62e7b4f90c8e..3c69f2d497f5 100644 --- a/mypy/visitor.py +++ b/mypy/visitor.py @@ -355,7 +355,8 @@ class NodeVisitor(Generic[T], ExpressionVisitor[T], StatementVisitor[T], Pattern methods. As all methods defined here return None by default, subclasses do not always need to override all the methods. - TODO make the default return value explicit + TODO: make the default return value explicit, then turn on + the warn_no_return flag in mypy_self_check.ini. """ # Not in superclasses: diff --git a/mypy_bootstrap.ini b/mypy_bootstrap.ini index 3a6eee6449d2..0085c9a806ec 100644 --- a/mypy_bootstrap.ini +++ b/mypy_bootstrap.ini @@ -13,3 +13,8 @@ warn_redundant_casts = True warn_unused_configs = True show_traceback = True always_true = MYPYC + +[mypy-mypy.visitor] +# See docstring for NodeVisitor for motivation. +warn_no_return = False +disable_error_code = return-value diff --git a/mypy_self_check.ini b/mypy_self_check.ini index 0852fd82cf47..ad7bfb31d4e6 100644 --- a/mypy_self_check.ini +++ b/mypy_self_check.ini @@ -12,3 +12,8 @@ plugins = misc/proper_plugin.py python_version = 3.7 exclude = mypy/typeshed/|mypyc/test-data/|mypyc/lib-rt/ enable_error_code = ignore-without-code,redundant-expr + +[mypy-mypy.visitor] +# See docstring for NodeVisitor for motivation. +warn_no_return = False +disable_error_code = return-value diff --git a/mypyc/test-data/commandline.test b/mypyc/test-data/commandline.test index cfd0d708bbda..6612df9e1886 100644 --- a/mypyc/test-data/commandline.test +++ b/mypyc/test-data/commandline.test @@ -171,7 +171,7 @@ class Nope(Trait1, Concrete2): # E: Non-trait bases must appear first in parent class NonExt2: @property # E: Property setters not supported in non-extension classes def test(self) -> int: - pass + return 0 @test.setter def test(self, x: int) -> None: diff --git a/mypyc/test/test_run.py b/mypyc/test/test_run.py index 0cca1890653e..63e4f153da40 100644 --- a/mypyc/test/test_run.py +++ b/mypyc/test/test_run.py @@ -192,6 +192,7 @@ def run_case_step(self, testcase: DataDrivenTestCase, incremental_step: int) -> options.python_version = sys.version_info[:2] options.export_types = True options.preserve_asts = True + options.allow_empty_bodies = True options.incremental = self.separate if "IncompleteFeature" in testcase.name: options.enable_incomplete_features = True diff --git a/mypyc/test/testutil.py b/mypyc/test/testutil.py index dc771b00551d..a2b12ce73e01 100644 --- a/mypyc/test/testutil.py +++ b/mypyc/test/testutil.py @@ -110,6 +110,7 @@ def build_ir_for_single_file2( options.python_version = (3, 6) options.export_types = True options.preserve_asts = True + options.allow_empty_bodies = True options.per_module_options["__main__"] = {"mypyc": True} source = build.BuildSource("main", "__main__", program_text) diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index e820a3a3c4fb..2831ba47caa0 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -314,8 +314,8 @@ class B(A): # E: Argument 1 of "f" is incompatible with supertype "A"; supertype defines the argument type as "int" \ # N: This violates the Liskov substitution principle \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides - pass - def g(self, x: int) -> int: pass + return 0 + def g(self, x: int) -> int: return 0 [out] [case testImplementingAbstractMethodWithMultipleBaseClasses] @@ -328,13 +328,13 @@ class J(metaclass=ABCMeta): @abstractmethod def g(self, x: str) -> str: pass class A(I, J): - def f(self, x: str) -> int: pass \ + def f(self, x: str) -> int: return 0 \ # E: Argument 1 of "f" is incompatible with supertype "I"; supertype defines the argument type as "int" \ # N: This violates the Liskov substitution principle \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides - def g(self, x: str) -> int: pass \ + def g(self, x: str) -> int: return 0 \ # E: Return type "int" of "g" incompatible with return type "str" in supertype "J" - def h(self) -> int: pass # Not related to any base class + def h(self) -> int: return 0 # Not related to any base class [out] [case testImplementingAbstractMethodWithExtension] @@ -345,7 +345,7 @@ class J(metaclass=ABCMeta): def f(self, x: int) -> int: pass class I(J): pass class A(I): - def f(self, x: str) -> int: pass \ + def f(self, x: str) -> int: return 0 \ # E: Argument 1 of "f" is incompatible with supertype "J"; supertype defines the argument type as "int" \ # N: This violates the Liskov substitution principle \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides @@ -376,11 +376,11 @@ class I(metaclass=ABCMeta): def h(self, a: 'I') -> A: pass class A(I): def h(self, a: 'A') -> 'I': # Fail - pass + return A() def f(self, a: 'I') -> 'I': - pass + return A() def g(self, a: 'A') -> 'A': - pass + return A() [out] main:11: error: Argument 1 of "h" is incompatible with supertype "I"; supertype defines the argument type as "I" main:11: note: This violates the Liskov substitution principle @@ -672,7 +672,7 @@ class A(metaclass=ABCMeta): def __gt__(self, other: 'A') -> int: pass [case testAbstractOperatorMethods2] -import typing +from typing import cast, Any from abc import abstractmethod, ABCMeta class A(metaclass=ABCMeta): @abstractmethod @@ -681,7 +681,8 @@ class B: @abstractmethod def __add__(self, other: 'A') -> int: pass class C: - def __add__(self, other: int) -> B: pass + def __add__(self, other: int) -> B: + return cast(Any, None) [out] [case testAbstractClassWithAnyBase] @@ -761,7 +762,7 @@ class A(metaclass=ABCMeta): def x(self) -> int: pass class B(A): @property - def x(self) -> int: pass + def x(self) -> int: return 0 b = B() b.x() # E: "int" not callable [builtins fixtures/property.pyi] @@ -775,7 +776,7 @@ class A(metaclass=ABCMeta): def x(self, v: int) -> None: pass class B(A): @property - def x(self) -> int: pass + def x(self) -> int: return 0 @x.setter def x(self, v: int) -> None: pass b = B() @@ -789,7 +790,7 @@ class A(metaclass=ABCMeta): def x(self) -> int: pass class B(A): @property - def x(self) -> str: pass # E: Signature of "x" incompatible with supertype "A" + def x(self) -> str: return "no" # E: Signature of "x" incompatible with supertype "A" b = B() b.x() # E: "str" not callable [builtins fixtures/property.pyi] @@ -850,7 +851,7 @@ class A(metaclass=ABCMeta): def x(self, v: int) -> None: pass class B(A): @property # E - def x(self) -> int: pass + def x(self) -> int: return 0 b = B() b.x.y # E [builtins fixtures/property.pyi] @@ -906,7 +907,7 @@ class C(Mixin, A): class A: @property def foo(cls) -> str: - pass + return "yes" class Mixin: foo = "foo" class C(Mixin, A): @@ -922,7 +923,7 @@ class Y(X): class A: @property def foo(cls) -> X: - pass + return X() class Mixin: foo = Y() class C(Mixin, A): @@ -934,7 +935,7 @@ class C(Mixin, A): class A: @property def foo(cls) -> str: - pass + return "no" class Mixin: foo = "foo" class C(A, Mixin): # E: Definition of "foo" in base class "A" is incompatible with definition in base class "Mixin" @@ -1024,7 +1025,7 @@ from abc import abstractmethod, ABCMeta from typing import Type, TypeVar T = TypeVar("T") -def deco(cls: Type[T]) -> Type[T]: ... +def deco(cls: Type[T]) -> Type[T]: return cls @deco class A(metaclass=ABCMeta): @@ -1050,3 +1051,551 @@ b: B b.x = 1 # E: Property "x" defined in "B" is read-only b.y = 1 [builtins fixtures/property.pyi] + + +-- Treatment of empty bodies in ABCs and protocols +-- ----------------------------------------------- + +[case testEmptyBodyProhibitedFunction] +from typing import overload, Union + +def func1(x: str) -> int: pass # E: Missing return statement +def func2(x: str) -> int: ... # E: Missing return statement +def func3(x: str) -> int: # E: Missing return statement + """Some function.""" + +@overload +def func4(x: int) -> int: ... +@overload +def func4(x: str) -> str: ... +def func4(x: Union[int, str]) -> Union[int, str]: # E: Missing return statement + pass + +@overload +def func5(x: int) -> int: ... +@overload +def func5(x: str) -> str: ... +def func5(x: Union[int, str]) -> Union[int, str]: # E: Missing return statement + """Some function.""" + +[case testEmptyBodyProhibitedMethodNonAbstract] +from typing import overload, Union + +class A: + def func1(self, x: str) -> int: pass # E: Missing return statement + def func2(self, x: str) -> int: ... # E: Missing return statement + def func3(self, x: str) -> int: # E: Missing return statement + """Some function.""" + +class B: + @classmethod + def func1(cls, x: str) -> int: pass # E: Missing return statement + @classmethod + def func2(cls, x: str) -> int: ... # E: Missing return statement + @classmethod + def func3(cls, x: str) -> int: # E: Missing return statement + """Some function.""" + +class C: + @overload + def func4(self, x: int) -> int: ... + @overload + def func4(self, x: str) -> str: ... + def func4(self, x: Union[int, str]) -> Union[int, str]: # E: Missing return statement + pass + + @overload + def func5(self, x: int) -> int: ... + @overload + def func5(self, x: str) -> str: ... + def func5(self, x: Union[int, str]) -> Union[int, str]: # E: Missing return statement + """Some function.""" +[builtins fixtures/classmethod.pyi] + +[case testEmptyBodyProhibitedPropertyNonAbstract] +class A: + @property + def x(self) -> int: ... # E: Missing return statement + @property + def y(self) -> int: ... # E: Missing return statement + @y.setter + def y(self, value: int) -> None: ... + +class B: + @property + def x(self) -> int: pass # E: Missing return statement + @property + def y(self) -> int: pass # E: Missing return statement + @y.setter + def y(self, value: int) -> None: pass + +class C: + @property + def x(self) -> int: # E: Missing return statement + """Some property.""" + @property + def y(self) -> int: # E: Missing return statement + """Some property.""" + @y.setter + def y(self, value: int) -> None: pass +[builtins fixtures/property.pyi] + +[case testEmptyBodyAllowedFunctionStub] +import stub +[file stub.pyi] +from typing import overload, Union + +def func1(x: str) -> int: pass +def func2(x: str) -> int: ... +def func3(x: str) -> int: + """Some function.""" + +[case testEmptyBodyAllowedMethodNonAbstractStub] +import stub +[file stub.pyi] +from typing import overload, Union + +class A: + def func1(self, x: str) -> int: pass + def func2(self, x: str) -> int: ... + def func3(self, x: str) -> int: + """Some function.""" + +class B: + @classmethod + def func1(cls, x: str) -> int: pass + @classmethod + def func2(cls, x: str) -> int: ... + @classmethod + def func3(cls, x: str) -> int: + """Some function.""" +[builtins fixtures/classmethod.pyi] + +[case testEmptyBodyAllowedPropertyNonAbstractStub] +import stub +[file stub.pyi] +class A: + @property + def x(self) -> int: ... + @property + def y(self) -> int: ... + @y.setter + def y(self, value: int) -> None: ... + +class B: + @property + def x(self) -> int: pass + @property + def y(self) -> int: pass + @y.setter + def y(self, value: int) -> None: pass + +class C: + @property + def x(self) -> int: + """Some property.""" + @property + def y(self) -> int: + """Some property.""" + @y.setter + def y(self, value: int) -> None: pass +[builtins fixtures/property.pyi] + +[case testEmptyBodyAllowedMethodAbstract] +from typing import overload, Union +from abc import abstractmethod + +class A: + @abstractmethod + def func1(self, x: str) -> int: pass + @abstractmethod + def func2(self, x: str) -> int: ... + @abstractmethod + def func3(self, x: str) -> int: + """Some function.""" + +class B: + @classmethod + @abstractmethod + def func1(cls, x: str) -> int: pass + @classmethod + @abstractmethod + def func2(cls, x: str) -> int: ... + @classmethod + @abstractmethod + def func3(cls, x: str) -> int: + """Some function.""" + +class C: + @overload + @abstractmethod + def func4(self, x: int) -> int: ... + @overload + @abstractmethod + def func4(self, x: str) -> str: ... + @abstractmethod + def func4(self, x: Union[int, str]) -> Union[int, str]: + pass + + @overload + @abstractmethod + def func5(self, x: int) -> int: ... + @overload + @abstractmethod + def func5(self, x: str) -> str: ... + @abstractmethod + def func5(self, x: Union[int, str]) -> Union[int, str]: + """Some function.""" +[builtins fixtures/classmethod.pyi] + +[case testEmptyBodyAllowedPropertyAbstract] +from abc import abstractmethod +class A: + @property + @abstractmethod + def x(self) -> int: ... + @property + @abstractmethod + def y(self) -> int: ... + @y.setter + @abstractmethod + def y(self, value: int) -> None: ... + +class B: + @property + @abstractmethod + def x(self) -> int: pass + @property + @abstractmethod + def y(self) -> int: pass + @y.setter + @abstractmethod + def y(self, value: int) -> None: pass + +class C: + @property + @abstractmethod + def x(self) -> int: + """Some property.""" + @property + @abstractmethod + def y(self) -> int: + """Some property.""" + @y.setter + @abstractmethod + def y(self, value: int) -> None: pass +[builtins fixtures/property.pyi] + +[case testEmptyBodyImplicitlyAbstractProtocol] +from typing import Protocol, overload, Union + +class P1(Protocol): + def meth(self) -> int: ... +class B1(P1): ... +class C1(P1): + def meth(self) -> int: + return 0 +B1() # E: Cannot instantiate abstract class "B1" with abstract attribute "meth" +C1() + +class P2(Protocol): + @classmethod + def meth(cls) -> int: ... +class B2(P2): ... +class C2(P2): + @classmethod + def meth(cls) -> int: + return 0 +B2() # E: Cannot instantiate abstract class "B2" with abstract attribute "meth" +C2() + +class P3(Protocol): + @overload + def meth(self, x: int) -> int: ... + @overload + def meth(self, x: str) -> str: ... +class B3(P3): ... +class C3(P3): + @overload + def meth(self, x: int) -> int: ... + @overload + def meth(self, x: str) -> str: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return 0 +B3() # E: Cannot instantiate abstract class "B3" with abstract attribute "meth" +C3() +[builtins fixtures/classmethod.pyi] + +[case testEmptyBodyImplicitlyAbstractProtocolProperty] +from typing import Protocol + +class P1(Protocol): + @property + def attr(self) -> int: ... +class B1(P1): ... +class C1(P1): + @property + def attr(self) -> int: + return 0 +B1() # E: Cannot instantiate abstract class "B1" with abstract attribute "attr" +C1() + +class P2(Protocol): + @property + def attr(self) -> int: ... + @attr.setter + def attr(self, value: int) -> None: ... +class B2(P2): ... +class C2(P2): + @property + def attr(self) -> int: return 0 + @attr.setter + def attr(self, value: int) -> None: pass +B2() # E: Cannot instantiate abstract class "B2" with abstract attribute "attr" +C2() +[builtins fixtures/property.pyi] + +[case testEmptyBodyImplicitlyAbstractProtocolStub] +from stub import P1, P2, P3, P4 + +class B1(P1): ... +class B2(P2): ... +class B3(P3): ... +class B4(P4): ... + +B1() +B2() +B3() +B4() # E: Cannot instantiate abstract class "B4" with abstract attribute "meth" + +[file stub.pyi] +from typing import Protocol, overload, Union +from abc import abstractmethod + +class P1(Protocol): + def meth(self) -> int: ... + +class P2(Protocol): + @classmethod + def meth(cls) -> int: ... + +class P3(Protocol): + @overload + def meth(self, x: int) -> int: ... + @overload + def meth(self, x: str) -> str: ... + +class P4(Protocol): + @abstractmethod + def meth(self) -> int: ... +[builtins fixtures/classmethod.pyi] + +[case testEmptyBodyUnsafeAbstractSuper] +# flags: --strict-optional +from stub import StubProto, StubAbstract +from typing import Protocol +from abc import abstractmethod + +class Proto(Protocol): + def meth(self) -> int: ... +class ProtoDef(Protocol): + def meth(self) -> int: return 0 + +class Abstract: + @abstractmethod + def meth(self) -> int: ... +class AbstractDef: + @abstractmethod + def meth(self) -> int: return 0 + +class SubProto(Proto): + def meth(self) -> int: + return super().meth() # E: Call to abstract method "meth" of "Proto" with trivial body via super() is unsafe +class SubProtoDef(ProtoDef): + def meth(self) -> int: + return super().meth() + +class SubAbstract(Abstract): + def meth(self) -> int: + return super().meth() # E: Call to abstract method "meth" of "Abstract" with trivial body via super() is unsafe +class SubAbstractDef(AbstractDef): + def meth(self) -> int: + return super().meth() + +class SubStubProto(StubProto): + def meth(self) -> int: + return super().meth() +class SubStubAbstract(StubAbstract): + def meth(self) -> int: + return super().meth() + +[file stub.pyi] +from typing import Protocol +from abc import abstractmethod + +class StubProto(Protocol): + def meth(self) -> int: ... +class StubAbstract: + @abstractmethod + def meth(self) -> int: ... + +[case testEmptyBodyUnsafeAbstractSuperProperty] +# flags: --strict-optional +from stub import StubProto, StubAbstract +from typing import Protocol +from abc import abstractmethod + +class Proto(Protocol): + @property + def attr(self) -> int: ... +class SubProto(Proto): + @property + def attr(self) -> int: return super().attr # E: Call to abstract method "attr" of "Proto" with trivial body via super() is unsafe + +class ProtoDef(Protocol): + @property + def attr(self) -> int: return 0 +class SubProtoDef(ProtoDef): + @property + def attr(self) -> int: return super().attr + +class Abstract: + @property + @abstractmethod + def attr(self) -> int: ... +class SubAbstract(Abstract): + @property + @abstractmethod + def attr(self) -> int: return super().attr # E: Call to abstract method "attr" of "Abstract" with trivial body via super() is unsafe + +class AbstractDef: + @property + @abstractmethod + def attr(self) -> int: return 0 +class SubAbstractDef(AbstractDef): + @property + @abstractmethod + def attr(self) -> int: return super().attr + +class SubStubProto(StubProto): + @property + def attr(self) -> int: return super().attr +class SubStubAbstract(StubAbstract): + @property + def attr(self) -> int: return super().attr + +[file stub.pyi] +from typing import Protocol +from abc import abstractmethod + +class StubProto(Protocol): + @property + def attr(self) -> int: ... +class StubAbstract: + @property + @abstractmethod + def attr(self) -> int: ... +[builtins fixtures/property.pyi] + +[case testEmptyBodyUnsafeAbstractSuperOverloads] +# flags: --strict-optional +from stub import StubProto +from typing import Protocol, overload, Union + +class ProtoEmptyImpl(Protocol): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + raise NotImplementedError +class ProtoDefImpl(Protocol): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return 0 +class ProtoNoImpl(Protocol): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + +class SubProtoEmptyImpl(ProtoEmptyImpl): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return super().meth(0) # E: Call to abstract method "meth" of "ProtoEmptyImpl" with trivial body via super() is unsafe +class SubProtoDefImpl(ProtoDefImpl): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return super().meth(0) +class SubStubProto(StubProto): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return super().meth(0) + +# TODO: it would be good to also give an error in this case. +class SubProtoNoImpl(ProtoNoImpl): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + def meth(self, x: Union[int, str]) -> Union[int, str]: + return super().meth(0) + +[file stub.pyi] +from typing import Protocol, overload + +class StubProto(Protocol): + @overload + def meth(self, x: str) -> str: ... + @overload + def meth(self, x: int) -> int: ... + +[builtins fixtures/exception.pyi] + +[case testEmptyBodyNoSuperWarningWithoutStrict] +# flags: --no-strict-optional +from typing import Protocol +from abc import abstractmethod + +class Proto(Protocol): + def meth(self) -> int: ... +class Abstract: + @abstractmethod + def meth(self) -> int: ... + +class SubProto(Proto): + def meth(self) -> int: + return super().meth() +class SubAbstract(Abstract): + def meth(self) -> int: + return super().meth() + +[case testEmptyBodyNoSuperWarningOptionalReturn] +# flags: --strict-optional +from typing import Protocol, Optional +from abc import abstractmethod + +class Proto(Protocol): + def meth(self) -> Optional[int]: pass +class Abstract: + @abstractmethod + def meth(self) -> Optional[int]: pass + +class SubProto(Proto): + def meth(self) -> Optional[int]: + return super().meth() +class SubAbstract(Abstract): + def meth(self) -> Optional[int]: + return super().meth() diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 401407c9d426..91341553133d 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -922,3 +922,16 @@ def f(d: D, s: str) -> None: [case testRecommendErrorCode] # type: ignore[whatever] # E: type ignore with error code is not supported for modules; use `# mypy: disable-error-code=...` [syntax] 1 + "asdf" + +[case testErrorCodeUnsafeSuper_no_empty] +# flags: --strict-optional +from abc import abstractmethod + +class Base: + @abstractmethod + def meth(self) -> int: + raise NotImplementedError() +class Sub(Base): + def meth(self) -> int: + return super().meth() # E: Call to abstract method "meth" of "Base" with trivial body via super() is unsafe [safe-super] +[builtins fixtures/exception.pyi] diff --git a/test-data/unit/diff.test b/test-data/unit/diff.test index 7369ea247e26..66adfaecd909 100644 --- a/test-data/unit/diff.test +++ b/test-data/unit/diff.test @@ -1484,3 +1484,16 @@ C = ParamSpec('C') [out] __main__.B __main__.C + +[case testEmptyBodySuper] +from abc import abstractmethod +class C: + @abstractmethod + def meth(self) -> int: ... +[file next.py] +from abc import abstractmethod +class C: + @abstractmethod + def meth(self) -> int: return 0 +[out] +__main__.C.meth diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index 9d8857301425..9f1821bda6a2 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -10031,3 +10031,46 @@ class C(B): ... == == main.py:4: note: Revealed type is "def () -> builtins.str" + +[case testAbstractBodyTurnsEmpty] +# flags: --strict-optional +from b import Base + +class Sub(Base): + def meth(self) -> int: + return super().meth() + +[file b.py] +from abc import abstractmethod +class Base: + @abstractmethod + def meth(self) -> int: return 0 + +[file b.py.2] +from abc import abstractmethod +class Base: + @abstractmethod + def meth(self) -> int: ... +[out] +== +main:6: error: Call to abstract method "meth" of "Base" with trivial body via super() is unsafe + +[case testAbstractBodyTurnsEmptyProtocol] +# flags: --strict-optional +from b import Base + +class Sub(Base): + def meth(self) -> int: + return super().meth() + +[file b.py] +from typing import Protocol +class Base(Protocol): + def meth(self) -> int: return 0 +[file b.py.2] +from typing import Protocol +class Base(Protocol): + def meth(self) -> int: ... +[out] +== +main:6: error: Call to abstract method "meth" of "Base" with trivial body via super() is unsafe From 1a4a8de14e806fa6d7e44a702c2d088af36e8bbd Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 20:00:35 +0100 Subject: [PATCH 02/13] Refactor --- mypy/checkmember.py | 46 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index ad8e95814b98..6221d753409c 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -296,23 +296,8 @@ def analyze_instance_member_access( # Look up the member. First look up the method dictionary. method = info.get_method(name) if method and not isinstance(method, Decorator): - unsafe_super = False if mx.is_super: - if isinstance(method, FuncDef) and method.is_trivial_body: - unsafe_super = True - impl = method - elif isinstance(method, OverloadedFuncDef): - if method.impl: - impl = method.impl if isinstance(method.impl, FuncDef) else method.impl.func - unsafe_super = impl.is_trivial_body - if unsafe_super: - ret_type = ( - impl.type.ret_type - if isinstance(impl.type, CallableType) - else AnyType(TypeOfAny.unannotated) - ) - if not subtypes.is_subtype(NoneType(), ret_type): - mx.msg.unsafe_super(method.name, method.info.name, mx.context) + validate_super_call(method, mx) if method.is_property: assert isinstance(method, OverloadedFuncDef) @@ -346,6 +331,25 @@ def analyze_instance_member_access( return analyze_member_var_access(name, typ, info, mx) +def validate_super_call(node: FuncBase, mx: MemberContext) -> None: + unsafe_super = False + if isinstance(node, FuncDef) and node.is_trivial_body: + unsafe_super = True + impl = node + elif isinstance(node, OverloadedFuncDef): + if node.impl: + impl = node.impl if isinstance(node.impl, FuncDef) else node.impl.func + unsafe_super = impl.is_trivial_body + if unsafe_super: + ret_type = ( + impl.type.ret_type + if isinstance(impl.type, CallableType) + else AnyType(TypeOfAny.unannotated) + ) + if not subtypes.is_subtype(NoneType(), ret_type): + mx.msg.unsafe_super(node.name, node.info.name, mx.context) + + def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: MemberContext) -> Type: # Class attribute. # TODO super? @@ -467,14 +471,8 @@ def analyze_member_var_access( if isinstance(vv, Decorator): # The associated Var node of a decorator contains the type. v = vv.var - if mx.is_super and vv.func.is_trivial_body: - ret_type = ( - vv.func.type.ret_type - if isinstance(vv.func.type, CallableType) - else AnyType(TypeOfAny.unannotated) - ) - if not subtypes.is_subtype(NoneType(), ret_type): - mx.msg.unsafe_super(vv.func.name, vv.func.info.name, mx.context) + if mx.is_super: + validate_super_call(vv.func, mx) if isinstance(vv, TypeInfo): # If the associated variable is a TypeInfo synthesize a Var node for From f4d7abdd8e972b178f56675dff85e6cc52f3d251 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 20:20:54 +0100 Subject: [PATCH 03/13] Force dedicated error code --- mypy/checker.py | 12 +++++++++--- mypy/errorcodes.py | 5 +++++ mypy/visitor.py | 2 +- mypy_bootstrap.ini | 3 +-- mypy_self_check.ini | 3 +-- test-data/unit/check-errorcodes.test | 8 ++++++++ 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index f0582a2d9d2e..f12a8997305e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1231,10 +1231,16 @@ def check_func_def( # entirely pass/Ellipsis/raise NotImplementedError. if isinstance(return_type, UninhabitedType): # This is a NoReturn function - self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn) + msg = message_registry.INVALID_IMPLICIT_RETURN else: - self.fail(message_registry.MISSING_RETURN_STATEMENT, defn) + msg = message_registry.MISSING_RETURN_STATEMENT + if body_is_trivial: + msg = msg._replace(code=codes.EMPTY_BODY) + self.fail(msg, defn) elif show_error: + msg = message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE + if body_is_trivial: + msg = msg._replace(code=codes.EMPTY_BODY) # similar to code in check_return_stmt self.check_subtype( subtype_label="implicitly returns", @@ -1242,7 +1248,7 @@ def check_func_def( supertype_label="expected", supertype=return_type, context=defn, - msg=message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE, + msg=msg, ) self.return_types.pop() diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 48d5f9c4e27c..a0f7a69b66a7 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -96,6 +96,11 @@ def __str__(self) -> str: UNUSED_COROUTINE: Final = ErrorCode( "unused-coroutine", "Ensure that all coroutines are used", "General" ) +EMPTY_BODY: Final = ErrorCode( + "empty-body", + "A dedicated error code to opt out return errors for empty/trivial bodies", + "General", +) SAFE_SUPER: Final = ErrorCode( "safe-super", "Warn about calls to abstract methods with empty/trivial bodies", "General" ) diff --git a/mypy/visitor.py b/mypy/visitor.py index 3c69f2d497f5..c5aa3caa8295 100644 --- a/mypy/visitor.py +++ b/mypy/visitor.py @@ -356,7 +356,7 @@ class NodeVisitor(Generic[T], ExpressionVisitor[T], StatementVisitor[T], Pattern subclasses do not always need to override all the methods. TODO: make the default return value explicit, then turn on - the warn_no_return flag in mypy_self_check.ini. + empty body checking in mypy_self_check.ini. """ # Not in superclasses: diff --git a/mypy_bootstrap.ini b/mypy_bootstrap.ini index 0085c9a806ec..c680990fbd9e 100644 --- a/mypy_bootstrap.ini +++ b/mypy_bootstrap.ini @@ -16,5 +16,4 @@ always_true = MYPYC [mypy-mypy.visitor] # See docstring for NodeVisitor for motivation. -warn_no_return = False -disable_error_code = return-value +disable_error_code = empty-body diff --git a/mypy_self_check.ini b/mypy_self_check.ini index ad7bfb31d4e6..620dbb7128be 100644 --- a/mypy_self_check.ini +++ b/mypy_self_check.ini @@ -15,5 +15,4 @@ enable_error_code = ignore-without-code,redundant-expr [mypy-mypy.visitor] # See docstring for NodeVisitor for motivation. -warn_no_return = False -disable_error_code = return-value +disable_error_code = empty-body diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 91341553133d..adf331fa1ca6 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -935,3 +935,11 @@ class Sub(Base): def meth(self) -> int: return super().meth() # E: Call to abstract method "meth" of "Base" with trivial body via super() is unsafe [safe-super] [builtins fixtures/exception.pyi] + +[case testDedicatedErrorCodeForEmpty_no_empty] +# flags: --strict-optional +from typing import Optional +def foo() -> int: ... # E: Missing return statement [empty-body] +def bar() -> None: ... +# This is consistent with how --warn-no-return behaves in general +def baz() -> Optional[int]: ... # E: Missing return statement [empty-body] From e86c18ee40ab7712706ad4f546f2831a2be23aa6 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 20:35:06 +0100 Subject: [PATCH 04/13] Fix docs build --- docs/source/class_basics.rst | 29 +++++++++++++++-------------- docs/source/error_code_list.rst | 20 +++++++++++--------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/docs/source/class_basics.rst b/docs/source/class_basics.rst index 3c769f9c9802..1d4164192318 100644 --- a/docs/source/class_basics.rst +++ b/docs/source/class_basics.rst @@ -312,20 +312,21 @@ Mypy allows you to omit the body for an abstract method, but if you do so, it is unsafe to call such method via ``super()``. For example: .. code-block:: python - from abc import abstractmethod - class Base: - @abstractmethod - def foo(self) -> int: pass - @abstractmethod - def bar(self) -> int: - return 0 - class Sub(Base): - def foo(self) -> int: - return super().foo() + 1 # error: Call to abstract method "foo" of "Base" - # with trivial body via super() is unsafe - @abstractmethod - def bar(self) -> int: - return super().bar() + 1 # This is OK however. + + from abc import abstractmethod + class Base: + @abstractmethod + def foo(self) -> int: pass + @abstractmethod + def bar(self) -> int: + return 0 + class Sub(Base): + def foo(self) -> int: + return super().foo() + 1 # error: Call to abstract method "foo" of "Base" + # with trivial body via super() is unsafe + @abstractmethod + def bar(self) -> int: + return super().bar() + 1 # This is OK however. A class can inherit any number of classes, both abstract and concrete. As with normal overrides, a dynamically typed method can diff --git a/docs/source/error_code_list.rst b/docs/source/error_code_list.rst index 2859f8cfce9f..a5dafe71970d 100644 --- a/docs/source/error_code_list.rst +++ b/docs/source/error_code_list.rst @@ -572,15 +572,17 @@ bodies are just empty. Calling such methods in subclasses via ``super()`` will cause runtime errors, so mypy prevents you from doing so: .. code-block:: python - from abc import abstractmethod - class Base: - @abstractmethod - def foo(self) -> int: ... - class Sub(Base): - def foo(self) -> int: - return super().foo() + 1 # error: Call to abstract method "foo" of "Base" with - # trivial body via super() is unsafe [safe-super] - Sub().foo() # This will crash at runtime. + + from abc import abstractmethod + class Base: + @abstractmethod + def foo(self) -> int: ... + class Sub(Base): + def foo(self) -> int: + return super().foo() + 1 # error: Call to abstract method "foo" of "Base" with + # trivial body via super() is unsafe [safe-super] + Sub().foo() # This will crash at runtime. + Mypy considers the following as trivial bodies: a ``pass`` statement, a literal ellipsis ``...``, a docstring, and a ``raise NotImplementedError`` statement. From 754bffda5db5b50ae98c5633537bb4071a88ab4b Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 20:37:18 +0100 Subject: [PATCH 05/13] Fix docs build --- docs/source/protocols.rst | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/source/protocols.rst b/docs/source/protocols.rst index 4fe2b8aca975..7f25d8dd9d09 100644 --- a/docs/source/protocols.rst +++ b/docs/source/protocols.rst @@ -378,13 +378,14 @@ omitting a value for an attribute or a method body will make it implicitly abstract: .. code-block:: python - class SomeProto(Protocol): - attr: int # Note, no right hand side - def method(self) -> str: ... # Literal ... here - class ExplicitSubclass(SomeProto): - pass - ExplicitSubclass() # error: Cannot instantiate abstract class 'ExplicitSubclass' - # with abstract attributes 'attr' and 'method' + + class SomeProto(Protocol): + attr: int # Note, no right hand side + def method(self) -> str: ... # Literal ... here + class ExplicitSubclass(SomeProto): + pass + ExplicitSubclass() # error: Cannot instantiate abstract class 'ExplicitSubclass' + # with abstract attributes 'attr' and 'method' Recursive protocols ******************* From 64ed35811f2800b35c5737b034cafd40fdd376c0 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 21:00:20 +0100 Subject: [PATCH 06/13] Fix a test --- test-data/unit/daemon.test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-data/unit/daemon.test b/test-data/unit/daemon.test index 370413ee774b..bb07361707b1 100644 --- a/test-data/unit/daemon.test +++ b/test-data/unit/daemon.test @@ -434,12 +434,12 @@ $ dmypy inspect --show attrs bar.py:10:1 --union-attrs [file foo.py] class B: - def b(self) -> int: ... + def b(self) -> int: return 0 a: int class C(B): a: int y: int - def x(self) -> int: ... + def x(self) -> int: return 0 v: C # line 9 if False: From 4461e9484f40b9f565f7e58989fdfb658a569ad0 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 21:24:02 +0100 Subject: [PATCH 07/13] Ignore generated methods --- mypy/checker.py | 7 +++++++ test-data/unit/check-attr.test | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index f12a8997305e..818b9dab259a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -63,6 +63,7 @@ ARG_STAR, CONTRAVARIANT, COVARIANT, + FUNC_NO_INFO, GDEF, IMPLICITLY_ABSTRACT, INVARIANT, @@ -1218,6 +1219,12 @@ def check_func_def( not allow_empty and not (isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT) and not self.is_stub + and ( + defn.info is FUNC_NO_INFO + # If we can't find the symbol, most likely there was already an error. + or defn.name in defn.info.names + and not defn.info.names[defn.name].plugin_generated + ) ) if self.options.warn_no_return: diff --git a/test-data/unit/check-attr.test b/test-data/unit/check-attr.test index c5b64ee61376..e2f97f5c708f 100644 --- a/test-data/unit/check-attr.test +++ b/test-data/unit/check-attr.test @@ -1,4 +1,4 @@ -[case testAttrsSimple] +[case testAttrsSimple_no_empty] import attr @attr.s class A: From 263b76001900ebffd7b3af7625e3af73232f95af Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sun, 25 Sep 2022 21:44:30 +0100 Subject: [PATCH 08/13] Add some special-casing to minimize fallout --- mypy/checker.py | 17 +++++++++++------ test-data/unit/check-abstract.test | 3 +++ test-data/unit/check-errorcodes.test | 5 +++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 818b9dab259a..57e643071cdd 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1219,14 +1219,19 @@ def check_func_def( not allow_empty and not (isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT) and not self.is_stub - and ( - defn.info is FUNC_NO_INFO - # If we can't find the symbol, most likely there was already an error. - or defn.name in defn.info.names - and not defn.info.names[defn.name].plugin_generated - ) ) + # Ignore plugin generated methods, these usually don't need any bodies. + if defn.info is not FUNC_NO_INFO and ( + defn.name not in defn.info.names or defn.info.names[defn.name].plugin_generated + ): + show_error = False + + # We want to minimize the fallout from checking empty bodies + # that was absent in many mypy versions. + if body_is_trivial and is_subtype(NoneType(), return_type): + show_error = False + if self.options.warn_no_return: if ( not self.current_node_deferred diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index 2831ba47caa0..558933fc1ae8 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -1057,6 +1057,7 @@ b.y = 1 -- ----------------------------------------------- [case testEmptyBodyProhibitedFunction] +# flags: --strict-optional from typing import overload, Union def func1(x: str) -> int: pass # E: Missing return statement @@ -1079,6 +1080,7 @@ def func5(x: Union[int, str]) -> Union[int, str]: # E: Missing return statement """Some function.""" [case testEmptyBodyProhibitedMethodNonAbstract] +# flags: --strict-optional from typing import overload, Union class A: @@ -1113,6 +1115,7 @@ class C: [builtins fixtures/classmethod.pyi] [case testEmptyBodyProhibitedPropertyNonAbstract] +# flags: --strict-optional class A: @property def x(self) -> int: ... # E: Missing return statement diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index adf331fa1ca6..ea65a45a0405 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -941,5 +941,6 @@ class Sub(Base): from typing import Optional def foo() -> int: ... # E: Missing return statement [empty-body] def bar() -> None: ... -# This is consistent with how --warn-no-return behaves in general -def baz() -> Optional[int]: ... # E: Missing return statement [empty-body] +# This is inconsistent with how --warn-no-return behaves in general +# but we want to minimize fallout of finally handling empty bodies. +def baz() -> Optional[int]: ... # OK From 94fa515d2c8f6539a3b06bb96415e33d8c7f770e Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 26 Sep 2022 14:11:42 +0100 Subject: [PATCH 09/13] Add a note for abstract classes; better testing --- mypy/checker.py | 34 +++++++++++++++++++++++------- test-data/unit/check-abstract.test | 17 +++++++++++++++ test-data/unit/lib-stub/abc.pyi | 2 +- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 57e643071cdd..f8f65efc7eed 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1232,6 +1232,13 @@ def check_func_def( if body_is_trivial and is_subtype(NoneType(), return_type): show_error = False + may_be_abstract = ( + body_is_trivial + and defn.info is not FUNC_NO_INFO + and defn.info.metaclass_type is not None + and defn.info.metaclass_type.type.fullname == "abc.ABCMeta" + ) + if self.options.warn_no_return: if ( not self.current_node_deferred @@ -1249,19 +1256,30 @@ def check_func_def( if body_is_trivial: msg = msg._replace(code=codes.EMPTY_BODY) self.fail(msg, defn) + if may_be_abstract: + self.note( + "If the method is meant to be abstract use @abc.abstractmethod", + defn, + ) elif show_error: msg = message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE if body_is_trivial: msg = msg._replace(code=codes.EMPTY_BODY) # similar to code in check_return_stmt - self.check_subtype( - subtype_label="implicitly returns", - subtype=NoneType(), - supertype_label="expected", - supertype=return_type, - context=defn, - msg=msg, - ) + if ( + not self.check_subtype( + subtype_label="implicitly returns", + subtype=NoneType(), + supertype_label="expected", + supertype=return_type, + context=defn, + msg=msg, + ) + and may_be_abstract + ): + self.note( + "If the method is meant to be abstract use @abc.abstractmethod", defn + ) self.return_types.pop() diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index 558933fc1ae8..cb655400d75a 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -1143,7 +1143,17 @@ class C: def y(self, value: int) -> None: pass [builtins fixtures/property.pyi] +[case testEmptyBodyNoteABCMeta] +# flags: --strict-optional +from abc import ABC + +class A(ABC): + def foo(self) -> int: # E: Missing return statement \ + # N: If the method is meant to be abstract use @abc.abstractmethod + ... + [case testEmptyBodyAllowedFunctionStub] +# flags: --strict-optional import stub [file stub.pyi] from typing import overload, Union @@ -1154,6 +1164,7 @@ def func3(x: str) -> int: """Some function.""" [case testEmptyBodyAllowedMethodNonAbstractStub] +# flags: --strict-optional import stub [file stub.pyi] from typing import overload, Union @@ -1175,6 +1186,7 @@ class B: [builtins fixtures/classmethod.pyi] [case testEmptyBodyAllowedPropertyNonAbstractStub] +# flags: --strict-optional import stub [file stub.pyi] class A: @@ -1205,6 +1217,7 @@ class C: [builtins fixtures/property.pyi] [case testEmptyBodyAllowedMethodAbstract] +# flags: --strict-optional from typing import overload, Union from abc import abstractmethod @@ -1252,6 +1265,7 @@ class C: [builtins fixtures/classmethod.pyi] [case testEmptyBodyAllowedPropertyAbstract] +# flags: --strict-optional from abc import abstractmethod class A: @property @@ -1290,6 +1304,7 @@ class C: [builtins fixtures/property.pyi] [case testEmptyBodyImplicitlyAbstractProtocol] +# flags: --strict-optional from typing import Protocol, overload, Union class P1(Protocol): @@ -1330,6 +1345,7 @@ C3() [builtins fixtures/classmethod.pyi] [case testEmptyBodyImplicitlyAbstractProtocolProperty] +# flags: --strict-optional from typing import Protocol class P1(Protocol): @@ -1359,6 +1375,7 @@ C2() [builtins fixtures/property.pyi] [case testEmptyBodyImplicitlyAbstractProtocolStub] +# flags: --strict-optional from stub import P1, P2, P3, P4 class B1(P1): ... diff --git a/test-data/unit/lib-stub/abc.pyi b/test-data/unit/lib-stub/abc.pyi index da90b588fca3..e60f709a5187 100644 --- a/test-data/unit/lib-stub/abc.pyi +++ b/test-data/unit/lib-stub/abc.pyi @@ -2,8 +2,8 @@ from typing import Type, Any, TypeVar T = TypeVar('T', bound=Type[Any]) -class ABC(type): pass class ABCMeta(type): def register(cls, tp: T) -> T: pass +class ABC(metaclass=ABCMeta): pass abstractmethod = object() abstractproperty = object() From dcc282686b482058625b49943d1a9258a21f2354 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 26 Sep 2022 16:33:06 +0100 Subject: [PATCH 10/13] Exclude type-check only --- mypy/checker.py | 5 +++++ mypy/nodes.py | 5 ++++- mypy/reachability.py | 4 ++++ test-data/unit/check-abstract.test | 8 ++++++++ test-data/unit/lib-stub/typing.pyi | 1 + 5 files changed, 22 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index f8f65efc7eed..cb36b0ff091c 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1227,6 +1227,11 @@ def check_func_def( ): show_error = False + # Ignore also definitions that appear in `if TYPE_CHECKING: ...` blocks. + # These can't be called at runtime anyway (similar to plugin-generated). + if isinstance(defn, FuncDef) and defn.is_mypy_only: + show_error = False + # We want to minimize the fallout from checking empty bodies # that was absent in many mypy versions. if body_is_trivial and is_subtype(NoneType(), return_type): diff --git a/mypy/nodes.py b/mypy/nodes.py index 6b7f25e7c6bc..a34ed99af615 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -782,6 +782,7 @@ class FuncDef(FuncItem, SymbolNode, Statement): "original_def", "deco_line", "is_trivial_body", + "is_mypy_only", ) # Note that all __init__ args must have default values @@ -803,8 +804,10 @@ def __init__( self.is_final = False # Original conditional definition self.original_def: None | FuncDef | Var | Decorator = None - # Used for error reporting (to keep backwad compatibility with pre-3.8) + # Used for error reporting (to keep backward compatibility with pre-3.8) self.deco_line: int | None = None + # Definitions that appear in if TYPE_CHECKING are marked with this flag. + self.is_mypy_only = False @property def name(self) -> str: diff --git a/mypy/reachability.py b/mypy/reachability.py index a688592a54b9..8602fc645e2b 100644 --- a/mypy/reachability.py +++ b/mypy/reachability.py @@ -13,6 +13,7 @@ CallExpr, ComparisonExpr, Expression, + FuncDef, IfStmt, Import, ImportAll, @@ -357,3 +358,6 @@ def visit_import_from(self, node: ImportFrom) -> None: def visit_import_all(self, node: ImportAll) -> None: node.is_mypy_only = True + + def visit_func_def(self, node: FuncDef) -> None: + node.is_mypy_only = True diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index cb655400d75a..dc03fda6dafa 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -1619,3 +1619,11 @@ class SubProto(Proto): class SubAbstract(Abstract): def meth(self) -> Optional[int]: return super().meth() + +[case testEmptyBodyTypeCheckingOnly] +# flags: --strict-optional +from typing import TYPE_CHECKING + +class C: + if TYPE_CHECKING: + def dynamic(self) -> int: ... # OK diff --git a/test-data/unit/lib-stub/typing.pyi b/test-data/unit/lib-stub/typing.pyi index 0a1bb42b936c..23d97704d934 100644 --- a/test-data/unit/lib-stub/typing.pyi +++ b/test-data/unit/lib-stub/typing.pyi @@ -27,6 +27,7 @@ NoReturn = 0 Never = 0 NewType = 0 ParamSpec = 0 +TYPE_CHECKING = 0 T = TypeVar('T') T_co = TypeVar('T_co', covariant=True) From e354aae64c9879203f0c6a0799eae70b50cbfaaa Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 29 Sep 2022 11:33:16 +0100 Subject: [PATCH 11/13] Address CR --- mypy/checker.py | 29 ++++++++++++++------------- mypy/main.py | 2 ++ mypy/message_registry.py | 3 +++ mypy/nodes.py | 7 ++++++- test-data/unit/check-abstract.test | 2 +- test-data/unit/check-incremental.test | 23 +++++++++++++++++++++ 6 files changed, 50 insertions(+), 16 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 4324d4323fe8..fa877ae1638a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1217,9 +1217,13 @@ def check_func_def( not body_is_trivial or # Allow empty bodies for abstract methods, overloads, in tests and stubs. - not allow_empty - and not (isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT) - and not self.is_stub + ( + not allow_empty + and not ( + isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT + ) + and not self.is_stub + ) ) # Ignore plugin generated methods, these usually don't need any bodies. @@ -1242,7 +1246,7 @@ def check_func_def( body_is_trivial and defn.info is not FUNC_NO_INFO and defn.info.metaclass_type is not None - and defn.info.metaclass_type.type.fullname == "abc.ABCMeta" + and defn.info.metaclass_type.type.has_base("abc.ABCMeta") ) if self.options.warn_no_return: @@ -1252,8 +1256,7 @@ def check_func_def( and show_error ): # Control flow fell off the end of a function that was - # declared to return a non-None type and is not - # entirely pass/Ellipsis/raise NotImplementedError. + # declared to return a non-None type. if isinstance(return_type, UninhabitedType): # This is a NoReturn function msg = message_registry.INVALID_IMPLICIT_RETURN @@ -1263,10 +1266,7 @@ def check_func_def( msg = msg._replace(code=codes.EMPTY_BODY) self.fail(msg, defn) if may_be_abstract: - self.note( - "If the method is meant to be abstract use @abc.abstractmethod", - defn, - ) + self.note(message_registry.EMPTY_BODY_ABSTRACT, defn) elif show_error: msg = message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE if body_is_trivial: @@ -1283,9 +1283,7 @@ def check_func_def( ) and may_be_abstract ): - self.note( - "If the method is meant to be abstract use @abc.abstractmethod", defn - ) + self.note(message_registry.EMPTY_BODY_ABSTRACT, defn) self.return_types.pop() @@ -6186,9 +6184,12 @@ def fail( self.msg.fail(msg, context, code=code) def note( - self, msg: str, context: Context, offset: int = 0, *, code: ErrorCode | None = None + self, msg: str | ErrorMessage, context: Context, offset: int = 0, *, code: ErrorCode | None = None ) -> None: """Produce a note.""" + if isinstance(msg, ErrorMessage): + self.msg.note(msg.value, context, code=msg.code) + return self.msg.note(msg, context, offset=offset, code=code) def iterable_item_type(self, instance: Instance) -> Type: diff --git a/mypy/main.py b/mypy/main.py index f0316737ec22..e859e4fed42a 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -999,6 +999,8 @@ def add_invertible_flag( "the contents of SHADOW_FILE instead.", ) add_invertible_flag("--fast-exit", default=True, help=argparse.SUPPRESS, group=internals_group) + # This flag is useful for mypy tests, where function bodies may be omitted. Plugin developers + # may want to use this as well in their tests. add_invertible_flag( "--allow-empty-bodies", default=False, help=argparse.SUPPRESS, group=internals_group ) diff --git a/mypy/message_registry.py b/mypy/message_registry.py index 9daa8528e7f6..df9bca43bbb3 100644 --- a/mypy/message_registry.py +++ b/mypy/message_registry.py @@ -33,6 +33,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage: # Type checker error message constants NO_RETURN_VALUE_EXPECTED: Final = ErrorMessage("No return value expected", codes.RETURN_VALUE) MISSING_RETURN_STATEMENT: Final = ErrorMessage("Missing return statement", codes.RETURN) +EMPTY_BODY_ABSTRACT: Final = ErrorMessage( + "If the method is meant to be abstract, use @abc.abstractmethod", codes.EMPTY_BODY +) INVALID_IMPLICIT_RETURN: Final = ErrorMessage("Implicit return in function which does not return") INCOMPATIBLE_RETURN_VALUE_TYPE: Final = ErrorMessage( "Incompatible return value type", codes.RETURN_VALUE diff --git a/mypy/nodes.py b/mypy/nodes.py index a34ed99af615..8c2306361d50 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -758,7 +758,12 @@ def is_dynamic(self) -> bool: return self.type is None -FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional", "is_trivial_body"] +FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + [ + "is_decorated", + "is_conditional", + "is_trivial_body", + "is_mypy_only", +] # Abstract status of a function NOT_ABSTRACT: Final = 0 diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index dc03fda6dafa..f67d9859397e 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -1149,7 +1149,7 @@ from abc import ABC class A(ABC): def foo(self) -> int: # E: Missing return statement \ - # N: If the method is meant to be abstract use @abc.abstractmethod + # N: If the method is meant to be abstract, use @abc.abstractmethod ... [case testEmptyBodyAllowedFunctionStub] diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index e4ab52e860a2..7da379f0be01 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -6058,3 +6058,26 @@ tmp/m.py:9: note: Expected: tmp/m.py:9: note: def update() -> bool tmp/m.py:9: note: Got: tmp/m.py:9: note: def update() -> str + +[case testAbstractBodyTurnsEmptyCoarse] +# flags: --strict-optional +from b import Base + +class Sub(Base): + def meth(self) -> int: + return super().meth() + +[file b.py] +from abc import abstractmethod +class Base: + @abstractmethod + def meth(self) -> int: return 0 + +[file b.py.2] +from abc import abstractmethod +class Base: + @abstractmethod + def meth(self) -> int: ... +[out] +[out2] +main:6: error: Call to abstract method "meth" of "Base" with trivial body via super() is unsafe From 489dc349d2bba1e37ad142fc47bd62d67b8cb93c Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 29 Sep 2022 12:06:03 +0100 Subject: [PATCH 12/13] Fix black --- mypy/checker.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index fa877ae1638a..8dac00bba23a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -6184,7 +6184,12 @@ def fail( self.msg.fail(msg, context, code=code) def note( - self, msg: str | ErrorMessage, context: Context, offset: int = 0, *, code: ErrorCode | None = None + self, + msg: str | ErrorMessage, + context: Context, + offset: int = 0, + *, + code: ErrorCode | None = None, ) -> None: """Produce a note.""" if isinstance(msg, ErrorMessage): From 821c42e66697769ea0100147e2999743fc826fab Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 29 Sep 2022 12:21:13 +0100 Subject: [PATCH 13/13] Try fixing mypyc build --- mypy/errorcodes.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index a0f7a69b66a7..897cb593a032 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -96,7 +96,9 @@ def __str__(self) -> str: UNUSED_COROUTINE: Final = ErrorCode( "unused-coroutine", "Ensure that all coroutines are used", "General" ) -EMPTY_BODY: Final = ErrorCode( +# TODO: why do we need the explicit type here? Without it mypyc CI builds fail with +# mypy/message_registry.py:37: error: Cannot determine type of "EMPTY_BODY" [has-type] +EMPTY_BODY: Final[ErrorCode] = ErrorCode( "empty-body", "A dedicated error code to opt out return errors for empty/trivial bodies", "General",