Infer return value of None for correctly inferred functions with no returns #983
Conversation
This file contains 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
nelfin
added a commit
to nelfin/pylint
that referenced
this pull request
May 1, 2021
4 tasks
nelfin
force-pushed
the
feature/infer-none-with-no-return
branch
from
616128c
to
ff719c8
Compare
|
Rebased to fix conflict with changes to |
nelfin
commented
May 2, 2021
nelfin
commented
May 2, 2021
nelfin
commented
May 2, 2021
cdce8p
reviewed
May 22, 2021
There was a problem hiding this comment.
All in all this looks like a reasonable change. Thanks for also opening pylint-dev/pylint#4428 to fix the pylint tests!
I ran some tests and didn't notice anything unusual, so should be ok to merge once all discussions are resolved.
cdce8p
added
the
pylint-tested
nelfin
force-pushed
the
feature/infer-none-with-no-return
branch
from
7ec6a68
to
d66e3d1
Compare
nelfin
added a commit
to nelfin/pylint
that referenced
this pull request
May 24, 2021
nelfin
force-pushed
the
feature/infer-none-with-no-return
branch
from
d66e3d1
to
6ba7778
Compare
Thanks @cdce8p ! |
Ref pylint-dev#485. If the function was inferred (unlike many compiler-builtins) and it contains no Return nodes, then the implicit return value is None.
Ref pylint-dev#663. This test did not actually check for regression of the issue fixed in 55076ca (i.e. it also passed on c87bea1 before the fix was applied). Additionally, it over-specified the behaviour it was attempting to check: whether the value returned from the context manager was Uninferable was not directly relevant to the test, so when this value changed due to unrelated fixes in inference, this test failed.
Ref pylint-dev#485. To avoid additional breakage, we optionally extend is_abstract to consider functions whose body is any raise statement (not just raise NotImplementedError)
nelfin
force-pushed
the
feature/infer-none-with-no-return
branch
from
6ba7778
to
ce10268
Compare
I updated the comment with a more in-depth explanation |
|
Thanks @nelfin ! |
cdce8p
pushed a commit
to pylint-dev/pylint
that referenced
this pull request
May 30, 2021
* Add regression tests of inference of implicit None return Ref pylint-dev/astroid#485. Depends on pylint-dev/astroid#983. * Update tests to return size from __len__ No reason why they can't when it's relevant, instead of disabling this warning
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.
Steps
Description
Functions with empty bodies or no return statements e.g.
def f(): passwere returned asUninferablefrominfer_call_resultType of Changes
Related Issue
Closes #485
Ref #663
Pylint PR: pylint-dev/pylint#4428