search

Shoobx / mypy-zope

Plugin for mypy to support zope.interface
MIT License
38 stars 11 forks source link

Attempt to reproduce #91 #92

Open kedder opened 1 year ago

DMRobertson commented 1 year ago

Try applying

diff --git a/tests/test_samples.py b/tests/test_samples.py
index f853009..6111e7e 100644
--- a/tests/test_samples.py
+++ b/tests/test_samples.py
@@ -26,6 +26,7 @@ def test_samples(samplefile, mypy_cache_dir):
     opts.show_traceback = True
     opts.namespace_packages = True
     opts.hide_error_codes = True
+    opts.warn_unreachable = True
     opts.plugins = ['mypy_zope:plugin']
     # Config file is needed to load plugins, it doesn't not exist and is not
     # supposed to.

To get the unreachable error

DMRobertson commented 1 year ago

Works for me locally:

=================================================== FAILURES ===================================================
________________________________________ test_samples[isinstance_impl] _________________________________________

samplefile = '/home/dmr/workspace/mypy-zope/tests/samples/isinstance_impl.py'
mypy_cache_dir = '/tmp/pytest-of-dmr/pytest-0/.mypy_cahe0'

    def test_samples(samplefile, mypy_cache_dir):
        opts = options.Options()
        opts.cache_dir = mypy_cache_dir
        opts.show_traceback = True
        opts.namespace_packages = True
        opts.hide_error_codes = True
        opts.warn_unreachable = True
        opts.plugins = ['mypy_zope:plugin']
        # Config file is needed to load plugins, it doesn't not exist and is not
        # supposed to.
        opts.config_file = '    not_existing_config.ini'

        try:
            base_dir = os.path.dirname(samplefile)
            source = BuildSource(samplefile,
                                 module=None,
                                 text=None,
                                 base_dir=base_dir)
            res = build.build(
                sources=[source],
                options=opts)
        except CompileError as e:
            assert False, e

        normalized = normalize_errors(res.errors, samplefile)
        actual = '\n'.join(normalized)
        expected = find_expected_output(samplefile)
>       assert actual == expected
E       assert 'isinstance_i...s unreachable' == 'isinstance_i....IFoo, None]"'
E         - isinstance_impl.py:19: note: Revealed type is "Union[__main__.IFoo, None]"
E         + isinstance_impl.py:19: note: Revealed type is "Union[__main__.IFoo, None]"
E         ?                                                                           +
E         + isinstance_impl.py:23: error: Statement is unreachable

tests/test_samples.py:50: AssertionError
DMRobertson commented 1 year ago

I spent a little time trying to track this down by sticking print statements into mypy.

I'm a bit suspicious of the fact that mypy decides to return in this elif branch: https://github.com/python/mypy/blob/db440ab063c3f01819a29d45e3e2288562f39891/mypy/checker.py#L6708-L6712

        elif not is_overlapping_types(
            current_type, proposed_type, prohibit_none_typevar_overlap=True, ignore_promotions=True
        ):
            # Expression is never of any type in proposed_type_ranges
            return UninhabitedType(), default

In this situation:

It seems a bit odd to conclude that # Expression is never of any type in proposed_type_ranges in this situation... but I don't fully understand mypy's logic and reasoning here.

kedder commented 1 year ago

Sorry for lack of attention to this, I was on my vacation for the last couple of weeks. Getting back to the usual rhythm now.

And thanks for digging in. What looks alarming in that snippet is ignore_promotions=True parameter. "Promotion" mechanism is how we pretend the implementation is a "kinda subclass" of an interface. We used to inject the interface into class hierarchy previously, so maybe that is why it worked before. Hmm...

kedder commented 1 year ago

@DMRobertson I did a pretty crazy hack to work around this (see the diffs). I'm both proud of it and afraid of it at the same time :) I suspect it could cause some memory leaking in the long-running dmypy instances, but it is the best I could come up with so far.

Would it be possible to check the fix on your codebase?

DMRobertson commented 1 year ago

