Mutate decorated functions: keep decorators on the trampoline only (#387)

src/mutmut/mutation/file_mutation.py

@@ -334,16 +334,18 @@ def _skip_node_and_children(self, node: cst.CSTNode) -> bool:
         ):
             return True
 
-        # ignore decorated functions, because
-        # 1) copying them for the trampoline setup can cause side effects (e.g. multiple @app.post("/foo") definitions)
-        # 2) decorators are executed when the function is defined, so we don't want to mutate their arguments and cause exceptions
-        # 3) @property decorators break the trampoline signature assignment (which expects it to be a function)
-        # Exception: @staticmethod and @classmethod are allowed because they are predictable and it's easy to set up trampolines for them
-        if isinstance(node, cst.FunctionDef) and len(node.decorators):
-            if len(node.decorators) == 1:
-                decorator = node.decorators[0].decorator
-                if isinstance(decorator, cst.Name) and decorator.value in ("staticmethod", "classmethod"):
-                    return False
+        # A decorated function is mutated: its decorators stay on the trampoline only, and the copies
+        # (original and mutants) carry none but the binding ones (see `function_trampoline_arrangement`).
+        # So a decorator runs once, as it would without mutmut (no second `@app.post("/foo")`), it never
+        # wraps a copy (`@property` sees the trampoline, a plain function), and its arguments are not
+        # mutated (decorator nodes are skipped below).
+        # Still skipped: decorators that define SEVERAL functions under one name, whose copies would collide,
+        # and caching decorators -- a cache on the trampoline answers every mutant from what the parent
+        # process computed before forking, so the mutants would never run (false survivors).
+        if isinstance(node, cst.FunctionDef) and any(
+            _defines_a_namesake(d.decorator) or _decorator_name(d.decorator) in _CACHING_DECORATORS
+            for d in node.decorators
+        ):
             return True
 
         # decorators are executed at definition time, so mutating them can raise
@@ -356,6 +358,43 @@ def _skip_node_and_children(self, node: cst.CSTNode) -> bool:
         return False
 
 
+#: Decorators after which another function of the same name follows in the same scope.
+_NAMESAKE_DECORATORS = frozenset({"overload"})
+_NAMESAKE_ATTRIBUTES = frozenset({"setter", "getter", "deleter", "register"})
+
+#: Decorators that memoise the decorated function's results.
+_CACHING_DECORATORS = frozenset({"cache", "lru_cache", "cached_property"})
+
+#: Decorators that decide how a function binds in its class; the copies need them to be called alike.
+BINDING_DECORATORS = frozenset({"staticmethod", "classmethod"})
+
+
+def _decorator_name(decorator: cst.BaseExpression) -> str | None:
+    """The last name of `decorator`: `overload`, `typing.overload` and `overload()` all give `overload`."""
+    if isinstance(decorator, cst.Call):
+        decorator = decorator.func
+    if isinstance(decorator, cst.Name):
+        return decorator.value
+    if isinstance(decorator, cst.Attribute):
+        return decorator.attr.value
+    return None
+
+
+def _defines_a_namesake(decorator: cst.BaseExpression) -> bool:
+    """`@overload`, `@x.setter` / `.getter` / `.deleter`, `@f.register`: a sibling shares the name."""
+    target = decorator.func if isinstance(decorator, cst.Call) else decorator
+    if isinstance(target, cst.Attribute) and target.attr.value in _NAMESAKE_ATTRIBUTES:
+        return True
+    return _decorator_name(decorator) in _NAMESAKE_DECORATORS
+
+
+def _binding_decorators(function: cst.FunctionDef) -> list[cst.Decorator]:
+    """The decorators a copy keeps: only those deciding how it binds (`staticmethod`, `classmethod`)."""
+    return [
+        d for d in function.decorators if isinstance(d.decorator, cst.Name) and d.decorator.value in BINDING_DECORATORS
+    ]
+
+
 def _is_enum_class(node: cst.ClassDef) -> bool:
     bases = (base.value.attr if isinstance(base.value, cst.Attribute) else base.value for base in node.bases)
     return any(isinstance(base, cst.Name) and base.value in ENUM_BASE_NAMES for base in bases)
