python-lsp / pylsp-mypy

Mypy plugin for the Python LSP Server.
MIT License
118 stars 35 forks source link

Fix dmypy crash: The `document.path` isn't needed for dmypy's `run` command #65

Closed StabbarN closed 1 year ago

StabbarN commented 1 year ago

Removing it from dmypy fixes the following crash:

some_full_file_path.py:
    1:1   error   Daemon crashed! ​mypy
                  Traceback (most recent call last):
                    File "mypy/dmypy_server.py", line 230, in serve
                    File "mypy/dmypy_server.py", line 277, in run_command
                    File "mypy/dmypy_server.py", line 345, in cmd_run
                    File "mypy/dmypy_server.py", line 414, in check
                    File "mypy/dmypy_server.py", line 663, in fine_grained_increment_follow_imports
                    File "mypy/server/update.py", line 267, in update
                    File "mypy/server/update.py", line 369, in update_one
                    File "mypy/server/update.py", line 452, in update_module
                    File "mypy/server/update.py", line 881, in propagate_changes_using_dependencies
                    File "mypy/server/update.py", line 1009, in reprocess_nodes
                    File "mypy/semanal_main.py", line 148, in semantic_analysis_for_targets
                    File "mypy/semanal_main.py", line 439, in apply_class_plugin_hooks
                    File "mypy/semanal_main.py", line 475, in apply_hooks_to_class
                    File "mypy/plugins/dataclasses.py", line 851, in dataclass_class_maker_callback
                    File "mypy/plugins/dataclasses.py", line 197, in transform
                    File "mypy/plugins/dataclasses.py", line 449, in collect_attributes
                    File "mypy/plugins/dataclasses.py", line 157, in deserialize
                    File "mypy/plugins/common.py", line 344, in deserialize_and_fixup_type
                    File "mypy/types.py", line 1356, in accept
                    File "mypy/fixup.py", line 209, in visit_instance
                    File "mypy/fixup.py", line 344, in lookup_fully_qualified_typeinfo
                    File "mypy/lookup.py", line 31, in lookup_fully_qualified
                  AssertionError: Cannot find module for SomeClass

Mypy version 1.3 but this has been an issue in earlier versions too.

I don't see why one would give the file argument to dmypy run. The daemon will detect what files have changed by itself.

Richardk2n commented 1 year ago

How does dmypy know which files to observe, if we do not pass the path?

StabbarN commented 1 year ago

One way for dmypy to know what files to observe is to read files in mypy.ini on first run.

I created a tiny test project to assert myself that it works.

main.py:

import second
if __name__ == "__main__":
    r = second.some_fn()
    h = 12
    test = r + h
    print(test)

second.py

def some_fn() -> int:
    return 1

mypy.ini:

[mypy]
files = main.py

To verify:

Richardk2n commented 1 year ago

This requires a config file. I want the file checked, that I have currently open.

Furthermore, passing files is perfectly fine/the intended use case according to mypy documentation. https://mypy.readthedocs.io/en/stable/mypy_daemon.html#basic-usage

Well we seem to pass the option in the wrong order, but I would not expect that to be the issue.

Can you tell me a little more about the crash (steps to reproduce), maybe we can find a fix without compromising running without a config file? Is this even our fault? Seems like a mypy issue.

StabbarN commented 1 year ago

I see, yeah, it's probably a tougher mypy issue. The rather long stacktrace hints that it's a nontrivial fix. I've been pondering about filing an issue, but first I've to update the large project to the latest version of mypy. I'll do that in 2-3 months from now.

StabbarN commented 1 year ago

Mypy 1.4.1 is still problematic but instead of crashing dmypy, it reports no errors.

dmypy with both a config file and specify a file seems to still be the culprit. That is dmypy --config-file mypy.ini file.py.

The bug lies outside this project so I'll close this PR.