diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py b/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py index 1fa9454ffd..b6cd1ea9a6 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py +++ b/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 diff --git a/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs b/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs index 1d7047da77..2a0992ac60 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs +++ b/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 { +pub(crate) fn auto_return_type( + function: &ast::StmtFunctionDef, + semantic: &SemanticModel, +) -> Option { // 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 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 diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap index 65f71dd345..41d76ce242 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap +++ b/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