fix-18886

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

@@ -2,7 +2,7 @@ use itertools::Itertools;
 use rustc_hash::FxHashSet;
 
 use ruff_python_ast::helpers::{
-    ReturnStatementVisitor, pep_604_union, typing_optional, typing_union,
+    ReturnStatementVisitor, map_callable, pep_604_union, typing_optional, typing_union,
 };
 use ruff_python_ast::name::Name;
 use ruff_python_ast::visitor::Visitor;
@@ -47,8 +47,95 @@ pub(crate) fn is_overload_impl(
     }
 }
 
+/// Returns `true` if all raise statements in the function body are `NotImplementedError`.
+fn only_raises_not_implemented_error(
+    function: &ast::StmtFunctionDef,
+    semantic: &SemanticModel,
+) -> bool {
+    use ruff_python_ast::statement_visitor::{StatementVisitor, walk_body};
+
+    struct NotImplementedErrorVisitor<'a> {
+        raises: Vec<&'a ast::StmtRaise>,
+    }
+
+    impl<'a> StatementVisitor<'a> for NotImplementedErrorVisitor<'a> {
+        fn visit_stmt(&mut self, stmt: &'a ast::Stmt) {
+            match stmt {
+                ast::Stmt::Raise(raise) => {
+                    self.raises.push(raise);
+                }
+                ast::Stmt::ClassDef(_) | ast::Stmt::FunctionDef(_) => {
+                    // Don't recurse into nested classes/functions
+                }
+                ast::Stmt::If(ast::StmtIf {
+                    body,
+                    elif_else_clauses,
+                    ..
+                }) => {
+                    walk_body(self, body);
+                    for clause in elif_else_clauses {
+                        walk_body(self, &clause.body);
+                    }
+                }
+                ast::Stmt::For(ast::StmtFor { body, orelse, .. })
+                | ast::Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
+                    walk_body(self, body);
+                    walk_body(self, orelse);
+                }
+                ast::Stmt::With(ast::StmtWith { body, .. }) => {
+                    walk_body(self, body);
+                }
+                ast::Stmt::Match(ast::StmtMatch { cases, .. }) => {
+                    for case in cases {
+                        walk_body(self, &case.body);
+                    }
+                }
+                ast::Stmt::Try(ast::StmtTry {
+                    body,
+                    handlers,
+                    orelse,
+                    finalbody,
+                    ..
+                }) => {
+                    walk_body(self, body);
+                    for handler in handlers {
+                        let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
+                            body,
+                            ..
+                        }) = handler;
+                        walk_body(self, body);
+                    }
+                    walk_body(self, orelse);
+                    walk_body(self, finalbody);
+                }
+                _ => {}
+            }
+        }
+    }
+
+    let mut visitor = NotImplementedErrorVisitor { raises: Vec::new() };
+    walk_body(&mut visitor, &function.body);
+
+    // If there are no raises, return false
+    if visitor.raises.is_empty() {
+        return false;
+    }
+
+    // Check if all raises are NotImplementedError
+    visitor.raises.iter().all(|raise| {
+        raise.exc.as_ref().is_some_and(|exc| {
+            semantic
+                .resolve_builtin_symbol(map_callable(exc))
+                .is_some_and(|name| matches!(name, "NotImplementedError" | "NotImplemented"))
+        })
+    })
+}
+
 /// 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();
@@ -65,8 +152,13 @@ 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);
 
-    // If every control flow path raises an exception, return `NoReturn`.
+    // If every control flow path raises an exception, check if it's NotImplementedError.
+    // If all raises are NotImplementedError, don't suggest NoReturn since these are
+    // abstract methods that should have the actual return type.
     if terminal == Terminal::Raise {
+        if only_raises_not_implemented_error(function, semantic) {
+            return None;
+        }
         return Some(AutoPythonType::Never);
     }
 

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

@@ -759,7 +759,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())
                         })
@@ -786,7 +786,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())
                         })
@@ -872,7 +872,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())
@@ -907,7 +907,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