@@ -543,7 +582,7 @@ def function_trampoline_arrangement(
     # trampoline with same signature, that forwards the calls to the activated mutant/original method
     # (put first, s.t. it stays next to @overload definitions of this function. mypy needs this)
     decorator_args = [cst.Arg(cst.Name(mutants_dict_name))]
-    if len(function.decorators) == 1 and m.matches(function.decorators[0].decorator, m.Name("classmethod")):
+    if any(m.matches(d.decorator, m.Name("classmethod")) for d in function.decorators):
         decorator_args.append(cst.Arg(cst.Name("True"), keyword=cst.Name("is_classmethod")))
     trampoline = function.with_changes(
         decorators=[
@@ -553,14 +592,17 @@ def function_trampoline_arrangement(
     )
     method_nodes.append(trampoline)
 
+    # the copies carry only the decorators that decide how they bind; the others act on the trampoline alone
+    copy = function.with_changes(decorators=_binding_decorators(function))
+
     # copy of original function
-    method_nodes.append(function.with_changes(name=cst.Name(mangled_name + "_orig")))
+    method_nodes.append(copy.with_changes(name=cst.Name(mangled_name + "_orig")))
 
     # mutated versions of the function
     for i, mutant in enumerate(mutants):
         mutant_name = f"{mangled_name}_{i + 1}"
         mutant_names.append(mutant_name)
-        mutated_method = function.with_changes(name=cst.Name(mutant_name))
+        mutated_method = copy.with_changes(name=cst.Name(mutant_name))
         mutated_method = cast(cst.FunctionDef, deep_replace(mutated_method, mutant.original_node, mutant.mutated_node))
         method_nodes.append(mutated_method)
 

tests/e2e/test_e2e_my_lib.py

@@ -81,6 +81,14 @@ def test_my_lib_result_snapshot():
                 "my_lib.xǁPointǁpragma_on_classmethod_decorator__mutmut_6": 1,
                 "my_lib.xǁPointǁpragma_on_classmethod_decorator__mutmut_7": 1,
                 "my_lib.xǁPointǁpragma_on_classmethod_decorator__mutmut_8": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_1": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_2": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_3": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_4": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_5": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_6": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_7": 1,
+                "my_lib.xǁPointǁskip_multi_decorator__mutmut_8": 1,
                 "my_lib.x_escape_sequences__mutmut_1": 1,
                 "my_lib.x_escape_sequences__mutmut_2": 0,
                 "my_lib.x_escape_sequences__mutmut_3": 1,

tests/mutation/test_mutation.py

@@ -1244,9 +1244,9 @@ def _mocked_runner_run_forced_failed(return_value=None, side_effect=None):
     return runner
 
 
-def test_do_not_mutate_top_level_decorators():
-    # Modifying top-level decorators could influence all mutations
-    # because they are executed at import time
+def test_decorated_bodies_are_mutated_but_never_their_decorators():
+    # Decorators run at import time, so mutating their arguments could break every mutant;
+    # the bodies they decorate are mutated like any other
     source = """
 @some_decorator(a = 2)
 def foo():
@@ -1261,10 +1261,62 @@ def x(self):
 """.strip()
 
     mutants = mutants_for_source(source)
-    for m in mutants:
-        print(m)  # pragma: no cover
 
-    assert not mutants
+    assert len(mutants) == 7
+    assert all("@some_decorator(a = 2)" in m and "@unique" in m and "@property" in m for m in mutants)
+
+
+@pytest.mark.parametrize(
+    "decorator",
+    [
+        "@overload",
+        "@typing.overload",
+        "@x.setter",
+        "@x.getter",
+        "@x.deleter",
+        "@process.register",
+        "@process.register(int)",
+    ],
+)
+def test_a_decorator_that_defines_a_namesake_is_not_mutated(decorator):
+    # Another function of the same name follows in the same scope: their copies would collide
+    source = f"""
+class A:
+    {decorator}
+    def x(self, value):
+        return value + 1
+""".strip()
+
+    assert mutants_for_source(source) == []
+
+
+@pytest.mark.parametrize("decorator", ["@cache", "@functools.cache", "@lru_cache(maxsize=2)", "@cached_property"])
+def test_a_caching_decorator_is_not_mutated(decorator):
+    # A cache on the trampoline answers every mutant from what the parent process computed
+    source = f"""
+class A:
+    {decorator}
+    def x(self):
+        return 1 + 2
+""".strip()
+
+    assert mutants_for_source(source) == []
+
+
+def test_the_copies_keep_only_the_binding_decorators():
+    source = """
+class A:
+    @register
+    @classmethod
+    def make(cls):
+        return 1 + 2
+""".strip()
+
+    code = mutated_module(source)
+
+    assert "@register\n    @classmethod\n    @_mutmut_mutated(" in code
+    assert "@classmethod\n    def xǁAǁmake__mutmut_orig(cls)" in code
+    assert "@register\n    @classmethod\n    def xǁAǁmake__mutmut_" not in code
 
 
 @pytest.mark.parametrize("decorator", ["@dataclass", "@dataclass(frozen=True)"])

tests/mutation/test_mutation_runtime.py

@@ -194,3 +194,118 @@ def foo(a: int, b: int = 2):
 
     assert foo(1) == 4
     assert foo(1, 10) == 11
+
+
+def _load(source, monkeypatch, mutant_of=None):
+    """`source` mutated and executed, with the first mutant of the function `mutant_of` active, or none."""
+    mutated_file = mutate_file_contents("test.py", source)
+    of_it = [m for m in mutated_file.mutant_names if mutant_of is not None and f"{mutant_of}__mutmut_" in m]
+    name = "test_module." + of_it[0] if of_it else "none"
+    assert (mutant_of is None) or of_it, f"no mutant of {mutant_of}"
+    monkeypatch.setenv("MUTANT_UNDER_TEST", name)
+    namespace = {"__name__": "test_module"}
+    exec(mutated_file.code, namespace)
+    return namespace, mutated_file.mutant_names
+
+
+def test_a_property_stays_a_property_and_its_mutants_run(monkeypatch):
+    source = """
+class Box:
+    def __init__(self, a, b):
+        self.a, self.b = a, b
+
+    @property
+    def total(self):
+        return self.a + self.b
+""".strip()
+
+    namespace, mutants = _load(source, monkeypatch)
+    assert isinstance(namespace["Box"].__dict__["total"], property)
+    assert namespace["Box"](2, 3).total == 5
+    assert mutants
+
+    namespace, _ = _load(source, monkeypatch, mutant_of="total")
+    assert namespace["Box"](2, 3).total == -1
+
+
+def test_a_context_manager_keeps_working_and_its_mutants_run(monkeypatch):
+    source = """
+from contextlib import contextmanager
+
+@contextmanager
+def scope(value):
+    yield value + 1
+""".strip()
+
+    namespace, _ = _load(source, monkeypatch)
+    with namespace["scope"](1) as inside:
+        assert inside == 2
+
+    namespace, _ = _load(source, monkeypatch, mutant_of="scope")
+    with namespace["scope"](1) as inside:
+        assert inside == 0
+
+
+def test_a_registering_decorator_runs_once_and_registers_the_trampoline(monkeypatch):
+    source = """
+REGISTRY = []
+
+def register(fn):
+    REGISTRY.append(fn)
+    return fn
+
+@register
+def double(x):
+    return x * 2
+""".strip()
+
+    namespace, _ = _load(source, monkeypatch)
+    assert [fn(3) for fn in namespace["REGISTRY"]] == [6]
+
+    namespace, _ = _load(source, monkeypatch, mutant_of="double")
+    assert [fn(3) for fn in namespace["REGISTRY"]] == [3 / 2]
+
+
+def test_a_classmethod_under_another_decorator_binds_to_the_subclass(monkeypatch):
+    source = """
+def marked(fn):
+    return fn
+
+class Base:
+    @marked
+    @classmethod
+    def create(cls):
+        return cls(1)
+
+class SubClass(Base):
+    def __init__(self, value):
+        self.value = value
+""".strip()
+
+    namespace, _ = _load(source, monkeypatch)
+    assert namespace["SubClass"].create().value == 1
+
+    namespace, _ = _load(source, monkeypatch, mutant_of="create")
+    assert namespace["SubClass"].create().value is None
+
+
+def test_an_override_method_is_mutated(monkeypatch):
+    source = """
+def override(fn):  # typing.override (3.12+) is an identity decorator too
+    return fn
+
+class Base:
+    def area(self):
+        return 0
+
+class Square(Base):
+    @override
+    def area(self):
+        return 2 * 2
+""".strip()
+
+    namespace, _ = _load(source, monkeypatch)
+    assert namespace["Square"]().area() == 4
+
+    namespace, _ = _load(source, monkeypatch, mutant_of="Squareǁarea")
+    assert namespace["Square"]().area() != 4