-
Notifications
You must be signed in to change notification settings - Fork 239
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
This comment has been minimized.
This comment has been minimized.
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.
Signed-off-by: Aryan Bagade <[email protected]>
ba00e77 to
7758b70
Compare
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Aryan Bagade <[email protected]>
|
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]
- ::error file=src/urllib3/_base_connection.py,line=98,col=32,endLine=98,endColumn=36,title=Pyrefly bad-return::Function declared to return `bool` but is missing an explicit `return`
- ::error file=src/urllib3/_base_connection.py,line=105,col=35,endLine=105,endColumn=39,title=Pyrefly bad-return::Function declared to return `bool` but is missing an explicit `return`
- ::error file=src/urllib3/_base_connection.py,line=109,col=45,endLine=109,endColumn=49,title=Pyrefly bad-return::Function declared to return `bool` but is missing an explicit `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]
- ::error file=cibuildwheel/environment.py,line=56,col=10,endLine=56,endColumn=13,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `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]
- ::error file=dulwich/object_store.py,line=317,col=27,endLine=317,endColumn=81,title=Pyrefly bad-return::Function declared to return `tuple[BytesIO, () -> None, () -> None]` but is missing an explicit `return`
- ::error file=dulwich/pack.py,line=216,col=49,endLine=216,endColumn=53,title=Pyrefly bad-return::Function declared to return `bool` but is missing an explicit `return`
- ::error file=dulwich/pack.py,line=219,col=62,endLine=219,endColumn=69,title=Pyrefly bad-return::Function declared to return `ShaFile` but is missing an explicit `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]
- ::error file=hydpy/auxs/calibtools.py,line=70,col=27,endLine=70,endColumn=32,title=Pyrefly bad-return::Function declared to return `float` but is missing an explicit `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]
- ::error file=rotkehlchen/chain/evm/accounting/structures.py,line=21,col=10,endLine=21,endColumn=13,title=Pyrefly bad-return::Function declared to return `int` but is missing an explicit `return`
streamlit (https://github.com/streamlit/streamlit)
- ERROR lib/streamlit/runtime/stats.py:45:30-33: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:50:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:55:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:60:23-26: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:64:32-35: Function declared to return `str` but is missing an explicit `return` [bad-return]
- ERROR lib/streamlit/runtime/stats.py:271:33-46: Function declared to return `Sequence[str]` but is missing an explicit `return` [bad-return]
- ::error file=lib/streamlit/runtime/stats.py,line=45,col=30,endLine=45,endColumn=33,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `return`
- ::error file=lib/streamlit/runtime/stats.py,line=50,col=23,endLine=50,endColumn=26,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `return`
- ::error file=lib/streamlit/runtime/stats.py,line=55,col=23,endLine=55,endColumn=26,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `return`
- ::error file=lib/streamlit/runtime/stats.py,line=60,col=23,endLine=60,endColumn=26,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `return`
- ::error file=lib/streamlit/runtime/stats.py,line=64,col=32,endLine=64,endColumn=35,title=Pyrefly bad-return::Function declared to return `str` but is missing an explicit `return`
- ::error file=lib/streamlit/runtime/stats.py,line=271,col=33,endLine=271,endColumn=46,title=Pyrefly bad-return::Function declared to return `Sequence[str]` but is missing an explicit `return`
pandas (https://github.com/pandas-dev/pandas)
- ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`
+ ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> int | ndarray[tuple[int], dtype[Any]] | ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | slice[int, int, Any] | slice[Any, Any, Any] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`
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]
- ::error file=setuptools/_vendor/importlib_metadata/_meta.py,line=41,col=23,endLine=41,endColumn=55,title=Pyrefly bad-return::Function declared to return `dict[str, list[str] | str]` but is missing an explicit `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]
- ::error file=django_test_migrations/contrib/pytest_plugin.py,line=39,col=62,endLine=39,endColumn=70,title=Pyrefly bad-return::Function declared to return `Migrator` but is missing an explicit `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]
- ::error file=scrapy/extensions/feedexport.py,line=116,col=39,endLine=116,endColumn=48,title=Pyrefly bad-return::Function declared to return `IO[bytes]` but is missing an explicit `return`
- ::error file=scrapy/spiderloader.py,line=35,col=55,endLine=35,endColumn=59,title=Pyrefly bad-return::Function declared to return `Self@SpiderLoaderProtocol` but is missing an explicit `return`
- ::error file=scrapy/spiderloader.py,line=38,col=41,endLine=38,endColumn=53,title=Pyrefly bad-return::Function declared to return `type[Spider]` but is missing an explicit `return`
- ::error file=scrapy/spiderloader.py,line=42,col=23,endLine=42,endColumn=32,title=Pyrefly bad-return::Function declared to return `list[str]` but is missing an explicit `return`
|
|
@samwgoldman has imported this pull request. If you are a Meta employee, you can view this in D90340690. |
kinto0
left a comment
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.
Review automatically exported from Phabricator review in Meta.
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