8000 Handle empty bodies safely (#13729) · python/mypy@ef22444 · GitHub
[go: up one dir, main page]
Skip to content

Commit ef22444

Browse files
ilevkivskyiilevkivskyi
authored
Handle empty bodies safely (#13729)
Fixes #2350 This essentially re-applies #8111 modulo various (logical) changes in master since then.The only important difference is that now I override few return-related error codes for empty bodies, to allow opting out easily in next few versions (I still keep the flag to simplify testing).
1 parent d560570 commit ef22444
  • visitor.py
  • test-data/unit

    • 37 files changed

    +957
    -50
    lines changed

    ‎docs/source/class_basics.rst‎

    Lines changed: 20 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -308,6 +308,26 @@ however:
    308308
    in this case, but any attempt to construct an instance will be
    309309
    flagged as an error.
    310310

    311+
    Mypy allows you to omit the body for an abstract method, but if you do so,
    312+
    it is unsafe to call such method via ``super()``. For example:
    313+
    314+
    .. code-block:: python
    315+
    316+
    from abc import abstractmethod
    317+
    class Base:
    318+
    @abstractmethod
    319+
    def foo(self) -> int: pass
    320+
    @abstractmethod
    321+
    def bar(self) -> int:
    322+
    return 0
    323+
    class Sub(Base):
    324+
    def foo(self) -> int:
    325+
    return super().foo() + 1 # error: Call to abstract method "foo" of "Base"
    326+
    # with trivial body via super() is unsafe
    327+
    @abstractmethod
    328+
    def bar(self) -> int:
    329+
    return super().bar() + 1 # This is OK however.
    330+
    311331
    A class can inherit any number of classes, both abstract and
    312332
    concrete. As with normal overrides, a dynamically typed method can
    313333
    override or implement a statically typed method defined in any base

    ‎docs/source/error_code_list.rst‎

    Lines changed: 22 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -564,6 +564,28 @@ Example:
    564564
    # Error: Cannot instantiate abstract class "Thing" with abstract attribute "save" [abstract]
    565565
    t = Thing()
    566566
    567+
    Check that call to an abstract method via super is valid [safe-super]
    568+
    ---------------------------------------------------------------------
    569+
    570+
    Abstract methods often don't have any default implementation, i.e. their
    571+
    bodies are just empty. Calling such methods in subclasses via ``super()``
    572+
    will cause runtime errors, so mypy prevents you from doing so:
    573+
    574+
    .. code-block:: python
    575+
    576+
    from abc import abstractmethod
    577+
    class Base:
    578+
    @abstractmethod
    579+
    def foo(self) -> int: ...
    580+
    class Sub(Base):
    581+
    def foo(self) -> int:
    582+
    return super().foo() + 1 # error: Call to abstract method "foo" of "Base" with
    583+
    # trivial body via super() is unsafe [safe-super]
    584+
    Sub().foo() # This will crash at runtime.
    585+
    586+
    Mypy considers the following as trivial bodies: a ``pass`` statement, a literal
    587+
    ellipsis ``...``, a docstring, and a ``raise NotImplementedError`` statement.
    588+
    567589
    Check the target of NewType [valid-newtype]
    568590
    -------------------------------------------
    569591

    ‎docs/source/protocols.rst‎

    Lines changed: 13 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -149,7 +149,19 @@ protocols. If you explicitly subclass these protocols you can inherit
    149149
    these default implementations. Explicitly including a protocol as a
    150150
    base class is also a way of documenting that your class implements a
    151151
    particular protocol, and it forces mypy to verify that your class
    152-
    implementation is actually compatible with the protocol.
    152+
    implementation is actually compatible with the protocol. In particular,
    153+
    omitting a value for an attribute or a method body will make it implicitly
    154+
    abstract:
    155+
    156+
    .. code-block:: python
    157+
    158+
    class SomeProto(Protocol):
    159+
    attr: int # Note, no right hand side
    160+
    def method(self) -> str: ... # Literal ... here
    161+
    class ExplicitSubclass(SomeProto):
    162+
    pass
    163+
    ExplicitSubclass() # error: Cannot instantiate abstract class 'ExplicitSubclass'
    164+
    # with abstract attributes 'attr' and 'method'
    153165
    154166
    Recursive protocols
    155167
    *******************

    ‎mypy/checker.py‎

    Lines changed: 88 additions & 21 deletions
    Original file line numberDiff line numberDiff line change
    @@ -63,13 +63,15 @@
    6363
    ARG_STAR,
    6464
    CONTRAVARIANT,
    6565
    COVARIANT,
    66+
    FUNC_NO_INFO,
    6667
    GDEF,
    6768
    IMPLICITLY_ABSTRACT,
    6869
    INVARIANT,
    6970
    IS_ABSTRACT,
    7071
    LDEF,
    7172
    LITERAL_TYPE,
    7273
    MDEF,
    74+
    NOT_ABSTRACT,
    7375
    AssertStmt,
    7476
    AssignmentExpr,
    7577
    AssignmentStmt,
    @@ -620,7 +622,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
    620622
    self.visit_decorator(cast(Decorator, defn.items[0]))
    621623
    for fdef in defn.items:
    622624
    assert isinstance(fdef, Decorator)
    623-
    self.check_func_item(fdef.func, name=fdef.func.name)
    625+
    self.check_func_item(fdef.func, name=fdef.func.name, allow_empty=True)
    624626
    if fdef.func.abstract_status in (IS_ABSTRACT, IMPLICITLY_ABSTRACT):
    625627
    num_abstract += 1
    626628
    if num_abstract not in (0, len(defn.items)):
    @@ -987,7 +989,11 @@ def _visit_func_def(self, defn: FuncDef) -> None:
    987989
    )
    988990

    989991
    def check_func_item(
    990-
    self, defn: FuncItem, type_override: CallableType | None = None, name: str | None = None
    992+
    self,
    993+
    defn: FuncItem,
    994+
    type_override: CallableType | None = None,
    995+
    name: str | None = None,
    996+
    allow_empty: bool = False,
    991997
    ) -> None:
    992998
    """Type check a function.
    993999
    @@ -1001,7 +1007,7 @@ def check_func_item(
    10011007
    typ = type_override.copy_modified(line=typ.line, column=typ.column)
    10021008
    if isinstance(typ, CallableType):
    10031009
    with self.enter_attribute_inference_context():
    1004-
    self.check_func_def(defn, typ, name)
    1010+
    self.check_func_def(defn, typ, name, allow_empty)
    10051011
    else:
    10061012
    raise RuntimeError("Not supported")
    10071013

    @@ -1018,7 +1024,9 @@ def enter_attribute_inference_context(self) -> Iterator[None]:
    10181024
    yield None
    10191025
    self.inferred_attribute_types = old_types
    10201026

    1021-
    def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> None:
    1027+
    def check_func_def(
    1028+
    self, defn: FuncItem, typ: CallableType, name: str | None, allow_empty: bool = False
    1029+
    ) -> None:
    10221030
    """Type check a function definition."""
    1023 178F 1031
    # Expand type variables with value restrictions to ordinary types.
    10241032
    expanded = self.expand_typevars(defn, typ)
    @@ -1190,7 +1198,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) ->
    11901198
    self.accept(item.body)
    11911199
    unreachable = self.binder.is_unreachable()
    11921200

    1193-
    if not unreachable and not body_is_trivial:
    1201+
    if not unreachable:
    11941202
    if defn.is_generator or is_named_instance(
    11951203
    self.return_types[-1], "typing.AwaitableGenerator"
    11961204
    ):
    @@ -1203,28 +1211,79 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) ->
    12031211
    return_type = self.return_types[-1]
    12041212
    return_type = get_proper_type(return_type)
    12051213

    1214+
    allow_empty = allow_empty or self.options.allow_empty_bodies
    1215+
    1216+
    show_error = (
    1217+
    not body_is_trivial
    1218+
    or
    1219+
    # Allow empty bodies for abstract methods, overloads, in tests and stubs.
    1220+
    (
    1221+
    not allow_empty
    1222+
    and not (
    1223+
    isinstance(defn, FuncDef) and defn.abstract_status != NOT_ABSTRACT
    1224+
    )
    1225+
    and not self.is_stub
    1226+
    )
    1227+
    )
    1228+
    1229+
    # Ignore plugin generated methods, these usually don't need any bodies.
    1230+
    if defn.info is not FUNC_NO_INFO and (
    1231+
    defn.name not in defn.info.names or defn.info.names[defn.name].plugin_generated
    1232+
    ):
    1233+
    show_error = False
    1234+
    1235+
    # Ignore also definitions that appear in `if TYPE_CHECKING: ...` blocks.
    1236+
    # These can't be called at runtime anyway (similar to plugin-generated).
    1237+
    if isinstance(defn, FuncDef) and defn.is_mypy_only:
    1238+
    show_error = False
    1239+
    1240+
    # We want to minimize the fallout from checking empty bodies
    1241+
    # that was absent in many mypy versions.
    1242+
    if body_is_trivial and is_subtype(NoneType(), return_type):
    1243+
    show_error = False
    1244+
    1245+
    may_be_abstract = (
    1246+
    body_is_trivial
    1247+
    and defn.info is not FUNC_NO_INFO
    1248+
    and defn.info.metaclass_type is not None
    1249+
    and defn.info.metaclass_type.type.has_base("abc.ABCMeta")
    1250+
    )
    1251+
    12061252
    if self.options.warn_no_return:
    1207-
    if not self.current_node_deferred and not isinstance(
    1208-
    return_type, (NoneType, AnyType)
    1253+
    if (
    1254+
    not self.current_node_deferred
    1255+
    and not isinstance(return_type, (NoneType, AnyType))
    1256+
    and show_error
    12091257
    ):
    12101258
    # Control flow fell off the end of a function that was
    1211-
    # declared to return a non-None type and is not
    1212-
    # entirely pass/Ellipsis/raise NotImplementedError.
    1259+
    # declared to return a non-None type.
    12131260
    if isinstance(return_type, UninhabitedType):
    12141261
    # This is a NoReturn function
    1215-
    self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn)
    1262+
    msg = message_registry.INVALID_IMPLICIT_RETURN
    12161263
    else:
    1217-
    self.fail(message_registry.MISSING_RETURN_STATEMENT, defn)
    1218-
    else:
    1264+
    msg = message_registry.MISSING_RETURN_STATEMENT
    1265+
    if body_is_trivial:
    1266+
    msg = msg._replace(code=codes.EMPTY_BODY)
    1267+
    self.fail(msg, defn)
    1268+
    if may_be_abstract:
    1269+
    self.note(message_registry.EMPTY_BODY_ABSTRACT, defn)
    1270+
    elif show_error:
    1271+
    msg = message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE
    1272+
    if body_is_trivial:
    1273+
    msg = msg._replace(code=codes.EMPTY_BODY)
    12191274
    # similar to code in check_return_stmt
    1220-
    self.check_subtype(
    1221-
    subtype_label="implicitly returns",
    1222-
    subtype=NoneType(),
    1223-
    supertype_label="expected",
    1224-
    supertype=return_type,
    1225-
    context=defn,
    1226-
    msg=message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE,
    1227-
    )
    1275+
    if (
    1276+
    not self.check_subtype(
    1277+
    subtype_label="implicitly returns",
    1278+
    subtype=NoneType(),
    1279+
    supertype_label="expected",
    1280+
    supertype=return_type,
    1281+
    context=defn,
    1282+
    msg=msg,
    1283+
    )
    1284+
    and may_be_abstract
    1285+
    ):
    1286+
    self.note(message_registry.EMPTY_BODY_ABSTRACT, defn)
    12281287

    12291288
    self.return_types.pop()
    12301289

    @@ -6125,9 +6184,17 @@ def fail(
    61256184
    self.msg.fail(msg, context, code=code)
    61266185

    61276186
    def note(
    6128-
    self, msg: str, context: Context, offset: int = 0, *, code: ErrorCode | None = None
    6187+
    self,
    6188+
    msg: str | ErrorMessage,
    6189+
    context: Context,
    6190+
    offset: int = 0,
    6191+
    *,
    6192+
    code: ErrorCode | None = None,
    61296193
    ) -> None:
    61306194
    """Produce a note."""
    6195+
    if isinstance(msg, ErrorMessage):
    6196+
    self.msg.note(msg.value, context, code=msg.code)
    6197+
    return
    61316198
    self.msg.note(msg, context, offset=offset, code=code)
    61326199

    61336200
    def iterable_item_type(self, instance: Instance) -> Type:

    ‎mypy/checkmember.py‎

    Lines changed: 24 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -296,6 +296,9 @@ def analyze_instance_member_access(
    296296
    # Look up the member. First look up the method dictionary.
    297297
    method = info.get_method(name)
    298298
    if method and not isinstance(method, Decorator):
    299+
    if mx.is_super:
    300+
    validate_super_call(method, mx)
    301+
    299302
    if method.is_property:
    300303
    assert isinstance(method, OverloadedFuncDef)
    301304
    first_item = cast(Decorator, method.items[0])
    @@ -328,6 +331,25 @@ def analyze_instance_member_access(
    328331
    return analyze_member_var_access(name, typ, info, mx)
    329332

    330333

    334+
    def validate_super_call(node: FuncBase, mx: MemberContext) -> None:
    335+
    unsafe_super = False
    336+
    if isinstance(node, FuncDef) and node.is_trivial_body:
    337+
    unsafe_super = True
    338+
    impl = node
    339+
    elif isinstance(node, OverloadedFuncDef):
    340+
    if node.impl:
    341+
    impl = node.impl if isinstance(node.impl, FuncDef) else node.impl.func
    342+
    unsafe_super = impl.is_trivial_body
    343+
    if unsafe_super:
    344+
    ret_type = (
    345+
    impl.type.ret_type
    346+
    if isinstance(impl.type, CallableType)
    347+
    else AnyType(TypeOfAny.unannotated)
    348+
    )
    349+
    if not subtypes.is_subtype(NoneType(), ret_type):
    350+
    mx.msg.unsafe_super(node.name, node.info.name, mx.context)
    351+
    352+
    331353
    def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: MemberContext) -> Type:
    332354
    # Class attribute.
    333355
    # TODO super?
    @@ -449,6 +471,8 @@ def analyze_member_var_access(
    449471
    if isinstance(vv, Decorator):
    450472
    # The associated Var node of a decorator contains the type.
    451473
    v = vv.var
    474+
    if mx.is_super:
    475+
    validate_super_call(vv.func, mx)
    452476

    453477
    if isinstance(vv, TypeInfo):
    454478
    # If the associated variable is a TypeInfo synthesize a Var node for

    ‎mypy/errorcodes.py‎

    Lines changed: 10 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -96,6 +96,16 @@ def __str__(self) -> str:
    57AF 9696
    UNUSED_COROUTINE: Final = ErrorCode(
    9797
    "unused-coroutine", "Ensure that all coroutines are used", "General"
    9898
    )
    99+
    # TODO: why do we need the explicit type here? Without it mypyc CI builds fail with
    100+
    # mypy/message_registry.py:37: error: Cannot determine type of "EMPTY_BODY" [has-type]
    101+
    EMPTY_BODY: Final[ErrorCode] = ErrorCode(
    102+
    "empty-body",
    103+
    "A dedicated error code to opt out return errors for empty/trivial bodies",
    104+
    "General",
    105+
    )
    106+
    SAFE_SUPER: Final = ErrorCode(
    107+
    "safe-super", "Warn about calls to abstract methods with empty/trivial bodies", "General"
    108+
    )
    99109

    100110
    # These error codes aren't enabled by default.
    101111
    NO_UNTYPED_DEF: Final[ErrorCode] = ErrorCode(

    ‎mypy/main.py‎

    Lines changed: 5 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -999,6 +999,11 @@ def add_invertible_flag(
    999999
    "the contents of SHADOW_FILE instead.",
    10001000
    )
    10011001
    add_invertible_flag("--fast-exit", default=True, help=argparse.SUPPRESS, group=internals_group)
    1002+
    # This flag is useful for mypy tests, where function bodies may be omitted. Plugin developers
    1003+
    # may want to use this as well in their tests.
    1004+
    add_invertible_flag(
    1005+
    "--allow-empty-bodies", default=False, help=argparse.SUPPRESS, group=internals_group
    1006+
    )
    10021007

    10031008
    report_group = parser.add_argument_group(
    10041009
    title="Report generation", description="Generate a report in the specified format."

    ‎mypy/message_registry.py‎

    Lines changed: 3 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -33,6 +33,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
    3333
    # Type checker error message constants
    3434
    NO_RETURN_VALUE_EXPECTED: Final = ErrorMessage("No return value expected", codes.RETURN_VALUE)
    3535
    MISSING_RETURN_STATEMENT: Final = ErrorMessage("Missing return statement", codes.RETURN)
    36+
    EMPTY_BODY_ABSTRACT: Final = ErrorMessage(
    37+
    "If the method is meant to be abstract, use @abc.abstractmethod", codes.EMPTY_BODY
    38+
    )
    3639
    INVALID_IMPLICIT_RETURN: Final = ErrorMessage("Implicit return in function which does not return")
    3740
    INCOMPATIBLE_RETURN_VALUE_TYPE: Final = ErrorMessage(
    3841
    "Incompatible return value type", codes.RETURN_VALUE

    ‎mypy/messages.py‎

    Lines changed: 8 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1231,6 +1231,14 @@ def first_argument_for_super_must_be_type(self, actual: Type, context: Context)
    12311231
    code=codes.ARG_TYPE,
    12321232
    )
    12331233

    1234+
    def unsafe_super(self, method: str, cls: str, ctx: Context) -> None:
    1235+
    self.fail(
    1236+
    'Call to abstract method "{}" of "{}" with trivial body'
    1237+
    " via super() is unsafe".format(method, cls),
    1238+
    ctx,
    1239+
    code=codes.SAFE_SUPER,
    1240+
    )
    1241+
    12341242
    def too_few_string_formatting_arguments(self, context: Context) -> None:
    12351243
    self.fail("Not enough arguments for format string", context, code=codes.STRING_FORMATTING)
    12361244

    ‎mypy/nodes.py‎

    Lines changed: 14 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -758,7 +758,12 @@ def is_dynamic(self) -> bool:
    758758
    return self.type is None
    759759

    760760

    761-
    FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + ["is_decorated", "is_conditional"]
    761+
    FUNCDEF_FLAGS: Final = FUNCITEM_FLAGS + [
    762+
    "is_decorated",
    763+
    "is_conditional",
    764+
    "is_trivial_body",
    765+
    "is_mypy_only",
    766+
    ]
    762767

    763768
    # Abstract status of a function
    764769
    NOT_ABSTRACT: Final = 0
    @@ -781,6 +786,8 @@ class FuncDef(FuncItem, SymbolNode, Statement):
    781786
    "abstract_status",
    782787
    "original_def",
    783788
    "deco_line",
    789+
    "is_trivial_body",
    790+
    "is_mypy_only",
    784791
    )
    785792

    786793
    # Note that all __init__ args must have default values
    @@ -796,11 +803,16 @@ def __init__(
    796803
    self.is_decorated = False
    797804
    self.is_conditional = False # Defined conditionally (within block)?
    798805
    self.abstract_status = NOT_ABSTRACT
    806+
    # Is this an abstract method with trivial body?
    807+
    # Such methods can't be called via super().
    808+
    self.is_trivial_body = False
    799809
    self.is_final = False
    800810
    # Original conditional definition
    801811
    self.original_def: None | FuncDef | Var | Decorator = None
    802-
    # Used for error reporting (to keep backwad compatibility with pre-3.8)
    812+
    # Used for error reporting (to keep backward compatibility with pre-3.8)
    803813
    self.deco_line: int | None = None
    814+
    # Definitions that appear in if TYPE_CHECKING are marked with this flag.
    815+
    self.is_mypy_only = False
    804816

    805817
    @property
    806818
    def name(self) -> str:

    0 commit comments

    Comments
     (0)
    0