Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 13 additions & 3 deletions crates/ruff_linter/src/rules/flake8_annotations/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 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, return `NoReturn`.
// If every control flow path raises an exception (other than NotImplementedError),
// suggest NoReturn.
if terminal == Terminal::Raise {
return Some(AutoPythonType::Never);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand All @@ -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())
})
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ 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 {
if terminal.is_always_raise() {
return;
}

Expand Down
Loading
Loading