Debugged extra_implementation signature checking

fileformats/core/extras.py

@@ -110,18 +110,34 @@ def type_match(mtype: ty.Union[str, type], ftype: ty.Union[str, type]) -> bool:
             if isinstance(mtype, str) and not isinstance(ftype, str):
                 mtype = eval(mtype, implementation.__globals__)
 
-            if morigin := ty.get_origin(mtype):
-                if forigin := ty.get_origin(ftype):
-                    if morigin != forigin:
+            if mtype is ty.Any or mtype == ftype:  # type: ignore[comparison-overlap]
+                return True
+
+            morigin = ty.get_origin(mtype)
+            forigin = ty.get_origin(ftype)
+
+            if morigin is not None or forigin is not None:
+                # Reduce to the concrete origin classes for a subclass
+                # comparison, e.g. `dict` for both `dict` and
+                # `ty.Dict[str, int]`, falling back to the annotation itself
+                # if it isn't a parameterised generic (e.g. a bare `dict`
+                # being matched against `ty.Mapping[str, int]`)
+                mcls = morigin if morigin is not None else mtype
+                fcls = forigin if forigin is not None else ftype
+                if inspect.isclass(mcls) and inspect.isclass(fcls):
+                    if not issubclass(fcls, mcls):
                         return False
+                elif mcls != fcls:
+                    # origins that aren't classes, e.g. typing.Union,
+                    # typing.Literal, etc.
+                    return False
+                margs = ty.get_args(mtype)
+                fargs = ty.get_args(ftype)
+                if margs and fargs:
                     return all(
-                        type_match(mt, ft)
-                        for mt, ft in zip_longest(
-                            ty.get_args(mtype), ty.get_args(ftype)
-                        )
+                        type_match(mt, ft) for mt, ft in zip_longest(margs, fargs)
                     )
-                else:
-                    return False
+                return True
 
             return (
                 mtype is ty.Any  # type: ignore[comparison-overlap]

fileformats/core/tests/test_extras.py

@@ -80,6 +80,110 @@ def woo_test_extra(woo: Woo, a: int, b: str) -> int:
             pass
 
 
+class WooKwargs(FileSet):
+    @extra
+    def test_extra_kwargs(self, a: int, **kwargs: int) -> float:
+        raise NotImplementedError
+
+
+def test_extra_signature_kwargs_match():
+    """A method with **kwargs is matched by an implementation with **kwargs of a
+    compatible type."""
+
+    @extra_implementation(WooKwargs.test_extra_kwargs)
+    def woo_test_extra_kwargs(woo: WooKwargs, a: int, **kwargs: int) -> float:
+        pass
+
+
+def test_extra_signature_kwargs_impl_missing():
+    """An implementation that drops the **kwargs present on the abstract method
+    should be rejected."""
+
+    with pytest.raises(TypeError, match="variable keywords vs non-variable keywords"):
+
+        @extra_implementation(WooKwargs.test_extra_kwargs)
+        def woo_test_extra_kwargs(woo: WooKwargs, a: int) -> float:
+            pass
+
+
+def test_extra_signature_kwargs_method_missing():
+    """An implementation that adds **kwargs not present on the abstract method
+    should be rejected."""
+
+    with pytest.raises(TypeError, match="non-variable keywords vs variable keywords"):
+
+        @extra_implementation(Woo.test_extra)
+        def woo_test_extra(
+            woo: Woo, a: int, b: float, c: ty.Optional[str] = None, **kwargs: ty.Any
+        ) -> float:
+            pass
+
+
+def test_extra_signature_kwargs_type_mismatch():
+    """**kwargs annotated with incompatible types between the method and the
+    implementation should be rejected."""
+
+    with pytest.raises(TypeError, match="Type of keyword args"):
+
+        @extra_implementation(WooKwargs.test_extra_kwargs)
+        def woo_test_extra_kwargs(woo: WooKwargs, a: int, **kwargs: str) -> float:
+            pass
+
+
+class WooGeneric(FileSet):
+    @extra
+    def test_extra_generic(self, a: ty.Mapping[str, int]) -> None:
+        raise NotImplementedError
+
+
+class WooGenericSubtype(WooGeneric):
+    pass
+
+
+class WooGenericPlain(WooGeneric):
+    pass
+
+
+def test_extra_signature_generic_exact_match():
+    """A ty.Mapping[str, int]-typed method is matched by an implementation typed
+    with the identical generic annotation."""
+
+    @extra_implementation(WooGeneric.test_extra_generic)
+    def woo_test_extra_generic(woo: WooGeneric, a: ty.Mapping[str, int]) -> None:
+        pass
+
+
+def test_extra_signature_generic_subtype_origin():
+    """A generic subtype of the method's generic origin should be accepted, e.g.
+    ty.MutableMapping[str, int] for a method typed as ty.Mapping[str, int]."""
+
+    @extra_implementation(WooGeneric.test_extra_generic)
+    def woo_test_extra_generic(
+        woo: WooGenericSubtype, a: ty.MutableMapping[str, int]
+    ) -> None:
+        pass
+
+
+def test_extra_signature_generic_plain_subclass():
+    """A plain (unparameterised) class that is a subclass of the method's generic
+    origin should be accepted, e.g. `dict` for a method typed as
+    ty.Mapping[str, int]."""
+
+    @extra_implementation(WooGeneric.test_extra_generic)
+    def woo_test_extra_generic(woo: WooGenericPlain, a: dict) -> None:
+        pass
+
+
+def test_extra_signature_generic_mismatch():
+    """An implementation typed with an unrelated generic should be rejected."""
+
+    with pytest.raises(TypeError, match="Type of 'a' arg"):
+
+        @extra_implementation(WooGeneric.test_extra_generic)
+        def woo_test_extra_generic(woo: WooGeneric, a: ty.Sequence[int]) -> None:
+            pass
+
+
 def test_vendor_extra_load(tmp_path: Path):
 
     fspath = tmp_path / "test.docx"