[ruff] Implement falsy-dict-get-fallback (RUF056) (#15160)

Co-authored-by: Micha Reiser <[email protected]>

crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py

@@ -0,0 +1,171 @@
+# Correct
+
+my_dict = {"key": "value", "other_key": "other_value"}
+
+# Using dict.get without a fallback
+value = my_dict.get("key")
+
+# Using dict.get with a truthy fallback
+value = my_dict.get("key", "default")
+value = my_dict.get("key", 42)
+value = my_dict.get("key", [1, 2, 3])
+value = my_dict.get("key", {"nested": "dict"})
+value = my_dict.get("key", set([1, 2]))
+value = my_dict.get("key", True)
+value = my_dict.get("key", "Non-empty string")
+value = my_dict.get("key", 3.14)
+value = my_dict.get("key", (1, 2))  # Tuples are truthy
+
+# Chained get calls with truthy fallbacks
+value1 = my_dict.get("key1", {'k': 'v'}).get("subkey")
+value2 = my_dict.get("key2", [1, 2, 3]).append(4)
+
+# Valid
+
+# Using dict.get with a falsy fallback: False
+value = my_dict.get("key", False)  
+
+# Using dict.get with a falsy fallback: empty string
+value = my_dict.get("key", "")  
+
+# Using dict.get with a falsy fallback: empty list
+value = my_dict.get("key", [])  
+
+# Using dict.get with a falsy fallback: empty dict
+value = my_dict.get("key", {})  
+
+# Using dict.get with a falsy fallback: empty set
+value = my_dict.get("key", set())  
+
+# Using dict.get with a falsy fallback: zero integer
+value = my_dict.get("key", 0)  
+
+# Using dict.get with a falsy fallback: zero float
+value = my_dict.get("key", 0.0)  
+
+# Using dict.get with a falsy fallback: None
+value = my_dict.get("key", None)  
+
+# Using dict.get with a falsy fallback via function call
+value = my_dict.get("key", list())  
+value = my_dict.get("key", dict())  
+value = my_dict.get("key", set())  
+
+# Reassigning with falsy fallback
+def get_value(d):
+    return d.get("key", False)  
+
+# Multiple dict.get calls with mixed fallbacks
+value1 = my_dict.get("key1", "default")
+value2 = my_dict.get("key2", 0)  
+value3 = my_dict.get("key3", "another default")
+
+# Using dict.get in a class with falsy fallback
+class MyClass:
+    def method(self):
+        return self.data.get("key", {})  
+
+# Using dict.get in a nested function with falsy fallback
+def outer():
+    def inner():
+        return my_dict.get("key", "")  
+    return inner()
+
+# Using dict.get with variable fallback that is falsy
+falsy_value = None
+value = my_dict.get("key", falsy_value)  
+
+# Using dict.get with variable fallback that is truthy
+truthy_value = "exists"
+value = my_dict.get("key", truthy_value)
+
+# Using dict.get with complex expressions as fallback
+value = my_dict.get("key", 0 or "default")  
+value = my_dict.get("key", [] if condition else {})  
+
+# testing dict.get call using kwargs
+value = my_dict.get(key="key", default=False)  
+value = my_dict.get(default=[], key="key")  
+
+
+# Edge Cases
+
+dicts = [my_dict, my_dict, my_dict]
+
+# Falsy fallback in a lambda
+get_fallback = lambda d: d.get("key", False)  
+
+# Falsy fallback in a list comprehension
+results = [d.get("key", "") for d in dicts]  
+
+# Falsy fallback in a generator expression
+results = (d.get("key", None) for d in dicts)  
+
+# Falsy fallback in a ternary expression
+value = my_dict.get("key", 0) if True else "default"  
+
+
+# Falsy fallback with inline comment
+value = my_dict.get("key", # comment1
+                     [] # comment2
+                     ) # comment3
+
+# Invalid
+# Invalid falsy fallbacks are when the call to dict.get is used in a boolean context
+
+# dict.get in ternary expression
+value = "not found" if my_dict.get("key", False) else "default"  # [RUF056]
+
+# dict.get in an if statement
+if my_dict.get("key", False):  # [RUF056]
+    pass
+
+# dict.get in compound if statement
+if True and my_dict.get("key", False):  # [RUF056]
+    pass
+
+if my_dict.get("key", False) or True:  # [RUF056]
+    pass
+
+# dict.get in an assert statement
+assert my_dict.get("key", False)  # [RUF056]
+
+# dict.get in a while statement
+while my_dict.get("key", False):  # [RUF056]
+    pass
+
+# dict.get in unary not expression
+value = not my_dict.get("key", False)  # [RUF056]
+
+# testing all falsy fallbacks
+value = not my_dict.get("key", False)  # [RUF056]
+value = not my_dict.get("key", [])  # [RUF056]
+value = not my_dict.get("key", list())  # [RUF056]
+value = not my_dict.get("key", {})  # [RUF056]
+value = not my_dict.get("key", dict())  # [RUF056]
+value = not my_dict.get("key", set())  # [RUF056]
+value = not my_dict.get("key", None)  # [RUF056]
+value = not my_dict.get("key", 0)  # [RUF056]
+value = not my_dict.get("key", 0.0)  # [RUF056]
+value = not my_dict.get("key", "")  # [RUF056]
+
+# testing dict.get call using kwargs
+value = not my_dict.get(key="key", default=False)  # [RUF056]
+value = not my_dict.get(default=[], key="key")  # [RUF056]
+
+# testing invalid dict.get call with inline comment
+value = not my_dict.get("key", # comment1
+                     [] # comment2
+                     ) # [RUF056]
+
+# testing invalid dict.get call with kwargs and inline comment
+value = not my_dict.get(key="key", # comment1
+                        default=False # comment2
+                        )  # [RUF056]
+value = not my_dict.get(default=[], # comment1
+                        key="key" # comment2
+                        )  # [RUF056]
+
+# testing invalid dict.get calls
+value = not my_dict.get(key="key", other="something", default=False)
+value = not my_dict.get(default=False, other="something", key="test")

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1111,6 +1111,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::PytestRaisesAmbiguousPattern) {
                 ruff::rules::pytest_raises_ambiguous_pattern(checker, call);
             }
