From 5d554edace07447bffffd6325bdbed8ba8ca609b Mon Sep 17 00:00:00 2001 From: Charlie Marsh <charlie.r.marsh@gmail.com> Date: Tue, 28 Nov 2023 13:42:31 -0800 Subject: [PATCH] Allow booleans in `@override` methods (#8882) Closes #8867. --- .../test/fixtures/flake8_boolean_trap/FBT.py | 8 ++++++ ...olean_default_value_positional_argument.rs | 24 ++++++++++++----- .../boolean_type_hint_positional_argument.rs | 26 ++++++++++++++----- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_boolean_trap/FBT.py b/crates/ruff_linter/resources/test/fixtures/flake8_boolean_trap/FBT.py index 58dcb4b5164b5..2c48a7d629fda 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_boolean_trap/FBT.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_boolean_trap/FBT.py @@ -106,3 +106,11 @@ def func(x: bool | str): def func(x: int | str): pass + + +from typing import override + + +@override +def func(x: bool): + pass diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs index 3a3618fa4d588..61134cd316b32 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs @@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; use ruff_python_ast::{Decorator, ParameterWithDefault, Parameters}; +use ruff_python_semantic::analyze::visibility; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -94,23 +95,18 @@ impl Violation for BooleanDefaultValuePositionalArgument { } } +/// FBT002 pub(crate) fn boolean_default_value_positional_argument( checker: &mut Checker, name: &str, decorator_list: &[Decorator], parameters: &Parameters, ) { + // Allow Boolean defaults in explicitly-allowed functions. if is_allowed_func_def(name) { return; } - if decorator_list.iter().any(|decorator| { - collect_call_path(&decorator.expression) - .is_some_and(|call_path| call_path.as_slice() == [name, "setter"]) - }) { - return; - } - for ParameterWithDefault { parameter, default, @@ -121,6 +117,20 @@ pub(crate) fn boolean_default_value_positional_argument( .as_ref() .is_some_and(|default| default.is_boolean_literal_expr()) { + // Allow Boolean defaults in setters. + if decorator_list.iter().any(|decorator| { + collect_call_path(&decorator.expression) + .is_some_and(|call_path| call_path.as_slice() == [name, "setter"]) + }) { + return; + } + + // Allow Boolean defaults in `@override` methods, since they're required to adhere to + // the parent signature. + if visibility::is_override(decorator_list, checker.semantic()) { + return; + } + checker.diagnostics.push(Diagnostic::new( BooleanDefaultValuePositionalArgument, parameter.name.range(), diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs index 1ba9a680d20fe..8cfefd02f8eac 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs @@ -4,6 +4,7 @@ use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; +use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; @@ -109,17 +110,11 @@ pub(crate) fn boolean_type_hint_positional_argument( decorator_list: &[Decorator], parameters: &Parameters, ) { + // Allow Boolean type hints in explicitly-allowed functions. if is_allowed_func_def(name) { return; } - if decorator_list.iter().any(|decorator| { - collect_call_path(&decorator.expression) - .is_some_and(|call_path| call_path.as_slice() == [name, "setter"]) - }) { - return; - } - for ParameterWithDefault { parameter, default: _, @@ -138,9 +133,26 @@ pub(crate) fn boolean_type_hint_positional_argument( continue; } } + + // Allow Boolean type hints in setters. + if decorator_list.iter().any(|decorator| { + collect_call_path(&decorator.expression) + .is_some_and(|call_path| call_path.as_slice() == [name, "setter"]) + }) { + return; + } + + // Allow Boolean defaults in `@override` methods, since they're required to adhere to + // the parent signature. + if visibility::is_override(decorator_list, checker.semantic()) { + return; + } + + // If `bool` isn't actually a reference to the `bool` built-in, return. if !checker.semantic().is_builtin("bool") { return; } + checker.diagnostics.push(Diagnostic::new( BooleanTypeHintPositionalArgument, parameter.name.range(),