inconsistent-return-statements not detected for while loop with flag

[mskrzypkows]

Bug description

    def get_the_answer(self):  # [inconsistent-return-statements]
        while self.is_running:
            read_message()
            if comunication_finished():
                return "done"

Configuration

No response

Command used

pylint a.py

Pylint output

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 9.97/10, +0.03)

Expected behavior

I expect to get message:
R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements) because when the 'self.is_running flag changes to false, I'll receive None instead of str which isn't desired behaviour and should be detected by pylint.

Pylint version

pylint 2.16.1
astroid 2.14.2
Python 3.10.9 (main, Dec 15 2022, 17:11:09) [Clang 14.0.0 (clang-1400.0.29.202)]

OS / Environment

Macos

Additional dependencies

No response

[nickdrozd]

In #1772, somebody complained that a warning in this situation was a false positive. So an exception was made for while loops. I think that was a bad idea, because now it's causing false negatives. The while exception should be removed, and the behavior should go back to the way it was. That would fix this false negative.

This is a case where it is impossible to reconcile the needs of every user. Some users want the tool to help them catch mistakes, and they will be bothered when mistakes aren't caught (like the present issue). Other users think their code is just fine the way it is, and they don't want the tool to suggest otherwise, so they will be bothered by what they perceive as "false positives" (the previous issue).

The fix here is easy, just revert the change from #1806.

[Pierre-Sassoulas]

If a while True: loop has no break, then the function can never go beyond the while loop, and we should not raise (as asked in #1772). On the other hand what we have here is not a while True loop, at least I suppose that self.is_running can become False so we should raise. I think we can reconcile both use-case. If we couldn't pylint need to favor false negative over false positive because it's less disruptive to have a false negative than a false positive. Less work for the user that do not need to disable message and less complaint for us.

[nickdrozd]

It's like when I step on the scale and see a number I don't like, and then I write an angry letter to the scale manufacturer 🙄

But I think your compromise is nice. If the while test is a truthy constant and the body contains no breaks, then we can deduce that anything after the loop is unreachable, and the warning won't be emitted. In any other circumstance, we will assume that the loop might terminate, and the warning will be emitted. Or in other words, the loop will be assumed to possibly terminate unless it is extremely obvious that it won't.