Unused function call extension #8732
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8732 +/- ##
==========================================
- Coverage 95.82% 95.80% -0.02%
==========================================
Files 173 174 +1
Lines 18381 18421 +40
==========================================
+ Hits 17613 17648 +35
- Misses 768 773 +5
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thank you for opening the pull request :)
Could you add a changelog entry (https://github.com/pylint-dev/pylint/blob/main/doc/whatsnew/fragments/5159.new_check), an example of bad/good code in the documentation and cover the missing line please (https://github.com/pylint-dev/pylint/tree/main/doc/data/messages/c/comparison-of-constants) ? There's an easier way to cover a lot of use cases with functional tests, see https://github.com/pylint-dev/pylint/blob/main/tests/functional/ext/check_elif/check_elif.py for example.
| msgs = { | ||
| "W5486": ( | ||
| "Function returned value which is never used", | ||
| "unused-return-value", |
There was a problem hiding this comment.
I just realized that we have a message for expression that aren't assigned : https://pylint.readthedocs.io/en/stable/user_guide/messages/warning/expression-not-assigned.html. Maybe grouping those two in the same checker and calling this one the same way would make sense:
| "unused-return-value", | |
| "function-return-not-assigned", |
There was a problem hiding this comment.
Relevant issues:
- False positive
expression-not-assignedwhen calling functions with no return inside ternary expression #8129 - expression-not-assigned should not be emitted for ternary operator #2380
We might want to check some ternary construct.
There was a problem hiding this comment.
Thanks for the review. Docs and examples added, switched to functional tests, message was renamed to function-return-not-assigned.
|
|
||
| some_var = "" | ||
| # next line should probably raise? | ||
| func_that_returns_something() if some_var else func_that_returns_none() |
There was a problem hiding this comment.
parent of all three func_that_returns_something some_var and func_that_returns_none is IfExpr. If some_var would be function call, I would consider it to be used while func_that_returns_something func_that_returns_none not. func_that_returns_something should raise function-return-not-assigned
There was a problem hiding this comment.
Thank you for your work on this. I think it need to take #7935 (comment) into account, seeing the truncated primer result, there's indeed too much false positive otherwise.
jacobtylerwalls
added
the
Waiting on author
|
This PR needs take over because because it has been open 8 weeks with no activity. |
github-actions
bot
added
the
Needs take over 🛎️
Type of Changes
Description
Adding extension to check for unassigned non-nullable return values from function/method calls. This update encourages proper handling of returned values, which is especially handy when working with immutable classes and collections.
Closes #7935