From 5812eb09f857e47775285c58a91e0d28f3be62b5 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sat, 20 Sep 2025 02:40:20 +0900 Subject: [PATCH 1/2] fix B004 to skip invalid hasattr/getattr calls --- .../test/fixtures/flake8_bugbear/B004.py | 18 ++++++++++++++++++ .../src/checkers/ast/analyze/expression.rs | 4 +++- .../rules/unreliable_callable_check.rs | 17 ++++++++++++++++- ...s__flake8_bugbear__tests__B004_B004.py.snap | 2 ++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py index b064101ed4cd36..92ec34823340b2 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py @@ -52,3 +52,21 @@ def __init__(self): self.__call__ = None assert hasattr(A(), "__call__") assert callable(A()) is False + +# https://github.com/astral-sh/ruff/issues/20440 +def test_invalid_hasattr_calls(): + hasattr(0, "__call__", 0) # 3 args - invalid + hasattr(0, "__call__", x=0) # keyword arg - invalid + hasattr(0, "__call__", 0, x=0) # 3 args + keyword - invalid + hasattr() # no args - invalid + hasattr(0) # 1 arg - invalid + hasattr(*(), "__call__", "extra") # unpacking - invalid + hasattr(*()) # unpacking - invalid + +def test_invalid_getattr_calls(): + getattr(0, "__call__", None, "extra") # 4 args - invalid + getattr(0, "__call__", default=None) # keyword arg - invalid + getattr() # no args - invalid + getattr(0) # 1 arg - invalid + getattr(*(), "__call__", None, "extra") # unpacking - invalid + getattr(*()) # unpacking - invalid diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 97dc1052dbbc05..26ea1cf986e866 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -717,7 +717,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { flake8_bugbear::rules::re_sub_positional_args(checker, call); } if checker.is_rule_enabled(Rule::UnreliableCallableCheck) { - flake8_bugbear::rules::unreliable_callable_check(checker, expr, func, args); + flake8_bugbear::rules::unreliable_callable_check( + checker, expr, func, args, keywords, + ); } if checker.is_rule_enabled(Rule::StripWithMultiCharacters) { flake8_bugbear::rules::strip_with_multi_characters(checker, expr, func, args); diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index b432ed369d2b3b..d60722867850c1 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -90,6 +90,7 @@ pub(crate) fn unreliable_callable_check( expr: &Expr, func: &Expr, args: &[Expr], + keywords: &[ast::Keyword], ) { let [obj, attr, ..] = args else { return; @@ -103,7 +104,21 @@ pub(crate) fn unreliable_callable_check( let Some(builtins_function) = checker.semantic().resolve_builtin_symbol(func) else { return; }; - if !matches!(builtins_function, "hasattr" | "getattr") { + + // Validate function arguments based on function name + let valid_args = match builtins_function { + "hasattr" => { + // hasattr should have exactly 2 positional arguments and no keywords + args.len() == 2 && keywords.is_empty() + } + "getattr" => { + // getattr should have 2 or 3 positional arguments and no keywords + (args.len() == 2 || args.len() == 3) && keywords.is_empty() + } + _ => return, + }; + + if !valid_args { return; } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap index 8170d5bcad622e..c6402908179719 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap @@ -156,4 +156,6 @@ help: Replace with `callable()` - assert hasattr(A(), "__call__") 53 + assert callable(A()) 54 | assert callable(A()) is False +55 | +56 | # https://github.com/astral-sh/ruff/issues/20440 note: This is an unsafe fix and may change runtime behavior From 7e8bf6cae77704892d2a729f43154722e1b2ca4e Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sat, 20 Sep 2025 03:36:10 +0900 Subject: [PATCH 2/2] move keywords check to the beginning --- .../flake8_bugbear/rules/unreliable_callable_check.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index d60722867850c1..4aa582533e92ec 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -92,6 +92,9 @@ pub(crate) fn unreliable_callable_check( args: &[Expr], keywords: &[ast::Keyword], ) { + if !keywords.is_empty() { + return; + } let [obj, attr, ..] = args else { return; }; @@ -109,11 +112,11 @@ pub(crate) fn unreliable_callable_check( let valid_args = match builtins_function { "hasattr" => { // hasattr should have exactly 2 positional arguments and no keywords - args.len() == 2 && keywords.is_empty() + args.len() == 2 } "getattr" => { // getattr should have 2 or 3 positional arguments and no keywords - (args.len() == 2 || args.len() == 3) && keywords.is_empty() + args.len() == 2 || args.len() == 3 } _ => return, };