Type ignore comments erroneously marked as unused by dmypy

[meshy]

There is currently a misbehaviour where "type: ignore" comments are erroneously marked as unused in re-runs of dmypy. There are also cases where errors disappear on the re-run.

As far as I can tell, this only happens in modules which contain an import that we don't know how to type (such as a module which does not exist), and a submodule which is unused.

There was a lot of commenting and investigation on this PR, but I hope that the committed tests and fixes illustrate and address the issue.

Related to #9655

[chadrik]

I ran into this issue testing a pre-release of 1.3 and it has created enough confusion on the team that I'm not sure we can roll it out until this is fixed. I would love to see this merged before release.

[JelleZijlstra]

The quotes shouldn't matter as this is a stub file, which isn't evaluated. The mutually recursive definition of Iterable and Iterator is indeed annoying to deal with, but mypy should be able to handle it.

[seddonym]

I'd like to try to help with this issue, and have some time next week.

From what I understand, the PR fixes the underlying issue, but causes some apparently unrelated tests to fail in a surprising way. (The odd behaviour is limited to tests and doesn't happen if we run mypy for real.)

I'm assuming this is a problem with the test machinery, and in particular with test fixtures that attempt to import Iterator.

My plan next week is to understand better what's going on by stepping through the test run in a debugger, but if anyone has any inkling as to what might be going on I'd be very grateful for any pointers. @chadrik or @JelleZijlstra I don't suppose you have any idea?

[seddonym]

I've created a simple test case to show what is breaking. This test passes (but it shouldn't).

[case testDemo]
[file a.py]
[file a.py.2]
# Comment to trigger reprocessing.
[builtins fixtures/list.pyi]
[out]
==
builtins.pyi:18: error: Name "Iterator" is not defined
builtins.pyi:18: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Iterator")

This test is simulating the addition of a comment to an otherwise empty file. We are using the list.pyi fixture. The first time the module is checked (above the ==) there are no errors, but the second time (below the ==) we get the "Iterator" is not defined error.

I'll continue to try to understand why this is...

[seddonym]

Good news - I think I've got somewhere. This WIP commit gets the number of failing tests down to only 3. 🥳

Background

I noticed while debugging that there were some additional errors in the error list following the first check run. These aren't surfaced anywhere obvious, I happened to find these by putting a breakpoint in the final statement of mypy.dmypy_server.Server.initialize_fine_grained and then drilling down to
self.fine_grained_manager.manager.errors.error_info_map.

Most of the errors were along these lines:

'Method "__getitem__" is not using @override but is overriding a method in class "typing.Sequence"'
'Method "__iter__" is not using @override but is overriding a method in class "typing.Iterable"'

These errors aren't present when I run it on master.

How I fixed it

I'm not sure if it's the correct thing to do, but I found adding from typing_extensions import override and then decorating the methods in question made the tests pass.

@chadrik @JelleZijlstra Do you have any concerns with this as a fix? If so, please advise what would be a better thing to do.

There were a couple of other test cases where instead the fix seemed to be just to add the appropriate builtins declaration in the test case.

Remaining failures

The last three are a bit different - I'll continue to look at these tomorrow.

Further context

The reason for the more visible Iterator errors seems to be that the semantic analyzer's globals aren't populated with the typing module when the second pass runs. I am not sure how this relates to the fix in question, I've only got limited understanding of what's going on so far.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[meshy]

I've now boiled this down the the minimal changes to fix the issue (though there are a lot of tests), and I think that this is finally ready for review!

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[seddonym]

Heroic effort @meshy!

Just left a couple of tiny comments, feel free to disregard. I'll leave approval to a maintainer!

[seddonym]

Great docstring, thanks.

[seddonym]

No big deal, but we could do this without any conditionals like this, if you prefer:

    seen_and_ancestors: set[str] = set()
    for module in seen:
        module_parts = module.split(".")
        for i in range(len(module_parts)):
            seen_and_ancestors.add(".".join(module_parts[:i + 1]))
    return seen_and_ancestors

[meshy]

I assumed that in some cases with many sub-modules that my way would be faster so my preference would be to keep it as-is, but I'm not strongly opposed to changing this if there's a strong will to do so?

[seddonym]

I think we might be able to drop this conditional and just always add the module_path.

[meshy]

It feels like it'd be quicker to skip this if we have a module with many submodules. I haven't measured this though.

[CoolCat467]

