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 4 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 @@ -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)
29 changes: 28 additions & 1 deletion crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
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.
Expand Down Expand Up @@ -43,6 +45,26 @@ 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<QualifiedName> = checker
.settings
.flake8_boolean_trap
.extend_allowed_calls
.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.
pub(super) fn is_allowed_func_def(name: &str) -> bool {
matches!(name, "__setitem__" | "__post_init__")
Expand All @@ -51,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) -> 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 +98,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) {
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
23 changes: 23 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,23 @@
//! Settings for the `flake8_boolean_trap` plugin.

use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt;

#[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 @@ -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
|
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: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
|
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
20 changes: 16 additions & 4 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
Expand Down
30 changes: 30 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
)]
Expand Down
Loading
Loading