Skip to content

Commit

Permalink
Allow Boolean positionals in setters (#9429)
Browse files Browse the repository at this point in the history
## Summary

Ignores Boolean trap enforcement for methods that appear to be setters
(as in the Qt and pygame APIs).

Closes #9287.
Closes #8923.
  • Loading branch information
charliermarsh committed Jan 8, 2024
1 parent 94968fe commit f419af4
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ def used(do):
bar.is_not(False)
next(iter([]), False)
sa.func.coalesce(tbl.c.valid, False)
setVisible(True)
set_visible(True)


class Registry:
Expand Down Expand Up @@ -114,3 +116,6 @@ def func(x: int | str):
@override
def func(x: bool):
pass


settings(True)
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
);
}
if checker.enabled(Rule::BooleanPositionalValueInCall) {
flake8_boolean_trap::rules::boolean_positional_value_in_call(checker, args, func);
flake8_boolean_trap::rules::boolean_positional_value_in_call(checker, call);
}
if checker.enabled(Rule::Debugger) {
flake8_debugger::rules::debugger_call(checker, expr, func);
Expand Down
26 changes: 21 additions & 5 deletions crates/ruff_linter/src/rules/flake8_boolean_trap/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,29 @@ 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(func: &Expr) -> bool {
if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func {
return is_allowed_func_call(attr);
pub(super) fn allow_boolean_trap(call: &ast::ExprCall) -> 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(),
_ => return false,
};

// If the function name is explicitly allowed, then the boolean trap is
// allowed.
if is_allowed_func_call(func_name) {
return true;
}

if let Expr::Name(ast::ExprName { id, .. }) = func {
return is_allowed_func_call(id);
// If the function appears to be a setter (e.g., `set_visible` or `setVisible`), then the
// boolean trap is allowed. We want to avoid raising a violation for cases in which the argument
// is positional-only and third-party, and this tends to be the case for setters.
if call.arguments.args.len() == 1 {
if func_name
.strip_prefix("set")
.is_some_and(|suffix| suffix.starts_with(|c: char| c == '_' || c.is_ascii_uppercase()))
{
return true;
}
}

false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Expr;
use ruff_python_ast as ast;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -45,11 +45,16 @@ impl Violation for BooleanPositionalValueInCall {
}
}

pub(crate) fn boolean_positional_value_in_call(checker: &mut Checker, args: &[Expr], func: &Expr) {
if allow_boolean_trap(func) {
pub(crate) fn boolean_positional_value_in_call(checker: &mut Checker, call: &ast::ExprCall) {
if allow_boolean_trap(call) {
return;
}
for arg in args.iter().filter(|arg| arg.is_boolean_literal_expr()) {
for arg in call
.arguments
.args
.iter()
.filter(|arg| arg.is_boolean_literal_expr())
{
checker
.diagnostics
.push(Diagnostic::new(BooleanPositionalValueInCall, arg.range()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ FBT.py:19:5: FBT001 Boolean-typed positional argument in function definition
21 | kwonly_nonvalued_nohint,
|

FBT.py:89:19: FBT001 Boolean-typed positional argument in function definition
FBT.py:91:19: FBT001 Boolean-typed positional argument in function definition
|
88 | # FBT001: Boolean positional arg in function definition
89 | def foo(self, value: bool) -> None:
90 | # FBT001: Boolean positional arg in function definition
91 | def foo(self, value: bool) -> None:
| ^^^^^ FBT001
90 | pass
92 | pass
|


Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@ FBT.py:57:17: FBT003 Boolean positional value in function call
59 | mylist.index(True)
|

FBT.py:69:38: FBT003 Boolean positional value in function call
|
67 | os.set_blocking(0, False)
68 | g_action.set_enabled(True)
69 | settings.set_enable_developer_extras(True)
| ^^^^ FBT003
70 | foo.is_(True)
71 | bar.is_not(False)
|
FBT.py:121:10: FBT003 Boolean positional value in function call
|
121 | settings(True)
| ^^^^ FBT003
|


Original file line number Diff line number Diff line change
Expand Up @@ -81,26 +81,26 @@ FBT.py:19:5: FBT001 Boolean-typed positional argument in function definition
21 | kwonly_nonvalued_nohint,
|

FBT.py:89:19: FBT001 Boolean-typed positional argument in function definition
FBT.py:91:19: FBT001 Boolean-typed positional argument in function definition
|
88 | # FBT001: Boolean positional arg in function definition
89 | def foo(self, value: bool) -> None:
90 | # FBT001: Boolean positional arg in function definition
91 | def foo(self, value: bool) -> None:
| ^^^^^ FBT001
90 | pass
92 | pass
|

FBT.py:99:10: FBT001 Boolean-typed positional argument in function definition
FBT.py:101:10: FBT001 Boolean-typed positional argument in function definition
|
99 | def func(x: Union[list, Optional[int | str | float | bool]]):
101 | def func(x: Union[list, Optional[int | str | float | bool]]):
| ^ FBT001
100 | pass
102 | pass
|

FBT.py:103:10: FBT001 Boolean-typed positional argument in function definition
FBT.py:105:10: FBT001 Boolean-typed positional argument in function definition
|
103 | def func(x: bool | str):
105 | def func(x: bool | str):
| ^ FBT001
104 | pass
106 | pass
|


0 comments on commit f419af4

Please sign in to comment.