From 829acf498dec75e31f4fedf8da223ee1dd807b7d Mon Sep 17 00:00:00 2001 From: Dylan Date: Sun, 8 Jun 2025 13:07:58 -0500 Subject: [PATCH] [`flake8-boolean-trap`] Stabilize lint `bool` suprtypes in `boolean-type-hint-positional-argument` (`FBT001`) (#18520) Feel free to complain about the rephrasing in the docs! --- crates/ruff_linter/src/preview.rs | 5 - .../src/rules/flake8_boolean_trap/mod.rs | 19 ---- .../boolean_type_hint_positional_argument.rs | 30 +---- ...e8_boolean_trap__tests__FBT001_FBT.py.snap | 15 ++- ...n_trap__tests__preview__FBT001_FBT.py.snap | 105 ------------------ 5 files changed, 18 insertions(+), 156 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__preview__FBT001_FBT.py.snap diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index 3bc09c0ea2..32f4b06a5b 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -23,11 +23,6 @@ pub(crate) const fn is_suspicious_function_reference_enabled(settings: &LinterSe settings.preview.is_enabled() } -// https://github.com/astral-sh/ruff/pull/7501 -pub(crate) const fn is_bool_subtype_of_annotation_enabled(settings: &LinterSettings) -> bool { - settings.preview.is_enabled() -} - // https://github.com/astral-sh/ruff/pull/10759 pub(crate) const fn is_comprehension_with_min_max_sum_enabled(settings: &LinterSettings) -> bool { settings.preview.is_enabled() diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs index 8268ca726b..3307cb949d 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs @@ -12,7 +12,6 @@ mod tests { use crate::registry::Rule; use crate::settings::LinterSettings; - use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -29,24 +28,6 @@ mod tests { Ok(()) } - #[test_case(Rule::BooleanTypeHintPositionalArgument, Path::new("FBT.py"))] - fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { - let snapshot = format!( - "preview__{}_{}", - rule_code.noqa_code(), - path.to_string_lossy() - ); - let diagnostics = test_path( - Path::new("flake8_boolean_trap").join(path).as_path(), - &settings::LinterSettings { - preview: PreviewMode::Enabled, - ..settings::LinterSettings::for_rule(rule_code) - }, - )?; - assert_messages!(snapshot, diagnostics); - Ok(()) - } - #[test] fn extend_allowed_callable() -> Result<()> { let diagnostics = test_path( 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 51bd830a66..24dff35099 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 @@ -7,12 +7,12 @@ use ruff_python_semantic::analyze::visibility; use crate::Violation; use crate::checkers::ast::Checker; -use crate::preview::is_bool_subtype_of_annotation_enabled; use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; /// ## What it does /// Checks for the use of boolean positional arguments in function definitions, -/// as determined by the presence of a `bool` type hint. +/// as determined by the presence of a type hint containing `bool` as an +/// evident subtype - e.g. `bool`, `bool | int`, `typing.Optional[bool]`, etc. /// /// ## Why is this bad? /// Calling a function with boolean positional arguments is confusing as the @@ -30,9 +30,6 @@ use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; /// Dunder methods that define operators are exempt from this rule, as are /// setters and `@override` definitions. /// -/// In [preview], this rule will also flag annotations that include boolean -/// variants, like `bool | int`. -/// /// ## Example /// /// ```python @@ -96,8 +93,6 @@ use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; /// ## References /// - [Python documentation: Calls](https://docs.python.org/3/reference/expressions.html#calls) /// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) -/// -/// [preview]: https://docs.astral.sh/ruff/preview/ #[derive(ViolationMetadata)] pub(crate) struct BooleanTypeHintPositionalArgument; @@ -128,14 +123,8 @@ pub(crate) fn boolean_type_hint_positional_argument( let Some(annotation) = parameter.annotation() else { continue; }; - if is_bool_subtype_of_annotation_enabled(checker.settings) { - if !match_annotation_to_complex_bool(annotation, checker.semantic()) { - continue; - } - } else { - if !match_annotation_to_literal_bool(annotation) { - continue; - } + if !match_annotation_to_complex_bool(annotation, checker.semantic()) { + continue; } // Allow Boolean type hints in setters. @@ -161,17 +150,6 @@ pub(crate) fn boolean_type_hint_positional_argument( } } -/// Returns `true` if the annotation is a boolean type hint (e.g., `bool`). -fn match_annotation_to_literal_bool(annotation: &Expr) -> bool { - match annotation { - // Ex) `True` - Expr::Name(name) => &name.id == "bool", - // Ex) `"True"` - Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value == "bool", - _ => false, - } -} - /// Returns `true` if the annotation is a boolean type hint (e.g., `bool`), or a type hint that /// includes boolean as a variant (e.g., `bool | int`). fn match_annotation_to_complex_bool(annotation: &Expr, semantic: &SemanticModel) -> bool { diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap index ea925637b5..ee91fa49f8 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT001_FBT.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs -snapshot_kind: text --- FBT.py:4:5: FBT001 Boolean-typed positional argument in function definition | @@ -89,3 +88,17 @@ FBT.py:90:19: FBT001 Boolean-typed positional argument in function definition | ^^^^^ FBT001 91 | pass | + +FBT.py:100:10: FBT001 Boolean-typed positional argument in function definition + | +100 | def func(x: Union[list, Optional[int | str | float | bool]]): + | ^ FBT001 +101 | pass + | + +FBT.py:104:10: FBT001 Boolean-typed positional argument in function definition + | +104 | def func(x: bool | str): + | ^ FBT001 +105 | pass + | diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__preview__FBT001_FBT.py.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__preview__FBT001_FBT.py.snap deleted file mode 100644 index d3ab33ec5c..0000000000 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__preview__FBT001_FBT.py.snap +++ /dev/null @@ -1,105 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs -snapshot_kind: text ---- -FBT.py:4:5: FBT001 Boolean-typed positional argument in function definition - | -2 | posonly_nohint, -3 | posonly_nonboolhint: int, -4 | posonly_boolhint: bool, - | ^^^^^^^^^^^^^^^^ FBT001 -5 | posonly_boolstrhint: "bool", -6 | /, - | - -FBT.py:5:5: FBT001 Boolean-typed positional argument in function definition - | -3 | posonly_nonboolhint: int, -4 | posonly_boolhint: bool, -5 | posonly_boolstrhint: "bool", - | ^^^^^^^^^^^^^^^^^^^ FBT001 -6 | /, -7 | offset, - | - -FBT.py:10:5: FBT001 Boolean-typed positional argument in function definition - | - 8 | posorkw_nonvalued_nohint, - 9 | posorkw_nonvalued_nonboolhint: int, -10 | posorkw_nonvalued_boolhint: bool, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -11 | posorkw_nonvalued_boolstrhint: "bool", -12 | posorkw_boolvalued_nohint=True, - | - -FBT.py:11:5: FBT001 Boolean-typed positional argument in function definition - | - 9 | posorkw_nonvalued_nonboolhint: int, -10 | posorkw_nonvalued_boolhint: bool, -11 | posorkw_nonvalued_boolstrhint: "bool", - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -12 | posorkw_boolvalued_nohint=True, -13 | posorkw_boolvalued_nonboolhint: int = True, - | - -FBT.py:14:5: FBT001 Boolean-typed positional argument in function definition - | -12 | posorkw_boolvalued_nohint=True, -13 | posorkw_boolvalued_nonboolhint: int = True, -14 | posorkw_boolvalued_boolhint: bool = True, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -15 | posorkw_boolvalued_boolstrhint: "bool" = True, -16 | posorkw_nonboolvalued_nohint=1, - | - -FBT.py:15:5: FBT001 Boolean-typed positional argument in function definition - | -13 | posorkw_boolvalued_nonboolhint: int = True, -14 | posorkw_boolvalued_boolhint: bool = True, -15 | posorkw_boolvalued_boolstrhint: "bool" = True, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -16 | posorkw_nonboolvalued_nohint=1, -17 | posorkw_nonboolvalued_nonboolhint: int = 2, - | - -FBT.py:18:5: FBT001 Boolean-typed positional argument in function definition - | -16 | posorkw_nonboolvalued_nohint=1, -17 | posorkw_nonboolvalued_nonboolhint: int = 2, -18 | posorkw_nonboolvalued_boolhint: bool = 3, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -19 | posorkw_nonboolvalued_boolstrhint: "bool" = 4, -20 | *, - | - -FBT.py:19:5: FBT001 Boolean-typed positional argument in function definition - | -17 | posorkw_nonboolvalued_nonboolhint: int = 2, -18 | posorkw_nonboolvalued_boolhint: bool = 3, -19 | posorkw_nonboolvalued_boolstrhint: "bool" = 4, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FBT001 -20 | *, -21 | kwonly_nonvalued_nohint, - | - -FBT.py:90:19: FBT001 Boolean-typed positional argument in function definition - | -89 | # FBT001: Boolean positional arg in function definition -90 | def foo(self, value: bool) -> None: - | ^^^^^ FBT001 -91 | pass - | - -FBT.py:100:10: FBT001 Boolean-typed positional argument in function definition - | -100 | def func(x: Union[list, Optional[int | str | float | bool]]): - | ^ FBT001 -101 | pass - | - -FBT.py:104:10: FBT001 Boolean-typed positional argument in function definition - | -104 | def func(x: bool | str): - | ^ FBT001 -105 | pass - |