-
Notifications
You must be signed in to change notification settings - Fork 235
Fix: Skip bad-return validation for Protocol methods (#1916) #1965
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
base: main
Are you sure you want to change the base?
Fix: Skip bad-return validation for Protocol methods (#1916) #1965
Conversation
Signed-off-by: Aryan Bagade <aryan@aryanbagade.com>
|
Diff from mypy_primer, showing the effect of this PR on open source code: urllib3 (https://github.com/urllib3/urllib3)
- ERROR src/urllib3/_base_connection.py:98:32-36: Function declared to return `bool` but is missing an explicit `return` [bad-return]
- ERROR src/urllib3/_base_connection.py:105:35-39: Function declared to return `bool` but is missing an explicit `return` [bad-return]
- ERROR src/urllib3/_base_connection.py:109:45-49: Function declared to return `bool` but is missing an explicit `return` [bad-return]
cibuildwheel (https://github.com/pypa/cibuildwheel)
- ERROR cibuildwheel/environment.py:56:10-13: Function declared to return `str` but is missing an explicit `return` [bad-return]
dulwich (https://github.com/dulwich/dulwich)
- ERROR dulwich/object_store.py:317:27-81: Function declared to return `tuple[BytesIO, () -> None, () -> None]` but is missing an explicit `return` [bad-return]
- ERROR dulwich/pack.py:216:49-53: Function declared to return `bool` but is missing an explicit `return` [bad-return]
- ERROR dulwich/pack.py:219:62-69: Function declared to return `ShaFile` but is missing an explicit `return` [bad-return]
hydpy (https://github.com/hydpy-dev/hydpy)
- ERROR hydpy/auxs/calibtools.py:70:27-32: Function declared to return `float` but is missing an explicit `return` [bad-return]
tornado (https://github.com/tornadoweb/tornado)
- ERROR tornado/ioloop.py:61:25-28: Function declared to return `int` but is missing an explicit `return` [bad-return]
- ERROR tornado/platform/asyncio.py:58:25-28: Function declared to return `int` but is missing an explicit `return` [bad-return]
- ERROR tornado/websocket.py:61:44-49: Function declared to return `bytes` but is missing an explicit `return` [bad-return]
- ERROR tornado/websocket.py:64:39-44: Function declared to return `bytes` but is missing an explicit `return` [bad-return]
- ERROR tornado/websocket.py:69:38-43: Function declared to return `bytes` but is missing an explicit `return` [bad-return]
- ERROR tornado/websocket.py:72:63-68: Function declared to return `bytes` but is missing an explicit `return` [bad-return]
rotki (https://github.com/rotki/rotki)
- ERROR rotkehlchen/chain/evm/accounting/structures.py:21:10-13: Function declared to return `int` but is missing an explicit `return` [bad-return]
streamlit (https://github.com/streamlit/streamlit)
- ERROR lib/streamlit/runtime/stats.py:44:30-33: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:49:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:54:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:59:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:63:32-35: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:270:33-46: Function declared to return `Sequence[str]` but is missing an explicit `return` [bad-return]
django-test-migrations (https://github.com/wemake-services/django-test-migrations)
- ERROR django_test_migrations/contrib/pytest_plugin.py:39:62-70: Function declared to return `Migrator` but is missing an explicit `return` [bad-return]
setuptools (https://github.com/pypa/setuptools)
- ERROR setuptools/_vendor/importlib_metadata/_meta.py:41:23-55: Function declared to return `dict[str, list[str] | str]` but is missing an explicit `return` [bad-return]
mypy (https://github.com/python/mypy)
- ERROR mypyc/test-data/fixtures/ir.py:18:26-30: Function declared to return `T_co` but is missing an explicit `return` [bad-return]
scrapy (https://github.com/scrapy/scrapy)
- ERROR scrapy/extensions/feedexport.py:116:39-48: Function declared to return `IO[bytes]` but is missing an explicit `return` [bad-return]
- ERROR scrapy/spiderloader.py:35:55-59: Function declared to return `Self@SpiderLoaderProtocol` but is missing an explicit `return` [bad-return]
- ERROR scrapy/spiderloader.py:38:41-53: Function declared to return `type[Spider]` but is missing an explicit `return` [bad-return]
- ERROR scrapy/spiderloader.py:42:23-32: Function declared to return `list[str]` but is missing an explicit `return` [bad-return]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I think we can do this a bit more simply, by instead adjusting the logic in BindingsBuilder::function_body, where we determine the stub_or_impl value. The class key is already available in that code, so we can also avoid threading it through the binding.
For what it's worth, it looks like we always treat ... as a stub. We did this to match pyright & because the conformance tests are written this way. The conformance tests were recently updated to avoid the need to accept ... always (python/typing#2134). For now, I think we should leave this behavior alone, but might be an interesting follow-up.
Summary
Protocol methods with only a docstring (no
...or implementation body) were incorrectly emitting "Function declared to return X but is missing an explicit return" errors. This is valid Python for Protocol definitions and is accepted by mypy and other type checkers.Changes:
class_keyfield toReturnTypeKind::ShouldValidateAnnotationto track the enclosing classsolve.rs, check if the class is a Protocol and skip return validation for Protocol methodsFixes #1916
Test Plan
test_protocol_method_with_docstringtest case covering:@propertywith docstring onlypasspython3 test.py- all tests passcargo test --lib protocol::- all 37 protocol tests pass