Handle raises with implicit alternate branches (#9377)

Closes
https://github.com/astral-sh/ruff/issues/9304#issuecomment-1874739740.

crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py

@@ -264,3 +264,41 @@ def func(x: int):
         if x > 0:
             return 1
     raise ValueError
+
+
+def func(x: int):
+    if x > 5:
+        raise ValueError
+    else:
+        pass
+
+
+def func(x: int):
+    if x > 5:
+        raise ValueError
+    elif x > 10:
+        pass
+
+
+def func(x: int):
+    if x > 5:
+        raise ValueError
+    elif x > 10:
+        return 5
+
+
+def func():
+    try:
+        return 5
+    except:
+        pass
+
+    raise ValueError
+
+
+def func(x: int):
+    match x:
+        case [1, 2, 3]:
+            return 1
+        case y:
+            return "foo"

crates/ruff_linter/src/rules/flake8_annotations/helpers.rs

@@ -89,7 +89,7 @@ pub(crate) fn auto_return_type(function: &ast::StmtFunctionDef) -> Option<AutoPy
     //     if x > 0:
     //         return 1
     // ```
-    if terminal == Terminal::ConditionalReturn || terminal == Terminal::None {
+    if terminal.has_implicit_return() {
         return_type = return_type.union(ResolvedPythonType::Atom(PythonType::None));
     }
 

crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type.snap

@@ -263,14 +263,14 @@ auto_return_type.py:82:5: ANN201 [*] Missing return type annotation for public f
 83 |     match x:
 84 |         case [1, 2, 3]:
    |
-   = help: Add return type annotation: `str | int`
+   = help: Add return type annotation: `str | int | None`
 
 ℹ Unsafe fix
 79 79 |         return 1
 80 80 | 
 81 81 | 
 82    |-def func(x: int):
-   82 |+def func(x: int) -> str | int:
+   82 |+def func(x: int) -> str | int | None:
 83 83 |     match x:
 84 84 |         case [1, 2, 3]:
 85 85 |             return 1
@@ -396,14 +396,14 @@ auto_return_type.py:137:5: ANN201 [*] Missing return type annotation for public
 138 |     try:
 139 |         return 1
     |
-    = help: Add return type annotation: `int`
+    = help: Add return type annotation: `int | None`
 
 ℹ Unsafe fix
 134 134 |         return 2
 135 135 | 
 136 136 | 
 137     |-def func(x: int):
-    137 |+def func(x: int) -> int:
+    137 |+def func(x: int) -> int | None:
 138 138 |     try:
 139 139 |         return 1
 140 140 |     except:
@@ -674,4 +674,99 @@ auto_return_type.py:262:5: ANN201 [*] Missing return type annotation for public
 264 264 |         if x > 0:
 265 265 |             return 1
 
+auto_return_type.py:269:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+269 | def func(x: int):
+    |     ^^^^ ANN201
+270 |     if x > 5:
+271 |         raise ValueError
+    |
+    = help: Add return type annotation: `None`
+
+ℹ Unsafe fix
+266 266 |     raise ValueError
+267 267 | 
+268 268 | 
+269     |-def func(x: int):
+    269 |+def func(x: int) -> None:
+270 270 |     if x > 5:
+271 271 |         raise ValueError
+272 272 |     else:
+
+auto_return_type.py:276:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+276 | def func(x: int):
+    |     ^^^^ ANN201
+277 |     if x > 5:
+278 |         raise ValueError
+    |
+    = help: Add return type annotation: `None`
+
+ℹ Unsafe fix
+273 273 |         pass
+274 274 | 
+275 275 | 
+276     |-def func(x: int):
+    276 |+def func(x: int) -> None:
+277 277 |     if x > 5:
+278 278 |         raise ValueError
+279 279 |     elif x > 10:
+
+auto_return_type.py:283:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+283 | def func(x: int):
+    |     ^^^^ ANN201
+284 |     if x > 5:
+285 |         raise ValueError
+    |
+    = help: Add return type annotation: `int | None`
+
+ℹ Unsafe fix
+280 280 |         pass
+281 281 | 
+282 282 | 
+283     |-def func(x: int):
+    283 |+def func(x: int) -> int | None:
+284 284 |     if x > 5:
+285 285 |         raise ValueError
+286 286 |     elif x > 10:
+
+auto_return_type.py:290:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+290 | def func():
+    |     ^^^^ ANN201
+291 |     try:
+292 |         return 5
+    |
+    = help: Add return type annotation: `int`
+
+ℹ Unsafe fix
+287 287 |         return 5
+288 288 | 
+289 289 | 
+290     |-def func():
+    290 |+def func() -> int:
+291 291 |     try:
+292 292 |         return 5
+293 293 |     except:
+
+auto_return_type.py:299:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+299 | def func(x: int):
+    |     ^^^^ ANN201
+300 |     match x:
+301 |         case [1, 2, 3]:
+    |
+    = help: Add return type annotation: `str | int`
+
+ℹ Unsafe fix
+296 296 |     raise ValueError
+297 297 | 
+298 298 | 
+299     |-def func(x: int):
+    299 |+def func(x: int) -> str | int:
+300 300 |     match x:
+301 301 |         case [1, 2, 3]:
+302 302 |             return 1
+
 

crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap

@@ -293,7 +293,7 @@ auto_return_type.py:82:5: ANN201 [*] Missing return type annotation for public f
 83 |     match x:
 84 |         case [1, 2, 3]:
    |
-   = help: Add return type annotation: `Union[str | int]`
+   = help: Add return type annotation: `Union[str | int | None]`
 
 ℹ Unsafe fix
    1  |+from typing import Union
@@ -305,7 +305,7 @@ auto_return_type.py:82:5: ANN201 [*] Missing return type annotation for public f
 80 81 | 
 81 82 | 
 82    |-def func(x: int):
-   83 |+def func(x: int) -> Union[str | int]:
+   83 |+def func(x: int) -> Union[str | int | None]:
 83 84 |     match x:
 84 85 |         case [1, 2, 3]:
 85 86 |             return 1
@@ -446,17 +446,22 @@ auto_return_type.py:137:5: ANN201 [*] Missing return type annotation for public
 138 |     try:
 139 |         return 1
     |
-    = help: Add return type annotation: `int`
+    = help: Add return type annotation: `Optional[int]`
 
 ℹ Unsafe fix
-134 134 |         return 2
+    1   |+from typing import Optional
-135 135 | 
+1   2   | def func():
-136 136 | 
+2   3   |     return 1
+3   4   | 
+--------------------------------------------------------------------------------
+134 135 |         return 2
+135 136 | 
+136 137 | 
 137     |-def func(x: int):
-    137 |+def func(x: int) -> int:
+    138 |+def func(x: int) -> Optional[int]:
-138 138 |     try:
+138 139 |     try:
-139 139 |         return 1
+139 140 |         return 1
-140 140 |     except:
+140 141 |     except:
 
 auto_return_type.py:146:5: ANN201 [*] Missing return type annotation for public function `func`
     |
@@ -755,4 +760,117 @@ auto_return_type.py:262:5: ANN201 [*] Missing return type annotation for public
 264 264 |         if x > 0:
 265 265 |             return 1
 
+auto_return_type.py:269:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+269 | def func(x: int):
+    |     ^^^^ ANN201
+270 |     if x > 5:
+271 |         raise ValueError
+    |
+    = help: Add return type annotation: `None`
+
+ℹ Unsafe fix
+266 266 |     raise ValueError
+267 267 | 
+268 268 | 
+269     |-def func(x: int):
+    269 |+def func(x: int) -> None:
+270 270 |     if x > 5:
+271 271 |         raise ValueError
+272 272 |     else:
+
+auto_return_type.py:276:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+276 | def func(x: int):
+    |     ^^^^ ANN201
+277 |     if x > 5:
+278 |         raise ValueError
+    |
+    = help: Add return type annotation: `None`
+
+ℹ Unsafe fix
+273 273 |         pass
+274 274 | 
+275 275 | 
+276     |-def func(x: int):
+    276 |+def func(x: int) -> None:
+277 277 |     if x > 5:
+278 278 |         raise ValueError
+279 279 |     elif x > 10:
+
+auto_return_type.py:283:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+283 | def func(x: int):
+    |     ^^^^ ANN201
+284 |     if x > 5:
+285 |         raise ValueError
+    |
+    = help: Add return type annotation: `Optional[int]`
+
+ℹ Unsafe fix
+214 214 |         return 1
+215 215 | 
+216 216 | 
+217     |-from typing import overload
+    217 |+from typing import overload, Optional
+218 218 | 
+219 219 | 
+220 220 | @overload
+--------------------------------------------------------------------------------
+280 280 |         pass
+281 281 | 
+282 282 | 
+283     |-def func(x: int):
+    283 |+def func(x: int) -> Optional[int]:
+284 284 |     if x > 5:
+285 285 |         raise ValueError
+286 286 |     elif x > 10:
+
+auto_return_type.py:290:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+290 | def func():
+    |     ^^^^ ANN201
+291 |     try:
+292 |         return 5
+    |
+    = help: Add return type annotation: `int`
+
+ℹ Unsafe fix
+287 287 |         return 5
+288 288 | 
+289 289 | 
+290     |-def func():
+    290 |+def func() -> int:
+291 291 |     try:
+292 292 |         return 5
+293 293 |     except:
+
+auto_return_type.py:299:5: ANN201 [*] Missing return type annotation for public function `func`
+    |
+299 | def func(x: int):
+    |     ^^^^ ANN201
+300 |     match x:
+301 |         case [1, 2, 3]:
+    |
+    = help: Add return type annotation: `Union[str | int]`
+
+ℹ Unsafe fix
+214 214 |         return 1
+215 215 | 
+216 216 | 
+217     |-from typing import overload
+    217 |+from typing import overload, Union
+218 218 | 
+219 219 | 
+220 220 | @overload
+--------------------------------------------------------------------------------
+296 296 |     raise ValueError
+297 297 | 
+298 298 | 
+299     |-def func(x: int):
+    299 |+def func(x: int) -> Union[str | int]:
+300 300 |     match x:
+301 301 |         case [1, 2, 3]:
+302 302 |             return 1
+
 

crates/ruff_python_semantic/src/analyze/terminal.rs

@@ -2,14 +2,16 @@ use ruff_python_ast::{self as ast, ExceptHandler, Stmt};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Terminal {
-    /// There is no known terminal (e.g., an implicit return).
+    /// There is no known terminal.
     None,
+    /// There is an implicit return (e.g., a path that doesn't return).
+    Implicit,
     /// Every path through the function ends with a `raise` statement.
     Raise,
     /// No path through the function ends with a `return` statement.
     Return,
     /// Every path through the function ends with a `return` or `raise` statement.
-    Explicit,
+    RaiseOrReturn,
     /// At least one path through the function ends with a `return` statement.
     ConditionalReturn,
 }
@@ -20,6 +22,19 @@ impl Terminal {
         Self::from_body(&function.body)
     }
 
+    /// Returns `true` if the [`Terminal`] behavior includes at least one `return` path.
+    pub fn has_any_return(self) -> bool {
+        matches!(
+            self,
+            Self::Return | Self::RaiseOrReturn | Self::ConditionalReturn
+        )
+    }
+
+    /// Returns `true` if the [`Terminal`] behavior includes at least one implicit `return` path.
+    pub fn has_implicit_return(self) -> bool {
+        matches!(self, Self::None | Self::Implicit | Self::ConditionalReturn)
+    }
+
     /// Returns the [`Terminal`] behavior of the body, if it can be determined.
     fn from_body(stmts: &[Stmt]) -> Terminal {
         let mut terminal = Terminal::None;
@@ -32,10 +47,10 @@ impl Terminal {
                         continue;
                     }
 
-                    terminal = terminal.union(Self::from_body(body));
+                    terminal = terminal.and_then(Self::from_body(body));
 
                     if !sometimes_breaks(body) {
-                        terminal = terminal.union(Self::from_body(orelse));
+                        terminal = terminal.and_then(Self::from_body(orelse));
                     }
                 }
                 Stmt::If(ast::StmtIf {
@@ -43,7 +58,7 @@ impl Terminal {
                     elif_else_clauses,
                     ..
                 }) => {
-                    let branch_terminal = Terminal::combine(
+                    let branch_terminal = Terminal::branches(
                         std::iter::once(Self::from_body(body)).chain(
                             elif_else_clauses
                                 .iter()
@@ -55,96 +70,176 @@ impl Terminal {
                     // `else`)...
                     if elif_else_clauses.iter().any(|clause| clause.test.is_none()) {
                         // And all branches return, then the `if` statement returns.
-                        terminal = terminal.union(branch_terminal);
+                        terminal = terminal.and_then(branch_terminal);
-                    } else if branch_terminal.has_return() {
+                    } else if branch_terminal.has_any_return() {
                         // Otherwise, if any branch returns, we know this can't be a
                         // non-returning function.
-                        terminal = terminal.union(Terminal::ConditionalReturn);
+                        terminal = terminal.and_then(Terminal::ConditionalReturn);
                     }
                 }
                 Stmt::Match(ast::StmtMatch { cases, .. }) => {
-                    // Note: we assume the `match` is exhaustive.
+                    let branch_terminal = terminal.and_then(Terminal::branches(
-                    terminal = terminal.union(Terminal::combine(
                         cases.iter().map(|case| Self::from_body(&case.body)),
                     ));
+
+                    // If the `match` is known to be exhaustive (by way of including a wildcard
+                    // pattern)...
+                    if cases.iter().any(is_wildcard) {
+                        // And all branches return, then the `match` statement returns.
+                        terminal = terminal.and_then(branch_terminal);
+                    } else {
+                        // Otherwise, if any branch returns, we know this can't be a
+                        // non-returning function.
+                        if branch_terminal.has_any_return() {
+                            terminal = terminal.and_then(Terminal::ConditionalReturn);
+                        }
+                    }
                 }
                 Stmt::Try(ast::StmtTry {
+                    body,
                     handlers,
                     orelse,
                     finalbody,
                     ..
                 }) => {
-                    // If the `finally` block returns, the `try` block must also return.
+                    // If the body returns, then this can't be a non-returning function. We assume
-                    terminal = terminal.union(Self::from_body(finalbody));
+                    // that _any_ statement in the body could raise an exception, so we don't
+                    // consider the body to be exhaustive. In other words, we assume the exception
+                    // handlers exist for a reason.
+                    let body_terminal = Self::from_body(body);
+                    if body_terminal.has_any_return() {
+                        terminal = terminal.and_then(Terminal::ConditionalReturn);
+                    }
 
-                    // If the else block and all the handlers return, the `try` block must also
+                    // If the `finally` block returns, the `try` block must also return. (Similarly,
-                    // return.
+                    // if the `finally` block raises, the `try` block must also raise.)
-                    let branch_terminal =
+                    terminal = terminal.and_then(Self::from_body(finalbody));
-                        Terminal::combine(std::iter::once(Self::from_body(orelse)).chain(
+
-                            handlers.iter().map(|handler| {
+                    let branch_terminal = Terminal::branches(handlers.iter().map(|handler| {
                         let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
                             body,
                             ..
                         }) = handler;
                         Self::from_body(body)
-                            }),
+                    }));
-                        ));
 
                     if orelse.is_empty() {
-                        // If there's no `else`, we may fall through.
+                        // If there's no `else`, we may fall through, so only mark that this can't
-                        if branch_terminal.has_return() {
+                        // be a non-returning function if any of the branches return.
-                            terminal = terminal.union(Terminal::ConditionalReturn);
+                        if branch_terminal.has_any_return() {
+                            terminal = terminal.and_then(Terminal::ConditionalReturn);
                         }
                     } else {
-                        // If there's an `else`, we may not fall through.
+                        // If there's an `else`, we won't fall through. If all the handlers and
-                        terminal = terminal.union(branch_terminal);
+                        // the `else` block return,, the `try` block also returns.
+                        terminal =
+                            terminal.and_then(branch_terminal.branch(Terminal::from_body(orelse)));
                     }
                 }
                 Stmt::With(ast::StmtWith { body, .. }) => {
-                    terminal = terminal.union(Self::from_body(body));
+                    terminal = terminal.and_then(Self::from_body(body));
                 }
                 Stmt::Return(_) => {
-                    terminal = terminal.union(Terminal::Explicit);
+                    terminal = terminal.and_then(Terminal::RaiseOrReturn);
                 }
                 Stmt::Raise(_) => {
-                    terminal = terminal.union(Terminal::Raise);
+                    terminal = terminal.and_then(Terminal::Raise);
                 }
                 _ => {}
             }
         }
-        terminal
+
+        match terminal {
+            Terminal::None => Terminal::Implicit,
+            _ => terminal,
+        }
     }
 
-    /// Returns `true` if the [`Terminal`] behavior includes at least one `return` path.
+    /// Combine two [`Terminal`] operators, with one appearing after the other.
-    fn has_return(self) -> bool {
+    fn and_then(self, other: Self) -> Self {
-        matches!(
-            self,
-            Self::Return | Self::Explicit | Self::ConditionalReturn
-        )
-    }
-
-    /// Combine two [`Terminal`] operators.
-    fn union(self, other: Self) -> Self {
         match (self, other) {
+            // If one of the operators is `None`, the result is the other operator.
             (Self::None, other) => other,
             (other, Self::None) => other,
-            (Self::Explicit, _) => Self::Explicit,
+
-            (_, Self::Explicit) => Self::Explicit,
+            // If one of the operators is `Implicit`, the result is the other operator.
+            (Self::Implicit, other) => other,
+            (other, Self::Implicit) => other,
+
+            // If both operators are conditional returns, the result is a conditional return.
             (Self::ConditionalReturn, Self::ConditionalReturn) => Self::ConditionalReturn,
-            (Self::Raise, Self::ConditionalReturn) => Self::Explicit,
+
-            (Self::ConditionalReturn, Self::Raise) => Self::Explicit,
+            // If one of the operators is `Raise`, then the function ends with an explicit `raise`
+            // or `return` statement.
+            (Self::Raise, Self::ConditionalReturn) => Self::RaiseOrReturn,
+            (Self::ConditionalReturn, Self::Raise) => Self::RaiseOrReturn,
+
+            // If one of the operators is `Return`, then the function returns.
             (Self::Return, Self::ConditionalReturn) => Self::Return,
             (Self::ConditionalReturn, Self::Return) => Self::Return,
+
+            // All paths through the function end with a `raise` statement.
             (Self::Raise, Self::Raise) => Self::Raise,
+
+            // All paths through the function end with a `return` statement.
             (Self::Return, Self::Return) => Self::Return,
-            (Self::Raise, Self::Return) => Self::Explicit,
+
-            (Self::Return, Self::Raise) => Self::Explicit,
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::Raise, Self::Return) => Self::RaiseOrReturn,
+
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::Return, Self::Raise) => Self::RaiseOrReturn,
+
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::RaiseOrReturn, _) => Self::RaiseOrReturn,
+            (_, Self::RaiseOrReturn) => Self::RaiseOrReturn,
+        }
+    }
+
+    /// Combine two [`Terminal`] operators from different branches.
+    fn branch(self, other: Self) -> Self {
+        match (self, other) {
+            // If one of the operators is `None`, the result is the other operator.
+            (Self::None, other) => other,
+            (other, Self::None) => other,
+
+            // If one of the operators is `Implicit`, the other operator should be downgraded.
+            (Self::Implicit, Self::Implicit) => Self::Implicit,
+            (Self::Implicit, Self::Raise) => Self::Implicit,
+            (Self::Raise, Self::Implicit) => Self::Implicit,
+            (Self::Implicit, Self::Return) => Self::ConditionalReturn,
+            (Self::Return, Self::Implicit) => Self::ConditionalReturn,
+            (Self::Implicit, Self::RaiseOrReturn) => Self::ConditionalReturn,
+            (Self::RaiseOrReturn, Self::Implicit) => Self::ConditionalReturn,
+            (Self::Implicit, Self::ConditionalReturn) => Self::ConditionalReturn,
+            (Self::ConditionalReturn, Self::Implicit) => Self::ConditionalReturn,
+
+            // If both operators are conditional returns, the result is a conditional return.
+            (Self::ConditionalReturn, Self::ConditionalReturn) => Self::ConditionalReturn,
+
+            (Self::Raise, Self::ConditionalReturn) => Self::RaiseOrReturn,
+            (Self::ConditionalReturn, Self::Raise) => Self::RaiseOrReturn,
+
+            (Self::Return, Self::ConditionalReturn) => Self::Return,
+            (Self::ConditionalReturn, Self::Return) => Self::Return,
+
+            // All paths through the function end with a `raise` statement.
+            (Self::Raise, Self::Raise) => Self::Raise,
+            // All paths through the function end with a `return` statement.
+            (Self::Return, Self::Return) => Self::Return,
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::Raise, Self::Return) => Self::RaiseOrReturn,
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::Return, Self::Raise) => Self::RaiseOrReturn,
+            // All paths through the function end with a `return` or `raise` statement.
+            (Self::RaiseOrReturn, _) => Self::RaiseOrReturn,
+            (_, Self::RaiseOrReturn) => Self::RaiseOrReturn,
         }
     }
 
     /// Combine a series of [`Terminal`] operators.
-    fn combine(iter: impl Iterator<Item = Terminal>) -> Terminal {
+    fn branches(iter: impl Iterator<Item = Terminal>) -> Terminal {
-        iter.fold(Terminal::None, Self::union)
+        iter.fold(Terminal::None, Terminal::branch)
     }
 }
 
@@ -153,7 +248,7 @@ fn sometimes_breaks(stmts: &[Stmt]) -> bool {
     for stmt in stmts {
         match stmt {
             Stmt::For(ast::StmtFor { body, orelse, .. }) => {
-                if Terminal::from_body(body).has_return() {
+                if Terminal::from_body(body).has_any_return() {
                     return false;
                 }
                 if sometimes_breaks(orelse) {
@@ -161,7 +256,7 @@ fn sometimes_breaks(stmts: &[Stmt]) -> bool {
                 }
             }
             Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
-                if Terminal::from_body(body).has_return() {
+                if Terminal::from_body(body).has_any_return() {
                     return false;
                 }
                 if sometimes_breaks(orelse) {
@@ -232,3 +327,24 @@ fn always_breaks(stmts: &[Stmt]) -> bool {
     }
     false
 }
+
+/// Returns true if the [`MatchCase`] is a wildcard pattern.
+fn is_wildcard(pattern: &ast::MatchCase) -> bool {
+    /// Returns true if the [`Pattern`] is a wildcard pattern.
+    fn is_wildcard_pattern(pattern: &ast::Pattern) -> bool {
+        match pattern {
+            ast::Pattern::MatchValue(_)
+            | ast::Pattern::MatchSingleton(_)
+            | ast::Pattern::MatchSequence(_)
+            | ast::Pattern::MatchMapping(_)
+            | ast::Pattern::MatchClass(_)
+            | ast::Pattern::MatchStar(_) => false,
+            ast::Pattern::MatchAs(ast::PatternMatchAs { pattern, .. }) => pattern.is_none(),
+            ast::Pattern::MatchOr(ast::PatternMatchOr { patterns, .. }) => {
+                patterns.iter().all(is_wildcard_pattern)
+            }
+        }
+    }
+
+    pattern.guard.is_none() && is_wildcard_pattern(&pattern.pattern)
+}