Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading