More helpful type guards #14238

A5rocks · 2022-12-03T05:48:18Z

A TODO I found while looking at something else got me down this rabbit hole, and I noticed mypy's subpar performance in these two cases.

Short descriptions from before I did this, so may be technically inaccurate:

check when defining a type guard whether the function is right or not
follow kw args if they go to a regular positional first arg

~~This may fix an issue but I have not conducted any searches.~~
Fixes #13199

> Type checkers should assume that type narrowing should be applied to the expression that is passed as the first positional argument to a user-defined type guard. [...] > > If a type guard function is implemented as an instance method or class method, the first positional argument maps to the second parameter (after “self” or “cls”).

A5rocks · 2022-12-03T06:29:16Z

mypy primer results seem to be narrowing on self:

    @abstractmethod
    def is_error(self) -> TypeGuard[Error[_TSource, _TError]]:
        """Returns `True` if the result is an `Error` value."""

        raise NotImplementedError

This seems... wrong? Firstly, the PEP says this (as mentioned in one of my commit messages):

If a type guard function is implemented as an instance method or class method, the first positional argument maps to the second parameter (after “self” or “cls”).

This was even explicitly rejected! https://peps.python.org/pep-0647/#narrowing-of-implicit-self-and-cls-parameters

Secondly, mypy doesn't even support this!! At the moment:

from typing import TypeGuard

class A:
  def n(self) -> TypeGuard[int]:
    return False

a = A()
if a.n():
  reveal_type(a)  # N: Revealed type is "__main__.A"

However, I realize I missed staticmethod special casing. Will fix.

from typing_extensions import TypeGuard

class Z:
    @staticmethod
    def typeguard(h: object) -> TypeGuard[int]:  # E: TypeGuard functions must have a positional argument
        return isinstance(h, int)

x: object
if Z().typeguard(x):
    reveal_type(x)
if Z.typeguard(x):
    reveal_type(x)

sobolevn

Thanks! I have a couple of questions, this is not a full review.

mypy/checker.py

mypy/semanal.py

A5rocks · 2022-12-10T06:27:35Z

OK with that tests should be added as requested. I've got one additional question that I've moved over to #14273 because it may cause more debate. Hopefully this as it stands is alright!

test-data/unit/check-typeguard.test

github-actions · 2022-12-11T06:36:32Z

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

sobolevn · 2022-12-11T07:11:42Z

Cases that error:

   
    @abstractmethod
    def is_error(self) -> TypeGuard[Error[_TSource, _TError]]:
        """Returns `True` if the result is an `Error` value."""

        raise NotImplementedError

    @abstractmethod
    def is_ok(self) -> TypeGuard[Ok[_TSource, _TError]]:
        """Returns `True` if the result is an `Ok` value."""

        raise NotImplementedError

A5rocks · 2022-12-11T07:25:32Z

Yep! As mentioned before, that is invalid as according to the PEP and mypy doesn't support it anyways. Having an error at definition time is likely beneficial.

A5rocks added 2 commits Dec 3, 2022

Start allowing keyword arguments to type guards

3dda329