Skip to content

Commit 3c9021f

Browse files
guptaarnavMichaReiser
guptaarnav
and
MichaReiser
authored
[ruff] Implement falsy-dict-get-fallback (RUF056) (#15160)
Co-authored-by: Micha Reiser <[email protected]>
1 parent 68d2466 commit 3c9021f

File tree

51 files changed

+897
-88
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+897
-88
lines changed

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

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
# Correct
2+
3+
my_dict = {"key": "value", "other_key": "other_value"}
4+
5+
# Using dict.get without a fallback
6+
value = my_dict.get("key")
7+
8+
# Using dict.get with a truthy fallback
9+
value = my_dict.get("key", "default")
10+
value = my_dict.get("key", 42)
11+
value = my_dict.get("key", [1, 2, 3])
12+
value = my_dict.get("key", {"nested": "dict"})
13+
value = my_dict.get("key", set([1, 2]))
14+
value = my_dict.get("key", True)
15+
value = my_dict.get("key", "Non-empty string")
16+
value = my_dict.get("key", 3.14)
17+
value = my_dict.get("key", (1, 2)) # Tuples are truthy
18+
19+
# Chained get calls with truthy fallbacks
20+
value1 = my_dict.get("key1", {'k': 'v'}).get("subkey")
21+
value2 = my_dict.get("key2", [1, 2, 3]).append(4)
22+
23+
# Valid
24+
25+
# Using dict.get with a falsy fallback: False
26+
value = my_dict.get("key", False)
27+
28+
# Using dict.get with a falsy fallback: empty string
29+
value = my_dict.get("key", "")
30+
31+
# Using dict.get with a falsy fallback: empty list
32+
value = my_dict.get("key", [])
33+
34+
# Using dict.get with a falsy fallback: empty dict
35+
value = my_dict.get("key", {})
36+
37+
# Using dict.get with a falsy fallback: empty set
38+
value = my_dict.get("key", set())
39+
40+
# Using dict.get with a falsy fallback: zero integer
41+
value = my_dict.get("key", 0)
42+
43+
# Using dict.get with a falsy fallback: zero float
44+
value = my_dict.get("key", 0.0)
45+
46+
# Using dict.get with a falsy fallback: None
47+
value = my_dict.get("key", None)
48+
49+
# Using dict.get with a falsy fallback via function call
50+
value = my_dict.get("key", list())
51+
value = my_dict.get("key", dict())
52+
value = my_dict.get("key", set())
53+
54+
# Reassigning with falsy fallback
55+
def get_value(d):
56+
return d.get("key", False)
57+
58+
# Multiple dict.get calls with mixed fallbacks
59+
value1 = my_dict.get("key1", "default")
60+
value2 = my_dict.get("key2", 0)
61+
value3 = my_dict.get("key3", "another default")
62+
63+
# Using dict.get in a class with falsy fallback
64+
class MyClass:
65+
def method(self):
66+
return self.data.get("key", {})
67+
68+
# Using dict.get in a nested function with falsy fallback
69+
def outer():
70+
def inner():
71+
return my_dict.get("key", "")
72+
return inner()
73+
74+
# Using dict.get with variable fallback that is falsy
75+
falsy_value = None
76+
value = my_dict.get("key", falsy_value)
77+
78+
# Using dict.get with variable fallback that is truthy
79+
truthy_value = "exists"
80+
value = my_dict.get("key", truthy_value)
81+
82+
# Using dict.get with complex expressions as fallback
83+
value = my_dict.get("key", 0 or "default")
84+
value = my_dict.get("key", [] if condition else {})
85+
86+
# testing dict.get call using kwargs
87+
value = my_dict.get(key="key", default=False)
88+
value = my_dict.get(default=[], key="key")
89+
90+
91+
# Edge Cases
92+
93+
dicts = [my_dict, my_dict, my_dict]
94+
95+
# Falsy fallback in a lambda
96+
get_fallback = lambda d: d.get("key", False)
97+
98+
# Falsy fallback in a list comprehension
99+
results = [d.get("key", "") for d in dicts]
100+
101+
# Falsy fallback in a generator expression
102+
results = (d.get("key", None) for d in dicts)
103+
104+
# Falsy fallback in a ternary expression
105+
value = my_dict.get("key", 0) if True else "default"
106+
107+
108+
# Falsy fallback with inline comment
109+
value = my_dict.get("key", # comment1
110+
[] # comment2
111+
) # comment3
112+
113+
# Invalid
114+
# Invalid falsy fallbacks are when the call to dict.get is used in a boolean context
115+
116+
# dict.get in ternary expression
117+
value = "not found" if my_dict.get("key", False) else "default" # [RUF056]
118+
119+
# dict.get in an if statement
120+
if my_dict.get("key", False): # [RUF056]
121+
pass
122+
123+
# dict.get in compound if statement
124+
if True and my_dict.get("key", False): # [RUF056]
125+
pass
126+
127+
if my_dict.get("key", False) or True: # [RUF056]
128+
pass
129+
130+
# dict.get in an assert statement
131+
assert my_dict.get("key", False) # [RUF056]
132+
133+
# dict.get in a while statement
134+
while my_dict.get("key", False): # [RUF056]
135+
pass
136+
137+
# dict.get in unary not expression
138+
value = not my_dict.get("key", False) # [RUF056]
139+
140+
# testing all falsy fallbacks
141+
value = not my_dict.get("key", False) # [RUF056]
142+
value = not my_dict.get("key", []) # [RUF056]
143+
value = not my_dict.get("key", list()) # [RUF056]
144+
value = not my_dict.get("key", {}) # [RUF056]
145+
value = not my_dict.get("key", dict()) # [RUF056]
146+
value = not my_dict.get("key", set()) # [RUF056]
147+
value = not my_dict.get("key", None) # [RUF056]
148+
value = not my_dict.get("key", 0) # [RUF056]
149+
value = not my_dict.get("key", 0.0) # [RUF056]
150+
value = not my_dict.get("key", "") # [RUF056]
151+
152+
# testing dict.get call using kwargs
153+
value = not my_dict.get(key="key", default=False) # [RUF056]
154+
value = not my_dict.get(default=[], key="key") # [RUF056]
155+
156+
# testing invalid dict.get call with inline comment
157+
value = not my_dict.get("key", # comment1
158+
[] # comment2
159+
) # [RUF056]
160+
161+
# testing invalid dict.get call with kwargs and inline comment
162+
value = not my_dict.get(key="key", # comment1
163+
default=False # comment2
164+
) # [RUF056]
165+
value = not my_dict.get(default=[], # comment1
166+
key="key" # comment2
167+
) # [RUF056]
168+
169+
# testing invalid dict.get calls
170+
value = not my_dict.get(key="key", other="something", default=False)
171+
value = not my_dict.get(default=False, other="something", key="test")

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,6 +1111,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
11111111
if checker.enabled(Rule::PytestRaisesAmbiguousPattern) {
11121112
ruff::rules::pytest_raises_ambiguous_pattern(checker, call);
11131113
}
1114+
if checker.enabled(Rule::FalsyDictGetFallback) {
1115+
ruff::rules::falsy_dict_get_fallback(checker, expr);
1116+
}
11141117
}
11151118
Expr::Dict(dict) => {
11161119
if checker.any_enabled(&[

‎crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
991991
(Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
992992
(Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
993993
(Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),
994+
(Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback),
994995
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
995996
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
996997

‎crates/ruff_linter/src/renamer.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,10 @@ impl Renamer {
293293
let qualified_name = semantic.resolve_qualified_name(func)?;
294294

295295
let name_argument = match qualified_name.segments() {
296-
["collections", "namedtuple"] => arguments.find_argument("typename", 0),
296+
["collections", "namedtuple"] => arguments.find_argument_value("typename", 0),
297297

298298
["typing" | "typing_extensions", "TypeVar" | "ParamSpec" | "TypeVarTuple" | "NewType" | "TypeAliasType"] => {
299-
arguments.find_argument("name", 0)
299+
arguments.find_argument_value("name", 0)
300300
}
301301

302302
["enum", "Enum" | "IntEnum" | "StrEnum" | "ReprEnum" | "Flag" | "IntFlag"]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub(crate) fn async_zero_sleep(checker: &mut Checker, call: &ExprCall) {
6262
return;
6363
}
6464

65-
let Some(arg) = call.arguments.find_argument("seconds", 0) else {
65+
let Some(arg) = call.arguments.find_argument_value("seconds", 0) else {
6666
return;
6767
};
6868

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ pub(crate) fn blocking_process_invocation(checker: &mut Checker, call: &ast::Exp
148148
}
149149

150150
fn is_p_wait(call: &ast::ExprCall, semantic: &SemanticModel) -> bool {
151-
let Some(arg) = call.arguments.find_argument("mode", 0) else {
151+
let Some(arg) = call.arguments.find_argument_value("mode", 0) else {
152152
return true;
153153
};
154154

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub(crate) fn long_sleep_not_forever(checker: &mut Checker, call: &ExprCall) {
6767
return;
6868
}
6969

70-
let Some(arg) = call.arguments.find_argument("seconds", 0) else {
70+
let Some(arg) = call.arguments.find_argument_value("seconds", 0) else {
7171
return;
7272
};
7373

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub(crate) fn django_extra(checker: &mut Checker, call: &ast::ExprCall) {
6262

6363
fn is_call_insecure(call: &ast::ExprCall) -> bool {
6464
for (argument_name, position) in [("select", 0), ("where", 1), ("tables", 3)] {
65-
if let Some(argument) = call.arguments.find_argument(argument_name, position) {
65+
if let Some(argument) = call.arguments.find_argument_value(argument_name, position) {
6666
match argument_name {
6767
"select" => match argument {
6868
Expr::Dict(dict) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub(crate) fn django_raw_sql(checker: &mut Checker, call: &ast::ExprCall) {
5252
{
5353
if !call
5454
.arguments
55-
.find_argument("sql", 0)
55+
.find_argument_value("sql", 0)
5656
.is_some_and(Expr::is_string_literal_expr)
5757
{
5858
checker

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ fn detect_insecure_hashlib_calls(
115115

116116
match hashlib_call {
117117
HashlibCall::New => {
118-
let Some(name_arg) = call.arguments.find_argument("name", 0) else {
118+
let Some(name_arg) = call.arguments.find_argument_value("name", 0) else {
119119
return;
120120
};
121121
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) {
159159
_ => None,
160160
})
161161
.and_then(|(argument_name, position)| {
162-
call.arguments.find_argument(argument_name, position)
162+
call.arguments.find_argument_value(argument_name, position)
163163
})
164164
else {
165165
return;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal
5353
return;
5454
}
5555

56-
let Some(policy_argument) = call.arguments.find_argument("policy", 0) else {
56+
let Some(policy_argument) = call.arguments.find_argument_value("policy", 0) else {
5757
return;
5858
};
5959

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
906906
["six", "moves", "urllib", "request", "Request"] => {
907907
// If the `url` argument is a string literal or an f-string, allow `http` and `https` schemes.
908908
if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
909-
if call.arguments.find_argument("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
909+
if call.arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
910910
return None;
911911
}
912912
}
@@ -916,11 +916,11 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
916916
["urllib", "request", "urlopen" | "urlretrieve" ] |
917917
["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" ] => {
918918
if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
919-
match call.arguments.find_argument("url", 0) {
919+
match call.arguments.find_argument_value("url", 0) {
920920
// If the `url` argument is a `urllib.request.Request` object, allow `http` and `https` schemes.
921921
Some(Expr::Call(ExprCall { func, arguments, .. })) => {
922922
if checker.semantic().resolve_qualified_name(func.as_ref()).is_some_and(|name| name.segments() == ["urllib", "request", "Request"]) {
923-
if arguments.find_argument("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
923+
if arguments.find_argument_value("url", 0).and_then(leading_chars).is_some_and(has_http_prefix) {
924924
return None;
925925
}
926926
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub(crate) fn unsafe_yaml_load(checker: &mut Checker, call: &ast::ExprCall) {
6565
.resolve_qualified_name(&call.func)
6666
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["yaml", "load"]))
6767
{
68-
if let Some(loader_arg) = call.arguments.find_argument("Loader", 1) {
68+
if let Some(loader_arg) = call.arguments.find_argument_value("Loader", 1) {
6969
if !checker
7070
.semantic()
7171
.resolve_qualified_name(loader_arg)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ fn extract_cryptographic_key(
114114
Some((CryptographicKey::Rsa { key_size }, range))
115115
}
116116
"ec" => {
117-
let argument = call.arguments.find_argument("curve", 0)?;
117+
let argument = call.arguments.find_argument_value("curve", 0)?;
118118
let ExprAttribute { attr, value, .. } = argument.as_attribute_expr()?;
119119
let qualified_name = checker.semantic().resolve_qualified_name(value)?;
120120
if matches!(
@@ -150,7 +150,7 @@ fn extract_cryptographic_key(
150150
}
151151

152152
fn extract_int_argument(call: &ExprCall, name: &str, position: usize) -> Option<(u16, TextRange)> {
153-
let argument = call.arguments.find_argument(name, position)?;
153+
let argument = call.arguments.find_argument_value(name, position)?;
154154
let Expr::NumberLiteral(ast::ExprNumberLiteral {
155155
value: ast::Number::Int(i),
156156
..

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ pub(crate) fn no_explicit_stacklevel(checker: &mut Checker, call: &ast::ExprCall
6060
return;
6161
}
6262

63-
if call.arguments.find_argument("stacklevel", 2).is_some()
63+
if call
64+
.arguments
65+
.find_argument_value("stacklevel", 2)
66+
.is_some()
6467
|| call
6568
.arguments
6669
.args

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub(crate) fn call_datetime_fromtimestamp(checker: &mut Checker, call: &ast::Exp
9191
return;
9292
}
9393

94-
let antipattern = match call.arguments.find_argument("tz", 1) {
94+
let antipattern = match call.arguments.find_argument_value("tz", 1) {
9595
Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
9696
Some(_) => return,
9797
None => DatetimeModuleAntipattern::NoTzArgumentPassed,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ pub(crate) fn call_datetime_now_without_tzinfo(checker: &mut Checker, call: &ast
8686
return;
8787
}
8888

89-
let antipattern = match call.arguments.find_argument("tz", 0) {
89+
let antipattern = match call.arguments.find_argument_value("tz", 0) {
9090
Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
9191
Some(_) => return,
9292
None => DatetimeModuleAntipattern::NoTzArgumentPassed,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ pub(crate) fn call_datetime_without_tzinfo(checker: &mut Checker, call: &ast::Ex
8080
return;
8181
}
8282

83-
let antipattern = match call.arguments.find_argument("tzinfo", 7) {
83+
let antipattern = match call.arguments.find_argument_value("tzinfo", 7) {
8484
Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
8585
Some(_) => return,
8686
None => DatetimeModuleAntipattern::NoTzArgumentPassed,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub(crate) fn locals_in_render_function(checker: &mut Checker, call: &ast::ExprC
5959
return;
6060
}
6161

62-
if let Some(argument) = call.arguments.find_argument("context", 2) {
62+
if let Some(argument) = call.arguments.find_argument_value("context", 2) {
6363
if is_locals_call(argument, checker.semantic()) {
6464
checker.diagnostics.push(Diagnostic::new(
6565
DjangoLocalsInRenderFunction,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ pub(crate) fn invalid_get_logger_argument(checker: &mut Checker, call: &ast::Exp
6262
return;
6363
}
6464

65-
let Some(Expr::Name(expr @ ast::ExprName { id, .. })) = call.arguments.find_argument("name", 0)
65+
let Some(Expr::Name(expr @ ast::ExprName { id, .. })) =
66+
call.arguments.find_argument_value("name", 0)
6667
else {
6768
return;
6869
};

0 commit comments

Comments
 (0)