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

cached runs of dmypy stop reporting "unused type: ignore" warnings #9655

Open
rggjan opened this issue Oct 28, 2020 · 22 comments
Open

cached runs of dmypy stop reporting "unused type: ignore" warnings #9655

rggjan opened this issue Oct 28, 2020 · 22 comments
Labels
bug mypy got something wrong topic-daemon dmypy topic-type-ignore # type: ignore comments

Comments

@rggjan
Copy link

rggjan commented Oct 28, 2020

Using mypy 0.782, I run
dmypy run .

on my codebase. The first time it runs, it correctly reports a few

error: unused 'type: ignore' comment

same as mypy. However, when rerunning dmypy run ., these errors disappear, while all other errors remain.

For some reason, this type of warning (warn_unused_ignores) seems to be missed sometimes when using the cache.

Unfortunately, it doesn't seem trivial to reproduce, so I can't give a reproducible project right now...

@rggjan rggjan added the bug mypy got something wrong label Oct 28, 2020
@JelleZijlstra JelleZijlstra added topic-daemon dmypy topic-type-ignore # type: ignore comments labels Mar 19, 2022
@jgillard
Copy link

I noticed this today with mypy 0.942, so this is still an issue.

> dmypy run
Success: no issues found in 604 source files

> mypy --cache-dir=/dev/null
apps/matchmaking/services.py:260: error: Unused "type: ignore" comment
Found 1 error in 1 file (checked 604 source files)

> mypy --version
mypy 0.942

@peterbygrave
Copy link

This is still a problem in 0.991

@AlexWaygood
Copy link
Member

Has anybody managed to find a reproducible example yet?

@meshy
Copy link
Contributor

meshy commented Feb 20, 2023

I have recently encountered this on 1.0.1. I have not yet produced a minimal example.

In my case the bulk of the unused type: ignore warnings which disappear are on lines that have been typed by the django-stubs project. I wonder if it may have something to do with that project, or with plugins in general.

@meshy

This comment was marked as outdated.

@meshy
Copy link
Contributor

meshy commented Mar 3, 2023

I have a new example that doesn't have any external requirements.

unused/__init__.py
(empty file)

unused/empty.py
(empty file)

example.py

from unused.empty import *
a = 1  # type: ignore

Instructions (in a clean virtualenv)

$ dmypy stop
Daemon stopped

$ dmypy run -- --package example --warn-unused-ignores
Daemon started
example.py:2: error: Unused "type: ignore" comment
Found 1 error in 1 file (checked 46 source files)

$ dmypy run -- --package example --warn-unused-ignores
Success: no issues found in 1 source file

From what I can tell this issue happens when we call fine_grained_manager.update([], to_delete) in dmypy_server.Server.fine_grained_increment_follow_imports.

In this case, to_delete is [('unused', '/.../mypy-9655/unused/__init__.py')].

For some reason, when this update is made, the Unused "type: ignore" comment also goes missing.

Next up: dig into mypy, and work out what's going on.

EDIT: I have created a failing test based on this example, and continue to investigate a fix.

meshy added a commit to meshy/mypy that referenced this issue Mar 4, 2023
This test currently fails with this error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: Unused "type: ignore" comment (diff)
      == Return code: 1                              (diff)
    Actual:
      (empty)

It demonstrates a bug that when an module is removed using
`FineGrainedBuildManager.update` because it is not "seen" by
`fine_grained_increment_follow_imports`, then "unused type: ignore"
warnings disappear from subsequent checks.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Mar 12, 2023
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: "type: ignore" comment without error code  [ignore-without-code] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that '"type: ignore" comment without error code'
errors currently disappear in the same way that 'Unused "type: ignore"'
errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Mar 12, 2023
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: "type: ignore" comment without error code  [ignore-without-code] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that '"type: ignore" comment without error code'
errors currently disappear in the same way that 'Unused "type: ignore"'
errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Mar 12, 2023
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:4: error: Name "a" may be undefined  [possibly-undefined] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that possibly-undefined errors currently disappear
in the same way that 'Unused "type: ignore"' errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Mar 13, 2023
Before this change, if a module was removed with `update` in the
`FineGrainedBuildManager`, then some errors would not re-appear.

This change ensures that three types of error are maintained when
running `reprocess_nodes`:

- possibly undefined vars
- unused ignores
- ignores without codes

By adding targets to ErrorInfos, this also fixes an issue where unused
ignore and ignores without codes errors didn't get re-processed when
they were the only issue in the file.

Fixes python#9655
meshy added a commit to meshy/mypy that referenced this issue Mar 13, 2023
Before this change, if a module was removed with `update` in the
`FineGrainedBuildManager`, then some errors would not re-appear.

This change ensures that three types of error are maintained when
running `reprocess_nodes`:

- possibly undefined vars
- unused ignores
- ignores without codes

