Allow `NoReturn`-like functions for `__str__`, `__len__`, etc. (#11017)

## Summary

If the method always raises, we shouldn't raise a diagnostic for
"returning a value of the wrong type".

Closes https://github.com/astral-sh/ruff/issues/11016.
This commit is contained in:
Charlie Marsh 2024-04-18 18:55:15 -04:00 committed by GitHub
parent e751b4ea82
commit 33529c049e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 107 additions and 61 deletions

View File

@ -21,12 +21,6 @@ class BytesNoReturn:
print("ruff") # [invalid-bytes-return]
class BytesWrongRaise:
def __bytes__(self):
print("raise some error")
raise NotImplementedError # [invalid-bytes-return]
# TODO: Once Ruff has better type checking
def return_bytes():
return "some string"
@ -63,3 +57,9 @@ class Bytes4:
class Bytes5:
def __bytes__(self):
raise NotImplementedError
class Bytes6:
def __bytes__(self):
print("raise some error")
raise NotImplementedError

View File

@ -26,12 +26,6 @@ class LengthNegative:
return -42 # [invalid-length-return]
class LengthWrongRaise:
def __len__(self):
print("raise some error")
raise NotImplementedError # [invalid-length-return]
# TODO: Once Ruff has better type checking
def return_int():
return "3"
@ -68,3 +62,9 @@ class Length4:
class Length5:
def __len__(self):
raise NotImplementedError
class Length6:
def __len__(self):
print("raise some error")
raise NotImplementedError

View File

@ -47,3 +47,14 @@ class Str2:
class Str3:
def __str__(self): ...
class Str4:
def __str__(self):
raise RuntimeError("__str__ not allowed")
class Str5:
def __str__(self): # PLE0307 (returns None if x <= 0)
if x > 0:
raise RuntimeError("__str__ not allowed")

View File

@ -5,6 +5,7 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast};
use ruff_python_semantic::analyze::function_type::is_stub;
use ruff_python_semantic::analyze::terminal::Terminal;
use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;
@ -43,7 +44,7 @@ impl Violation for InvalidBoolReturnType {
}
}
/// E0307
/// PLE0304
pub(crate) fn invalid_bool_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
if function_def.name.as_str() != "__bool__" {
return;
@ -57,19 +58,29 @@ pub(crate) fn invalid_bool_return(checker: &mut Checker, function_def: &ast::Stm
return;
}
// Determine the terminal behavior (i.e., implicit return, no return, etc.).
let terminal = Terminal::from_function(function_def);
// If every control flow path raises an exception, ignore the function.
if terminal == Terminal::Raise {
return;
}
// If there are no return statements, add a diagnostic.
if terminal == Terminal::Implicit {
checker.diagnostics.push(Diagnostic::new(
InvalidBoolReturnType,
function_def.identifier(),
));
return;
}
let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(&function_def.body);
visitor.returns
};
if returns.is_empty() {
checker.diagnostics.push(Diagnostic::new(
InvalidBoolReturnType,
function_def.identifier(),
));
}
for stmt in returns {
if let Some(value) = stmt.value.as_deref() {
if !matches!(

View File

@ -5,6 +5,7 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast};
use ruff_python_semantic::analyze::function_type::is_stub;
use ruff_python_semantic::analyze::terminal::Terminal;
use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;
@ -43,7 +44,7 @@ impl Violation for InvalidBytesReturnType {
}
}
/// E0308
/// PLE0308
pub(crate) fn invalid_bytes_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
if function_def.name.as_str() != "__bytes__" {
return;
@ -57,19 +58,29 @@ pub(crate) fn invalid_bytes_return(checker: &mut Checker, function_def: &ast::St
return;
}
// Determine the terminal behavior (i.e., implicit return, no return, etc.).
let terminal = Terminal::from_function(function_def);
// If every control flow path raises an exception, ignore the function.
if terminal == Terminal::Raise {
return;
}
// If there are no return statements, add a diagnostic.
if terminal == Terminal::Implicit {
checker.diagnostics.push(Diagnostic::new(
InvalidBytesReturnType,
function_def.identifier(),
));
return;
}
let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(&function_def.body);
visitor.returns
};
if returns.is_empty() {
checker.diagnostics.push(Diagnostic::new(
InvalidBytesReturnType,
function_def.identifier(),
));
}
for stmt in returns {
if let Some(value) = stmt.value.as_deref() {
if !matches!(

View File

@ -5,6 +5,7 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::analyze::function_type::is_stub;
use ruff_python_semantic::analyze::terminal::Terminal;
use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;
@ -63,19 +64,29 @@ pub(crate) fn invalid_length_return(checker: &mut Checker, function_def: &ast::S
return;
}
// Determine the terminal behavior (i.e., implicit return, no return, etc.).
let terminal = Terminal::from_function(function_def);
// If every control flow path raises an exception, ignore the function.
if terminal == Terminal::Raise {
return;
}
// If there are no return statements, add a diagnostic.
if terminal == Terminal::Implicit {
checker.diagnostics.push(Diagnostic::new(
InvalidLengthReturnType,
function_def.identifier(),
));
return;
}
let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(&function_def.body);
visitor.returns
};
if returns.is_empty() {
checker.diagnostics.push(Diagnostic::new(
InvalidLengthReturnType,
function_def.identifier(),
));
}
for stmt in returns {
if let Some(value) = stmt.value.as_deref() {
if is_negative_integer(value)

View File

@ -5,6 +5,7 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast};
use ruff_python_semantic::analyze::function_type::is_stub;
use ruff_python_semantic::analyze::terminal::Terminal;
use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;
@ -57,19 +58,29 @@ pub(crate) fn invalid_str_return(checker: &mut Checker, function_def: &ast::Stmt
return;
}
// Determine the terminal behavior (i.e., implicit return, no return, etc.).
let terminal = Terminal::from_function(function_def);
// If every control flow path raises an exception, ignore the function.
if terminal == Terminal::Raise {
return;
}
// If there are no return statements, add a diagnostic.
if terminal == Terminal::Implicit {
checker.diagnostics.push(Diagnostic::new(
InvalidStrReturnType,
function_def.identifier(),
));
return;
}
let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(&function_def.body);
visitor.returns
};
if returns.is_empty() {
checker.diagnostics.push(Diagnostic::new(
InvalidStrReturnType,
function_def.identifier(),
));
}
for stmt in returns {
if let Some(value) = stmt.value.as_deref() {
if !matches!(

View File

@ -59,7 +59,7 @@ impl Violation for NonSlotAssignment {
}
}
/// E0237
/// PLE0237
pub(crate) fn non_slot_assignment(checker: &mut Checker, class_def: &ast::StmtClassDef) {
let semantic = checker.semantic();

View File

@ -40,12 +40,3 @@ invalid_return_type_length.py:26:16: PLE0303 `__len__` does not return a non-neg
26 | return -42 # [invalid-length-return]
| ^^^ PLE0303
|
invalid_return_type_length.py:30:9: PLE0303 `__len__` does not return a non-negative integer
|
29 | class LengthWrongRaise:
30 | def __len__(self):
| ^^^^^^^ PLE0303
31 | print("raise some error")
32 | raise NotImplementedError # [invalid-length-return]
|

View File

@ -32,3 +32,12 @@ invalid_return_type_str.py:21:16: PLE0307 `__str__` does not return `str`
21 | return False
| ^^^^^ PLE0307
|
invalid_return_type_str.py:58:9: PLE0307 `__str__` does not return `str`
|
57 | class Str5:
58 | def __str__(self): # PLE0307 (returns None if x <= 0)
| ^^^^^^^ PLE0307
59 | if x > 0:
60 | raise RuntimeError("__str__ not allowed")
|

View File

@ -32,12 +32,3 @@ invalid_return_type_bytes.py:20:9: PLE0308 `__bytes__` does not return `bytes`
| ^^^^^^^^^ PLE0308
21 | print("ruff") # [invalid-bytes-return]
|
invalid_return_type_bytes.py:25:9: PLE0308 `__bytes__` does not return `bytes`
|
24 | class BytesWrongRaise:
25 | def __bytes__(self):
| ^^^^^^^^^ PLE0308
26 | print("raise some error")
27 | raise NotImplementedError # [invalid-bytes-return]
|