python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.46k stars 2.83k forks source link

dmypy not reporting an issue that mypy finds #15677

Open brettcannon opened 1 year ago

brettcannon commented 1 year ago

Bug Report

packaging on  enriched-metadata [!⇡] via 🐍 v3.11.4 (.venv) 
❯ PYTHONPATH=/mnt/c/Users/brcan/.vscode-insiders/extensions/ms-python.mypy-type-checker-2023.1.11921011/bundled/libs /hom
e/brettcannon/Repositories/pypa/packaging/.venv/bin/python -m mypy --no-color-output --no-error-summary --show-absolute-p
ath --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end /home/brettcannon/Reposi
tories/pypa/packaging/src/packaging/metadata.py
/home/brettcannon/Repositories/pypa/packaging/src/packaging/metadata.py:303:18:303:24: error: Item "None" of "list[Any] | None" has no attribute "__iter__" (not iterable)  [union-attr]
/home/brettcannon/Repositories/pypa/packaging/src/packaging/metadata.py:604:5:624:86: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
⏎                                                                                                                   

packaging on  enriched-metadata [+⇡] via 🐍 v3.11.4 (.venv) 
❯ PYTHONPATH=/mnt/c/Users/brcan/.vscode-insiders/extensions/ms-python.mypy-type-checker-2023.1.11921011/bundled/libs
 /home/brettcannon/Repositories/pypa/packaging/.venv/bin/python -m mypy.dmypy --status-file /tmp/.vscode.dmypy_statu
s/status-55f8413f-94ce-437f-9911-110647253f9d.json run -- --no-color-output --no-error-summary --show-absolute-path 
--show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end /home/brettcannon/Repos
itories/pypa/packaging/src/packaging/metadata.py

packaging on  enriched-metadata [+⇡] via 🐍 v3.11.4 (.venv)

To Reproduce

See above.

Expected Behavior

Issues reported by mypy to also be reported by dmypy.

Actual Behavior

dmypy not printing any issues.

Your Environment

hauntsaninja commented 1 year ago

Hmm, I wasn't able to reproduce. I ran several variants of:

~/dev/packaging main λ python -m mypy --version
mypy 1.4.1 (compiled: yes)
~/dev/packaging main λ python -m mypy --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/metadata.py
/Users/shantanu/dev/packaging/src/packaging/metadata.py:283:18:283:24: error: Item "None" of "list[Any] | None" has no attribute "__iter__" (not iterable)  [union-attr]
~/dev/packaging main λ python -m mypy.dmypy --status-file asdf.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/metadata.py
Daemon started
/Users/shantanu/dev/packaging/src/packaging/metadata.py:283:18:283:24: error: Item "None" of "list[Any] | None" has no attribute "__iter__" (not iterable)  [union-attr]

(Also curious that you're using uncompiled mypy, don't expect that to make a difference, but I'll try with that later)

karthiknadig commented 1 year ago

You need to first run it on a file that does not have any reported problems:

(.venv) C:\GIT\demo\packaging>python -m mypy.dmypy --version 
dmypy 1.4.1

(.venv) C:\GIT\demo\packaging>python -m mypy --version
mypy 1.4.1 (compiled: yes)

(.venv) C:\GIT\demo\packaging>python -m mypy.dmypy --status-file asdf2.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/metadata.py
Daemon started
C:\GIT\demo\packaging\src\packaging\metadata.py:283:18:283:24: error: Item "None" of "Optional[List[Any]]" has no attribute "__iter__" (not iterable)  [union-attr]

(.venv) C:\GIT\demo\packaging>python -m mypy.dmypy --status-file asdf2.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/metadata.py
C:\GIT\demo\packaging\src\packaging\metadata.py:283:18:283:24: error: Item "None" of "Optional[List[Any]]" has no attribute "__iter__" (not iterable)  [union-attr]

(.venv) C:\GIT\demo\packaging>python -m mypy.dmypy --status-file asdf2.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/markers.py 

(.venv) C:\GIT\demo\packaging>python -m mypy.dmypy --status-file asdf2.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --follow-imports=normal --show-error-end src/packaging/metadata.py

(.venv) C:\GIT\demo\packaging>
cpnielsen commented 1 year ago

I think I have found the underlying cause, though I am not sure what the correct solution is. I tested, and adding --follow-imports=skip makes this issue disappear, but is not helpful if you do want to follow imports in general. I may be over-explaining the issue here, but hopefully it can help fix it.

Test project setup (was using an existing project when I came across the issue):

I tested both with entire code base and individual "base.py" as input to the following command:

poetry run python -m mypy.dmypy --status-file test.json run -- --no-color-output --no-error-summary --show-absolute-path --show-column-numbers --show-error-code --no-pretty --show-error-end [input]

I also tested this by activating the virtualenv and running it without poetry run, same results

In the initial run when the daemon loads, it does a full build (calling mypy.build.build) and caching it using the FineGrainedManager for later updates. Running it repeatedly without file updates just returns a cached response and does not reveal the issue. Changing any affected file(s), invalidating the cache, causes all errors to disappear for the changed file.

When run with --follow-imports=normal (the default), the dmypy_server.Server.fine_grained_increment_follow_imports method handles the heavy lifting. It has the side-effect that all changed modules are marked as followed (all calls to fine_grained_manager.update is given followed = True.

The calls from there are: FineGrainedManager.update -> FineGrainedManager.update_one -> FineGrainedManager.update_module-> update_module_isolated. Here it creates a new 1-item BuildSource list, with followed=True (passed down the call chain). It then removes the given module from the graph and calls load_graph with the single source and the old graph and an empty list of new modules.

Root cause

At this point a State instance is created with the module name ("id"), path, source, and crucially root_source=not bs.followed (bs being the BuildSource). When it is initialised, we hit the error where it sets ignore_all to True, which makes all errors for the file disappear.

In my case, this is the mix of things causing this:

In short, mypy marks it as a "silently imported module that is not a root source" and instructs it to ignore all errors.

It looks like this was discovered and fixed in this PR previously: https://github.com/python/mypy/pull/14060.

Note: I am not sure what the cause is for @karthiknadig as he is using relative paths, but I can only assume it's related.

Suggested fix

It seems like the initial section of fine_grained_increment_follow_imports need to detect changed modules that match the input sources, and update them with followed=False initially, and then the remaining modules (if any) from find_reachable_changed_modules can be updated with followed=True. However, I am not sure of the exact implications, so hopefully someone more knowledgable can pitch in.

@JukkaL You did the initial fix, I hope this might make sense to you and that you might have a good idea of how this could be fixed. It is breaking the mypy-type-checker in VS Code currently (it always passes in files with their absolute path, so it breaks for any Poetry projects at least), which is how I discovered this issue.

hauntsaninja commented 4 days ago

To repro with latest mypy master:

set -x

rm -rf packaging
git clone https://github.com/hauntsaninja/packaging.git
git -C packaging checkout ed29588

pkill -f dmypy
python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py
python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py
python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/metadata.py
python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py

pkill -f dmypy
rm -rf packaging

Gives:

...
+ python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py
Daemon started
packaging/src/packaging/specifiers.py:248: error: Unused "type: ignore" comment  [unused-ignore]
Found 1 error in 1 file (checked 1 source file)
+ python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py
packaging/src/packaging/specifiers.py:248: error: Unused "type: ignore" comment  [unused-ignore]
Found 1 error in 1 file (checked 1 source file)
+ python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/metadata.py
Success: no issues found in 1 source file
+ python -m mypy.dmypy run -- --config-file packaging/pyproject.toml packaging/src/packaging/specifiers.py
Success: no issues found in 1 source file