From bd82ac4e540af62a1f358f6d2b475e86a3a0e19c Mon Sep 17 00:00:00 2001 From: giulio d'erme Date: Sat, 3 Oct 2026 16:28:37 +0200 Subject: [PATCH] Do not mutate typing.Protocol classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutating a method of a Protocol adds the generated mutant methods (xǁFooǁbar__mutmut_orig, xǁFooǁbar__mutmut_1, ...) as members of the protocol. Classes that implement it structurally then no longer conform, so the type checker reports an error wherever one is used as the protocol. An error in an original method body crashes the type check filter with "Could not find mutant for type error"; an error inside a mutant function discards that unrelated mutant as caught by the type checker. Method stubs are never executed either, so mutants of their default arguments could not be killed. Skip any class that lists typing.Protocol or typing_extensions.Protocol among its bases, plain or subscripted. The base is resolved through the module's imports (aliases and star imports included), so other classes named Protocol, such as asyncio.Protocol or Twisted's, are still mutated, and so is a subclass of a protocol that does not list Protocol itself. Fixes #590 --- HISTORY.rst | 5 ++ src/mutmut/mutation/file_mutation.py | 60 ++++++++++++++++ tests/mutation/test_mutation.py | 100 +++++++++++++++++++++++++++ 3 files changed, 165 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index f94da6bc..65ac8b5a 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,6 +1,11 @@ Changelog --------- +Unreleased +~~~~~~~~~~ + +* Fix ``typing.Protocol`` classes being mutated. The generated mutant methods became members of the protocol, so classes implementing it no longer conformed, and ``type_check_command`` either crashed with "Could not find mutant for type error" or discarded unrelated mutants as caught by the type checker. Methods of protocols, including default implementations, are no longer mutated (`#590`) + 3.8.0 ~~~~~~~~~~ diff --git a/src/mutmut/mutation/file_mutation.py b/src/mutmut/mutation/file_mutation.py index 3613565a..9f91e512 100644 --- a/src/mutmut/mutation/file_mutation.py +++ b/src/mutmut/mutation/file_mutation.py @@ -16,6 +16,7 @@ import libcst as cst import libcst.matchers as m +from libcst.helpers import get_full_name_for_node from libcst.metadata import MetadataWrapper from libcst.metadata import PositionProvider @@ -217,8 +218,13 @@ def __init__( ) self.ignored_classes: set[str] = set() self.ignored_functions: set[str] = set() + # names that `typing.Protocol` is imported as (`Protocol`, or an alias), and that its modules are imported as + self._protocol_names: set[str] = set() + self._protocol_module_names: set[str] = set() def on_visit(self, node: cst.CSTNode) -> bool: + self._record_protocol_import(node) + if self._skip_node_and_children(node): return False @@ -334,6 +340,17 @@ def _skip_node_and_children(self, node: cst.CSTNode) -> bool: return False return True + # ignore Protocol classes, because + # 1) the trampoline setup adds the mutant methods (`xǁFooǁbar__mutmut_1`, ...) as members of the protocol, + # so classes that implement it structurally no longer conform and type checkers report errors at every + # place where they are used as the protocol, which either crashes the type check filter or gets + # attributed to unrelated mutants + # 2) method stubs are never executed, so mutants of their default arguments cannot be killed + # This also leaves method bodies of protocols unmutated, including default implementations + # that explicit subclasses inherit, because mutating them changes the members in the same way. + if isinstance(node, cst.ClassDef) and self._is_protocol_class(node): + return True + # decorators are executed at definition time, so mutating them can raise # exceptions on import. A decorated class is still recursed into, because # its decorator is kept on the original class and only its methods get @@ -343,6 +360,49 @@ def _skip_node_and_children(self, node: cst.CSTNode) -> bool: return False + def _is_protocol_class(self, node: cst.ClassDef) -> bool: + """Does this class list `typing.Protocol` (or `Protocol[T]`, or the `typing_extensions` one) among its bases? + + A class is only a protocol if it lists `Protocol` explicitly, so subclasses of a protocol + that do not are regular classes and still get mutated. The base is resolved through the + imports, so other classes named `Protocol` (`asyncio.Protocol`, ...) are not matched.""" + for base in node.bases: + value = base.value + if isinstance(value, cst.Subscript): + value = value.value + if isinstance(value, cst.Name) and value.value in self._protocol_names: + return True + if ( + isinstance(value, cst.Attribute) + and value.attr.value == "Protocol" + and isinstance(value.value, cst.Name) + and value.value.value in self._protocol_module_names + ): + return True + return False + + def _record_protocol_import(self, node: cst.CSTNode) -> None: + """Record the names under which `typing.Protocol` and its modules are imported.""" + if isinstance(node, cst.Import): + for alias in node.names: + if alias.evaluated_name in PROTOCOL_MODULES: + self._protocol_module_names.add(alias.evaluated_alias or alias.evaluated_name) + elif ( + isinstance(node, cst.ImportFrom) + and not node.relative + and node.module is not None + and get_full_name_for_node(node.module) in PROTOCOL_MODULES + ): + if isinstance(node.names, cst.ImportStar): + self._protocol_names.add("Protocol") + return + for alias in node.names: + if alias.evaluated_name == "Protocol": + self._protocol_names.add(alias.evaluated_alias or "Protocol") + + +PROTOCOL_MODULES = {"typing", "typing_extensions"} + def _walk(node: cst.CSTNode) -> Iterator[cst.CSTNode]: """Yield `node` and every node below it.""" diff --git a/tests/mutation/test_mutation.py b/tests/mutation/test_mutation.py index 0ff0d198..e69fc291 100644 --- a/tests/mutation/test_mutation.py +++ b/tests/mutation/test_mutation.py @@ -2,6 +2,7 @@ import pathlib import shutil import subprocess +import sys import tempfile from collections import defaultdict from collections.abc import Sequence @@ -1241,6 +1242,105 @@ def sum(self): assert mutants == [source.replace("self.x + self.y", "self.x - self.y")] +@pytest.mark.parametrize( + "imports, bases", + [ + ("from typing import Protocol", "Protocol"), + ("import typing", "typing.Protocol"), + ("import typing as t", "t.Protocol"), + ("from typing import Protocol as P", "P"), + ("import typing_extensions", "typing_extensions.Protocol"), + ("from typing_extensions import Protocol", "Protocol"), + ("from typing import *", "Protocol"), + ("from typing import Protocol", "Protocol[T]"), + ("import typing", "Generic[T], typing.Protocol[T]"), + ("from typing import Protocol", "Protocol, metaclass=Meta"), + ], +) +def test_do_not_mutate_protocol_classes(imports, bases): + # Mutating a method of a Protocol adds the generated mutant methods as members + # of the protocol, so implementations of it no longer conform (#590) + source = f""" +{imports} + +class Source({bases}): + def read(self, size: int = 10) -> str: ... + + def read_twice(self) -> str: + return self.read() + self.read() +""".strip() + + assert mutants_for_source(source) == [] + assert "xǁSourceǁ" not in mutated_module(source) + + +@pytest.mark.parametrize( + "source", + [ + pytest.param( + """ +from typing import Protocol, runtime_checkable + +@runtime_checkable +class Source(Protocol): + def read(self, size: int = 10) -> str: ... +""", + id="runtime_checkable", + ), + pytest.param( + """ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Protocol +else: + Protocol = object + +class Source(Protocol): + def read(self, size: int = 10) -> str: ... +""", + id="type_checking_import", + ), + pytest.param( + """ +from typing import Protocol + +class Source[T](Protocol): + def read(self, size: int = 10) -> T: ... +""", + id="pep695_type_parameters", + marks=pytest.mark.skipif(sys.version_info < (3, 12), reason="PEP 695 syntax needs Python 3.12"), + ), + ], +) +def test_do_not_mutate_protocol_class_forms(source): + assert "xǁSourceǁ" not in mutated_module(source.strip()) + + +@pytest.mark.parametrize( + "imports, bases", + [ + # a subclass of a protocol that does not list Protocol itself is a regular class + ("from typing import Protocol\nclass Source(Protocol): ...", "Source"), + # other classes named Protocol are regular base classes + ("import asyncio", "asyncio.Protocol"), + ("from twisted.internet.protocol import Protocol", "Protocol"), + ("from .typing import Protocol", "Protocol"), + ], +) +def test_mutate_classes_that_are_not_protocols(imports, bases): + source = f""" +{imports} + +class File({bases}): + def read(self, size: int = 10) -> str: + return "x" * size +""".strip() + + assert mutants_for_source(source) + assert "xǁFileǁread__mutmut_orig" in mutated_module(source) + + def test_do_not_mutate_pattern_single_line(patch_config): source = 'logger.info("hello")' patch_config("do_not_mutate_patterns", [r"logger\.\w+\("])