Validate contextmanager return types against the context manager - #378
Validate contextmanager return types against the context manager#378Muhtasim-Munif-Fahim wants to merge 1 commit into
Conversation
`contextlib.contextmanager` and `contextlib.asynccontextmanager` wrap the
decorated generator function with `functools.wraps`, which copies
`__annotations__` over verbatim. The recorded return type therefore describes
what the *generator* yields, while calling the decorated function actually
returns a context manager.
`_validate_return_type` read that annotation straight off the template, so
mocking such a function rejected every legitimate return value:
TypeCheckError: type of return must be
typing.AsyncGenerator[foo.Foo, NoneType]; got
<class 'testslide.core.strict_mock.FooStrictMock'> instead
Detect the decoration by comparing the template against its `__wrapped__`:
contextlib turns a generator function into one that is no longer a generator
function, which distinguishes it from `functools.wraps` in general. The
expected type then becomes `contextlib.AbstractContextManager` or
`AbstractAsyncContextManager`.
The check is redirected, not disabled: a non-context-manager return value is
still rejected, which the third new test asserts.
Adds a `ContextManagerTarget` fixture plus decorated sync and async factories
to tests/sample_module.py, and three examples to the `_validate_return_type`
context. The two positive ones fail without this change.
Fixes facebook#193
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Fixes #193. The issue is from 2020 but still reproduces on current
main(555422d) — repro below.Cause
contextlib.contextmanagerandcontextlib.asynccontextmanagerwrap the decorated generator function withfunctools.wraps, which copies__annotations__over verbatim. So forthe recorded return type is
AsyncGenerator[Foo, None]— what the generator yields — while callingget_cm()actually returns an_AsyncGeneratorContextManager._validate_return_typereads that annotation straight off the template, so mocking the function rejects every legitimate return value:There is no return value that could satisfy it — the annotation describes something the caller never receives.
Reproduction
Exactly the files from #193;
testslide test_foo.pyfails before this change and passes after.foo.py / test_foo.py
Approach
Detect the decoration by comparing the template against its
__wrapped__: contextlib turns a generator function into one that is no longer a generator function, which is what distinguishes it fromfunctools.wrapsin general. The expected type then becomescontextlib.AbstractContextManager/AbstractAsyncContextManager.I used the unparameterized ABCs deliberately. The yielded type is produced later by
__aenter__, so it is not present in the value being validated and cannot be checked at this point; claimingAsyncContextManager[Foo]would be checking something we cannot see. Happy to change if you would rather keep the parameter for readability.The check is redirected, not disabled — a non-context-manager return value is still rejected.
fails_for_context_manager_template_given_a_non_context_managerasserts that, so this cannot silently rot into "no validation".Tests
Adds a
ContextManagerTargetfixture (implements both the sync and async protocols) plus decorated factories totests/sample_module.py, and three examples to the_validate_return_typecontext. The two positive ones fail without the change with exactly the error from the issue.Verified on Windows / Python 3.13:
lib,mock_callable,mock_async_callable,strict_mock,mock_constructor,patch_attribute: 2865 successful, 4 failed, 62 skipped — identical baseline of 4 pre-existing failures before and after; the 3 new successes are mine.cli_unittesterroring, which isimport ptybeing Unix-only, and reproduces on a clean checkout.ruff checkback to the same 22 pre-existing findings;flake8 --select=F,C90clean. I left the pre-existingruff formatdeviation insample_module.py(a lambda inTarget.__init__) untouched so the diff stays on topic.🤖 Generated with Claude Code