mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 13:51:37 +00:00
[pylint
] Implement invalid-length-returned
(E0303
) (#10963)
Add pylint rule invalid-length-returned (PLE0303) See https://github.com/astral-sh/ruff/issues/970 for rules Test Plan: `cargo test` TBD: from the description: "Strictly speaking `bool` is a subclass of `int`, thus returning `True`/`False` is valid. To be consistent with other rules (e.g. [PLE0305](https://github.com/astral-sh/ruff/pull/10962) invalid-index-returned), ruff will raise, compared to pylint which will not raise."
This commit is contained in:
parent
b23414e3cc
commit
9f01ac3f87
8 changed files with 242 additions and 0 deletions
70
crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_length.py
vendored
Normal file
70
crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_length.py
vendored
Normal file
|
@ -0,0 +1,70 @@
|
||||||
|
# These testcases should raise errors
|
||||||
|
|
||||||
|
|
||||||
|
class Bool:
|
||||||
|
def __len__(self):
|
||||||
|
return True # [invalid-length-return]
|
||||||
|
|
||||||
|
|
||||||
|
class Float:
|
||||||
|
def __len__(self):
|
||||||
|
return 3.05 # [invalid-length-return]
|
||||||
|
|
||||||
|
|
||||||
|
class Str:
|
||||||
|
def __len__(self):
|
||||||
|
return "ruff" # [invalid-length-return]
|
||||||
|
|
||||||
|
|
||||||
|
class LengthNoReturn:
|
||||||
|
def __len__(self):
|
||||||
|
print("ruff") # [invalid-length-return]
|
||||||
|
|
||||||
|
|
||||||
|
class LengthNegative:
|
||||||
|
def __len__(self):
|
||||||
|
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"
|
||||||
|
|
||||||
|
|
||||||
|
class ComplexReturn:
|
||||||
|
def __len__(self):
|
||||||
|
return return_int() # [invalid-length-return]
|
||||||
|
|
||||||
|
|
||||||
|
# These testcases should NOT raise errors
|
||||||
|
|
||||||
|
|
||||||
|
class Length:
|
||||||
|
def __len__(self):
|
||||||
|
return 42
|
||||||
|
|
||||||
|
|
||||||
|
class Length2:
|
||||||
|
def __len__(self):
|
||||||
|
x = 42
|
||||||
|
return x
|
||||||
|
|
||||||
|
|
||||||
|
class Length3:
|
||||||
|
def __len__(self): ...
|
||||||
|
|
||||||
|
|
||||||
|
class Length4:
|
||||||
|
def __len__(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class Length5:
|
||||||
|
def __len__(self):
|
||||||
|
raise NotImplementedError
|
|
@ -97,6 +97,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::InvalidBoolReturnType) {
|
if checker.enabled(Rule::InvalidBoolReturnType) {
|
||||||
pylint::rules::invalid_bool_return(checker, name, body);
|
pylint::rules::invalid_bool_return(checker, name, body);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::InvalidLengthReturnType) {
|
||||||
|
pylint::rules::invalid_length_return(checker, function_def);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::InvalidBytesReturnType) {
|
if checker.enabled(Rule::InvalidBytesReturnType) {
|
||||||
pylint::rules::invalid_bytes_return(checker, function_def);
|
pylint::rules::invalid_bytes_return(checker, function_def);
|
||||||
}
|
}
|
||||||
|
|
|
@ -241,6 +241,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment),
|
(Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment),
|
||||||
(Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases),
|
(Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases),
|
||||||
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
|
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
|
||||||
|
(Pylint, "E0303") => (RuleGroup::Preview, rules::pylint::rules::InvalidLengthReturnType),
|
||||||
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
|
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
|
||||||
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
|
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
|
||||||
(Pylint, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType),
|
(Pylint, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType),
|
||||||
|
|
|
@ -77,6 +77,10 @@ mod tests {
|
||||||
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
|
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
|
||||||
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
|
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
|
||||||
#[test_case(Rule::InvalidBoolReturnType, Path::new("invalid_return_type_bool.py"))]
|
#[test_case(Rule::InvalidBoolReturnType, Path::new("invalid_return_type_bool.py"))]
|
||||||
|
#[test_case(
|
||||||
|
Rule::InvalidLengthReturnType,
|
||||||
|
Path::new("invalid_return_type_length.py")
|
||||||
|
)]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
Rule::InvalidBytesReturnType,
|
Rule::InvalidBytesReturnType,
|
||||||
Path::new("invalid_return_type_bytes.py")
|
Path::new("invalid_return_type_bytes.py")
|
||||||
|
|
|
@ -0,0 +1,110 @@
|
||||||
|
use ruff_diagnostics::{Diagnostic, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast::helpers::ReturnStatementVisitor;
|
||||||
|
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::type_inference::{NumberLike, PythonType, ResolvedPythonType};
|
||||||
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for `__len__` implementations that return values other than a non-negative
|
||||||
|
/// `integer`.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// The `__len__` method should return a non-negative `integer`. Returning a different
|
||||||
|
/// value may cause unexpected behavior.
|
||||||
|
///
|
||||||
|
/// Note: `bool` is a subclass of `int`, so it's technically valid for `__len__` to
|
||||||
|
/// return `True` or `False`. However, for consistency with other rules, Ruff will
|
||||||
|
/// still raise when `__len__` returns a `bool`.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// class Foo:
|
||||||
|
/// def __len__(self):
|
||||||
|
/// return "2"
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// class Foo:
|
||||||
|
/// def __len__(self):
|
||||||
|
/// return 2
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// ## References
|
||||||
|
/// - [Python documentation: The `__len__` method](https://docs.python.org/3/reference/datamodel.html#object.__len__)
|
||||||
|
#[violation]
|
||||||
|
pub struct InvalidLengthReturnType;
|
||||||
|
|
||||||
|
impl Violation for InvalidLengthReturnType {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("`__len__` does not return a non-negative integer")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// E0303
|
||||||
|
pub(crate) fn invalid_length_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
|
||||||
|
if function_def.name.as_str() != "__len__" {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if !checker.semantic().current_scope().kind.is_class() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if is_stub(function_def, checker.semantic()) {
|
||||||
|
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)
|
||||||
|
|| !matches!(
|
||||||
|
ResolvedPythonType::from(value),
|
||||||
|
ResolvedPythonType::Unknown
|
||||||
|
| ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
|
||||||
|
)
|
||||||
|
{
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(Diagnostic::new(InvalidLengthReturnType, value.range()));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Disallow implicit `None`.
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(Diagnostic::new(InvalidLengthReturnType, stmt.range()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the given expression is a negative integer.
|
||||||
|
fn is_negative_integer(value: &Expr) -> bool {
|
||||||
|
matches!(
|
||||||
|
value,
|
||||||
|
Expr::UnaryOp(ast::ExprUnaryOp {
|
||||||
|
op: ast::UnaryOp::USub,
|
||||||
|
..
|
||||||
|
})
|
||||||
|
)
|
||||||
|
}
|
|
@ -31,6 +31,7 @@ pub(crate) use invalid_bool_return::*;
|
||||||
pub(crate) use invalid_bytes_return::*;
|
pub(crate) use invalid_bytes_return::*;
|
||||||
pub(crate) use invalid_envvar_default::*;
|
pub(crate) use invalid_envvar_default::*;
|
||||||
pub(crate) use invalid_envvar_value::*;
|
pub(crate) use invalid_envvar_value::*;
|
||||||
|
pub(crate) use invalid_length_return::*;
|
||||||
pub(crate) use invalid_str_return::*;
|
pub(crate) use invalid_str_return::*;
|
||||||
pub(crate) use invalid_string_characters::*;
|
pub(crate) use invalid_string_characters::*;
|
||||||
pub(crate) use iteration_over_set::*;
|
pub(crate) use iteration_over_set::*;
|
||||||
|
@ -130,6 +131,7 @@ mod invalid_bool_return;
|
||||||
mod invalid_bytes_return;
|
mod invalid_bytes_return;
|
||||||
mod invalid_envvar_default;
|
mod invalid_envvar_default;
|
||||||
mod invalid_envvar_value;
|
mod invalid_envvar_value;
|
||||||
|
mod invalid_length_return;
|
||||||
mod invalid_str_return;
|
mod invalid_str_return;
|
||||||
mod invalid_string_characters;
|
mod invalid_string_characters;
|
||||||
mod iteration_over_set;
|
mod iteration_over_set;
|
||||||
|
|
|
@ -0,0 +1,51 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||||
|
---
|
||||||
|
invalid_return_type_length.py:6:16: PLE0303 `__len__` does not return a non-negative integer
|
||||||
|
|
|
||||||
|
4 | class Bool:
|
||||||
|
5 | def __len__(self):
|
||||||
|
6 | return True # [invalid-length-return]
|
||||||
|
| ^^^^ PLE0303
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_length.py:11:16: PLE0303 `__len__` does not return a non-negative integer
|
||||||
|
|
|
||||||
|
9 | class Float:
|
||||||
|
10 | def __len__(self):
|
||||||
|
11 | return 3.05 # [invalid-length-return]
|
||||||
|
| ^^^^ PLE0303
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_length.py:16:16: PLE0303 `__len__` does not return a non-negative integer
|
||||||
|
|
|
||||||
|
14 | class Str:
|
||||||
|
15 | def __len__(self):
|
||||||
|
16 | return "ruff" # [invalid-length-return]
|
||||||
|
| ^^^^^^ PLE0303
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_length.py:20:9: PLE0303 `__len__` does not return a non-negative integer
|
||||||
|
|
|
||||||
|
19 | class LengthNoReturn:
|
||||||
|
20 | def __len__(self):
|
||||||
|
| ^^^^^^^ PLE0303
|
||||||
|
21 | print("ruff") # [invalid-length-return]
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_length.py:26:16: PLE0303 `__len__` does not return a non-negative integer
|
||||||
|
|
|
||||||
|
24 | class LengthNegative:
|
||||||
|
25 | def __len__(self):
|
||||||
|
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]
|
||||||
|
|
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -3267,6 +3267,7 @@
|
||||||
"PLE03",
|
"PLE03",
|
||||||
"PLE030",
|
"PLE030",
|
||||||
"PLE0302",
|
"PLE0302",
|
||||||
|
"PLE0303",
|
||||||
"PLE0304",
|
"PLE0304",
|
||||||
"PLE0307",
|
"PLE0307",
|
||||||
"PLE0308",
|
"PLE0308",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue