Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement invalid-hash-returned (PLE0309) #10961

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# These testcases should raise errors


class Bool:
def __hash__(self):
return True # [invalid-hash-return]


class Float:
def __hash__(self):
return 3.05 # [invalid-hash-return]


class Str:
def __hash__(self):
return "ruff" # [invalid-hash-return]


class HashNoReturn:
def __hash__(self):
print("ruff") # [invalid-hash-return]


class HashWrongRaise:
def __hash__(self):
print("raise some error")
raise NotImplementedError # [invalid-hash-return]


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


class ComplexReturn:
def __hash__(self):
return return_int() # [invalid-hash-return]



# These testcases should NOT raise errors


class Hash:
def __hash__(self):
return 7741


class Hash2:
def __hash__(self):
x = 7741
return x


class Hash3:
def __hash__(self): ...


class Has4:
def __hash__(self):
pass


class Hash5:
def __hash__(self):
raise NotImplementedError
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::InvalidHashReturnType) {
pylint::rules::invalid_hash_return(checker, function_def);
}
if checker.enabled(Rule::InvalidStrReturnType) {
pylint::rules::invalid_str_return(checker, function_def);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
(Pylint, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType),
(Pylint, "E0309") => (RuleGroup::Preview, rules::pylint::rules::InvalidHashReturnType),
(Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject),
(Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat),
(Pylint, "E0643") => (RuleGroup::Preview, rules::pylint::rules::PotentialIndexError),
Expand Down
9 changes: 5 additions & 4 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ mod tests {
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.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::InvalidLengthReturnType,
Path::new("invalid_return_type_length.py")
)]
#[test_case(
Rule::InvalidBytesReturnType,
Path::new("invalid_return_type_bytes.py")
)]
#[test_case(Rule::InvalidHashReturnType, Path::new("invalid_return_type_hash.py"))]
#[test_case(
Rule::InvalidLengthReturnType,
Path::new("invalid_return_type_length.py")
)]
#[test_case(Rule::InvalidStrReturnType, Path::new("invalid_return_type_str.py"))]
#[test_case(Rule::DuplicateBases, Path::new("duplicate_bases.py"))]
#[test_case(Rule::InvalidCharacterBackspace, Path::new("invalid_characters.py"))]
Expand Down
96 changes: 96 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/invalid_hash_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
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::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__hash__` implementations that return a type other than `integer`.
///
/// ## Why is this bad?
/// The `__hash__` 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 `__hash__` to
/// return `True` or `False`. However, for consistency with other rules, Ruff will
/// still raise when `__hash__` returns a `bool`.
///
/// ## Example
/// ```python
/// class Foo:
/// def __hash__(self):
/// return "2"
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __hash__(self):
/// return 2
/// ```
///
///
/// ## References
/// - [Python documentation: The `__hash__` method](https://docs.python.org/3/reference/datamodel.html#object.__hash__)
#[violation]
pub struct InvalidHashReturnType;

impl Violation for InvalidHashReturnType {
#[derive_message_formats]
fn message(&self) -> String {
format!("`__hash__` does not return `integer`")
}
}

/// E0309
pub(crate) fn invalid_hash_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
if function_def.name.as_str() != "__hash__" {
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(
InvalidHashReturnType,
function_def.identifier(),
));
}

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(InvalidHashReturnType, value.range()));
}
} else {
// Disallow implicit `None`.
checker
.diagnostics
.push(Diagnostic::new(InvalidHashReturnType, stmt.range()));
}
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub(crate) use invalid_bool_return::*;
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_length_return::*;
pub(crate) use invalid_str_return::*;
pub(crate) use invalid_string_characters::*;
Expand Down Expand Up @@ -131,6 +132,7 @@ mod invalid_bool_return;
mod invalid_bytes_return;
mod invalid_envvar_default;
mod invalid_envvar_value;
mod invalid_hash_return;
mod invalid_length_return;
mod invalid_str_return;
mod invalid_string_characters;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
invalid_return_type_hash.py:6:16: PLE0309 `__hash__` does not return `integer`
|
4 | class Bool:
5 | def __hash__(self):
6 | return True # [invalid-hash-return]
| ^^^^ PLE0309
|

invalid_return_type_hash.py:11:16: PLE0309 `__hash__` does not return `integer`
|
9 | class Float:
10 | def __hash__(self):
11 | return 3.05 # [invalid-hash-return]
| ^^^^ PLE0309
|

invalid_return_type_hash.py:16:16: PLE0309 `__hash__` does not return `integer`
|
14 | class Str:
15 | def __hash__(self):
16 | return "ruff" # [invalid-hash-return]
| ^^^^^^ PLE0309
|

invalid_return_type_hash.py:20:9: PLE0309 `__hash__` does not return `integer`
|
19 | class HashNoReturn:
20 | def __hash__(self):
| ^^^^^^^^ PLE0309
21 | print("ruff") # [invalid-hash-return]
|

invalid_return_type_hash.py:25:9: PLE0309 `__hash__` does not return `integer`
|
24 | class HashWrongRaise:
25 | def __hash__(self):
| ^^^^^^^^ PLE0309
26 | print("raise some error")
27 | raise NotImplementedError # [invalid-hash-return]
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.