PyHamcrest icon indicating copy to clipboard operation
PyHamcrest copied to clipboard

Matchers should be contravariant

Open mezuzza opened this issue 3 years ago • 7 comments

Currently, the Matcher class has a type arg T as it should, but Matchers are currently invariant with respect to T. However, I think it's quite clear that the following should be legal code:

matcher: Matcher[str] = has_length(greater_than(0))

I've recreated that definition here:

from typing import Any, Generic, Sequence, Sized, TypeVar

T = TypeVar("T")


class Matcher(Generic[T]):
  def matches(self, _: T) -> bool:
    ...


def greater_than(_: Any) -> Matcher[Any]:
  ...


def has_length(_: int | Matcher[int]) -> Matcher[Sized]:
  ...


matcher: Matcher[str] = has_length(greater_than(0))

Pyright provides the following error:

» pyright test.py
[...]
test.py
  test.py:23:25 - error: Expression of type "Matcher[Sized]" cannot be assigned to declared type "Matcher[str]"
    "Matcher[Sized]" is incompatible with "Matcher[str]"
      TypeVar "T@Matcher" is invariant
        "Sized" is incompatible with "str" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations
Completed in 0.582sec

Changing line 3 to T = TypeVar('T', contravariant=True) fixes the issue.

python version: 3.10.8 pyhamcrest version: 2.0.4 pyright version: 1.1.280

mezuzza avatar Nov 27 '22 20:11 mezuzza

Thanks, I'll look into this in a few days if no one else gets to it first.

brunns avatar Nov 28 '22 09:11 brunns

Thanks a lot. I think this should fix a number of type problems when using the library.

mezuzza avatar Nov 28 '22 13:11 mezuzza

Ah, I remember now. So, I did try this back when I added typing, but the trouble is getting the contravariant matcher to work with assert_that(). I was never able to work it out.

brunns avatar Dec 08 '22 10:12 brunns

Maybe I'm missing something, but

from typing import Any, Generic, Sized, TypeVar

T = TypeVar("T", contravariant=True)


class Matcher(Generic[T]):
  def matches(self, _: T) -> bool:
    ...


def greater_than(_: Any) -> Matcher[Any]:
  ...


def has_length(_: int | Matcher[int]) -> Matcher[Sized]:
  ...


matcher: Matcher[str] = has_length(greater_than(0))


T2 = TypeVar("T2")


def assert_that(
  actual_or_assertion: T2, matcher: Matcher[T2], reason: str = ""
) -> None:
  ...


assert_that("hi there bob", has_length(greater_than(0)))

Doesn't give me any errors.

Was there a specific case that fails?

mezuzza avatar Dec 08 '22 20:12 mezuzza

It may be be me who is missing something! Or perhaps the overrides are the problem? But in any case, I can't get this working in the real codebase.

Do you want to try it? I'm sure a PR would be gratefully received as long as it's backward compatible.

brunns avatar Dec 08 '22 20:12 brunns

I can give it a shot later when I get some time. Probably won't be for a bit though.

mezuzza avatar Dec 15 '22 00:12 mezuzza

Thanks, I appreciate it.

brunns avatar Dec 15 '22 06:12 brunns