Do not mutate typing.Protocol classes

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
 ~~~~~~~~~~
 

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."""

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+\("])