Skip to content

Commit

Permalink
[pylint] Implement invalid-bool-return-type (E304) (#10377)
Browse files Browse the repository at this point in the history
## Summary

Implement `E304` in the issue #970. Throws an error when the returning value
of `__bool__` method is not boolean.

Reference: https://pylint.readthedocs.io/en/stable/user_guide/messages/error/invalid-bool-returned.html

## Test Plan

Add test cases and run `cargo test`
  • Loading branch information
boolean-light authored Mar 13, 2024
1 parent 32d6f84 commit c269c1a
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# These testcases should raise errors

class Float:
def __bool__(self):
return 3.05 # [invalid-bool-return]

class Int:
def __bool__(self):
return 0 # [invalid-bool-return]


class Str:
def __bool__(self):
x = "ruff"
return x # [invalid-bool-return]

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

class ComplexReturn:
def __bool__(self):
return return_int() # [invalid-bool-return]



# These testcases should NOT raise errors

class Bool:
def __bool__(self):
return True


class Bool2:
def __bool__(self):
x = True
return x
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 @@ -91,6 +91,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
checker.diagnostics.push(diagnostic);
}
}
if checker.enabled(Rule::InvalidBoolReturnType) {
pylint::rules::invalid_bool_return(checker, name, body);
}
if checker.enabled(Rule::InvalidStrReturnType) {
pylint::rules::invalid_str_return(checker, name, body);
}
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 @@ -240,6 +240,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment),
(Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases),
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
(Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject),
(Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ mod tests {
#[test_case(Rule::ImportSelf, Path::new("import_self/module.py"))]
#[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::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
78 changes: 78 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::ReturnStatementVisitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::Stmt;
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 `__bool__` implementations that return a type other than `bool`.
///
/// ## Why is this bad?
/// The `__bool__` method should return a `bool` object. Returning a different
/// type may cause unexpected behavior.
///
/// ## Example
/// ```python
/// class Foo:
/// def __bool__(self):
/// return 2
/// ```
///
/// Use instead:
/// ```python
/// class Foo:
/// def __bool__(self):
/// return True
/// ```
///
/// ## References
/// - [Python documentation: The `__bool__` method](https://docs.python.org/3/reference/datamodel.html#object.__bool__)
#[violation]
pub struct InvalidBoolReturnType;

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

/// E0307
pub(crate) fn invalid_bool_return(checker: &mut Checker, name: &str, body: &[Stmt]) {
if name != "__bool__" {
return;
}

if !checker.semantic().current_scope().kind.is_class() {
return;
}

let returns = {
let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(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::Bool))
) {
checker
.diagnostics
.push(Diagnostic::new(InvalidBoolReturnType, value.range()));
}
} else {
// Disallow implicit `None`.
checker
.diagnostics
.push(Diagnostic::new(InvalidBoolReturnType, 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 @@ -25,6 +25,7 @@ pub(crate) use import_private_name::*;
pub(crate) use import_self::*;
pub(crate) use invalid_all_format::*;
pub(crate) use invalid_all_object::*;
pub(crate) use invalid_bool_return::*;
pub(crate) use invalid_envvar_default::*;
pub(crate) use invalid_envvar_value::*;
pub(crate) use invalid_str_return::*;
Expand Down Expand Up @@ -113,6 +114,7 @@ mod import_private_name;
mod import_self;
mod invalid_all_format;
mod invalid_all_object;
mod invalid_bool_return;
mod invalid_envvar_default;
mod invalid_envvar_value;
mod invalid_str_return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
invalid_return_type_bool.py:5:16: PLE0304 `__bool__` does not return `bool`
|
3 | class Float:
4 | def __bool__(self):
5 | return 3.05 # [invalid-bool-return]
| ^^^^ PLE0304
6 |
7 | class Int:
|

invalid_return_type_bool.py:9:16: PLE0304 `__bool__` does not return `bool`
|
7 | class Int:
8 | def __bool__(self):
9 | return 0 # [invalid-bool-return]
| ^ PLE0304
|
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.

0 comments on commit c269c1a

Please sign in to comment.