+            if checker.enabled(Rule::FalsyDictGetFallback) {
+                ruff::rules::falsy_dict_get_fallback(checker, expr);
+            }
         }
         Expr::Dict(dict) => {
             if checker.any_enabled(&[

crates/ruff_linter/src/codes.rs

@@ -991,6 +991,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
         (Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
         (Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),
+        (Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
 

crates/ruff_linter/src/renamer.rs

@@ -293,10 +293,10 @@ impl Renamer {
         let qualified_name = semantic.resolve_qualified_name(func)?;
 
         let name_argument = match qualified_name.segments() {
-            ["collections", "namedtuple"] => arguments.find_argument("typename", 0),
+            ["collections", "namedtuple"] => arguments.find_argument_value("typename", 0),
 
             ["typing" | "typing_extensions", "TypeVar" | "ParamSpec" | "TypeVarTuple" | "NewType" | "TypeAliasType"] => {
-                arguments.find_argument("name", 0)
+                arguments.find_argument_value("name", 0)
             }
 
             ["enum", "Enum" | "IntEnum" | "StrEnum" | "ReprEnum" | "Flag" | "IntFlag"]

crates/ruff_linter/src/rules/flake8_async/rules/async_zero_sleep.rs

@@ -62,7 +62,7 @@ pub(crate) fn async_zero_sleep(checker: &mut Checker, call: &ExprCall) {
         return;
     }
 
-    let Some(arg) = call.arguments.find_argument("seconds", 0) else {
+    let Some(arg) = call.arguments.find_argument_value("seconds", 0) else {
         return;
     };
 

crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs

@@ -148,7 +148,7 @@ pub(crate) fn blocking_process_invocation(checker: &mut Checker, call: &ast::Exp
 }
 
 fn is_p_wait(call: &ast::ExprCall, semantic: &SemanticModel) -> bool {
-    let Some(arg) = call.arguments.find_argument("mode", 0) else {
+    let Some(arg) = call.arguments.find_argument_value("mode", 0) else {
         return true;
     };
 

crates/ruff_linter/src/rules/flake8_async/rules/long_sleep_not_forever.rs

@@ -67,7 +67,7 @@ pub(crate) fn long_sleep_not_forever(checker: &mut Checker, call: &ExprCall) {
         return;
     }
 
-    let Some(arg) = call.arguments.find_argument("seconds", 0) else {
+    let Some(arg) = call.arguments.find_argument_value("seconds", 0) else {
         return;
     };
 

crates/ruff_linter/src/rules/flake8_bandit/rules/bad_file_permissions.rs

@@ -71,7 +71,7 @@ pub(crate) fn bad_file_permissions(checker: &mut Checker, call: &ast::ExprCall)
         .resolve_qualified_name(&call.func)
         .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["os", "chmod"]))
     {
-        if let Some(mode_arg) = call.arguments.find_argument("mode", 1) {
+        if let Some(mode_arg) = call.arguments.find_argument_value("mode", 1) {
             match parse_mask(mode_arg, checker.semantic()) {
                 // The mask couldn't be determined (e.g., it's dynamic).
                 Ok(None) => {}

crates/ruff_linter/src/rules/flake8_bandit/rules/django_extra.rs

@@ -62,7 +62,7 @@ pub(crate) fn django_extra(checker: &mut Checker, call: &ast::ExprCall) {
 
 fn is_call_insecure(call: &ast::ExprCall) -> bool {
     for (argument_name, position) in [("select", 0), ("where", 1), ("tables", 3)] {
-        if let Some(argument) = call.arguments.find_argument(argument_name, position) {
+        if let Some(argument) = call.arguments.find_argument_value(argument_name, position) {
             match argument_name {
                 "select" => match argument {
                     Expr::Dict(dict) => {

crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs

@@ -52,7 +52,7 @@ pub(crate) fn django_raw_sql(checker: &mut Checker, call: &ast::ExprCall) {
     {
         if !call
             .arguments
-            .find_argument("sql", 0)
+            .find_argument_value("sql", 0)
             .is_some_and(Expr::is_string_literal_expr)
         {
             checker

crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs

@@ -115,7 +115,7 @@ fn detect_insecure_hashlib_calls(
 
     match hashlib_call {
         HashlibCall::New => {
-            let Some(name_arg) = call.arguments.find_argument("name", 0) else {
+            let Some(name_arg) = call.arguments.find_argument_value("name", 0) else {
                 return;
             };
             let Some(hash_func_name) = string_literal(name_arg) else {
@@ -159,7 +159,7 @@ fn detect_insecure_crypt_calls(checker: &mut Checker, call: &ast::ExprCall) {
             _ => None,
         })
         .and_then(|(argument_name, position)| {
-            call.arguments.find_argument(argument_name, position)
+            call.arguments.find_argument_value(argument_name, position)
         })
     else {
         return;

crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs

@@ -53,7 +53,7 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal
         return;
     }
 
-    let Some(policy_argument) = call.arguments.find_argument("policy", 0) else {
+    let Some(policy_argument) = call.arguments.find_argument_value("policy", 0) else {
         return;
     };
 

crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs

@@ -906,7 +906,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             ["six", "moves", "urllib", "request", "Request"] => {
                 // If the `url` argument is a string literal or an f-string, allow `http` and `https` schemes.
                 if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
-                    if call.arguments.find_argument("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
+                    if call.arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
                         return None;
                     }
                 }
@@ -916,11 +916,11 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             ["urllib", "request", "urlopen" | "urlretrieve" ] |
             ["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" ] => {
                 if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
-                    match call.arguments.find_argument("url", 0) {
+                    match call.arguments.find_argument_value("url", 0) {
                         // If the `url` argument is a `urllib.request.Request` object, allow `http` and `https` schemes.
                         Some(Expr::Call(ExprCall { func, arguments, .. })) => {
                             if checker.semantic().resolve_qualified_name(func.as_ref()).is_some_and(|name| name.segments() == ["urllib", "request", "Request"]) {
-                                if arguments.find_argument("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
+                                if arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
                                     return None;
                                 }
                             }

crates/ruff_linter/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs

@@ -65,7 +65,7 @@ pub(crate) fn unsafe_yaml_load(checker: &mut Checker, call: &ast::ExprCall) {
         .resolve_qualified_name(&call.func)
         .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["yaml", "load"]))
     {
-        if let Some(loader_arg) = call.arguments.find_argument("Loader", 1) {
+        if let Some(loader_arg) = call.arguments.find_argument_value("Loader", 1) {
             if !checker
                 .semantic()
                 .resolve_qualified_name(loader_arg)

crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs

@@ -114,7 +114,7 @@ fn extract_cryptographic_key(
                     Some((CryptographicKey::Rsa { key_size }, range))
                 }
                 "ec" => {
-                    let argument = call.arguments.find_argument("curve", 0)?;
+                    let argument = call.arguments.find_argument_value("curve", 0)?;
                     let ExprAttribute { attr, value, .. } = argument.as_attribute_expr()?;
                     let qualified_name = checker.semantic().resolve_qualified_name(value)?;
                     if matches!(
@@ -150,7 +150,7 @@ fn extract_cryptographic_key(
 }
 
 fn extract_int_argument(call: &ExprCall, name: &str, position: usize) -> Option<(u16, TextRange)> {
-    let argument = call.arguments.find_argument(name, position)?;
+    let argument = call.arguments.find_argument_value(name, position)?;
     let Expr::NumberLiteral(ast::ExprNumberLiteral {
         value: ast::Number::Int(i),
         ..

crates/ruff_linter/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs

@@ -60,7 +60,10 @@ pub(crate) fn no_explicit_stacklevel(checker: &mut Checker, call: &ast::ExprCall
         return;
     }
 
-    if call.arguments.find_argument("stacklevel", 2).is_some()
+    if call
+        .arguments
+        .find_argument_value("stacklevel", 2)
+        .is_some()
         || call
             .arguments
             .args

crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs

@@ -91,7 +91,7 @@ pub(crate) fn call_datetime_fromtimestamp(checker: &mut Checker, call: &ast::Exp
         return;
     }
 
-    let antipattern = match call.arguments.find_argument("tz", 1) {
+    let antipattern = match call.arguments.find_argument_value("tz", 1) {
         Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
         Some(_) => return,
         None => DatetimeModuleAntipattern::NoTzArgumentPassed,

crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs

@@ -86,7 +86,7 @@ pub(crate) fn call_datetime_now_without_tzinfo(checker: &mut Checker, call: &ast
         return;
     }
 
-    let antipattern = match call.arguments.find_argument("tz", 0) {
+    let antipattern = match call.arguments.find_argument_value("tz", 0) {
         Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
         Some(_) => return,
         None => DatetimeModuleAntipattern::NoTzArgumentPassed,

crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs

@@ -80,7 +80,7 @@ pub(crate) fn call_datetime_without_tzinfo(checker: &mut Checker, call: &ast::Ex
         return;
     }
 
-    let antipattern = match call.arguments.find_argument("tzinfo", 7) {
+    let antipattern = match call.arguments.find_argument_value("tzinfo", 7) {
         Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
         Some(_) => return,
         None => DatetimeModuleAntipattern::NoTzArgumentPassed,

crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs

@@ -59,7 +59,7 @@ pub(crate) fn locals_in_render_function(checker: &mut Checker, call: &ast::ExprC
         return;
     }
 
-    if let Some(argument) = call.arguments.find_argument("context", 2) {
+    if let Some(argument) = call.arguments.find_argument_value("context", 2) {
         if is_locals_call(argument, checker.semantic()) {
             checker.diagnostics.push(Diagnostic::new(
                 DjangoLocalsInRenderFunction,

crates/ruff_linter/src/rules/flake8_logging/rules/invalid_get_logger_argument.rs

@@ -62,7 +62,8 @@ pub(crate) fn invalid_get_logger_argument(checker: &mut Checker, call: &ast::Exp
         return;
     }
 
-    let Some(Expr::Name(expr @ ast::ExprName { id, .. })) = call.arguments.find_argument("name", 0)
+    let Some(Expr::Name(expr @ ast::ExprName { id, .. })) =
+        call.arguments.find_argument_value("name", 0)
     else {
         return;
     };