pylint
pylint copied to clipboard
pylint-dev
False negative ``expression-not-assigned`` on fonction calls that return values
Current problem
This is a request to implement a check similar to the [[nodiscard]] function attribute in C++. A typical mistake is when people assume that the method modifies the current object while it actually creates and returns a copy of self. Calling this method without using the return value doesn't make sense and could be easily caught.
An example with sqlalchemy. Instead of:
if (id):
query.filter(row.id == id)
You should write:
if (id):
query = query.filter(row.id == id)
The problem is the first version looks legit: query.filter reads as "filter the query", so it's hard to spot this mistake. I've seen even experienced sqlalchemy users fall into this trap once in a while.
Desired solution
I'd suggest something like # pylint: enable=unused-return-value pragma for functions with a subsequent analysis of the call site.
There's already a very similar warning expression-not-assigned / W0106, but it specifically ignores the function calls. Which makes sense because in many cases functions have side effects besides just the return value. In some cases though they're limited to the return value and it would be nice to catch these.
Additional context
https://stackoverflow.com/questions/51858215/how-to-make-pylint-report-unused-return-values https://stackoverflow.com/questions/74785720/python-prevent-discarding-of-the-function-return-value
It seem like it could be considered like a false negative of expression-not-assigned, but a call can have side effect so right now we're not raising this message on calls.
It seem like it could be considered like a false negative of expression-not-assigned, but a call can have side effect so right now we're not raising this message on calls.
This is an option. The thing is, I'm really sure this should be enabled on a per-function basis, otherwise it's going to be 99% noise. But annotations for a few key functions in popular libraries (like the @_generative decorator in the sqlalchemy) will make a huge difference.
And from that perspective it makes sense to have a separate warning ID, just not to mess with defaults for expression-not-assigned.
Sounds like an option to list the calls that should definitely be assigned to something would be nice (calls-that-need-assignment ?). Amusingly it would make https://pylint.pycqa.org/en/latest/user_guide/messages/warning/useless-with-lock.html useless if we simply add threading.Lock()to this list 😄
This feature would also be very valuable when using libraries with return codes/bools where you need to remember to check them all the time :sweat:
Very much in favour of this!
No discard is quite a common warning in other languages: https://www.avanderlee.com/swift/discardableresult/
