Skip to content

Commit

Permalink
Small updates
Browse files Browse the repository at this point in the history
  • Loading branch information
tibor-reiss committed Apr 18, 2024
1 parent e95aaab commit 04fe643
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -1,42 +1,51 @@
# These testcases should raise errors


class Bool:
"""pylint would not raise, but ruff does - see explanation in the docs"""
def __index__(self):
return True # [invalid-index-return]


class Float:
def __index__(self):
return 3.05 # [invalid-index-return]


class Dict:
def __index__(self):
return {"1": "1"} # [invalid-index-return]


class Str:
def __index__(self):
return "ruff" # [invalid-index-return]


class IndexNoReturn:
def __index__(self):
print("ruff") # [invalid-index-return]


class IndexWrongRaise:
def __index__(self):
print("raise some error")
raise NotImplementedError # [invalid-index-return]


# TODO: Once Ruff has better type checking
def return_index():
return "3"


class ComplexReturn:
def __index__(self):
return return_index() # [invalid-index-return]


# These testcases should NOT raise errors


class Index:
def __index__(self):
return 0
Expand All @@ -47,14 +56,17 @@ def __index__(self):
x = 1
return x


class Index3:
def __index__(self):
...


class Index4:
def __index__(self):
pass


class Index5:
def __index__(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 @@ -104,7 +104,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::invalid_bytes_return(checker, function_def);
}
if checker.enabled(Rule::InvalidIndexReturnType) {
pylint::rules::invalid_index_return(checker, name, body);
pylint::rules::invalid_index_return(checker, function_def);
}
if checker.enabled(Rule::InvalidStrReturnType) {
pylint::rules::invalid_str_return(checker, function_def);
Expand Down
40 changes: 15 additions & 25 deletions crates/ruff_linter/src/rules/pylint/rules/invalid_index_return.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
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::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;

Expand All @@ -15,6 +17,12 @@ use crate::checkers::ast::Checker;
/// The `__index__` method should return an `integer`. Returning a different
/// type may cause unexpected behavior.
///
/// Note: `bool` is a subclass of `int`, so it's technically valid for `__index__` to
/// return `True` or `False`. However, a DeprecationWarning (`DeprecationWarning:
/// __index__ returned non-int (type bool)`) for such cases was already introduced,
/// thus this is a conscious difference between the original pylint rule and the
/// current ruff implementation.
///
/// ## Example
/// ```python
/// class Foo:
Expand All @@ -29,10 +37,7 @@ use crate::checkers::ast::Checker;
/// return 2
/// ```
///
/// Note: Strictly speaking `bool` is a subclass of `int`, thus returning `True`/`False` is valid.
/// However, a DeprecationWarning (`DeprecationWarning: __index__ returned non-int (type bool)`)
/// for such cases was already introduced, thus this is a conscious difference between the original
/// pylint rule and the current ruff implementation.
///
/// ## References
/// - [Python documentation: The `__index__` method](https://docs.python.org/3/reference/datamodel.html#object.__index__)
#[violation]
Expand All @@ -46,44 +51,29 @@ impl Violation for InvalidIndexReturnType {
}

/// E0305
pub(crate) fn invalid_index_return(checker: &mut Checker, name: &str, body: &[Stmt]) {
if name != "__index__" {
pub(crate) fn invalid_index_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
if function_def.name.as_str() != "__index__" {
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(
InvalidIndexReturnType,
body.last().unwrap().range(),
function_def.identifier(),
));
}

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_index.py:6:16: PLE0305 `__index__` does not return `integer`
invalid_return_type_index.py:7:16: PLE0305 `__index__` does not return `integer`
|
4 | """pylint would not raise, but ruff does - see explanation in the docs"""
5 | def __index__(self):
6 | return True # [invalid-index-return]
5 | """pylint would not raise, but ruff does - see explanation in the docs"""
6 | def __index__(self):
7 | return True # [invalid-index-return]
| ^^^^ PLE0305
7 |
8 | class Float:
|

invalid_return_type_index.py:10:16: PLE0305 `__index__` does not return `integer`
invalid_return_type_index.py:12:16: PLE0305 `__index__` does not return `integer`
|
8 | class Float:
9 | def __index__(self):
10 | return 3.05 # [invalid-index-return]
10 | class Float:
11 | def __index__(self):
12 | return 3.05 # [invalid-index-return]
| ^^^^ PLE0305
11 |
12 | class Dict:
|

invalid_return_type_index.py:14:16: PLE0305 `__index__` does not return `integer`
invalid_return_type_index.py:17:16: PLE0305 `__index__` does not return `integer`
|
12 | class Dict:
13 | def __index__(self):
14 | return {"1": "1"} # [invalid-index-return]
15 | class Dict:
16 | def __index__(self):
17 | return {"1": "1"} # [invalid-index-return]
| ^^^^^^^^^^ PLE0305
15 |
16 | class Str:
|

invalid_return_type_index.py:18:16: PLE0305 `__index__` does not return `integer`
invalid_return_type_index.py:22:16: PLE0305 `__index__` does not return `integer`
|
16 | class Str:
17 | def __index__(self):
18 | return "ruff" # [invalid-index-return]
20 | class Str:
21 | def __index__(self):
22 | return "ruff" # [invalid-index-return]
| ^^^^^^ PLE0305
19 |
20 | class IndexNoReturn:
|

invalid_return_type_index.py:22:9: PLE0305 `__index__` does not return `integer`
invalid_return_type_index.py:26:9: PLE0305 `__index__` does not return `integer`
|
20 | class IndexNoReturn:
21 | def __index__(self):
22 | print("ruff") # [invalid-index-return]
| ^^^^^^^^^^^^^ PLE0305
23 |
24 | class IndexWrongRaise:
25 | class IndexNoReturn:
26 | def __index__(self):
| ^^^^^^^^^ PLE0305
27 | print("ruff") # [invalid-index-return]
|

invalid_return_type_index.py:27:9: PLE0305 `__index__` does not return `integer`
|
25 | def __index__(self):
26 | print("raise some error")
27 | raise NotImplementedError # [invalid-index-return]
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PLE0305
28 |
29 | # TODO: Once Ruff has better type checking
invalid_return_type_index.py:31:9: PLE0305 `__index__` does not return `integer`
|
30 | class IndexWrongRaise:
31 | def __index__(self):
| ^^^^^^^^^ PLE0305
32 | print("raise some error")
33 | raise NotImplementedError # [invalid-index-return]
|

0 comments on commit 04fe643

Please sign in to comment.