Apologies for the delay in getting back to you. I'm trying this out on https://github.com/matrix-org/synapse/pull/15313, as well as locally.

It looks like this PR fixes all-but-one of the unreachable errors we had when I raised #91. Many thanks for your efforts!

EDIT: I don't seem able to reproduce that outcome now... not sure what is happening!

I haven't had the chance to dig into the remaining error yet; when I get time I'll see if I can reduce that to a minimal example for you.

Before:

dmr on titan in synapse on  dmr/test-mypy-zope-92 is 📦 v1.80.0rc2 via 🐍 v3.11.2 (matrix-synapse-py3.11) via 🦀 v1.68.0 took 1m8s 
2023-03-23 13:58:55 ✗ 1 ERROR  $ pip install mypy-zope==0.9.0 && rm -r .mypy_cache/ && mypy
Collecting mypy-zope==0.9.0
  Using cached mypy_zope-0.9.0-py3-none-any.whl (32 kB)
Requirement already satisfied: mypy==1.0.0 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib64/python3.11/site-packages (from mypy-zope==0.9.0) (1.0.0)
Requirement already satisfied: zope.interface in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib64/python3.11/site-packages (from mypy-zope==0.9.0) (5.4.0)
Requirement already satisfied: zope.schema in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy-zope==0.9.0) (6.2.0)
Requirement already satisfied: typing-extensions>=3.10 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy==1.0.0->mypy-zope==0.9.0) (4.5.0)
Requirement already satisfied: mypy-extensions>=0.4.3 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy==1.0.0->mypy-zope==0.9.0) (0.4.3)
Requirement already satisfied: setuptools in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from zope.interface->mypy-zope==0.9.0) (65.5.1)
Requirement already satisfied: zope.event in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from zope.schema->mypy-zope==0.9.0) (4.5.0)
Installing collected packages: mypy-zope
  Attempting uninstall: mypy-zope
    Found existing installation: mypy-zope 0.9.1.dev0
    Uninstalling mypy-zope-0.9.1.dev0:
      Successfully uninstalled mypy-zope-0.9.1.dev0
Successfully installed mypy-zope-0.9.0

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: pip install --upgrade pip
tests/http/test_proxyagent.py:649: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:768: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:833: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:846: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:854: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:152: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:471: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:61: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:95: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:130: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:156: error: Statement is unreachable  [unreachable]
Found 11 errors in 3 files (checked 793 source files)

After:

dmr on titan in synapse on  dmr/test-mypy-zope-92 is 📦 v1.80.0rc2 via 🐍 v3.11.2 (matrix-synapse-py3.11) via 🦀 v1.68.0 took 6s 
2023-03-23 13:57:09 ✗ 1 ERROR  $ pip install git+https://github.com/Shoobx/mypy-zope.git#fix-unreachable && rm -r .mypy_cache/ && mypy
Collecting git+https://github.com/Shoobx/mypy-zope.git#fix-unreachable
  Cloning https://github.com/Shoobx/mypy-zope.git to /tmp/pip-req-build-8pceb57w
  Running command git clone --filter=blob:none --quiet https://github.com/Shoobx/mypy-zope.git /tmp/pip-req-build-8pceb57w
  Resolved https://github.com/Shoobx/mypy-zope.git to commit 6945ee0ea814724712b75338c1b1967950a78f06
  Preparing metadata (setup.py) ... done
Requirement already satisfied: mypy==1.0.0 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib64/python3.11/site-packages (from mypy-zope==0.9.1.dev0) (1.0.0)
Requirement already satisfied: zope.interface in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib64/python3.11/site-packages (from mypy-zope==0.9.1.dev0) (5.4.0)
Requirement already satisfied: zope.schema in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy-zope==0.9.1.dev0) (6.2.0)
Requirement already satisfied: typing-extensions>=3.10 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy==1.0.0->mypy-zope==0.9.1.dev0) (4.5.0)
Requirement already satisfied: mypy-extensions>=0.4.3 in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from mypy==1.0.0->mypy-zope==0.9.1.dev0) (0.4.3)
Requirement already satisfied: setuptools in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from zope.interface->mypy-zope==0.9.1.dev0) (65.5.1)
Requirement already satisfied: zope.event in /home/dmr/.cache/pypoetry/virtualenvs/matrix-synapse-YNqvS2_4-py3.11/lib/python3.11/site-packages (from zope.schema->mypy-zope==0.9.1.dev0) (4.5.0)

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: pip install --upgrade pip
tests/http/federation/test_matrix_federation_agent.py:152: error: Statement is unreachable  [unreachable]
Found 1 error in 1 file (checked 793 source files)

