mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:25:17 +00:00
[pylint
] Implement invalid-index-returned
(PLE0305
) (#10962)
Add pylint rule invalid-index-returned (PLE0305) See https://github.com/astral-sh/ruff/issues/970 for rules Test Plan: `cargo test`
This commit is contained in:
parent
97acf1d59b
commit
27902b7130
8 changed files with 234 additions and 0 deletions
73
crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_index.py
vendored
Normal file
73
crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_index.py
vendored
Normal file
|
@ -0,0 +1,73 @@
|
||||||
|
# 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]
|
||||||
|
|
||||||
|
|
||||||
|
# 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
|
||||||
|
|
||||||
|
|
||||||
|
class Index2:
|
||||||
|
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
|
||||||
|
|
||||||
|
|
||||||
|
class Index6:
|
||||||
|
def __index__(self):
|
||||||
|
print("raise some error")
|
||||||
|
raise NotImplementedError
|
|
@ -103,6 +103,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
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);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::InvalidIndexReturnType) {
|
||||||
|
pylint::rules::invalid_index_return(checker, function_def);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::InvalidHashReturnType) {
|
if checker.enabled(Rule::InvalidHashReturnType) {
|
||||||
pylint::rules::invalid_hash_return(checker, function_def);
|
pylint::rules::invalid_hash_return(checker, function_def);
|
||||||
}
|
}
|
||||||
|
|
|
@ -243,6 +243,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(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, "E0303") => (RuleGroup::Preview, rules::pylint::rules::InvalidLengthReturnType),
|
||||||
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
|
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
|
||||||
|
(Pylint, "E0305") => (RuleGroup::Preview, rules::pylint::rules::InvalidIndexReturnType),
|
||||||
(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),
|
||||||
(Pylint, "E0309") => (RuleGroup::Preview, rules::pylint::rules::InvalidHashReturnType),
|
(Pylint, "E0309") => (RuleGroup::Preview, rules::pylint::rules::InvalidHashReturnType),
|
||||||
|
|
|
@ -81,6 +81,10 @@ mod tests {
|
||||||
Rule::InvalidBytesReturnType,
|
Rule::InvalidBytesReturnType,
|
||||||
Path::new("invalid_return_type_bytes.py")
|
Path::new("invalid_return_type_bytes.py")
|
||||||
)]
|
)]
|
||||||
|
#[test_case(
|
||||||
|
Rule::InvalidIndexReturnType,
|
||||||
|
Path::new("invalid_return_type_index.py")
|
||||||
|
)]
|
||||||
#[test_case(Rule::InvalidHashReturnType, Path::new("invalid_return_type_hash.py"))]
|
#[test_case(Rule::InvalidHashReturnType, Path::new("invalid_return_type_hash.py"))]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
Rule::InvalidLengthReturnType,
|
Rule::InvalidLengthReturnType,
|
||||||
|
|
|
@ -0,0 +1,109 @@
|
||||||
|
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};
|
||||||
|
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;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for `__index__` implementations that return a type other than `integer`.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// 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:
|
||||||
|
/// def __index__(self):
|
||||||
|
/// return "2"
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// class Foo:
|
||||||
|
/// def __index__(self):
|
||||||
|
/// return 2
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// ## References
|
||||||
|
/// - [Python documentation: The `__index__` method](https://docs.python.org/3/reference/datamodel.html#object.__index__)
|
||||||
|
#[violation]
|
||||||
|
pub struct InvalidIndexReturnType;
|
||||||
|
|
||||||
|
impl Violation for InvalidIndexReturnType {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("`__index__` does not return an integer")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// E0305
|
||||||
|
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 is_stub(function_def, checker.semantic()) {
|
||||||
|
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(
|
||||||
|
InvalidIndexReturnType,
|
||||||
|
function_def.identifier(),
|
||||||
|
));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let returns = {
|
||||||
|
let mut visitor = ReturnStatementVisitor::default();
|
||||||
|
visitor.visit_body(&function_def.body);
|
||||||
|
visitor.returns
|
||||||
|
};
|
||||||
|
|
||||||
|
for stmt in returns {
|
||||||
|
if let Some(value) = stmt.value.as_deref() {
|
||||||
|
if !matches!(
|
||||||
|
ResolvedPythonType::from(value),
|
||||||
|
ResolvedPythonType::Unknown
|
||||||
|
| ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
|
||||||
|
) {
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(Diagnostic::new(InvalidIndexReturnType, value.range()));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Disallow implicit `None`.
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(Diagnostic::new(InvalidIndexReturnType, stmt.range()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -32,6 +32,7 @@ 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_hash_return::*;
|
pub(crate) use invalid_hash_return::*;
|
||||||
|
pub(crate) use invalid_index_return::*;
|
||||||
pub(crate) use invalid_length_return::*;
|
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::*;
|
||||||
|
@ -133,6 +134,7 @@ mod invalid_bytes_return;
|
||||||
mod invalid_envvar_default;
|
mod invalid_envvar_default;
|
||||||
mod invalid_envvar_value;
|
mod invalid_envvar_value;
|
||||||
mod invalid_hash_return;
|
mod invalid_hash_return;
|
||||||
|
mod invalid_index_return;
|
||||||
mod invalid_length_return;
|
mod invalid_length_return;
|
||||||
mod invalid_str_return;
|
mod invalid_str_return;
|
||||||
mod invalid_string_characters;
|
mod invalid_string_characters;
|
||||||
|
|
|
@ -0,0 +1,41 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||||
|
---
|
||||||
|
invalid_return_type_index.py:8:16: PLE0305 `__index__` does not return an integer
|
||||||
|
|
|
||||||
|
7 | def __index__(self):
|
||||||
|
8 | return True # [invalid-index-return]
|
||||||
|
| ^^^^ PLE0305
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_index.py:13:16: PLE0305 `__index__` does not return an integer
|
||||||
|
|
|
||||||
|
11 | class Float:
|
||||||
|
12 | def __index__(self):
|
||||||
|
13 | return 3.05 # [invalid-index-return]
|
||||||
|
| ^^^^ PLE0305
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_index.py:18:16: PLE0305 `__index__` does not return an integer
|
||||||
|
|
|
||||||
|
16 | class Dict:
|
||||||
|
17 | def __index__(self):
|
||||||
|
18 | return {"1": "1"} # [invalid-index-return]
|
||||||
|
| ^^^^^^^^^^ PLE0305
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_index.py:23:16: PLE0305 `__index__` does not return an integer
|
||||||
|
|
|
||||||
|
21 | class Str:
|
||||||
|
22 | def __index__(self):
|
||||||
|
23 | return "ruff" # [invalid-index-return]
|
||||||
|
| ^^^^^^ PLE0305
|
||||||
|
|
|
||||||
|
|
||||||
|
invalid_return_type_index.py:27:9: PLE0305 `__index__` does not return an integer
|
||||||
|
|
|
||||||
|
26 | class IndexNoReturn:
|
||||||
|
27 | def __index__(self):
|
||||||
|
| ^^^^^^^^^ PLE0305
|
||||||
|
28 | print("ruff") # [invalid-index-return]
|
||||||
|
|
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -3269,6 +3269,7 @@
|
||||||
"PLE0302",
|
"PLE0302",
|
||||||
"PLE0303",
|
"PLE0303",
|
||||||
"PLE0304",
|
"PLE0304",
|
||||||
|
"PLE0305",
|
||||||
"PLE0307",
|
"PLE0307",
|
||||||
"PLE0308",
|
"PLE0308",
|
||||||
"PLE0309",
|
"PLE0309",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue