Hi David, I would like to work on this. Would this be an OK first issue to pick up?

You are certainly welcome to try! However, I do not think that this is a particularly beginner friendly task. We might have some issues labeled with "help wanted" that might be more suitable for a first contribution.

Got it. I will try to find another issue, or another minor task in this area (I do see some TODOs in the tests, for example).

However, one thing is not clear to me. You mention in the issue description that replacing sys.exit(1) with while True: pass does not give an error. I tried that out, and indeed there is no error.

However, the following also does not give any error:

def f() -> int:
  while True:
    pass

(Notice that I changed the return value to int.)

Here, if the "while True never returns anything" inference was being done correctly, the above should have errored out, right?

Moreover, the following gives an error:

# "Object of type `Literal["foo"]` is not assignable to return type `int`"
def f() -> int:
  while True:
    pass

  return "foo"

... which makes me wonder if red-knot does not correctly understand the while True: pass case at all. Am I missing something?

Edit: looking at astral-sh/ruff#15676, which introduced the unreachability-detection, I don't see a "while True with no break" condition.

However, the following also does not give any error:

def f() -> int:
while True:
pass

(Notice that I changed the return value to int.)

Because there are no reachable return statements, and the end of function scope is not visible, it makes sense that there are no diagnostics emitted currently, because we aren't checking the return value at all.

However, in this case, the correct thing to do would be to enforce that the function's annotated return type is Never, isn't it?
@sharkdp does this seem fit for a separate issue?

@abhijeetbodas2001 Sorry for responding so late, I must have missed the notification about your original message while I was out on vacation.

Here, if the "while True never returns anything" inference was being done correctly, the above should have errored out, right?

I don't think so. A function annotated with int that loops endlessly is not incorrect. You can think of the endless loop as returning Never, but Never is assignable to the return type int (Never is assignable to any type), so nothing wrong there.

Moreover, the following gives an error:

Yes, we don't silence all errors in unreachable code. And in this case, it's arguable if that would be an improvement. If the loop were not endless, that return statement would be wrong. So flagging it does not seem like unwanted behavior to me.

(Never is assignable to any type)

Ah! I missed this. That makes sense then. Thanks!

Currently NoReturn cannot be used to narrow a type. Which is a relatively common pattern in for example Flask examples. Where abort() can be used to return an error code early. This can be used to make sure that the request contains valid data.

For reference here is another example that currently doesnt work.

from typing import NoReturn, reveal_type

def raise_error() -> NoReturn:
    raise Exception('test')


def test(x: str | None) -> None:
    if x is None:
        raise_error()
    
    reveal_type(x)
    # Should be "str" is "str | None"

Same issue, different symptoms. I have this simplified example that's causing warning[possibly-unbound-attribute]: Attribute ignoreon typeQCloseEvent | None is possibly unbound since ty doesn't see that

from PySide6 import QtGui
from PySide6.QtWidgets import QMainWindow

class MainWindow(QMainWindow):
    def closeEvent(self, event: QtGui.QCloseEvent | None = None):
        """Exit safely when closing the window."""

        if event is None or False:  # `or False` is actually some other condition, simplified for this example
            sys.exit(1)  # Here ty should see that `event` can't be `None` if this branch wasn't taken

        # ... some more checks and user prompts happened here

        # Fallthrough case: Prevent program from closing.
        event.ignore()

Also for os.execl, os.execle, os.execlp, os.execlpe, os.execvp, os.execvpe.

Funcs in os.py that mention "replacing the current process" in their doc.

def restartLow() -> typing.NoReturn:
	"""Will not return from the function."""
	os.execl(
		sys.executable,
		sys.executable,
		*sys.argv,
	)

def execl(file, *args):
    """execl(file, *args)

    Execute the executable file with argument list args, replacing the
    current process. """
    execv(file, args)