By adding targets to ErrorInfos, this also fixes an issue where unused
ignore and ignores without codes errors didn't get re-processed when
they were the only issue in the file.

Fixes python#9655
@meshy
Copy link
Contributor

meshy commented Mar 17, 2023

My PR now contains a fix for this that I hope should get things working for most people.

ilevkivskyi pushed a commit that referenced this issue Apr 9, 2023
This adds a commit which fixes issue
#9655 wherein some types of error
would be lost when a file was re-processed by dmypy. Regression tests
are also included.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".

<!-- If this pull request fixes an issue, add "Fixes #NNN" with the
issue number. -->


<!--
Checklist:
- Read the [Contributing
Guidelines](https:/python/mypy/blob/master/CONTRIBUTING.md)
- Add tests for all changed behaviour.
- If you can't add a test, please explain why and how you verified your
changes work.
- Make sure CI passes.
- Please do not force push to the PR once it has been reviewed.
-->

---------

Co-authored-by: AlexWaygood <[email protected]>
@ilevkivskyi
Copy link
Member

This should be fixed by #14835

@meshy
Copy link
Contributor

meshy commented Apr 27, 2023

While I think things have improved, I'm not sure that it was entirely fixed by that PR.

I've produced two more PRs (failing tests only) with similar cases, though I haven't yet been able to get to the bottom of them yet:

@ilevkivskyi
Copy link
Member

OK, I see, let's then keep this open to track those.

@ilevkivskyi ilevkivskyi reopened this Apr 27, 2023
@meshy
Copy link
Contributor

meshy commented Apr 27, 2023

Thanks :)

JukkaL added a commit that referenced this issue May 4, 2023
This reverts commit a9ee618.

The original fix doesn't work in all cases. Reverting it since fixing
remaining issues is non-trivial, and it's better not to include regressions
or partially implemented features in mypy 1.3. See #9655 for context.
JukkaL added a commit that referenced this issue May 4, 2023
…#15179)

This reverts commit a9ee618.

The original fix doesn't work in all cases. Reverting it since fixing
remaining issues is non-trivial, and it's better not to include
regressions or partially implemented features in mypy 1.3. See #9655 for
context.
wesleywright pushed a commit that referenced this issue May 4, 2023
…#15179)

This reverts commit a9ee618.

The original fix doesn't work in all cases. Reverting it since fixing
remaining issues is non-trivial, and it's better not to include
regressions or partially implemented features in mypy 1.3. See #9655 for
context.
meshy added a commit to meshy/mypy that referenced this issue Dec 20, 2023
This test currently fails with this error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: Unused "type: ignore" comment (diff)
      == Return code: 1                              (diff)
    Actual:
      (empty)

It demonstrates a bug that when an module is removed using
`FineGrainedBuildManager.update` because it is not "seen" by
`fine_grained_increment_follow_imports`, then "unused type: ignore"
warnings disappear from subsequent checks.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Dec 20, 2023
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: "type: ignore" comment without error code  [ignore-without-code] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that '"type: ignore" comment without error code'
errors currently disappear in the same way that 'Unused "type: ignore"'
errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Dec 20, 2023
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:4: error: Name "a" may be undefined  [possibly-undefined] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that possibly-undefined errors currently disappear
in the same way that 'Unused "type: ignore"' errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Dec 20, 2023
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
meshy added a commit to meshy/mypy that referenced this issue Dec 20, 2023
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy. Regression tests
are also included.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
@m-honel-ywh2bt
Copy link

Still facing the issue with python 3.12.1 / mypy 1.7.1.

Using meshy's example in a clean virtualenv:

$ dmypy stop
Daemon stopped

$ dmypy --version
dmypy 1.7.1

$ python --version
Python 3.12.1

$ dmypy run -- --package example --warn-unused-ignores
Daemon started
example.py:2: error: Unused "type: ignore" comment  [unused-ignore]
Found 1 error in 1 file (checked 1 source file)

$ dmypy run -- --package example --warn-unused-ignores
Success: no issues found in 1 source file

@meshy
Copy link
Contributor

meshy commented Dec 22, 2023

@m-honel-ywh2bt we're making progress on this!

Just a couple of days ago we solved the examples in #15043. Testing on my local code base suggests that we may have missed some edge-cases there though, so I'm trying to produce some more minimal examples for those.

There are also some cases in #15049 that we haven't found the root cause of yet. I hold onto the hope that we'll get to the bottom of this!

meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This test currently fails with this error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: Unused "type: ignore" comment (diff)
      == Return code: 1                              (diff)
    Actual:
      (empty)

