From c269c1a7063bb27e75dc81c2a598eab13aa8b754 Mon Sep 17 00:00:00 2001 From: boolean <61526956+boolean-light@users.noreply.github.com> Date: Wed, 13 Mar 2024 23:13:45 +0900 Subject: [PATCH] [`pylint`] Implement `invalid-bool-return-type` (`E304`) (#10377) ## 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` --- .../pylint/invalid_return_type_bool.py | 37 +++++++++ .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../rules/pylint/rules/invalid_bool_return.rs | 78 +++++++++++++++++++ .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + ...__PLE0304_invalid_return_type_bool.py.snap | 20 +++++ ruff.schema.json | 1 + 8 files changed, 143 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py new file mode 100644 index 0000000000000..31ab90b738a2e --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py @@ -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 \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index ea1335bbc0cd1..191456bf0766b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -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); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ef05dcb053128..c471c890a0f1a 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index e03e9303b3f26..0cbd284ddbc8f 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -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"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs new file mode 100644 index 0000000000000..d677b86e51832 --- /dev/null +++ b/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. +/// +/// ## 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())); + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 34289564eed02..8f6d40554d2ad 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -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::*; @@ -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; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap new file mode 100644 index 0000000000000..b28107c0851d8 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap @@ -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 + | diff --git a/ruff.schema.json b/ruff.schema.json index 5d44debca2f2f..d0f0f9a6394e0 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3219,6 +3219,7 @@ "PLE03", "PLE030", "PLE0302", + "PLE0304", "PLE0307", "PLE06", "PLE060",