stubtest: error for pos-only differences for dunder methods, too

[AlexWaygood]

Description

stubtest currently does not complain if a dunder method in a stub file has an argument that should be positional-only, but is marked in the stub as being positional-or-keyword. This PR proposes to remove this exception.

Removing the exception will increase our confidence in the accuracy of typeshed stubs with regard to positional-only arguments.

The rationale for the exception was the very large number of complaints stubtest had when checking typeshed. However, I have been using this patch over the past few days to file a series of PRs to typeshed, in order to minimise the impact of this PR:

• Fix positional-only differences in types typeshed#7220
• Fix positional-only differences in sys typeshed#7221
• Fix positional-only differences in ctypes typeshed#7223
• Fix positional-only differences in _tkinter typeshed#7224
• Fix positional-only differences in xml typeshed#7225
• Fix positional-only differences in many stdlib modules typeshed#7226
• Fix positional-only differences in threading.local typeshed#7227

As a result, this PR means that only the following new hits are generated when running stubtest on typeshed:

+ error: sqlite3.Connection.__exit__ is inconsistent, stub argument "t" should be positional-only (rename with a leading double underscore, i.e. "__type")
+ error: sqlite3.Connection.__exit__ is inconsistent, stub argument "exc" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Connection.__exit__ is inconsistent, stub argument "tb" should be positional-only (rename with a leading double underscore, i.e. "__traceback")
+ error: sqlite3.Row.__eq__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Row.__ge__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Row.__getitem__ is inconsistent, stub argument "index" should be positional-only (rename with a leading double underscore, i.e. "__key")
+ error: sqlite3.Row.__gt__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Row.__le__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Row.__lt__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.Row.__ne__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Connection.__exit__ is inconsistent, stub argument "t" should be positional-only (rename with a leading double underscore, i.e. "__type")
+ error: sqlite3.dbapi2.Connection.__exit__ is inconsistent, stub argument "exc" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Connection.__exit__ is inconsistent, stub argument "tb" should be positional-only (rename with a leading double underscore, i.e. "__traceback")
+ error: sqlite3.dbapi2.Row.__eq__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Row.__ge__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Row.__getitem__ is inconsistent, stub argument "index" should be positional-only (rename with a leading double underscore, i.e. "__key")
+ error: sqlite3.dbapi2.Row.__gt__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Row.__le__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Row.__lt__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")
+ error: sqlite3.dbapi2.Row.__ne__ is inconsistent, stub argument "other" should be positional-only (rename with a leading double underscore, i.e. "__value")

All of these remaining hits will be resolved by one final outstanding PR to typeshed (EDIT: now merged -- thanks @JelleZijlstra!):

• Fix positional-only differences in sqlite3 typeshed#7222.

The full command I ran to obtain this diff was the following (with ../typeshed being the path to my clone of typeshed):

> python -m mypy.stubtest --check-typeshed --custom-typeshed-dir ../typeshed --allowlist ../typeshed/tests/stubtest_allowlists/py3_common.txt --allowlist ../typeshed/tests/stubtest_allowlists/py310.txt --allowlist ../typeshed/tests/stubtest_allowlists/win32.txt --allowlist ../typeshed/tests/stubtest_allowlists/win32-py310.txt > pos_only_report.txt

I ran this command without this patch applied, and with this patch applied. The command was run on Windows, with Python 3.10.

Test Plan

The existing exception does not appear to be tested; this PR does not appear to break any existing tests. I'm happy to add some tests for this change, if they'd be appreciated, however :)

Design question

We could add a new command-line option to turn this exception back on. Thoughts on that?

[AlexWaygood]

I think that's the longest rationale I've ever written for deleting a single line of code.

[AlexWaygood]

cc. @hauntsaninja

[JelleZijlstra]

Thanks for reminding me to merge that PR, and thanks for all the fixes!

I don't think this needs a new config option; we should just remove the special case. But I'll let @hauntsaninja weigh in.

[hauntsaninja]

Thanks for making all of these fixes, looks good!

If you're interested in pulling on this, there are a couple other places where we special case dunders (grep for is_dunder and SPECIAL_DUNDERS). In particular, I think the SPECIAL_DUNDERS check in verify_typeinfo could be productively removed.

On the design question: I don't think we need a flag to turn these specifically back off. stubtest already has a --ignore-positional-only for people who don't find this class of error useful. More generally, stubtest operates in a somewhat murky area and so is forced to have several opinions. If we get feedback that those opinions are bad, we should change, but I don't want to accrete complexity preemptively.

[AlexWaygood]

If you're interested in pulling on this, there are a couple other places where we special case dunders (grep for is_dunder and SPECIAL_DUNDERS). In particular, I think the SPECIAL_DUNDERS check in verify_typeinfo could be productively removed.

I already have a second patch waiting in the wings, I just figured I'd do the smaller one first ;)

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[AlexWaygood]

What's the easiest way to run the tests in teststubtest.py from the command line btw, for future reference?

[hauntsaninja]

pytest ./mypy/test/teststubtest.py should work :-)

[AlexWaygood]

pytest ./mypy/test/teststubtest.py should work :-)

Thanks! I'm still relatively new to pytest 🤦‍♂️

[hauntsaninja]

mypy's test setup is also very custom!