It demonstrates a bug that when an module is removed using
`FineGrainedBuildManager.update` because it is not "seen" by
`fine_grained_increment_follow_imports`, then "unused type: ignore"
warnings disappear from subsequent checks.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: "type: ignore" comment without error code  [ignore-without-code] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that '"type: ignore" comment without error code'
errors currently disappear in the same way that 'Unused "type: ignore"'
errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:4: error: Name "a" may be undefined  [possibly-undefined] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that possibly-undefined errors currently disappear
in the same way that 'Unused "type: ignore"' errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
@meshy
Copy link
Contributor

meshy commented Jan 2, 2024

If anyone following this thread is able to check if #15043 works for them, I'd appreciate the feedback.

@m-honel-ywh2bt
Copy link

@meshy Using #15043, I can confirm that Unused "type: ignore" comment errors seem to appear everytime dmypy is ran, without having to restart the daemon. 👌🏻

meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
meshy added a commit to meshy/mypy that referenced this issue Jan 2, 2024
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
@JukkaL
Copy link
Collaborator

JukkaL commented Feb 5, 2024

I think there is another case where type ignore comments still aren't being reported. Here are a few failing test cases:

[case testUnusedTypeIgnorePreservedAfterChangeInOtherModule1]
# flags: --warn-unused-ignores
[file main.py]
import a

def f() -> None:
    1()
    a = 1  # type: ignore

[file a.py]
[file a.py.2]
x = 1

[out]
main.py:4: error: "int" not callable
main.py:5: error: Unused "type: ignore" comment
==
main.py:4: error: "int" not callable
main.py:5: error: Unused "type: ignore" comment

[case testUnusedTypeIgnorePreservedAfterChangeInOtherModule2]
# flags: --warn-unused-ignores
[file main.py]
import a

def f() -> None:
    1()
    a = 1  # type: ignore

def g() -> None:
    1()  # type: ignore

[file a.py]
[file a.py.2]
x = 1

[out]
main.py:4: error: "int" not callable
main.py:5: error: Unused "type: ignore" comment
==
main.py:4: error: "int" not callable
main.py:5: error: Unused "type: ignore" comment

The error on line 5 isn't reported on the second step.

The root cause is that we don't report unused type ignores in nodes only processed using reprocess_nodes, i.e. when following fine-grained dependencies or reprocessing nodes that previously generated errors.

I think this is a potential way to fix this (but I haven't thought about this very carefully yet):

  • Collect a set of all the triggered/processed nodes in propagate_changes_using_dependencies.
  • At the end of a daemon update, perform the equivalent of generate_unused_ignore_notes, but only report errors within the previously triggered nodes. The filtering step seems necessary, as some parts of modules are often not processed during a fine-grained increment.

@meshy
Copy link
Contributor

meshy commented Feb 5, 2024

@JukkaL good catch.

Do you think this blocks #15043, or could we tackle that case separately?

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 5, 2024

Ah, I forgot to mention that the test cases I mentioned above aren't specifically covered by #15043 yet. It's okay to leave fixing these for another PR, but we shouldn't close this issue until all known cases are fixed.

@meshy
Copy link
Contributor

meshy commented Feb 5, 2024

OK great, thank you. I've removed the "fixes" comment from that PR so we avoid closing this issue.

meshy added a commit to meshy/mypy that referenced this issue Feb 8, 2024
This test currently fails with this error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: Unused "type: ignore" comment (diff)
      == Return code: 1                              (diff)
    Actual:
      (empty)

It demonstrates a bug that when an module is removed using
`FineGrainedBuildManager.update` because it is not "seen" by
`fine_grained_increment_follow_imports`, then "unused type: ignore"
warnings disappear from subsequent checks.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Feb 8, 2024
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:2: error: "type: ignore" comment without error code  [ignore-without-code] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that '"type: ignore" comment without error code'
errors currently disappear in the same way that 'Unused "type: ignore"'
errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Feb 8, 2024
This test fails with the error:

    AssertionError: Command 3 (dmypy check -- bar.py) did not give expected output
    --- Captured stderr call ---
    Expected:
      bar.py:4: error: Name "a" may be undefined  [possibly-undefined] (diff)
      == Return code: 1                             (diff)
    Actual:
      (empty)

This test illustrates that possibly-undefined errors currently disappear
in the same way that 'Unused "type: ignore"' errors do as described in python#9655.

Ref: python#9655
meshy added a commit to meshy/mypy that referenced this issue Feb 8, 2024
This which fixes issue
python#9655 wherein some types of error
would be lost when a file was re-processed by dmypy.

This also fixes another error where sometimes files would not be
re-processed by dmypy if the only error in the file was either "unused
type ignore" or "ignore without code".
meshy added a commit to meshy/mypy that referenced this issue Feb 8, 2024
This catches a regression caused by the previous attempt to fix python#9655
where "type: ignore" comments are erroneously marked as unused in
re-runs of dmypy.

Ref: python#14835
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-daemon dmypy topic-type-ignore # type: ignore comments
Projects
None yet
Development

No branches or pull requests

10 participants