I suspect it could cause some memory leaking in the long-running dmypy instances

Afraid I don't have any information to report here: I don't use dmypy myself. I'm not sure if anyone else on the team does either (@clokep maybe?)

clokep commented 1 year ago

Afraid I don't have any information to report here: I don't use dmypy myself. I'm not sure if anyone else on the team does either (@clokep maybe?)

I do not use it. Sorry. 😢

DMRobertson commented 1 year ago

Erm, I have just reran the commands

pip install git+https://github.com/Shoobx/mypy-zope.git#fix-unreachable && rm -r .mypy_cache/ && mypy

after which I saw the full suite of 11 errors. So I am not sure why I only saw one error in the sample above. Some kind of caching problem? (But I explicitly nuked the cache...)

kedder commented 1 year ago

Erm, I have just reran the commands

pip install git+https://github.com/Shoobx/mypy-zope.git#fix-unreachable && rm -r .mypy_cache/ && mypy

after which I saw the full suite of 11 errors. So I am not sure why I only saw one error in the sample above. Some kind of caching problem? (But I explicitly nuked the cache...)

Hm, you mean changes in this branch doesn't make any difference for you? I wonder how can we ensure you are actually getting the right branch. Could you check out the branch in a separate directory and install it via pip install -e ../mypy-zope-unreachable (just to be 100% sure it is being picked up).

DMRobertson commented 1 year ago

Hm, you mean changes in this branch doesn't make any difference for you?

It looks like it :(

I had one mypy run where it seemed to make a difference, but I'm unable to reproduce that---even with an editable install of mypy-zope checked out on this branch.

I reduced the test failures I saw in our project to a case. It fails for me on this branch of mypy-zope: do you see the same?

diff --git a/tests/samples/dmr_unreachable.py b/tests/samples/dmr_unreachable.py
index e69de29..36f2b13 100644
--- a/tests/samples/dmr_unreachable.py
+++ b/tests/samples/dmr_unreachable.py
@@ -0,0 +1,31 @@
+from typing import Optional
+
+from zope.interface import Interface, implementer
+
+
+class IFoo(Interface):
+    pass
+
+
+@implementer(IFoo)
+class BaseFoo:
+    pass
+
+
+class ChildFoo(BaseFoo):
+    pass
+
+
+class IFooFactory(Interface):
+    def build() -> Optional[IFoo]:
+        pass
+
+
+def build_and_use_foo(client_factory: IFooFactory) -> None:
+    client_protocol = client_factory.build()
+    assert isinstance(client_protocol, ChildFoo)
+    print("Hello")
+
+
+"""
+"""
\ No newline at end of file
kedder commented 1 year ago

@DMRobertson just for the sake of experiment, could you copy https://github.com/Shoobx/mypy-zope/blob/96518e2725401adb8ecbe177441971666037fd81/tests/samples/isinstance_impl.py into the working directory of your project and run mypy on that file?

DMRobertson commented 1 year ago

@DMRobertson just for the sake of experiment, could you copy https://github.com/Shoobx/mypy-zope/blob/96518e2725401adb8ecbe177441971666037fd81/tests/samples/isinstance_impl.py into the working directory of your project and run mypy on that file?

Certainly! This is matrix-org/synapse@f031e40 (#15313). Mypy did not produce any errors for isintance_impl.py, see https://github.com/matrix-org/synapse/actions/runs/4531921476/jobs/7982662739#step:5:12