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

Mypy's --warn-unreachable option erroneously considers some isinstance calls to be always False with mypy-zope 0.9.0 #91

Open
DMRobertson opened this issue Feb 10, 2023 · 6 comments

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Feb 10, 2023

With mypy 1.0.0 and mypy-zope 0.9.0, the typechecker seems to think that that isinstance(x, T) is always false when T implements some interface Iand x: Optional[I]. For example, the source

from typing import Optional, reveal_type
from zope.interface import implementer, Interface


class IFoo(Interface):
    ...


@implementer(IFoo)
class MyFoo:
    ...


def make_foo() -> Optional[IFoo]:
    return MyFoo()


x = make_foo()
reveal_type(x)
assert isinstance(x, MyFoo)
print("hello")

yields

$ mypy temp.py
temp.py:19: note: Revealed type is "Union[temp.IFoo, None]"
temp.py:21: error: Statement is unreachable  [unreachable]
Found 1 error in 1 file (checked 1 source file)

but at runtime, we see that the print statement is reachable:

$ python temp.py
Runtime type is 'MyFoo'
hello

Would guess that #90 is related to this somehow.

@kedder
Copy link
Member

kedder commented Feb 10, 2023

Hm, but isn't it right though? There is no way for mypy to know if x is an instance of MyFoo. It is an instance of IFoo according to the description (or maybe None). It may be MyFoo, or any other implementation of IFoo.

@DMRobertson
Copy link
Contributor Author

There is no way for mypy to know if x is an instance of MyFoo. It is an instance of IFoo according to the description (or maybe None). It may be MyFoo, or any other implementation of IFoo.

I agree with this. But mypy cannot conclude from this that isinstance(x, MyFoo) must be False in all circumstances---which is what I think leads to the unreachable error.

@kedder
Copy link
Member

kedder commented Feb 10, 2023

That's fair. I'll take to look.

@DMRobertson
Copy link
Contributor Author

Many thanks! FWIW we've only seen this problem when checking our test, so it's not the end of the world as is!

DMRobertson pushed a commit to matrix-org/synapse that referenced this issue Feb 10, 2023
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]
```
kedder added a commit that referenced this issue Feb 10, 2023
@kedder
Copy link
Member

kedder commented Feb 10, 2023

It is strange, but I can't repro it - see #92. @DMRobertson are you using some special mypy configuration to trigger the problem?

@DMRobertson
Copy link
Contributor Author

Ahh, sorry. I should have double-checked this with a clean environment.

I think you need the --warn-unreachable command line flag to see this behaviour. (I was using Synapse's mypy config which has this turned on by default.)

@DMRobertson DMRobertson changed the title Mypy erroneously considers some isinstance calls to be always False with mypy-zope 0.9.0 Mypy's --warn-unreachable option erroneously considers some isinstance calls to be always False with mypy-zope 0.9.0 Feb 10, 2023
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
kedder added a commit that referenced this issue Mar 13, 2023
kedder added a commit that referenced this issue Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants