diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_index.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_index.py new file mode 100644 index 0000000000..6ceba7e7ef --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_index.py @@ -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 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index db30aeb3f7..2b6468bfd7 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -103,6 +103,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::InvalidBytesReturnType) { 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) { pylint::rules::invalid_hash_return(checker, function_def); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index f5c30294eb..bfc392cfc8 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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, "E0303") => (RuleGroup::Preview, rules::pylint::rules::InvalidLengthReturnType), (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, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType), (Pylint, "E0309") => (RuleGroup::Preview, rules::pylint::rules::InvalidHashReturnType), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index def31a22c2..b20b17dac5 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -81,6 +81,10 @@ mod tests { Rule::InvalidBytesReturnType, 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::InvalidLengthReturnType, diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_index_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_index_return.rs new file mode 100644 index 0000000000..14fe3dd791 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_index_return.rs @@ -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())); + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 362a00f839..16f31a9d5a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -32,6 +32,7 @@ pub(crate) use invalid_bytes_return::*; pub(crate) use invalid_envvar_default::*; pub(crate) use invalid_envvar_value::*; pub(crate) use invalid_hash_return::*; +pub(crate) use invalid_index_return::*; pub(crate) use invalid_length_return::*; pub(crate) use invalid_str_return::*; pub(crate) use invalid_string_characters::*; @@ -133,6 +134,7 @@ mod invalid_bytes_return; mod invalid_envvar_default; mod invalid_envvar_value; mod invalid_hash_return; +mod invalid_index_return; mod invalid_length_return; mod invalid_str_return; mod invalid_string_characters; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0305_invalid_return_type_index.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0305_invalid_return_type_index.py.snap new file mode 100644 index 0000000000..367f15b8b7 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0305_invalid_return_type_index.py.snap @@ -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] + | diff --git a/ruff.schema.json b/ruff.schema.json index cb3bdae453..9c2d65110c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3269,6 +3269,7 @@ "PLE0302", "PLE0303", "PLE0304", + "PLE0305", "PLE0307", "PLE0308", "PLE0309",