[flake8-boolean-trap] Add setting for user defined allowed boolean trap (#10531)

## Summary

Add a setting `extend-allowed-calls` to allow users to define their own
list of calls which allow boolean traps.

Resolves #10485.
Resolves #10356.

## Test Plan

Extended text fixture and added setting test.
This commit is contained in:
Auguste Lalande 2024-03-29 20:26:12 -04:00 committed by GitHub
parent 9f56902719
commit 3c48913473
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 288 additions and 49 deletions

View file

@ -31,8 +31,7 @@ def function(
kwonly_nonboolvalued_boolhint: bool = 1,
kwonly_nonboolvalued_boolstrhint: "bool" = 1,
**kw,
):
...
): ...
def used(do):
@ -131,4 +130,27 @@ class Fit:
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)

View file

@ -1,4 +1,9 @@
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 {
@ -43,6 +48,24 @@ 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,
semantic: &SemanticModel,
settings: &LinterSettings,
) -> bool {
semantic
.resolve_qualified_name(call.func.as_ref())
.is_some_and(|qualified_name| {
settings
.flake8_boolean_trap
.extend_allowed_calls
.iter()
.map(|target| QualifiedName::from_dotted_name(target))
.any(|target| qualified_name == target)
})
}
/// 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 +74,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, 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(),
@ -76,5 +99,10 @@ pub(super) fn allow_boolean_trap(call: &ast::ExprCall) -> bool {
}
}
// If the call is explicitly allowed by the user, then the boolean trap is allowed.
if is_user_allowed_func_call(call, checker.semantic(), checker.settings) {
return true;
}
false
}

View file

@ -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,22 @@ 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![
"django.db.models.Value".to_string(),
"pydantic.Field".to_string(),
],
},
..LinterSettings::for_rule(Rule::BooleanPositionalValueInCall)
},
)?;
assert_messages!(diagnostics);
Ok(())
}
}

View file

@ -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/)
@ -46,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) {
if allow_boolean_trap(call, checker) {
return;
}
for arg in call

View file

@ -0,0 +1,25 @@
//! Settings for the `flake8-boolean-trap` plugin.
use std::fmt;
use ruff_macros::CacheKey;
use crate::display_settings;
#[derive(Debug, CacheKey, Default)]
pub struct Settings {
pub extend_allowed_calls: Vec<String>,
}
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(())
}
}

View file

@ -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
|

View file

@ -1,37 +1,61 @@
---
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
|
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:156:23: FBT003 Boolean positional value in function call
|
155 | class Settings(BaseSettings):
156 | foo: bool = Field(True, exclude=True)
| ^^^^ FBT003
|

View file

@ -0,0 +1,35 @@
---
source: crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs
---
FBT.py:41:11: FBT003 Boolean positional value in function call
|
41 | used("a", True)
| ^^^^ FBT003
42 | used(do=True)
|
FBT.py:56:11: FBT003 Boolean positional value in function call
|
54 | {}.pop(True, False)
55 | dict.fromkeys(("world",), True)
56 | {}.deploy(True, False)
| ^^^^ FBT003
57 | getattr(someobj, attrname, False)
58 | mylist.index(True)
|
FBT.py:56:17: FBT003 Boolean positional value in function call
|
54 | {}.pop(True, False)
55 | dict.fromkeys(("world",), True)
56 | {}.deploy(True, False)
| ^^^^^ FBT003
57 | getattr(someobj, attrname, False)
58 | mylist.index(True)
|
FBT.py:120:10: FBT003 Boolean positional value in function call
|
120 | settings(True)
| ^^^^ FBT003
|

View file

@ -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
|

View file

@ -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(),

View file

@ -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<Flake8AnnotationsOptions>,
pub flake8_bandit: Option<Flake8BanditOptions>,
pub flake8_boolean_trap: Option<Flake8BooleanTrapOptions>,
pub flake8_bugbear: Option<Flake8BugbearOptions>,
pub flake8_builtins: Option<Flake8BuiltinsOptions>,
pub flake8_comprehensions: Option<Flake8ComprehensionsOptions>,
@ -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");
}

View file

@ -809,6 +809,10 @@ pub struct LintCommonOptions {
#[option_group]
pub flake8_bandit: Option<Flake8BanditOptions>,
/// Options for the `flake8-boolean-trap` plugin.
#[option_group]
pub flake8_boolean_trap: Option<Flake8BooleanTrapOptions>,
/// Options for the `flake8-bugbear` plugin.
#[option_group]
pub flake8_bugbear: Option<Flake8BugbearOptions>,
@ -1046,6 +1050,32 @@ 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 {
/// 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 = [\"pydantic.Field\", \"django.db.models.Value\"]"
)]
pub extend_allowed_calls: Option<Vec<String>>,
}
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,
)]

39
ruff.schema.json generated
View file

@ -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": "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"
],
"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": [