Skip to content

Commit

Permalink
[flake8-boolean-trap] Add setting for user defined allowed boolean …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
augustelalande authored Mar 30, 2024
1 parent 9f56902 commit 3c48913
Show file tree
Hide file tree
Showing 13 changed files with 288 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ def function(
kwonly_nonboolvalued_boolhint: bool = 1,
kwonly_nonboolvalued_boolstrhint: "bool" = 1,
**kw,
):
...
): ...


def used(do):
Expand Down Expand Up @@ -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)
30 changes: 29 additions & 1 deletion crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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__")
Expand All @@ -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(),
Expand All @@ -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
}
20 changes: 20 additions & 0 deletions crates/ruff_linter/src/rules/flake8_boolean_trap/mod.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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};

Expand Down Expand Up @@ -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(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/)
Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions crates/ruff_linter/src/rules/flake8_boolean_trap/settings.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
|


Original file line number Diff line number Diff line change
@@ -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
|
Original file line number Diff line number Diff line change
@@ -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
|
Original file line number Diff line number Diff line change
Expand Up @@ -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
|


14 changes: 8 additions & 6 deletions crates/ruff_linter/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
Loading

0 comments on commit 3c48913

Please sign in to comment.