Skip to content

Commit

Permalink
Small tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 18, 2024
1 parent 0d66b12 commit b09ac0b
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 69 deletions.
Original file line number Diff line number Diff line change
@@ -1,58 +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):
...
def __len__(self): ...


class Length4:
def __len__(self):
pass


class Length5:
def __len__(self):
raise NotImplementedError
raise NotImplementedError
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::invalid_bool_return(checker, name, body);
}
if checker.enabled(Rule::InvalidLengthReturnType) {
pylint::rules::invalid_length_return(checker, name, body);
pylint::rules::invalid_length_return(checker, function_def);
}
if checker.enabled(Rule::InvalidBytesReturnType) {
pylint::rules::invalid_bytes_return(checker, function_def);
Expand Down
43 changes: 17 additions & 26 deletions crates/ruff_linter/src/rules/pylint/rules/invalid_length_return.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
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, Stmt};
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 other than a non-negative `integer`.
/// 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:
Expand All @@ -29,9 +36,7 @@ use crate::checkers::ast::Checker;
/// return 2
/// ```
///
/// Note: Strictly speaking `bool` is a subclass of `int`, thus returning `True`/`False` is valid.
/// To be consistent with other rules (e.g. PLE0305 invalid-index-returned), ruff will raise, compared
/// to pylint which will not raise.
///
/// ## References
/// - [Python documentation: The `__len__` method](https://docs.python.org/3/reference/datamodel.html#object.__len__)
#[violation]
Expand All @@ -40,49 +45,34 @@ pub struct InvalidLengthReturnType;
impl Violation for InvalidLengthReturnType {
#[derive_message_formats]
fn message(&self) -> String {
format!("`__len__` does not return a non-negative `integer`")
format!("`__len__` does not return a non-negative integer")
}
}

/// E0303
pub(crate) fn invalid_length_return(checker: &mut Checker, name: &str, body: &[Stmt]) {
if name != "__len__" {
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 body.len() == 1
&& (matches!(&body[0], Stmt::Expr(expr) if expr.value.is_ellipsis_literal_expr())
|| body[0].is_pass_stmt()
|| body[0].is_raise_stmt())
{
return;
}

let body_without_comments = body
.iter()
.filter(|stmt| !matches!(stmt, Stmt::Expr(expr) if expr.value.is_string_literal_expr()))
.collect::<Vec<_>>();
if body_without_comments.is_empty() {
return;
}
if body_without_comments.len() == 1 && body_without_comments[0].is_raise_stmt() {
if is_stub(function_def, checker.semantic()) {
return;
}

let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(body);
visitor.visit_body(&function_def.body);
visitor.returns
};

if returns.is_empty() {
checker.diagnostics.push(Diagnostic::new(
InvalidLengthReturnType,
body.last().unwrap().range(),
function_def.identifier(),
));
}

Expand All @@ -108,6 +98,7 @@ pub(crate) fn invalid_length_return(checker: &mut Checker, name: &str, body: &[S
}
}

/// Returns `true` if the given expression is a negative integer.
fn is_negative_integer(value: &Expr) -> bool {
matches!(
value,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,62 +1,51 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
invalid_return_type_length.py:5:16: PLE0303 `__len__` does not return a non-negative `integer`
invalid_return_type_length.py:6:16: PLE0303 `__len__` does not return a non-negative integer
|
3 | class Bool:
4 | def __len__(self):
5 | return True # [invalid-length-return]
4 | class Bool:
5 | def __len__(self):
6 | return True # [invalid-length-return]
| ^^^^ PLE0303
6 |
7 | class Float:
|

invalid_return_type_length.py:9:16: PLE0303 `__len__` does not return a non-negative `integer`
invalid_return_type_length.py:11:16: PLE0303 `__len__` does not return a non-negative integer
|
7 | class Float:
8 | def __len__(self):
9 | return 3.05 # [invalid-length-return]
9 | class Float:
10 | def __len__(self):
11 | return 3.05 # [invalid-length-return]
| ^^^^ PLE0303
10 |
11 | class Str:
|

invalid_return_type_length.py:13:16: PLE0303 `__len__` does not return a non-negative `integer`
invalid_return_type_length.py:16:16: PLE0303 `__len__` does not return a non-negative integer
|
11 | class Str:
12 | def __len__(self):
13 | return "ruff" # [invalid-length-return]
14 | class Str:
15 | def __len__(self):
16 | return "ruff" # [invalid-length-return]
| ^^^^^^ PLE0303
14 |
15 | class LengthNoReturn:
|

invalid_return_type_length.py:17:9: PLE0303 `__len__` does not return a non-negative `integer`
invalid_return_type_length.py:20:9: PLE0303 `__len__` does not return a non-negative integer
|
15 | class LengthNoReturn:
16 | def __len__(self):
17 | print("ruff") # [invalid-length-return]
| ^^^^^^^^^^^^^ PLE0303
18 |
19 | class LengthNegative:
19 | class LengthNoReturn:
20 | def __len__(self):
| ^^^^^^^ PLE0303
21 | print("ruff") # [invalid-length-return]
|

invalid_return_type_length.py:21:16: PLE0303 `__len__` does not return a non-negative `integer`
invalid_return_type_length.py:26:16: PLE0303 `__len__` does not return a non-negative integer
|
19 | class LengthNegative:
20 | def __len__(self):
21 | return -42 # [invalid-length-return]
24 | class LengthNegative:
25 | def __len__(self):
26 | return -42 # [invalid-length-return]
| ^^^ PLE0303
22 |
23 | class LengthWrongRaise:
|

invalid_return_type_length.py:26:9: PLE0303 `__len__` does not return a non-negative `integer`
|
24 | def __len__(self):
25 | print("raise some error")
26 | raise NotImplementedError # [invalid-length-return]
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PLE0303
27 |
28 | # TODO: Once Ruff has better type checking
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]
|

0 comments on commit b09ac0b

Please sign in to comment.