Merge 0c87832e8a into 665f68036c

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

@@ -302,3 +302,26 @@ def func(x: int):
             return 1
         case y:
             return "foo"
+
+
+# Test cases for NotImplementedError - should not get NoReturn auto-fix
+def func():
+    raise NotImplementedError
+
+
+class Class:
+    @staticmethod
+    def func():
+        raise NotImplementedError
+
+    @classmethod
+    def method(cls):
+        raise NotImplementedError
+
+    def instance_method(self):
+        raise NotImplementedError
+
+
+# Test case: function that raises other exceptions should still get NoReturn
+def func():
+    raise ValueError

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

@@ -48,7 +48,10 @@ pub(crate) fn is_overload_impl(
 }
 
 /// Given a function, guess its return type.
-pub(crate) fn auto_return_type(function: &ast::StmtFunctionDef) -> Option<AutoPythonType> {
+pub(crate) fn auto_return_type(
+    function: &ast::StmtFunctionDef,
+    semantic: &SemanticModel,
+) -> Option<AutoPythonType> {
     // Collect all the `return` statements.
     let returns = {
         let mut visitor = ReturnStatementVisitor::default();
@@ -63,9 +66,16 @@ pub(crate) fn auto_return_type(function: &ast::StmtFunctionDef) -> Option<AutoPy
     };
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function);
+    let terminal = Terminal::from_function(function, semantic);
 
-    // If every control flow path raises an exception, return `NoReturn`.
+    // If every control flow path raises NotImplementedError, don't suggest NoReturn
+    // since these are abstract methods that should have the actual return type.
+    if terminal == Terminal::RaiseNotImplemented {
+        return None;
+    }
+
+    // If every control flow path raises an exception (other than NotImplementedError),
+    // suggest NoReturn.
     if terminal == Terminal::Raise {
         return Some(AutoPythonType::Never);
     }

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

@@ -762,7 +762,7 @@ pub(crate) fn definition(
                 let return_type = if is_stub_function(function, checker) {
                     None
                 } else {
-                    auto_return_type(function)
+                    auto_return_type(function, checker.semantic())
                         .and_then(|return_type| {
                             return_type.into_expression(checker, function.parameters.start())
                         })
@@ -789,7 +789,7 @@ pub(crate) fn definition(
                 let return_type = if is_stub_function(function, checker) {
                     None
                 } else {
-                    auto_return_type(function)
+                    auto_return_type(function, checker.semantic())
                         .and_then(|return_type| {
                             return_type.into_expression(checker, function.parameters.start())
                         })
@@ -875,7 +875,7 @@ pub(crate) fn definition(
                         let return_type = if is_stub_function(function, checker) {
                             None
                         } else {
-                            auto_return_type(function)
+                            auto_return_type(function, checker.semantic())
                                 .and_then(|return_type| {
                                     return_type
                                         .into_expression(checker, function.parameters.start())
@@ -910,7 +910,7 @@ pub(crate) fn definition(
                         let return_type = if is_stub_function(function, checker) {
                             None
                         } else {
-                            auto_return_type(function)
+                            auto_return_type(function, checker.semantic())
                                 .and_then(|return_type| {
                                     return_type
                                         .into_expression(checker, function.parameters.start())

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

@@ -775,3 +775,71 @@ help: Add return type annotation: `str | int`
 301 |         case [1, 2, 3]:
 302 |             return 1
 note: This is an unsafe fix and may change runtime behavior
+
+ANN201 Missing return type annotation for public function `func`
+   --> auto_return_type.py:308:5
+    |
+307 | # Test cases for NotImplementedError - should not get NoReturn auto-fix
+308 | def func():
+    |     ^^^^
+309 |     raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN205 Missing return type annotation for staticmethod `func`
+   --> auto_return_type.py:314:9
+    |
+312 | class Class:
+313 |     @staticmethod
+314 |     def func():
+    |         ^^^^
+315 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN206 Missing return type annotation for classmethod `method`
+   --> auto_return_type.py:318:9
+    |
+317 |     @classmethod
+318 |     def method(cls):
+    |         ^^^^^^
+319 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN201 Missing return type annotation for public function `instance_method`
+   --> auto_return_type.py:321:9
+    |
+319 |         raise NotImplementedError
+320 |
+321 |     def instance_method(self):
+    |         ^^^^^^^^^^^^^^^
+322 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN201 [*] Missing return type annotation for public function `func`
+   --> auto_return_type.py:326:5
+    |
+325 | # Test case: function that raises other exceptions should still get NoReturn
+326 | def func():
+    |     ^^^^
+327 |     raise ValueError
+    |
+help: Add return type annotation: `Never`
+214 |         return 1
+215 | 
+216 | 
+    - from typing import overload
+217 + from typing import overload, Never
+218 | 
+219 | 
+220 | @overload
+--------------------------------------------------------------------------------
+323 | 
+324 | 
+325 | # Test case: function that raises other exceptions should still get NoReturn
+    - def func():
+326 + def func() -> Never:
+327 |     raise ValueError
+note: This is an unsafe fix and may change runtime behavior

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

@@ -879,3 +879,71 @@ help: Add return type annotation: `Union[str, int]`
 301 |         case [1, 2, 3]:
 302 |             return 1
 note: This is an unsafe fix and may change runtime behavior
+
+ANN201 Missing return type annotation for public function `func`
+   --> auto_return_type.py:308:5
+    |
+307 | # Test cases for NotImplementedError - should not get NoReturn auto-fix
+308 | def func():
+    |     ^^^^
+309 |     raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN205 Missing return type annotation for staticmethod `func`
+   --> auto_return_type.py:314:9
+    |
+312 | class Class:
+313 |     @staticmethod
+314 |     def func():
+    |         ^^^^
+315 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN206 Missing return type annotation for classmethod `method`
+   --> auto_return_type.py:318:9
+    |
+317 |     @classmethod
+318 |     def method(cls):
+    |         ^^^^^^
+319 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN201 Missing return type annotation for public function `instance_method`
+   --> auto_return_type.py:321:9
+    |
+319 |         raise NotImplementedError
+320 |
+321 |     def instance_method(self):
+    |         ^^^^^^^^^^^^^^^
+322 |         raise NotImplementedError
+    |
+help: Add return type annotation
+
+ANN201 [*] Missing return type annotation for public function `func`
+   --> auto_return_type.py:326:5
+    |
+325 | # Test case: function that raises other exceptions should still get NoReturn
+326 | def func():
+    |     ^^^^
+327 |     raise ValueError
+    |
+help: Add return type annotation: `NoReturn`
+214 |         return 1
+215 | 
+216 | 
+    - from typing import overload
+217 + from typing import overload, NoReturn
+218 | 
+219 | 
+220 | @overload
+--------------------------------------------------------------------------------
+323 | 
+324 | 
+325 | # Test case: function that raises other exceptions should still get NoReturn
+    - def func():
+326 + def func() -> NoReturn:
+327 |     raise ValueError
+note: This is an unsafe fix and may change runtime behavior

crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs

@@ -60,7 +60,7 @@ pub(crate) fn invalid_bool_return(checker: &Checker, function_def: &ast::StmtFun
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs

@@ -60,7 +60,7 @@ pub(crate) fn invalid_bytes_return(checker: &Checker, function_def: &ast::StmtFu
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_linter/src/rules/pylint/rules/invalid_hash_return.rs

@@ -64,7 +64,7 @@ pub(crate) fn invalid_hash_return(checker: &Checker, function_def: &ast::StmtFun
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_linter/src/rules/pylint/rules/invalid_index_return.rs

@@ -66,7 +66,7 @@ pub(crate) fn invalid_index_return(checker: &Checker, function_def: &ast::StmtFu
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_linter/src/rules/pylint/rules/invalid_length_return.rs

@@ -65,7 +65,7 @@ pub(crate) fn invalid_length_return(checker: &Checker, function_def: &ast::StmtF
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_linter/src/rules/pylint/rules/invalid_str_return.rs

@@ -60,7 +60,7 @@ pub(crate) fn invalid_str_return(checker: &Checker, function_def: &ast::StmtFunc
     }
 
     // Determine the terminal behavior (i.e., implicit return, no return, etc.).
-    let terminal = Terminal::from_function(function_def);
+    let terminal = Terminal::from_function(function_def, checker.semantic());
 
     // If every control flow path raises an exception, ignore the function.
     if terminal == Terminal::Raise {

crates/ruff_python_semantic/src/analyze/terminal.rs

@@ -1,5 +1,8 @@
+use ruff_python_ast::helpers::map_callable;
 use ruff_python_ast::{self as ast, ExceptHandler, Stmt};
 
+use crate::model::SemanticModel;
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Terminal {
     /// There is no known terminal.
@@ -8,6 +11,8 @@ pub enum Terminal {
     Implicit,
     /// Every path through the function ends with a `raise` statement.
     Raise,
+    /// Every path through the function ends with a `raise NotImplementedError` statement.
+    RaiseNotImplemented,
     /// No path through the function ends with a `return` statement.
     Return,
     /// Every path through the function ends with a `return` or `raise` statement.
@@ -18,8 +23,8 @@ pub enum Terminal {
 
 impl Terminal {
     /// Returns the [`Terminal`] behavior of the function, if it can be determined.
-    pub fn from_function(function: &ast::StmtFunctionDef) -> Terminal {
+    pub fn from_function(function: &ast::StmtFunctionDef, semantic: &SemanticModel) -> Terminal {
-        Self::from_body(&function.body)
+        Self::from_body(&function.body, Some(semantic))
     }
 
     /// Returns `true` if the [`Terminal`] behavior includes at least one `return` path.
@@ -36,7 +41,7 @@ impl Terminal {
     }
 
     /// Returns the [`Terminal`] behavior of the body, if it can be determined.
-    fn from_body(stmts: &[Stmt]) -> Terminal {
+    fn from_body(stmts: &[Stmt], semantic: Option<&SemanticModel>) -> Terminal {
         let mut terminal = Terminal::None;
 
         for stmt in stmts {
@@ -47,10 +52,10 @@ impl Terminal {
                         continue;
                     }
 
-                    terminal = terminal.and_then(Self::from_body(body));
+                    terminal = terminal.and_then(Self::from_body(body, semantic));
 
                     if !sometimes_breaks(body) {
-                        terminal = terminal.and_then(Self::from_body(orelse));
+                        terminal = terminal.and_then(Self::from_body(orelse, semantic));
                     }
                 }
                 Stmt::If(ast::StmtIf {
@@ -59,10 +64,10 @@ impl Terminal {
                     ..
                 }) => {
                     let branch_terminal = Terminal::branches(
-                        std::iter::once(Self::from_body(body)).chain(
+                        std::iter::once(Self::from_body(body, semantic)).chain(
                             elif_else_clauses
                                 .iter()
-                                .map(|clause| Self::from_body(&clause.body)),
+                                .map(|clause| Self::from_body(&clause.body, semantic)),
                         ),
                     );
 
@@ -79,7 +84,9 @@ impl Terminal {
                 }
                 Stmt::Match(ast::StmtMatch { cases, .. }) => {
                     let branch_terminal = terminal.and_then(Terminal::branches(
-                        cases.iter().map(|case| Self::from_body(&case.body)),
+                        cases
+                            .iter()
+                            .map(|case| Self::from_body(&case.body, semantic)),
                     ));
 
                     // If the `match` is known to be exhaustive (by way of including a wildcard
@@ -106,21 +113,21 @@ impl Terminal {
                     // 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);
+                    let body_terminal = Self::from_body(body, semantic);
                     if body_terminal.has_any_return() {
                         terminal = terminal.and_then(Terminal::ConditionalReturn);
                     }
 
                     // If the `finally` block returns, the `try` block must also return. (Similarly,
                     // if the `finally` block raises, the `try` block must also raise.)
-                    terminal = terminal.and_then(Self::from_body(finalbody));
+                    terminal = terminal.and_then(Self::from_body(finalbody, semantic));
 
                     let branch_terminal = Terminal::branches(handlers.iter().map(|handler| {
                         let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
                             body,
                             ..
                         }) = handler;
-                        Self::from_body(body)
+                        Self::from_body(body, semantic)
                     }));
 
                     if orelse.is_empty() {
@@ -132,18 +139,35 @@ impl Terminal {
                     } else {
                         // If there's an `else`, we won't fall through. If all the handlers and
                         // the `else` block return,, the `try` block also returns.
-                        terminal =
+                        terminal = terminal.and_then(
-                            terminal.and_then(branch_terminal.branch(Terminal::from_body(orelse)));
+                            branch_terminal.branch(Terminal::from_body(orelse, semantic)),
+                        );
                     }
                 }
                 Stmt::With(ast::StmtWith { body, .. }) => {
-                    terminal = terminal.and_then(Self::from_body(body));
+                    terminal = terminal.and_then(Self::from_body(body, semantic));
                 }
                 Stmt::Return(_) => {
                     terminal = terminal.and_then(Terminal::RaiseOrReturn);
                 }
-                Stmt::Raise(_) => {
+                Stmt::Raise(raise) => {
-                    terminal = terminal.and_then(Terminal::Raise);
+                    // Check if this raise is NotImplementedError (only if semantic model is available)
+                    let is_not_implemented = semantic
+                        .and_then(|sem| {
+                            raise.exc.as_ref().and_then(|exc| {
+                                sem.resolve_builtin_symbol(map_callable(exc))
+                                    .filter(|name| {
+                                        matches!(*name, "NotImplementedError" | "NotImplemented")
+                                    })
+                            })
+                        })
+                        .is_some();
+
+                    if is_not_implemented {
+                        terminal = terminal.and_then(Terminal::RaiseNotImplemented);
+                    } else {
+                        terminal = terminal.and_then(Terminal::Raise);
+                    }
                 }
                 _ => {}
             }
@@ -174,21 +198,32 @@ impl Terminal {
             (Self::Raise, Self::ConditionalReturn) => Self::RaiseOrReturn,
             (Self::ConditionalReturn, Self::Raise) => Self::RaiseOrReturn,
 
+            // If one of the operators is `RaiseNotImplemented`, treat it like `Raise` for combinations
+            (Self::RaiseNotImplemented, Self::ConditionalReturn) => Self::RaiseOrReturn,
+            (Self::ConditionalReturn, Self::RaiseNotImplemented) => 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 `raise NotImplementedError` statement.
+            (Self::RaiseNotImplemented, Self::RaiseNotImplemented) => Self::RaiseNotImplemented,
+            // Mixed raises: if one is NotImplementedError and one is regular Raise, result is Raise
+            (Self::RaiseNotImplemented, Self::Raise) => Self::Raise,
+            (Self::Raise, Self::RaiseNotImplemented) => 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,
+            (Self::RaiseNotImplemented, Self::Return) => Self::RaiseOrReturn,
 
             // All paths through the function end with a `return` or `raise` statement.
             (Self::Return, Self::Raise) => Self::RaiseOrReturn,
+            (Self::Return, Self::RaiseNotImplemented) => Self::RaiseOrReturn,
 
             // All paths through the function end with a `return` or `raise` statement.
             (Self::RaiseOrReturn, _) => Self::RaiseOrReturn,
@@ -207,6 +242,8 @@ impl Terminal {
             (Self::Implicit, Self::Implicit) => Self::Implicit,
             (Self::Implicit, Self::Raise) => Self::Implicit,
             (Self::Raise, Self::Implicit) => Self::Implicit,
+            (Self::Implicit, Self::RaiseNotImplemented) => Self::Implicit,
+            (Self::RaiseNotImplemented, Self::Implicit) => Self::Implicit,
             (Self::Implicit, Self::Return) => Self::ConditionalReturn,
             (Self::Return, Self::Implicit) => Self::ConditionalReturn,
             (Self::Implicit, Self::RaiseOrReturn) => Self::ConditionalReturn,
@@ -219,18 +256,27 @@ impl Terminal {
 
             (Self::Raise, Self::ConditionalReturn) => Self::RaiseOrReturn,
             (Self::ConditionalReturn, Self::Raise) => Self::RaiseOrReturn,
+            (Self::RaiseNotImplemented, Self::ConditionalReturn) => Self::RaiseOrReturn,
+            (Self::ConditionalReturn, Self::RaiseNotImplemented) => 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 `raise NotImplementedError` statement.
+            (Self::RaiseNotImplemented, Self::RaiseNotImplemented) => Self::RaiseNotImplemented,
+            // Mixed raises: if one is NotImplementedError and one is regular Raise, result is Raise
+            (Self::RaiseNotImplemented, Self::Raise) => Self::Raise,
+            (Self::Raise, Self::RaiseNotImplemented) => 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,
+            (Self::RaiseNotImplemented, Self::Return) => Self::RaiseOrReturn,
             // All paths through the function end with a `return` or `raise` statement.
             (Self::Return, Self::Raise) => Self::RaiseOrReturn,
+            (Self::Return, Self::RaiseNotImplemented) => Self::RaiseOrReturn,
             // All paths through the function end with a `return` or `raise` statement.
             (Self::RaiseOrReturn, _) => Self::RaiseOrReturn,
             (_, Self::RaiseOrReturn) => Self::RaiseOrReturn,
@@ -248,7 +294,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_any_return() {
+                if Terminal::from_body(body, None).has_any_return() {
                     return false;
                 }
                 if sometimes_breaks(orelse) {
@@ -256,7 +302,7 @@ fn sometimes_breaks(stmts: &[Stmt]) -> bool {
                 }
             }
             Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
-                if Terminal::from_body(body).has_any_return() {
+                if Terminal::from_body(body, None).has_any_return() {
                     return false;
                 }
                 if sometimes_breaks(orelse) {