typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

Disable mypy overload-overlap warnings?

Open srittau opened this issue 1 year ago • 6 comments

These mypy warnings are overzealous, as it's often quite useful to have partial overlaps: We ignore this warning in 192 places in typeshed at the moment, and I don't think we gain any benefit from having it enabled.

Ideally, mypy would have a separate warning for overloads that are completely shadowed by another overload, though.

srittau avatar Jun 20 '24 14:06 srittau

We should check how many become unused with the mypy master branch, as the way the lint works at mypy has just been completely reworked in https://github.com/python/mypy/pull/17392.

AlexWaygood avatar Jun 20 '24 15:06 AlexWaygood

That sounds promising!

srittau avatar Jun 20 '24 15:06 srittau

So yes, these are mypy's new complaints regarding overload-overlap if you use mypy master, fiddle with mypy's master branch so that you get rid of the thing where it stops --warn-unused-ignores from working inside typeshed directories, then fiddle with our mypy_test.py so that it ignores the override error code (because there's a bunch of new errors from that error code on mypy master which are unrelated to the thing we're discussing here), and then run python tests/mypy_test.py stdlib -p3.13:

stdlib/typing.pyi:1056: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/types.pyi:322: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/types.pyi:573: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:77: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:127: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:133: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:139: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:158: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:164: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/re.pyi:175: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/importlib/metadata/__init__.pyi:49: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/enum.pyi:108: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/enum.pyi:244: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/collections/__init__.pyi:289: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:171: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:1735: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/builtins.pyi:1745: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tempfile.pyi:464: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tempfile.pyi:474: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/urllib/parse.pyi:201: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [overload-overlap]
stdlib/urllib/parse.pyi:207: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [overload-overlap]
stdlib/unittest/mock.pyi:278: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:339: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:385: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:396: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:620: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:659: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/turtle.pyi:669: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tkinter/ttk.pyi:1043: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/tkinter/ttk.pyi:1087: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/urllib/error.pyi:17: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:76: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:118: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:122: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/multiprocessing/sharedctypes.pyi:123: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:151: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:153: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:157: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:166: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:176: error: Unused "type: ignore" comment  [unused-ignore]
stdlib/asyncio/tasks.pyi:189: error: Unused "type: ignore" comment  [unused-ignore]

And in our third-party stubs we have:

stubs/WebOb/webob/request.pyi:44: error: Unused "type: ignore" comment  [unused-ignore]
stubs/gevent/gevent/timeout.pyi:46: error: Unused "type: ignore" comment  [unused-ignore]
stubs/icalendar/icalendar/parser_tools.pyi:11: error: Unused "type: ignore" comment  [unused-ignore]
stubs/icalendar/icalendar/parser_tools.pyi:15: error: Unused "type: ignore" comment  [unused-ignore]
stubs/olefile/olefile/olefile.pyi:168: error: Unused "type: ignore" comment  [unused-ignore]
stubs/olefile/olefile/olefile.pyi:170: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/xml/_functions_overloads.pyi:80: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:40: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:158: error: Unused "type: ignore" comment  [unused-ignore]
stubs/openpyxl/openpyxl/descriptors/nested.pyi:274: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pika/pika/adapters/twisted_connection.pyi:26: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pika/pika/adapters/twisted_connection.pyi:27: error: Unused "type: ignore" comment  [unused-ignore]
stubs/pycocotools/pycocotools/mask.pyi:25: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:127: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:136: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:155: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:159: error: Unused "type: ignore" comment  [unused-ignore]
stubs/setuptools/pkg_resources/__init__.pyi:196: error: Unused "type: ignore" comment  [unused-ignore]
stubs/ExifRead/exifread/utils.pyi:9: error: Unused "type: ignore" comment  [unused-ignore]

So in total that's around 60 that will go away altogether. That will still leave us with quite a few errors ignored in typeshed, and I agree we don't get much value from the check. (We already have pyright's version switched off, because it was even noisier than mypy's.)

Diff I made to mypy to get --warn-unused-ignore working:

--- a/mypy/errors.py
+++ b/mypy/errors.py
@@ -680,11 +680,6 @@ class Errors:
                 self.has_blockers.remove(path)
 
     def generate_unused_ignore_errors(self, file: str) -> None:
-        if (
-            is_typeshed_file(self.options.abs_custom_typeshed_dir if self.options else None, file)
-            or file in self.ignored_files
-        ):
-            return
         ignored_lines = self.ignored_lines[file]
         used_ignored_lines = self.used_ignored_lines[file]
         for line, ignored_codes in ignored_lines.items():

Diff I made to our tests:

diff --git a/tests/mypy_test.py b/tests/mypy_test.py
index cafaca811..431439509 100755
--- a/tests/mypy_test.py
+++ b/tests/mypy_test.py
@@ -270,6 +270,8 @@ def run_mypy(
             "ignore-without-code",
             "--enable-error-code",
             "redundant-self",
+            "--disable-error-code",
+            "override",
             "--config-file",
             temp.name,
         ]

AlexWaygood avatar Jun 20 '24 16:06 AlexWaygood

My gut feeling says that we should disable the test even after the PR (as you also wrote), but maybe it's a good idea to do this after the next mypy release, as a baseline.

srittau avatar Jun 20 '24 17:06 srittau

Tbh, I kind of like these, despite the fact that we often have no choice but to ignore them. When reading stubs, they alert me to useful facts about the way overloads are structured. I would vote we prune down the ignores to match the state following https://github.com/python/mypy/pull/17392 which gets rid of some of the least helpful ones (I'll open a PR soon so we can take a look).

hauntsaninja avatar Aug 02 '24 07:08 hauntsaninja

I'm actually now quite strongly opposed to disabling these warnings, given the spectrum of views in https://discuss.python.org/t/draft-typing-spec-chapter-for-overloads . Until we have specified behaviour of overloads, I think it's very useful to clearly call out areas where typeshed's needs in practice can inform that specification

hauntsaninja avatar Aug 17 '24 01:08 hauntsaninja