Testing out installing this branch of your repo locally, I found the repeat dmypy runs on a module I installed in edit mode (pip install -e <module>) were still ignoring errors, and then attempting to preform a dmypy run on another file that used that module resulted in a crash.

dmypy --status-file="<my_home_folder>/.idlerc/mypy/dmypy.json" run --timeout=1800 --log-file="<my_home_folder>/.idlerc/mypy/log.txt" --export-types "<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py" -- --warn-redundant-casts --disallow-untyped-calls --cache-dir="<my_home_folder>/.idlerc/mypy" --cache-fine-grained --warn-unused-ignores --show-traceback --no-error-summary --show-column-numbers --no-color-output --show-absolute-path --hide-error-context --strict --warn-unreachable --soft-error-limit=-1 --disallow-untyped-defs --show-error-end --no-implicit-reexport --show-error-codes

<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:539:9:539:27: error: Property "enabled" defined in "Connection" is read-only [misc]
<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:658:13:658:30: error: Property "enabled" defined in "Connection" is read-only [misc]
<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:745:13:745:30: error: Property "enabled" defined in "Connection" is read-only [misc]
<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:933:38:933:38: error: Cannot use a covariant type variable as a parameter [misc]
<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:1320:26:1320:35: error: Only concrete class can be given where "type[BasePlayer[Any]]" is expected [type-abstract]
<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py:1323:37:1323:46: error: Only concrete class can be given where "type[BasePlayer[Any]]" is expected [type-abstract]

dmypy --status-file="<my_home_folder>/.idlerc/mypy/dmypy.json" run --timeout=1800 --log-file="<my_home_folder>/.idlerc/mypy/log.txt" --export-types "<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/src/neat/neat.py" -- --warn-redundant-casts --disallow-untyped-calls --cache-dir="<my_home_folder>/.idlerc/mypy" --cache-fine-grained --warn-unused-ignores --show-traceback --no-error-summary --show-column-numbers --no-color-output --show-absolute-path --hide-error-context --strict --warn-unreachable --soft-error-limit=-1 --disallow-untyped-defs --show-error-end --no-implicit-reexport --show-error-codes

No content returned

dmypy --status-file="<my_home_folder>/.idlerc/mypy/dmypy.json" run --timeout=1800 --log-file="<my_home_folder>/.idlerc/mypy/log.txt" --export-types "<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/examples/minimax/checkers_ai.py" -- --disallow-untyped-defs --hide-error-context --no-implicit-reexport --no-color-output --show-error-end --show-absolute-path --cache-fine-grained --no-error-summary --soft-error-limit=-1 --warn-unused-ignores --disallow-untyped-calls --warn-unreachable --show-column-numbers --show-traceback --warn-redundant-casts --cache-dir="<my_home_folder>/.idlerc/mypy" --strict --show-error-codes

