Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False postives when decorating functions with signature that uses ParamSpec within a decoarator #12909

Closed
PIG208 opened this issue May 30, 2022 · 1 comment · Fixed by #14677
Labels
bug mypy got something wrong topic-paramspec PEP 612, ParamSpec, Concatenate

Comments

@PIG208
Copy link

PIG208 commented May 30, 2022

Bug Report

When using a decorator on the decorated function within the definition of another decorator, mypy reports false positives.

To Reproduce

from typing import Callable, TypeVar
from typing_extensions import ParamSpec, Concatenate

P = ParamSpec("P")
T = TypeVar("T")


def identity(func: Callable[P, T]) -> Callable[P, T]:
    def _wrapped(*args: P.args, **kwargs: P.kwargs) -> T:
        return func(*args, **kwargs)

    return _wrapped


def int_identity(func: Callable[Concatenate[int, P], T]) -> Callable[Concatenate[int, P], T]:
    # identify should preserve the signature of _wrapped
    # error: Argument 1 to "identity" has incompatible type "Callable[[Arg(int, 'number'), **P], T]"; expected "Callable[[Arg(int, 'number'), **P], T]"
    @identity
    def _wrapped(number: int, *args: P.args, **kwargs: P.kwargs) -> T:
        return func(number, *args, **kwargs)

    return _wrapped

Expected Behavior

The code should pass type-check with no errors.

Actual Behavior

Got error:
error: Argument 1 to "identity" has incompatible type "Callable[[Arg(int, 'number'), **P], T]"; expected "Callable[[Arg(int, 'number'), **P], T]"

Your Environment

  • Mypy version used: 0.960
  • Mypy command-line flags: --strict
  • Python version used: Python 3.8.10
  • Operating system and version: WSL2 - Ubuntu 20.04
@PIG208 PIG208 added the bug mypy got something wrong label May 30, 2022
@AlexWaygood AlexWaygood added the topic-paramspec PEP 612, ParamSpec, Concatenate label May 30, 2022
@bbatliner
Copy link

I think #12806 is a duplicate of this. And I have an example of my own to contribute:

from typing import ParamSpec, Callable, TypeVar
from functools import lru_cache, _lru_cache_wrapper

P = ParamSpec("P")
R = TypeVar("R")


def memoize(f: Callable[P, R]) -> "_lru_cache_wrapper[R]":
    return lru_cache(None)(f)


def function(f: Callable[P, R]) -> None:
    # error: Argument 1 to "memoize" has incompatible type "Callable[[Arg(int, 'a'), **P], None]"; expected "Callable[[Arg(int, 'a'), **P], None]"
    @memoize
    def inner_function(a: int, *args: P.args, **kwargs: P.kwargs) -> None:
        ...
    ...

The "incompatible types" are stringified exactly the same.

PIG208 added a commit to PIG208/zulip that referenced this issue Jul 28, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Jul 28, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Jul 28, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 1, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 4, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 5, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 5, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 5, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 5, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 7, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
timabbott pushed a commit to PIG208/zulip that referenced this issue Aug 9, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 10, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 10, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 10, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 10, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/zulip that referenced this issue Aug 11, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
andersk pushed a commit to PIG208/zulip that referenced this issue Aug 12, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
andersk pushed a commit to PIG208/zulip that referenced this issue Aug 12, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
timabbott pushed a commit to zulip/zulip that referenced this issue Aug 12, 2022
This refactors `rate_limit` so that we no longer use it as a decorator.
This is a workaround to python/mypy#12909 as
`rate_limit` previous expects different parameters than its callers.

Our approach to test logging handlers also needs to be updated because
the view function is not decorated by `rate_limit`.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
DMRobertson pushed a commit to matrix-org/synapse that referenced this issue Feb 16, 2023
I tried to workaround the ignores but found it too much trouble.

I think the corresponding issue is
python/mypy#12909. The mypy repo has a PR
claiming to fix this (python/mypy#14677) which
might mean this gets resolved soon?
DMRobertson pushed a commit to matrix-org/synapse that referenced this issue Feb 16, 2023
* Update mypy and mypy-zope
* Remove unused ignores

These used to suppress

```
synapse/storage/engines/__init__.py:28: error: "__new__" must return a
class instance (got "NoReturn")  [misc]
```

and

```
synapse/http/matrixfederationclient.py:1270: error: "BaseException" has no attribute "reasons"  [attr-defined]
```

(note that we check `hasattr(e, "reasons")` above)

* Avoid empty body warnings, sometimes by marking methods as abstract

E.g.

```
tests/handlers/test_register.py:58: error: Missing return statement  [empty-body]
tests/handlers/test_register.py:108: error: Missing return statement  [empty-body]
```

* Suppress false positive about `JaegerConfig`

Complaint was

```
synapse/logging/opentracing.py:450: error: Function "Type[Config]" could always be true in boolean context  [truthy-function]
```

* Fix not calling `is_state()`

Oops!

```
tests/rest/client/test_third_party_rules.py:428: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
```

* Suppress false positives from ParamSpecs

````
synapse/logging/opentracing.py:971: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]"  [arg-type]
synapse/logging/opentracing.py:1017: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]"  [arg-type]
````

* Drive-by improvement to `wrapping_logic` annotation

* Workaround false "unreachable" positives

See Shoobx/mypy-zope#91

```
tests/http/test_proxyagent.py:626: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:762: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:826: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:838: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:845: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:151: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:452: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:60: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:93: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:127: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:152: error: Statement is unreachable  [unreachable]
```

* Changelog

* Tweak DBAPI2 Protocol to be accepted by mypy 1.0

Some extra context in:
- matrix-org/python-canonicaljson#57
- python/mypy#6002
- https://mypy.readthedocs.io/en/latest/common_issues.html#covariant-subtyping-of-mutable-protocol-members-is-rejected

* Pull in updated canonicaljson lib

so the protocol check just works

* Improve comments in opentracing

I tried to workaround the ignores but found it too much trouble.

I think the corresponding issue is
python/mypy#12909. The mypy repo has a PR
claiming to fix this (python/mypy#14677) which
might mean this gets resolved soon?

* Better annotation for INTERACTIVE_AUTH_CHECKERS

* Drive-by AUTH_TYPE annotation, to remove an ignore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-paramspec PEP 612, ParamSpec, Concatenate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants