hamcrest / PyHamcrest

Hamcrest matchers for Python
http://hamcrest.org/
Other
766 stars 111 forks source link

Matchers should be contravariant #222

Open mezuzza opened 1 year ago

mezuzza commented 1 year ago

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

brunns commented 1 year ago

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

mezuzza commented 1 year ago

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

brunns commented 1 year ago

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.

mezuzza commented 1 year ago

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?

brunns commented 1 year ago

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.

mezuzza commented 1 year ago

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

brunns commented 1 year ago

Thanks, I appreciate it.