comments and tests

Author: abhijeetbodas2001

crates/ty_python_semantic/resources/mdtest/terminal_statements.md

@@ -572,6 +572,8 @@ def f():
 
 ## Calls to functions returning `Never` / `NoReturn`
 
+These calls should be treated as terminal statements.
+
 ### No implicit return
 
 If we see a call to a function returning `Never`, we should be able to understand that the function
@@ -614,9 +616,42 @@ def g(x: int | None):
         sys.exit(1)
 
     # TODO: should be just int, not int | None
+    # See https://github.com/astral-sh/ty/issues/685
     reveal_type(x)  # revealed: int | None
 ```
 
+### Possibly unresolved diagnostics
+
+If the codepath on which a variable is not defined eventually returns `Never`, use of the variable
+should not give any diagnostics.
+
+```py
+import sys
+
+def _(flag: bool):
+    if flag:
+        x = 3
+    else:
+        sys.exit()
+
+    x  # No possibly-unresolved-references diagnostic here.
+```
+
+Similarly, there shouldn't be any diagnostics if the `except` block of a `try/except` construct has
+a call with `NoReturn`.
+
+```py
+import sys
+
+def _():
+    try:
+        x = 3
+    except:
+        sys.exit()
+
+    x  # No possibly-unresolved-references diagnostic here.
+```
+
 ### Bindings after call
 
 These should be understood to be unreachable.
@@ -651,6 +686,7 @@ def f(x): ...
 def _() -> NoReturn:
     f(3)
 
+# This should be an error because of implicitly returning `None`
 # error: [invalid-return-type]
 def _() -> NoReturn:
     f("")

crates/ty_python_semantic/src/semantic_index/builder.rs

@@ -1910,9 +1910,21 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
 
                 self.visit_expr(value);
 
-                // These constraints are expensive
-                if !self.source_type.is_stub() && self.in_function_scope() {
-                    if let ast::Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
+                // If the statement is a call, it could possibly be a call to a function
+                // marked with `NoReturn` (for example, `sys.exit()`). In this case, we use a special
+                // kind of constraint to mark the following code as unreachable.
+                //
+                // Ideally, these constraints should be added for every call expression, even those in
+                // sub-expressions and in the module-level scope. But doing so makes the number of
+                // such constraints so high that it significantly degrades performance. We thus cut
+                // scope here and add these constraints only at statement level function calls,
+                // like `sys.exit()`, and not within sub-expression like `3 + sys.exit()` etc.
+                //
+                // We also only add these inside function scopes, since considering module-level
+                // constraints can affect the the type of imported symbols, leading to a lot more
+                // work in third-party code.
+                if let ast::Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
+                    if !self.source_type.is_stub() && self.in_function_scope() {
                         let callable = self.add_standalone_expression(func);
                         let call_expr = self.add_standalone_expression(value.as_ref());
 

crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs

@@ -689,6 +689,13 @@ impl ReachabilityConstraints {
                 callable,
                 call_expr,
             }) => {
+                // We first infer just the type of the callable. In the most likely case that the
+                // function is not marked with `NoReturn`, or that it always returns `NoReturn`,
+                // doing so allows us to avoid the more expensive work of inferring the entire call
+                // expression (which could involve inferring argument types to possibly run the overload
+                // selection algorithm).
+                // Avoiding this on the happy-path is important because these constraints can be
+                // very large in number, since we add them on all statement level function calls.
                 let ty = infer_expression_type(db, callable);
 
                 let overloads_iterator =