Skip to content

Commit

Permalink
Ignore setters in flake8-boolean-trap (#3092)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 21, 2023
1 parent 37df07d commit d9fd78d
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,11 @@ def __init__(self) -> None:
# FBT001: Boolean positional arg in function definition
def __setitem__(self, switch: Switch, value: bool) -> None:
self._switches[switch.value] = value

@foo.setter
def foo(self, value: bool) -> None:
pass

# FBT001: Boolean positional arg in function definition
def foo(self, value: bool) -> None:
pass
12 changes: 10 additions & 2 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,12 @@ where
.rules
.enabled(&Rule::BooleanPositionalArgInFunctionDefinition)
{
flake8_boolean_trap::rules::check_positional_boolean_in_def(self, name, args);
flake8_boolean_trap::rules::check_positional_boolean_in_def(
self,
name,
decorator_list,
args,
);
}

if self
Expand All @@ -715,7 +720,10 @@ where
.enabled(&Rule::BooleanDefaultValueInFunctionDefinition)
{
flake8_boolean_trap::rules::check_boolean_default_value_in_function_definition(
self, name, args,
self,
name,
decorator_list,
args,
);
}

Expand Down
28 changes: 26 additions & 2 deletions crates/ruff/src/rules/flake8_boolean_trap/rules.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use ruff_macros::{define_violation, derive_message_formats};
use rustpython_parser::ast::{Arguments, Constant, Expr, ExprKind};

use ruff_macros::{define_violation, derive_message_formats};

use crate::ast::helpers::collect_call_path;
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::{Diagnostic, DiagnosticKind};
Expand Down Expand Up @@ -91,10 +93,23 @@ fn add_if_boolean(checker: &mut Checker, arg: &Expr, kind: DiagnosticKind) {
}
}

pub fn check_positional_boolean_in_def(checker: &mut Checker, name: &str, arguments: &Arguments) {
pub fn check_positional_boolean_in_def(
checker: &mut Checker,
name: &str,
decorator_list: &[Expr],
arguments: &Arguments,
) {
if FUNC_DEF_NAME_ALLOWLIST.contains(&name) {
return;
}

if decorator_list.iter().any(|expr| {
let call_path = collect_call_path(expr);
call_path.as_slice() == [name, "setter"]
}) {
return;
}

for arg in arguments.posonlyargs.iter().chain(arguments.args.iter()) {
if arg.node.annotation.is_none() {
continue;
Expand Down Expand Up @@ -125,11 +140,20 @@ pub fn check_positional_boolean_in_def(checker: &mut Checker, name: &str, argume
pub fn check_boolean_default_value_in_function_definition(
checker: &mut Checker,
name: &str,
decorator_list: &[Expr],
arguments: &Arguments,
) {
if FUNC_DEF_NAME_ALLOWLIST.contains(&name) {
return;
}

if decorator_list.iter().any(|expr| {
let call_path = collect_call_path(expr);
call_path.as_slice() == [name, "setter"]
}) {
return;
}

for arg in &arguments.defaults {
add_if_boolean(checker, arg, BooleanDefaultValueInFunctionDefinition.into());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: src/rules/flake8_boolean_trap/mod.rs
source: crates/ruff/src/rules/flake8_boolean_trap/mod.rs
expression: diagnostics
---
- kind:
Expand Down Expand Up @@ -82,4 +82,14 @@ expression: diagnostics
column: 45
fix: ~
parent: ~
- kind:
BooleanPositionalArgInFunctionDefinition: ~
location:
row: 77
column: 18
end_location:
row: 77
column: 29
fix: ~
parent: ~

7 changes: 2 additions & 5 deletions crates/ruff/src/visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,8 @@ fn method_visibility(stmt: &Stmt) -> Visibility {
// Is this a setter or deleter?
if decorator_list.iter().any(|expr| {
let call_path = collect_call_path(expr);
if call_path.len() > 1 {
call_path[0] == name
} else {
false
}
call_path.as_slice() == [name, "setter"]
|| call_path.as_slice() == [name, "deleter"]
}) {
return Visibility::Private;
}
Expand Down

0 comments on commit d9fd78d

Please sign in to comment.