Repository navigation
Infer every value of a context manager's first yield - #3443
Open
Parsiffall1 wants to merge 3 commits into
Open
Parsiffall1 wants to merge 3 commits into
Parsiffall1 wants to merge 3 commits into
Conversation
_infer_context_manager took only the first value inferred for the first yield of a contextlib.contextmanager generator, so the target of "with ... as x" lost the other possible values. Infer all values of that first yield instead, still ignoring later yields. Closes pylint-dev#3440
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Changes
Description
For a
contextlib.contextmanagergenerator,_infer_context_managerdidnext(inferred.infer_yield_types()), so theastarget kept only the first value inferred for the firstyield. When that yield can infer to several values, the others were dropped. With the example from the issue,capturedinferred only toTextIOWrapper, and pylint reportedInstance of 'TextIOWrapper' has no 'getvalue' member.This change takes the first
yieldof the generator (same selection asFunctionDef.infer_yield_result) and yields every value inferred for it, using the generator's call context as before. Later yields are still ignored, astest_inferring_with_contextlib_contextmanagerexpects.Tests:
test_inferring_with_contextlib_contextmanager_all_first_yield_values:yield 1 if flag else "a"now infers to both1and"a", and a secondyieldis still ignored. Fails onmain, passes with the fix.test_inferring_with_contextlib_contextmanager_uses_call_arguments: the call arguments are still used to infer the yielded value.pytest tests: all pass, excepttest_identify_old_namespace_package_protocol, which fails the same way onmainin my environment becausepkg_resourcesisn't installed. pre-commit (ruff, black, pylint, mypy, pyright) passes. The new lines have full branch coverage.pylint -Eon_pytestandpip/_internal, before and after: the only difference is one less false positive (_pytest/config/__init__.py:507: Instance of 'TextIOWrapper' has no 'mode' member).Note: #3323 changes how
yield fromis inferred ininfer_yield_result. This path now infers the first yield directly, so whichever PR lands second needs to keep both behaviours. The tests in each PR would catch it.Closes #3440