Fix useless-return false negatives #9492
Conversation
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #9492 +/- ##
==========================================
- Coverage 95.83% 95.82% -0.01%
==========================================
Files 173 173
Lines 18808 18811 +3
==========================================
+ Hits 18024 18026 +2
- Misses 784 785 +1
|
This comment has been minimized.
This comment has been minimized.
DanielNoord
left a comment
DanielNoord
left a comment
There was a problem hiding this comment.
The primer run looks good as well!
Nice and simple change π Let's add some more tests though!
| return | ||
|
|
||
|
|
||
| def function5(parameter): # [useless-return] |
There was a problem hiding this comment.
I'd like to see some cases where there is actual code after these excepts and if ... else statements to make sure we are not raising false positives.
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Looking good ! I'm wondering what happens if there's a if/return construct in the middle of a for or while loop. (Then we should not raise because the return isn't useless afaiu). Could we add the same test cases but inside a loop ?
Returns inside try or if/else conditions were not being detected as useless. Said nodes are now checked for return statements to ensure their last component is not a useless return as well. Closes pylint-dev#9449.
This useless return was discovered after improving the useless-return checker. It has been removed to fix the warning.
crazybolillo
force-pushed
the
i9449-anton
branch
from
220cf1b to
3f4349f
Compare
This comment has been minimized.
This comment has been minimized.
crazybolillo
force-pushed
the
i9449-anton
branch
from
3f4349f to
43ddc01
Compare
This comment has been minimized.
This comment has been minimized.
|
I added some more tests. Is this what you had in mind? Or am I missing some other cases? |
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Thank you for adding the test case, I was thinking of something like this (in fact most of your test cases could do in a loop context)
Test suite has been improved with more tests to verify no bugs have been introduced by the recent changes to the checker.
crazybolillo
force-pushed
the
i9449-anton
branch
from
43ddc01 to
579b41a
Compare
|
Great change ! |
3 participants
Type of Changes
Description
Returns inside try or if/else conditions were not being detected as useless. Said nodes are now checked for return statements to ensure their last component is not a useless return as well.
Closes #9449.