Skip to content

Mutate methods decorated with @override - #620

Open
Culaii wants to merge 1 commit into
boxed:mainfrom
Culaii:fix/mutate-override-methods
Open

Culaii wants to merge 1 commit into
boxed:mainfrom
Culaii:fix/mutate-override-methods

Conversation

@Culaii

@Culaii Culaii commented Oct 30, 2026

Copy link
Copy Markdown

Refs #387.

Problem

Since #476, a method with a single @staticmethod / @classmethod is mutated; any other decorator keeps it out. In a typed codebase checked with mypy's explicit-override (or pyright's reportImplicitOverride), every overriding method carries @typing.override (PEP 698), so all of them silently drop out of mutation testing. Reproduction on main (857019a):

from typing import override

class Shape:
    def area(self) -> int:
        return 0

class Plain(Shape):
    def area(self) -> int:
        return 2 * 3

class Marked(Shape):
    @override
    def area(self) -> int:
        return 2 * 3

Plain.area gets 3 mutants; Marked.area, identical apart from the decorator, gets none. In our project that was ~213 mutants across six modules.

Change

override is allowed, alone or stacked with @staticmethod / @classmethod, in the forms @override, @typing.override and @typing_extensions.override. None of the reasons in the decorator rule applies to it: it returns the function itself, only setting __override__; it takes no arguments; it does not change the signature. The trampoline setup already copies the decorators onto the trampoline and the mutants, so nothing else changes there, except that the classmethod detection now looks at every decorator, so @classmethod @override still binds to the subclass (is_classmethod).

Any other decorator still keeps the function out, including override combined with @property, and the attribute form on another module (@other.override).

Tests

  • test_mutate_override_methods: the three spellings, and stacked with staticmethod / classmethod (red on main).
  • test_do_not_mutate_override_methods_with_other_decorators: the neighbours that must stay skipped.
  • test_override_classmethod_subclass_runtime: a @classmethod @override called through a subclass, with the mutant active and inactive, and the __override__ marker kept (red on main; also red if only the skip rule changes and the classmethod detection does not).

Full suite: 483 passed, 4 skipped; pre-commit clean.

A possible follow-up

This only covers override. The general direction @Otto-AA proposed in #387 (decorators kept on the trampoline only, mutants left undecorated) would cover most other decorators too; I'd be happy to work on it if you want to go that way, but it seemed better as a separate change with its own design discussion.

🤖 Generated with Claude Code

`typing.override` (PEP 698) returns the function itself and only sets
`__override__`, so copying it onto the trampoline and the mutants has none of
the side effects the decorator rule guards against. It is now allowed on its
own and stacked with `@staticmethod` / `@classmethod`; any other decorator
still keeps the function out.

The classmethod detection of the trampoline looks at every decorator, so a
`@classmethod @override` method still binds to the subclass it is called on.

Refs boxed#387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zyv

zyv commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

It would be good to have this; then maybe I can get rid of some of my monkeypatching. FYI, my combination so far is:

ALLOWED = frozenset({"property", "abstractmethod", "register", "override"})
"""Decorators safe to apply again to a copied function body, as they do not cache a result or register a second time"""

NATIVE = frozenset({"staticmethod", "classmethod"})
"""Decorators instrumented by mutmut when alone, with `classmethod` bound through an argument of mutmut's trampoline"""

This PR covers the combination of NATIVE (staticmethod and classmethod) with override. The rest can also be done, but I guess with more careful trampoline arrangement, so maybe doing it step by step is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants