Allow booleans in @override methods (#8882)

Closes #8867.
astral-sh · Nov 28, 2023 · 5d554ed · 5d554ed
1 parent 4126888
commit 5d554ed
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 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
@@ -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/...f_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs b/...f_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/.../ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs b/.../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(),