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-bool-return-type (E304) #10377

Merged
merged 4 commits into from Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -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]
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved

# 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
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
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
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
@@ -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.
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
///
/// ## 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
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
@@ -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.