From 3593b4cfc55875fe8b6abcd0ac2c79097b06560d Mon Sep 17 00:00:00 2001 From: augustelalande Date: Fri, 22 Mar 2024 20:24:43 -0400 Subject: [PATCH 1/5] add setting for user defined allowed boolean trap --- .../test/fixtures/flake8_boolean_trap/FBT.py | 23 +++++++++++ .../src/rules/flake8_boolean_trap/helpers.rs | 17 +++++++- .../src/rules/flake8_boolean_trap/mod.rs | 24 ++++++++++++ .../rules/boolean_positional_value_in_call.rs | 2 +- .../src/rules/flake8_boolean_trap/settings.rs | 31 +++++++++++++++ ...e8_boolean_trap__tests__FBT003_FBT.py.snap | 25 ++++++++++++ ..._trap__tests__extend_allowed_callable.snap | 4 ++ crates/ruff_linter/src/settings/mod.rs | 14 ++++--- crates/ruff_workspace/src/configuration.rs | 20 ++++++++-- crates/ruff_workspace/src/options.rs | 27 +++++++++++++ ruff.schema.json | 39 +++++++++++++++++++ 11 files changed, 214 insertions(+), 12 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs create mode 100644 crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap 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 e4dcf05345856a..d3e963c4759c8f 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 @@ -132,3 +132,26 @@ def __post_init__(self, force: bool) -> None: print(force) Fit(force=True) + + +# https://github.com/astral-sh/ruff/issues/10356 +from django.db.models import Case, Q, Value, When + + +qs.annotate( + is_foo_or_bar=Case( + When(Q(is_foo=True) | Q(is_bar=True)), + then=Value(True), + ), + default=Value(False), +) + + +# https://github.com/astral-sh/ruff/issues/10485 +from pydantic import Field +from pydantic_settings import BaseSettings + + +class Settings(BaseSettings): + + foo: bool = Field(True, exclude=True) diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs index 49f61539e8d4e4..2ff551a37b92f8 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs @@ -1,3 +1,4 @@ +use crate::settings::LinterSettings; use ruff_python_ast::{self as ast, Expr}; /// Returns `true` if a function call is allowed to use a boolean trap. @@ -43,6 +44,14 @@ pub(super) fn is_allowed_func_call(name: &str) -> bool { ) } +/// Returns `true` if a function call is allowed by the user to use a boolean trap. +pub(super) fn is_user_allowed_func_call(name: &str, settings: &LinterSettings) -> bool { + settings + .flake8_boolean_trap + .extend_allowed_calls + .contains(&name.to_string()) +} + /// Returns `true` if a function definition is allowed to use a boolean trap. pub(super) fn is_allowed_func_def(name: &str) -> bool { matches!(name, "__setitem__" | "__post_init__") @@ -51,7 +60,7 @@ pub(super) fn is_allowed_func_def(name: &str) -> bool { /// Returns `true` if an argument is allowed to use a boolean trap. To return /// `true`, the function name must be explicitly allowed, and the argument must /// be either the first or second argument in the call. -pub(super) fn allow_boolean_trap(call: &ast::ExprCall) -> bool { +pub(super) fn allow_boolean_trap(call: &ast::ExprCall, settings: &LinterSettings) -> bool { let func_name = match call.func.as_ref() { Expr::Attribute(ast::ExprAttribute { attr, .. }) => attr.as_str(), Expr::Name(ast::ExprName { id, .. }) => id.as_str(), @@ -76,5 +85,11 @@ pub(super) fn allow_boolean_trap(call: &ast::ExprCall) -> bool { } } + // If the function name is explicitly allowed by the user, then the boolean trap is + // allowed. + if is_user_allowed_func_call(func_name, settings) { + return true; + } + false } 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 ef132129d4739b..fcd69c9db678cb 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs @@ -1,6 +1,7 @@ //! Rules from [flake8-boolean-trap](https://pypi.org/project/flake8-boolean-trap/). mod helpers; pub(crate) mod rules; +pub mod settings; #[cfg(test)] mod tests { @@ -11,6 +12,7 @@ mod tests { use crate::registry::Rule; use crate::settings::types::PreviewMode; + use crate::settings::LinterSettings; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -44,4 +46,26 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test] + fn extend_allowed_callable() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_boolean_trap/FBT.py"), + &LinterSettings { + flake8_boolean_trap: super::settings::Settings { + extend_allowed_calls: vec![ + "Value".to_string(), + "Field".to_string(), + "settings".to_string(), + "deploy".to_string(), + "used".to_string(), + ], + ..super::settings::Settings::default() + }, + ..LinterSettings::for_rule(Rule::BooleanPositionalValueInCall) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs index 9d5c9697e1b260..f59e45cf6a42b4 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs @@ -46,7 +46,7 @@ impl Violation for BooleanPositionalValueInCall { } pub(crate) fn boolean_positional_value_in_call(checker: &mut Checker, call: &ast::ExprCall) { - if allow_boolean_trap(call) { + if allow_boolean_trap(call, checker.settings) { return; } for arg in call diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs new file mode 100644 index 00000000000000..e42a79c50d7ab2 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs @@ -0,0 +1,31 @@ +//! Settings for the `flake8_boolean_trap` plugin. + +use crate::display_settings; +use ruff_macros::CacheKey; +use std::fmt; + +#[derive(Debug, CacheKey)] +pub struct Settings { + pub extend_allowed_calls: Vec, +} + +impl Default for Settings { + fn default() -> Self { + Self { + extend_allowed_calls: Vec::new(), + } + } +} + +impl fmt::Display for Settings { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + display_settings! { + formatter = f, + namespace = "linter.flake8_boolean_trap", + fields = [ + self.extend_allowed_calls | array, + ] + } + Ok(()) + } +} diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap index af7c5002a07adf..40003339b8c86f 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap @@ -34,4 +34,29 @@ FBT.py:121:10: FBT003 Boolean positional value in function call | ^^^^ FBT003 | +FBT.py:144:20: FBT003 Boolean positional value in function call + | +142 | is_foo_or_bar=Case( +143 | When(Q(is_foo=True) | Q(is_bar=True)), +144 | then=Value(True), + | ^^^^ FBT003 +145 | ), +146 | default=Value(False), + | +FBT.py:146:19: FBT003 Boolean positional value in function call + | +144 | then=Value(True), +145 | ), +146 | default=Value(False), + | ^^^^^ FBT003 +147 | ) + | + +FBT.py:157:23: FBT003 Boolean positional value in function call + | +155 | class Settings(BaseSettings): +156 | +157 | foo: bool = Field(True, exclude=True) + | ^^^^ FBT003 + | diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap new file mode 100644 index 00000000000000..20c4f4ea94bc5e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs +--- + diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 99d5a481e85692..510dc3a5126346 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -16,11 +16,11 @@ use ruff_macros::CacheKey; use crate::line_width::LineLength; use crate::registry::{Linter, Rule}; use crate::rules::{ - flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins, flake8_comprehensions, - flake8_copyright, flake8_errmsg, flake8_gettext, flake8_implicit_str_concat, - flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_self, - flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming, - pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, + flake8_annotations, flake8_bandit, flake8_boolean_trap, flake8_bugbear, flake8_builtins, + flake8_comprehensions, flake8_copyright, flake8_errmsg, flake8_gettext, + flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes, + flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, + pep8_naming, pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, }; use crate::settings::types::{ExtensionMapping, FilePatternSet, PerFileIgnores, PythonVersion}; use crate::{codes, RuleSelector}; @@ -237,6 +237,7 @@ pub struct LinterSettings { // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_bandit: flake8_bandit::settings::Settings, + pub flake8_boolean_trap: flake8_boolean_trap::settings::Settings, pub flake8_bugbear: flake8_bugbear::settings::Settings, pub flake8_builtins: flake8_builtins::settings::Settings, pub flake8_comprehensions: flake8_comprehensions::settings::Settings, @@ -399,16 +400,17 @@ impl LinterSettings { typing_modules: vec![], flake8_annotations: flake8_annotations::settings::Settings::default(), flake8_bandit: flake8_bandit::settings::Settings::default(), + flake8_boolean_trap: flake8_boolean_trap::settings::Settings::default(), flake8_bugbear: flake8_bugbear::settings::Settings::default(), flake8_builtins: flake8_builtins::settings::Settings::default(), flake8_comprehensions: flake8_comprehensions::settings::Settings::default(), flake8_copyright: flake8_copyright::settings::Settings::default(), flake8_errmsg: flake8_errmsg::settings::Settings::default(), + flake8_gettext: flake8_gettext::settings::Settings::default(), flake8_implicit_str_concat: flake8_implicit_str_concat::settings::Settings::default(), flake8_import_conventions: flake8_import_conventions::settings::Settings::default(), flake8_pytest_style: flake8_pytest_style::settings::Settings::default(), flake8_quotes: flake8_quotes::settings::Settings::default(), - flake8_gettext: flake8_gettext::settings::Settings::default(), flake8_self: flake8_self::settings::Settings::default(), flake8_tidy_imports: flake8_tidy_imports::settings::Settings::default(), flake8_type_checking: flake8_type_checking::settings::Settings::default(), diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 5414515e3d7956..0745eb3d70b932 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -40,10 +40,11 @@ use ruff_python_formatter::{ }; use crate::options::{ - Flake8AnnotationsOptions, Flake8BanditOptions, Flake8BugbearOptions, Flake8BuiltinsOptions, - Flake8ComprehensionsOptions, Flake8CopyrightOptions, Flake8ErrMsgOptions, Flake8GetTextOptions, - Flake8ImplicitStrConcatOptions, Flake8ImportConventionsOptions, Flake8PytestStyleOptions, - Flake8QuotesOptions, Flake8SelfOptions, Flake8TidyImportsOptions, Flake8TypeCheckingOptions, + Flake8AnnotationsOptions, Flake8BanditOptions, Flake8BooleanTrapOptions, Flake8BugbearOptions, + Flake8BuiltinsOptions, Flake8ComprehensionsOptions, Flake8CopyrightOptions, + Flake8ErrMsgOptions, Flake8GetTextOptions, Flake8ImplicitStrConcatOptions, + Flake8ImportConventionsOptions, Flake8PytestStyleOptions, Flake8QuotesOptions, + Flake8SelfOptions, Flake8TidyImportsOptions, Flake8TypeCheckingOptions, Flake8UnusedArgumentsOptions, FormatOptions, IsortOptions, LintCommonOptions, LintOptions, McCabeOptions, Options, Pep8NamingOptions, PyUpgradeOptions, PycodestyleOptions, PydocstyleOptions, PyflakesOptions, PylintOptions, @@ -292,6 +293,10 @@ impl Configuration { .flake8_bandit .map(Flake8BanditOptions::into_settings) .unwrap_or_default(), + flake8_boolean_trap: lint + .flake8_boolean_trap + .map(Flake8BooleanTrapOptions::into_settings) + .unwrap_or_default(), flake8_bugbear: lint .flake8_bugbear .map(Flake8BugbearOptions::into_settings) @@ -609,6 +614,7 @@ pub struct LintConfiguration { // Plugins pub flake8_annotations: Option, pub flake8_bandit: Option, + pub flake8_boolean_trap: Option, pub flake8_bugbear: Option, pub flake8_builtins: Option, pub flake8_comprehensions: Option, @@ -713,6 +719,7 @@ impl LintConfiguration { // Plugins flake8_annotations: options.common.flake8_annotations, flake8_bandit: options.common.flake8_bandit, + flake8_boolean_trap: options.common.flake8_boolean_trap, flake8_bugbear: options.common.flake8_bugbear, flake8_builtins: options.common.flake8_builtins, flake8_comprehensions: options.common.flake8_comprehensions, @@ -1127,6 +1134,7 @@ impl LintConfiguration { // Plugins flake8_annotations: self.flake8_annotations.combine(config.flake8_annotations), flake8_bandit: self.flake8_bandit.combine(config.flake8_bandit), + flake8_boolean_trap: self.flake8_boolean_trap.combine(config.flake8_boolean_trap), flake8_bugbear: self.flake8_bugbear.combine(config.flake8_bugbear), flake8_builtins: self.flake8_builtins.combine(config.flake8_builtins), flake8_comprehensions: self @@ -1358,6 +1366,10 @@ fn warn_about_deprecated_top_level_lint_options( used_options.push("flake8-bandit"); } + if top_level_options.flake8_boolean_trap.is_some() { + used_options.push("flake8-boolean-trap"); + } + if top_level_options.flake8_bugbear.is_some() { used_options.push("flake8-bugbear"); } diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 3c29a5e9f44e1c..55c9e3dbb18c4e 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -809,6 +809,10 @@ pub struct LintCommonOptions { #[option_group] pub flake8_bandit: Option, + /// Options for the `flake8-boolean-trap` plugin. + #[option_group] + pub flake8_boolean_trap: Option, + /// Options for the `flake8-bugbear` plugin. #[option_group] pub flake8_bugbear: Option, @@ -1046,6 +1050,29 @@ impl Flake8BanditOptions { } } +#[derive( + Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize, OptionsMetadata, CombineOptions, +)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct Flake8BooleanTrapOptions { + /// A list of calls with which to allow a boolean trap. + #[option( + default = "[]", + value_type = "list[str]", + example = "extend-allowed-calls = [\"Field\", \"Value\"]" + )] + pub extend_allowed_calls: Option>, +} + +impl Flake8BooleanTrapOptions { + pub fn into_settings(self) -> ruff_linter::rules::flake8_boolean_trap::settings::Settings { + ruff_linter::rules::flake8_boolean_trap::settings::Settings { + extend_allowed_calls: self.extend_allowed_calls.unwrap_or_default(), + } + } +} + #[derive( Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize, OptionsMetadata, CombineOptions, )] diff --git a/ruff.schema.json b/ruff.schema.json index b3e88111c7f730..efd4b2a6f5b87f 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -226,6 +226,18 @@ } ] }, + "flake8-boolean-trap": { + "description": "Options for the `flake8-boolean-trap` plugin.", + "deprecated": true, + "anyOf": [ + { + "$ref": "#/definitions/Flake8BooleanTrapOptions" + }, + { + "type": "null" + } + ] + }, "flake8-bugbear": { "description": "Options for the `flake8-bugbear` plugin.", "deprecated": true, @@ -894,6 +906,22 @@ }, "additionalProperties": false }, + "Flake8BooleanTrapOptions": { + "type": "object", + "properties": { + "extend-allowed-calls": { + "description": "A list of calls with which to allow a boolean trap.", + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + }, "Flake8BugbearOptions": { "type": "object", "properties": { @@ -1914,6 +1942,17 @@ } ] }, + "flake8-boolean-trap": { + "description": "Options for the `flake8-boolean-trap` plugin.", + "anyOf": [ + { + "$ref": "#/definitions/Flake8BooleanTrapOptions" + }, + { + "type": "null" + } + ] + }, "flake8-bugbear": { "description": "Options for the `flake8-bugbear` plugin.", "anyOf": [ From deefdd921fc850d44bf2ced169f0f0bc0d30cd05 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Fri, 22 Mar 2024 20:40:59 -0400 Subject: [PATCH 2/5] update rule docs --- .../rules/boolean_positional_value_in_call.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs index f59e45cf6a42b4..6d8abf13360143 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs @@ -9,6 +9,9 @@ use crate::rules::flake8_boolean_trap::helpers::allow_boolean_trap; /// ## What it does /// Checks for boolean positional arguments in function calls. /// +/// Some functions are whitelisted by default. To extend the list of allowed calls +/// configure the [`lint.flake8-boolean-trap.extend-allowed-calls`] option. +/// /// ## Why is this bad? /// Calling a function with boolean positional arguments is confusing as the /// meaning of the boolean value is not clear to the caller, and to future @@ -32,6 +35,9 @@ use crate::rules::flake8_boolean_trap::helpers::allow_boolean_trap; /// func(flag=True) /// ``` /// +/// ## Options +/// - `lint.flake8-boolean-trap.extend-allowed-calls` +/// /// ## 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/) From a1e9c434b5d32df082cfd2e6c725183ef8d04735 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Sat, 23 Mar 2024 00:34:54 -0400 Subject: [PATCH 3/5] clippy --- .../ruff_linter/src/rules/flake8_boolean_trap/mod.rs | 1 - .../src/rules/flake8_boolean_trap/settings.rs | 10 +--------- 2 files changed, 1 insertion(+), 10 deletions(-) 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 fcd69c9db678cb..37ccc86bbc7a86 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs @@ -60,7 +60,6 @@ mod tests { "deploy".to_string(), "used".to_string(), ], - ..super::settings::Settings::default() }, ..LinterSettings::for_rule(Rule::BooleanPositionalValueInCall) }, diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs index e42a79c50d7ab2..b04f79c2df7031 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs @@ -4,19 +4,11 @@ use crate::display_settings; use ruff_macros::CacheKey; use std::fmt; -#[derive(Debug, CacheKey)] +#[derive(Debug, CacheKey, Default)] pub struct Settings { pub extend_allowed_calls: Vec, } -impl Default for Settings { - fn default() -> Self { - Self { - extend_allowed_calls: Vec::new(), - } - } -} - impl fmt::Display for Settings { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { display_settings! { From ecc1132454ef498cb5ba60949fe32b7ae3a606c1 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Wed, 27 Mar 2024 23:32:49 -0400 Subject: [PATCH 4/5] use symbol paths instead of functions names --- .../src/rules/flake8_boolean_trap/helpers.rs | 30 ++++++++++++------ .../src/rules/flake8_boolean_trap/mod.rs | 7 ++--- .../rules/boolean_positional_value_in_call.rs | 2 +- ..._trap__tests__extend_allowed_callable.snap | 31 +++++++++++++++++++ crates/ruff_workspace/src/options.rs | 7 +++-- ruff.schema.json | 2 +- 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs index 2ff551a37b92f8..9f778c73f248ea 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs @@ -1,4 +1,5 @@ -use crate::settings::LinterSettings; +use crate::checkers::ast::Checker; +use ruff_python_ast::name::QualifiedName; use ruff_python_ast::{self as ast, Expr}; /// Returns `true` if a function call is allowed to use a boolean trap. @@ -44,12 +45,24 @@ pub(super) fn is_allowed_func_call(name: &str) -> bool { ) } -/// Returns `true` if a function call is allowed by the user to use a boolean trap. -pub(super) fn is_user_allowed_func_call(name: &str, settings: &LinterSettings) -> bool { - settings +/// Returns `true` if a call is allowed by the user to use a boolean trap. +pub(super) fn is_user_allowed_func_call(call: &ast::ExprCall, checker: &Checker) -> bool { + let extend_immutable_calls: Vec = checker + .settings .flake8_boolean_trap .extend_allowed_calls - .contains(&name.to_string()) + .iter() + .map(|target| QualifiedName::from_dotted_name(target)) + .collect(); + + checker + .semantic() + .resolve_qualified_name(call.func.as_ref()) + .is_some_and(|qualified_name| { + extend_immutable_calls + .iter() + .any(|target| qualified_name == *target) + }) } /// Returns `true` if a function definition is allowed to use a boolean trap. @@ -60,7 +73,7 @@ pub(super) fn is_allowed_func_def(name: &str) -> bool { /// Returns `true` if an argument is allowed to use a boolean trap. To return /// `true`, the function name must be explicitly allowed, and the argument must /// be either the first or second argument in the call. -pub(super) fn allow_boolean_trap(call: &ast::ExprCall, settings: &LinterSettings) -> bool { +pub(super) fn allow_boolean_trap(call: &ast::ExprCall, checker: &Checker) -> bool { let func_name = match call.func.as_ref() { Expr::Attribute(ast::ExprAttribute { attr, .. }) => attr.as_str(), Expr::Name(ast::ExprName { id, .. }) => id.as_str(), @@ -85,9 +98,8 @@ pub(super) fn allow_boolean_trap(call: &ast::ExprCall, settings: &LinterSettings } } - // If the function name is explicitly allowed by the user, then the boolean trap is - // allowed. - if is_user_allowed_func_call(func_name, settings) { + // If the call is explicitly allowed by the user, then the boolean trap is allowed. + if is_user_allowed_func_call(call, checker) { return true; } 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 37ccc86bbc7a86..80c81bb3f4654c 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs @@ -54,11 +54,8 @@ mod tests { &LinterSettings { flake8_boolean_trap: super::settings::Settings { extend_allowed_calls: vec![ - "Value".to_string(), - "Field".to_string(), - "settings".to_string(), - "deploy".to_string(), - "used".to_string(), + "django.db.models.Value".to_string(), + "pydantic.Field".to_string(), ], }, ..LinterSettings::for_rule(Rule::BooleanPositionalValueInCall) diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs index 6d8abf13360143..e89b91c4929fd3 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_positional_value_in_call.rs @@ -52,7 +52,7 @@ impl Violation for BooleanPositionalValueInCall { } pub(crate) fn boolean_positional_value_in_call(checker: &mut Checker, call: &ast::ExprCall) { - if allow_boolean_trap(call, checker.settings) { + if allow_boolean_trap(call, checker) { return; } for arg in call diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap index 20c4f4ea94bc5e..9e80fe6c4da1dd 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap @@ -1,4 +1,35 @@ --- source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs --- +FBT.py:42:11: FBT003 Boolean positional value in function call + | +42 | used("a", True) + | ^^^^ FBT003 +43 | used(do=True) + | +FBT.py:57:11: FBT003 Boolean positional value in function call + | +55 | {}.pop(True, False) +56 | dict.fromkeys(("world",), True) +57 | {}.deploy(True, False) + | ^^^^ FBT003 +58 | getattr(someobj, attrname, False) +59 | mylist.index(True) + | + +FBT.py:57:17: FBT003 Boolean positional value in function call + | +55 | {}.pop(True, False) +56 | dict.fromkeys(("world",), True) +57 | {}.deploy(True, False) + | ^^^^^ FBT003 +58 | getattr(someobj, attrname, False) +59 | mylist.index(True) + | + +FBT.py:121:10: FBT003 Boolean positional value in function call + | +121 | settings(True) + | ^^^^ FBT003 + | diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 55c9e3dbb18c4e..bfa28301231bc5 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -1056,11 +1056,14 @@ impl Flake8BanditOptions { #[serde(deny_unknown_fields, rename_all = "kebab-case")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub struct Flake8BooleanTrapOptions { - /// A list of calls with which to allow a boolean trap. + /// Additional callable functions with which to allow boolean traps. + /// + /// Expects to receive a list of fully-qualified names (e.g., `pydantic.Field`, rather than + /// `Field`). #[option( default = "[]", value_type = "list[str]", - example = "extend-allowed-calls = [\"Field\", \"Value\"]" + example = "extend-allowed-calls = [\"pydantic.Field\", \"django.db.models.Value\"]" )] pub extend_allowed_calls: Option>, } diff --git a/ruff.schema.json b/ruff.schema.json index efd4b2a6f5b87f..3e327a5cd781df 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -910,7 +910,7 @@ "type": "object", "properties": { "extend-allowed-calls": { - "description": "A list of calls with which to allow a boolean trap.", + "description": "Additional callable functions with which to allow boolean traps.\n\nExpects to receive a list of fully-qualified names (e.g., `pydantic.Field`, rather than `Field`).", "type": [ "array", "null" From 18ab69e00b1d8186a960f193ce2d8df6915304cf Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 29 Mar 2024 20:17:50 -0400 Subject: [PATCH 5/5] Minor tweaks --- .../test/fixtures/flake8_boolean_trap/FBT.py | 5 +-- .../src/rules/flake8_boolean_trap/helpers.rs | 31 ++++++++------- .../src/rules/flake8_boolean_trap/settings.rs | 8 ++-- ...e8_boolean_trap__tests__FBT001_FBT.py.snap | 10 ++--- ...e8_boolean_trap__tests__FBT003_FBT.py.snap | 39 +++++++++---------- ..._trap__tests__extend_allowed_callable.snap | 34 ++++++++-------- ...n_trap__tests__preview__FBT001_FBT.py.snap | 22 +++++------ 7 files changed, 73 insertions(+), 76 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 d3e963c4759c8f..32074fb46765b2 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 @@ -31,8 +31,7 @@ def function( kwonly_nonboolvalued_boolhint: bool = 1, kwonly_nonboolvalued_boolstrhint: "bool" = 1, **kw, -): - ... +): ... def used(do): @@ -131,6 +130,7 @@ class Fit: def __post_init__(self, force: bool) -> None: print(force) + Fit(force=True) @@ -153,5 +153,4 @@ def __post_init__(self, force: bool) -> None: class Settings(BaseSettings): - foo: bool = Field(True, exclude=True) diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs index 9f778c73f248ea..62e6967f6243af 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs @@ -1,6 +1,9 @@ -use crate::checkers::ast::Checker; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::SemanticModel; + +use crate::checkers::ast::Checker; +use crate::settings::LinterSettings; /// Returns `true` if a function call is allowed to use a boolean trap. pub(super) fn is_allowed_func_call(name: &str) -> bool { @@ -46,22 +49,20 @@ pub(super) fn is_allowed_func_call(name: &str) -> bool { } /// Returns `true` if a call is allowed by the user to use a boolean trap. -pub(super) fn is_user_allowed_func_call(call: &ast::ExprCall, checker: &Checker) -> bool { - let extend_immutable_calls: Vec = checker - .settings - .flake8_boolean_trap - .extend_allowed_calls - .iter() - .map(|target| QualifiedName::from_dotted_name(target)) - .collect(); - - checker - .semantic() +pub(super) fn is_user_allowed_func_call( + call: &ast::ExprCall, + semantic: &SemanticModel, + settings: &LinterSettings, +) -> bool { + semantic .resolve_qualified_name(call.func.as_ref()) .is_some_and(|qualified_name| { - extend_immutable_calls + settings + .flake8_boolean_trap + .extend_allowed_calls .iter() - .any(|target| qualified_name == *target) + .map(|target| QualifiedName::from_dotted_name(target)) + .any(|target| qualified_name == target) }) } @@ -99,7 +100,7 @@ pub(super) fn allow_boolean_trap(call: &ast::ExprCall, checker: &Checker) -> boo } // If the call is explicitly allowed by the user, then the boolean trap is allowed. - if is_user_allowed_func_call(call, checker) { + if is_user_allowed_func_call(call, checker.semantic(), checker.settings) { return true; } diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs index b04f79c2df7031..3b88395c9847a1 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs @@ -1,9 +1,11 @@ -//! Settings for the `flake8_boolean_trap` plugin. +//! Settings for the `flake8-boolean-trap` plugin. -use crate::display_settings; -use ruff_macros::CacheKey; use std::fmt; +use ruff_macros::CacheKey; + +use crate::display_settings; + #[derive(Debug, CacheKey, Default)] pub struct Settings { pub extend_allowed_calls: Vec, 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 fe22f4c481a216..c14a748e79d6c8 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 @@ -81,12 +81,10 @@ FBT.py:19:5: FBT001 Boolean-typed positional argument in function definition 21 | kwonly_nonvalued_nohint, | -FBT.py:91:19: FBT001 Boolean-typed positional argument in function definition +FBT.py:90:19: FBT001 Boolean-typed positional argument in function definition | -90 | # FBT001: Boolean positional arg in function definition -91 | def foo(self, value: bool) -> None: +89 | # FBT001: Boolean positional arg in function definition +90 | def foo(self, value: bool) -> None: | ^^^^^ FBT001 -92 | pass +91 | pass | - - diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap index 40003339b8c86f..4985aa37f2c1da 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__FBT003_FBT.py.snap @@ -1,36 +1,36 @@ --- source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs --- -FBT.py:42:11: FBT003 Boolean positional value in function call +FBT.py:41:11: FBT003 Boolean positional value in function call | -42 | used("a", True) +41 | used("a", True) | ^^^^ FBT003 -43 | used(do=True) +42 | used(do=True) | -FBT.py:57:11: FBT003 Boolean positional value in function call +FBT.py:56:11: FBT003 Boolean positional value in function call | -55 | {}.pop(True, False) -56 | dict.fromkeys(("world",), True) -57 | {}.deploy(True, False) +54 | {}.pop(True, False) +55 | dict.fromkeys(("world",), True) +56 | {}.deploy(True, False) | ^^^^ FBT003 -58 | getattr(someobj, attrname, False) -59 | mylist.index(True) +57 | getattr(someobj, attrname, False) +58 | mylist.index(True) | -FBT.py:57:17: FBT003 Boolean positional value in function call +FBT.py:56:17: FBT003 Boolean positional value in function call | -55 | {}.pop(True, False) -56 | dict.fromkeys(("world",), True) -57 | {}.deploy(True, False) +54 | {}.pop(True, False) +55 | dict.fromkeys(("world",), True) +56 | {}.deploy(True, False) | ^^^^^ FBT003 -58 | getattr(someobj, attrname, False) -59 | mylist.index(True) +57 | getattr(someobj, attrname, False) +58 | mylist.index(True) | -FBT.py:121:10: FBT003 Boolean positional value in function call +FBT.py:120:10: FBT003 Boolean positional value in function call | -121 | settings(True) +120 | settings(True) | ^^^^ FBT003 | @@ -53,10 +53,9 @@ FBT.py:146:19: FBT003 Boolean positional value in function call 147 | ) | -FBT.py:157:23: FBT003 Boolean positional value in function call +FBT.py:156:23: FBT003 Boolean positional value in function call | 155 | class Settings(BaseSettings): -156 | -157 | foo: bool = Field(True, exclude=True) +156 | foo: bool = Field(True, exclude=True) | ^^^^ FBT003 | diff --git a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap index 9e80fe6c4da1dd..71b3e8df8d31d0 100644 --- a/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap +++ b/crates/ruff_linter/src/rules/flake8_boolean_trap/snapshots/ruff_linter__rules__flake8_boolean_trap__tests__extend_allowed_callable.snap @@ -1,35 +1,35 @@ --- source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs --- -FBT.py:42:11: FBT003 Boolean positional value in function call +FBT.py:41:11: FBT003 Boolean positional value in function call | -42 | used("a", True) +41 | used("a", True) | ^^^^ FBT003 -43 | used(do=True) +42 | used(do=True) | -FBT.py:57:11: FBT003 Boolean positional value in function call +FBT.py:56:11: FBT003 Boolean positional value in function call | -55 | {}.pop(True, False) -56 | dict.fromkeys(("world",), True) -57 | {}.deploy(True, False) +54 | {}.pop(True, False) +55 | dict.fromkeys(("world",), True) +56 | {}.deploy(True, False) | ^^^^ FBT003 -58 | getattr(someobj, attrname, False) -59 | mylist.index(True) +57 | getattr(someobj, attrname, False) +58 | mylist.index(True) | -FBT.py:57:17: FBT003 Boolean positional value in function call +FBT.py:56:17: FBT003 Boolean positional value in function call | -55 | {}.pop(True, False) -56 | dict.fromkeys(("world",), True) -57 | {}.deploy(True, False) +54 | {}.pop(True, False) +55 | dict.fromkeys(("world",), True) +56 | {}.deploy(True, False) | ^^^^^ FBT003 -58 | getattr(someobj, attrname, False) -59 | mylist.index(True) +57 | getattr(someobj, attrname, False) +58 | mylist.index(True) | -FBT.py:121:10: FBT003 Boolean positional value in function call +FBT.py:120:10: FBT003 Boolean positional value in function call | -121 | settings(True) +120 | settings(True) | ^^^^ FBT003 | 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 index 1f892a4d165330..ee91fa49f86587 100644 --- 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 @@ -81,26 +81,24 @@ FBT.py:19:5: FBT001 Boolean-typed positional argument in function definition 21 | kwonly_nonvalued_nohint, | -FBT.py:91:19: FBT001 Boolean-typed positional argument in function definition +FBT.py:90:19: FBT001 Boolean-typed positional argument in function definition | -90 | # FBT001: Boolean positional arg in function definition -91 | def foo(self, value: bool) -> None: +89 | # FBT001: Boolean positional arg in function definition +90 | def foo(self, value: bool) -> None: | ^^^^^ FBT001 -92 | pass +91 | pass | -FBT.py:101:10: FBT001 Boolean-typed positional argument in function definition +FBT.py:100:10: FBT001 Boolean-typed positional argument in function definition | -101 | def func(x: Union[list, Optional[int | str | float | bool]]): +100 | def func(x: Union[list, Optional[int | str | float | bool]]): | ^ FBT001 -102 | pass +101 | pass | -FBT.py:105:10: FBT001 Boolean-typed positional argument in function definition +FBT.py:104:10: FBT001 Boolean-typed positional argument in function definition | -105 | def func(x: bool | str): +104 | def func(x: bool | str): | ^ FBT001 -106 | pass +105 | pass | - -