Daemon crashed!
Traceback (most recent call last):
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/dmypy_server.py", line 236, in serve
resp = self.run_command(command, data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/dmypy_server.py", line 285, in run_command
ret = method(self, **data)
^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/dmypy_server.py", line 353, in cmd_run
return self.check(sources, export_types, is_tty, terminal_width)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/dmypy_server.py", line 429, in check
messages = self.fine_grained_increment_follow_imports(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/dmypy_server.py", line 631, in fine_grained_increment_follow_imports
messages = fine_grained_manager.update(changed, [], followed=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/server/update.py", line 267, in update
result = self.update_one(
^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/server/update.py", line 369, in update_one
result = self.update_module(next_id, next_path, next_id in removed_set, followed)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/server/update.py", line 431, in update_module
result = update_module_isolated(
^^^^^^^^^^^^^^^^^^^^^^^
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/server/update.py", line 670, in update_module_isolated
state.generate_unused_ignore_notes()
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/build.py", line 2583, in generate_unused_ignore_notes
self.manager.errors.generate_unused_ignore_errors(self.xpath)
File "<my_home_folder>/.local/lib/python3.12/site-packages/mypy/errors.py", line 690, in generate_unused_ignore_errors
if line in self.skipped_lines[file]:
~~~~~~~~~~~~~~~~~~^^^^^^
KeyError: '<my_home_folder>/Desktop/Python/Ports/NEAT-Template-Python/examples/minimax/checkers_ai.py'

Full Code Repository:
https://github.com/CoolCat467/NEAT-Template-Python/tree/checkers-minimax-test
Command runs:
https://github.com/CoolCat467/NEAT-Template-Python/blob/checkers-minimax-test/dmypy_test.sh

[meshy]

@CoolCat467 That's great info, thank you very much.

Have you tested that crash on Mypy's master branch? Is it definitely a problem introduced by these changes?

I'll see if I can reproduce those disappearing errors.

If you're able to produce a minimal example of any of these failures I'd really appreciate it!

[meshy]

@CoolCat467 I've reproduced a crash on the master branch using your example. The crash I got wasn't exactly the same, but it's possible that's because I didn't have exactly the same setup as you.

I don't think that the crash I got is related to these changes, though as I said, I didn't get the same one as you.

I've boiled it down to a minimal example, and added details of the crash in #14645 (comment)

[meshy]

@CoolCat467 I'm not able to reproduce your issue with the disappearing messages using the script in that repo.

Can I please ask for detailed instructions on how to reproduce it?

[CoolCat467]

@CoolCat467 I'm not able to reproduce your issue with the disappearing messages using the script in that repo.

Can I please ask for detailed instructions on how to reproduce it?

I have now updated https://github.com/CoolCat467/NEAT-Template-Python/dmypy_test.sh to use absolute paths and I get the key error reliably. I would be happy to help if you need more details!

Edit: I think the important detail was the fact I am telling dmypy absolute paths, because my use case of dmypy is as a code editor extension (idlemypyextension), where it might be quite important to tell the mypy daemon exactly which file we want it to check, because potentially the user changes projects without restarting the daemon and they might happen to have files named exactly the same way or whatnot.

[meshy]

@CoolCat467 Thank you for the clarification. I've got that working now, and can reproduce the dropped errors (on this branch and master). I'll try to turn this into a minimal test case.

Curiously, I'm now not seeing the crash on the third command, but I'll come back to that later.

[meshy]

The best minimal example I have so far:

pyproject.toml:

[build-system]
requires = ["setuptools >= 64"]
build-backend = "setuptools.build_meta"

[project]
name = "dmypy_example"
version = "0.0.1"
dependencies = []

src/foo.py (see notes at end):

a: str = 1

Commands:

# Create a new Python 3.12 virtualenv, then:
pip install --editable .
dmypy run --export-types $(pwd)/src/foo.py
dmypy run --export-types $(pwd)/src/foo.py

Results:

$ pip install --editable .
...

$ dmypy run --export-types $(pwd)/src/foo.py
Daemon started
src/foo.py:1: error: Incompatible types in assignment (expression has type "int", variable has type "str")  [assignment]
Found 1 error in 1 file (checked 1 source file)

$ dmypy run --export-types $(pwd)/src/foo.py
Success: no issues found in 1 source file

Notes:

• If I use a name other than src this doesn't seem to work. This is so odd that I feel like I must be doing something wrong.
• This doesn't happen if we don't pip install.
• It doesn't matter what the error in foo.py is. All errors seem to disappear in the second run.
• This happens on the master branch of Mypy too.

My conclusion:

It's disappointing that this branch doesn't fix this error, but I don't think that it's caused by this branch, so perhaps we should consider this a new bug for handling separately?

[CoolCat467]

It's disappointing that this branch doesn't fix this error, but I don't think that it's caused by this branch, so perhaps we should consider this a new bug for handling separately?

Yea, I think it would make sense to make a new issue for this in particular.

If I use a name other than src this doesn't seem to work. This is so odd that I feel like I must be doing something wrong.

I think this might be due to the project structure pip expects, but not completely sure

Also side note, thank you for showing the use of pwd, I wasn't aware that was a thing, that's super helpful instead of using python to figure out the path!

[meshy]

If I use a name other than src this doesn't seem to work. This is so odd that I feel like I must be doing something wrong.

I think this might be due to the project structure pip expects, but not completely sure

As far as I know, src is nothing to do with pip, it's a convention for structuring code that has been gaining momentum in the last few years. (Not a good one, in my opinion, but that's off topic.)

thank you for showing the use of pwd

I'm glad you found it useful! 🙂

It's disappointing that this branch doesn't fix this error, but I don't think that it's caused by this branch, so perhaps we should consider this a new bug for handling separately?

Yea, I think it would make sense to make a new issue for this in particular.

Thanks! I've opened a new issue so we can track it there. #16768

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[meshy]

Hi all! It's been a while since I've had a look at this (I got a bit overwhelmed with it, TBH).

I notice that this was considered for release in 1.9, but wasn't merged because "while the PR itself is apparently ok, it doesn't actually fix the issue completely that its trying to address".

Am I right in saying that the issue discovered in this comment is the blocker? Before I go about investigating that, are there any other blockers that we're aware of?