From 3c9021ffcba2f622594c4465be0080e7af80a7ce Mon Sep 17 00:00:00 2001 From: Arnav Gupta Date: Tue, 31 Dec 2024 05:40:51 -0500 Subject: [PATCH] [ruff] Implement falsy-dict-get-fallback (RUF056) (#15160) Co-authored-by: Micha Reiser --- .../resources/test/fixtures/ruff/RUF056.py | 171 ++++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/renamer.rs | 4 +- .../flake8_async/rules/async_zero_sleep.rs | 2 +- .../rules/blocking_process_invocation.rs | 2 +- .../rules/long_sleep_not_forever.rs | 2 +- .../rules/bad_file_permissions.rs | 2 +- .../rules/flake8_bandit/rules/django_extra.rs | 2 +- .../flake8_bandit/rules/django_raw_sql.rs | 2 +- .../rules/hashlib_insecure_hash_functions.rs | 4 +- .../rules/ssh_no_host_key_verification.rs | 2 +- .../rules/suspicious_function_call.rs | 6 +- .../flake8_bandit/rules/unsafe_yaml_load.rs | 2 +- .../rules/weak_cryptographic_key.rs | 4 +- .../rules/no_explicit_stacklevel.rs | 5 +- .../rules/call_datetime_fromtimestamp.rs | 2 +- .../rules/call_datetime_now_without_tzinfo.rs | 2 +- .../rules/call_datetime_without_tzinfo.rs | 2 +- .../rules/locals_in_render_function.rs | 2 +- .../rules/invalid_get_logger_argument.rs | 3 +- .../rules/logging_call.rs | 2 +- .../rules/flake8_pytest_style/rules/fail.rs | 4 +- .../flake8_pytest_style/rules/parametrize.rs | 10 +- .../rules/flake8_pytest_style/rules/patch.rs | 2 +- .../rules/flake8_pytest_style/rules/raises.rs | 2 +- .../rules/split_static_string.rs | 4 +- .../rules/invalid_pathlib_with_suffix.rs | 2 +- .../flake8_use_pathlib/rules/os_sep_split.rs | 2 +- .../rules/replaceable_by_pathlib.rs | 4 +- .../src/rules/pep8_naming/helpers.rs | 6 +- .../ruff_linter/src/rules/pylint/helpers.rs | 2 +- .../src/rules/pylint/rules/bad_open_mode.rs | 4 +- .../pylint/rules/invalid_envvar_default.rs | 2 +- .../pylint/rules/invalid_envvar_value.rs | 2 +- .../rules/unnecessary_list_index_lookup.rs | 21 +- .../pylint/rules/unspecified_encoding.rs | 18 +- .../pyupgrade/rules/redundant_open_modes.rs | 2 +- .../refurb/rules/unnecessary_enumerate.rs | 2 +- .../refurb/rules/unnecessary_from_float.rs | 4 +- .../rules/verbose_decimal_constructor.rs | 2 +- crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff/rules/falsy_dict_get_fallback.rs | 120 +++++ .../ruff/rules/map_int_version_parsing.rs | 2 +- .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../ruff/rules/quadratic_list_summation.rs | 2 +- .../ruff/rules/unnecessary_cast_to_int.rs | 4 +- .../rules/unnecessary_regular_expression.rs | 22 +- ..._rules__ruff__tests__RUF056_RUF056.py.snap | 485 ++++++++++++++++++ crates/ruff_python_ast/src/nodes.rs | 24 +- ruff.schema.json | 1 + 51 files changed, 897 insertions(+), 88 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py new file mode 100644 index 0000000000..831ec4bcea --- /dev/null +++ b/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") \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 64ab6a448b..d3e55f12a7 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/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(&[ diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index d4bff4099a..a90a87c630 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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), diff --git a/crates/ruff_linter/src/renamer.rs b/crates/ruff_linter/src/renamer.rs index 9d2c9d6c92..a5308d9029 100644 --- a/crates/ruff_linter/src/renamer.rs +++ b/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"] diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_zero_sleep.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_zero_sleep.rs index 31e1126e69..d97d33afcb 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_zero_sleep.rs +++ b/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; }; diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs b/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs index 2e7843c643..5d850e54cc 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/blocking_process_invocation.rs +++ b/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; }; diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/long_sleep_not_forever.rs b/crates/ruff_linter/src/rules/flake8_async/rules/long_sleep_not_forever.rs index 6d37fe2459..b86449db06 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/long_sleep_not_forever.rs +++ b/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; }; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/bad_file_permissions.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/bad_file_permissions.rs index 2fe7e159e4..4318ab99d6 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/bad_file_permissions.rs +++ b/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) => {} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/django_extra.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/django_extra.rs index 2473f69155..1972e5b6b5 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/django_extra.rs +++ b/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) => { diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs index ed75a1b05b..78578c5ccb 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/django_raw_sql.rs +++ b/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 diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs index d5a18e4446..6828c1c6d2 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs +++ b/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; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs index 996167379b..ab990527c2 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs +++ b/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; }; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index a5af59e8df..b97ca93bc7 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/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; } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs index 2bf7878d9c..d9c9e2c10f 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs +++ b/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) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs index 4f1e738b5c..04facbc188 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/weak_cryptographic_key.rs +++ b/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), .. diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs index fb3077283f..69962a28d5 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs +++ b/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 diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs index 7cf70c933c..b8df928d19 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs index 8ca0b3ca97..9a5f8a399f 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs index 5dac0ee87d..6fd99e9a7e 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs b/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs index b08adc2be1..c4677d031f 100644 --- a/crates/ruff_linter/src/rules/flake8_django/rules/locals_in_render_function.rs +++ b/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, diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/invalid_get_logger_argument.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/invalid_get_logger_argument.rs index 83b401f6a6..5a1a8eea3c 100644 --- a/crates/ruff_linter/src/rules/flake8_logging/rules/invalid_get_logger_argument.rs +++ b/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; }; diff --git a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs index fede8ba368..518e3e8225 100644 --- a/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff_linter/src/rules/flake8_logging_format/rules/logging_call.rs @@ -181,7 +181,7 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { // G001 - G004 let msg_pos = usize::from(matches!(logging_call_type, LoggingCallType::LogCall)); - if let Some(format_arg) = call.arguments.find_argument("msg", msg_pos) { + if let Some(format_arg) = call.arguments.find_argument_value("msg", msg_pos) { check_msg(checker, format_arg); } diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fail.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fail.rs index cbfad41ecd..1d756f237d 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fail.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fail.rs @@ -61,8 +61,8 @@ pub(crate) fn fail_call(checker: &mut Checker, call: &ast::ExprCall) { // `pytest.fail(msg="...")` (deprecated in pytest 7.0) if call .arguments - .find_argument("reason", 0) - .or_else(|| call.arguments.find_argument("msg", 0)) + .find_argument_value("reason", 0) + .or_else(|| call.arguments.find_argument_value("msg", 0)) .map_or(true, is_empty_or_null_string) { checker diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs index d8a073e591..cae2cf2f5c 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs @@ -864,12 +864,12 @@ pub(crate) fn parametrize(checker: &mut Checker, call: &ExprCall) { if checker.enabled(Rule::PytestParametrizeNamesWrongType) { let names = if checker.settings.preview.is_enabled() { - call.arguments.find_argument("argnames", 0) + call.arguments.find_argument_value("argnames", 0) } else { call.arguments.find_positional(0) }; let values = if checker.settings.preview.is_enabled() { - call.arguments.find_argument("argvalues", 1) + call.arguments.find_argument_value("argvalues", 1) } else { call.arguments.find_positional(1) }; @@ -879,12 +879,12 @@ pub(crate) fn parametrize(checker: &mut Checker, call: &ExprCall) { } if checker.enabled(Rule::PytestParametrizeValuesWrongType) { let names = if checker.settings.preview.is_enabled() { - call.arguments.find_argument("argnames", 0) + call.arguments.find_argument_value("argnames", 0) } else { call.arguments.find_positional(0) }; let values = if checker.settings.preview.is_enabled() { - call.arguments.find_argument("argvalues", 1) + call.arguments.find_argument_value("argvalues", 1) } else { call.arguments.find_positional(1) }; @@ -894,7 +894,7 @@ pub(crate) fn parametrize(checker: &mut Checker, call: &ExprCall) { } if checker.enabled(Rule::PytestDuplicateParametrizeTestCases) { if let Some(values) = if checker.settings.preview.is_enabled() { - call.arguments.find_argument("argvalues", 1) + call.arguments.find_argument_value("argvalues", 1) } else { call.arguments.find_positional(1) } { diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/patch.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/patch.rs index 7fb57225fb..b245d8724c 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/patch.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/patch.rs @@ -84,7 +84,7 @@ fn check_patch_call(call: &ast::ExprCall, index: usize) -> Option { range: _, } = call .arguments - .find_argument("new", index)? + .find_argument_value("new", index)? .as_lambda_expr()?; // Walk the lambda body. If the lambda uses the arguments, then it's valid. diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs index 82972ef54b..a693012d00 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs @@ -177,7 +177,7 @@ pub(crate) fn raises_call(checker: &mut Checker, call: &ast::ExprCall) { } if checker.enabled(Rule::PytestRaisesTooBroad) { - if let Some(exception) = call.arguments.find_argument("expected_exception", 0) { + if let Some(exception) = call.arguments.find_argument_value("expected_exception", 0) { if call .arguments .find_keyword("match") diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs index 7d6797188b..ec34e88c4e 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs @@ -67,7 +67,7 @@ pub(crate) fn split_static_string( ) { let ExprCall { arguments, .. } = call; - let maxsplit_arg = arguments.find_argument("maxsplit", 1); + let maxsplit_arg = arguments.find_argument_value("maxsplit", 1); let Some(maxsplit_value) = get_maxsplit_value(maxsplit_arg) else { return; }; @@ -79,7 +79,7 @@ pub(crate) fn split_static_string( Direction::Right }; - let sep_arg = arguments.find_argument("sep", 0); + let sep_arg = arguments.find_argument_value("sep", 0); let split_replacement = if let Some(sep) = sep_arg { match sep { Expr::NoneLiteral(_) => split_default(str_value, maxsplit_value), diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/invalid_pathlib_with_suffix.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/invalid_pathlib_with_suffix.rs index 5cdfd085f1..60790c16f8 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/invalid_pathlib_with_suffix.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/invalid_pathlib_with_suffix.rs @@ -89,7 +89,7 @@ pub(crate) fn invalid_pathlib_with_suffix(checker: &mut Checker, call: &ast::Exp return; } - let Some(ast::Expr::StringLiteral(string)) = arguments.find_argument("suffix", 0) else { + let Some(ast::Expr::StringLiteral(string)) = arguments.find_argument_value("suffix", 0) else { return; }; diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_sep_split.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_sep_split.rs index ef226675f1..e075a5c8df 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_sep_split.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_sep_split.rs @@ -82,7 +82,7 @@ pub(crate) fn os_sep_split(checker: &mut Checker, call: &ast::ExprCall) { return; } - let Some(sep) = call.arguments.find_argument("sep", 0) else { + let Some(sep) = call.arguments.find_argument_value("sep", 0) else { return; }; diff --git a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs index fd97c1fe3b..4331d77fff 100644 --- a/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs +++ b/crates/ruff_linter/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs @@ -115,7 +115,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) { // ``` if call .arguments - .find_argument("closefd", 6) + .find_argument_value("closefd", 6) .is_some_and(|expr| { !matches!( expr, @@ -124,7 +124,7 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) { }) || call .arguments - .find_argument("opener", 7) + .find_argument_value("opener", 7) .is_some_and(|expr| !expr.is_none_literal_expr()) || call .arguments diff --git a/crates/ruff_linter/src/rules/pep8_naming/helpers.rs b/crates/ruff_linter/src/rules/pep8_naming/helpers.rs index 41598653b3..2b69eddd7c 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/helpers.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/helpers.rs @@ -121,8 +121,8 @@ pub(super) fn is_django_model_import(name: &str, stmt: &Stmt, semantic: &Semanti // Match against, e.g., `apps.get_model("zerver", "Attachment")`. if let Some(unqualified_name) = UnqualifiedName::from_expr(func.as_ref()) { if matches!(unqualified_name.segments(), [.., "get_model"]) { - if let Some(argument) = - arguments.find_argument("model_name", arguments.args.len().saturating_sub(1)) + if let Some(argument) = arguments + .find_argument_value("model_name", arguments.args.len().saturating_sub(1)) { if let Some(string_literal) = argument.as_string_literal_expr() { if string_literal.value.to_str() == name { @@ -141,7 +141,7 @@ pub(super) fn is_django_model_import(name: &str, stmt: &Stmt, semantic: &Semanti qualified_name.segments(), ["django", "utils", "module_loading", "import_string"] ) { - if let Some(argument) = arguments.find_argument("dotted_path", 0) { + if let Some(argument) = arguments.find_argument_value("dotted_path", 0) { if let Some(string_literal) = argument.as_string_literal_expr() { if let Some((.., model)) = string_literal.value.to_str().rsplit_once('.') { if model == name { diff --git a/crates/ruff_linter/src/rules/pylint/helpers.rs b/crates/ruff_linter/src/rules/pylint/helpers.rs index 75777a40fd..e0bbc25980 100644 --- a/crates/ruff_linter/src/rules/pylint/helpers.rs +++ b/crates/ruff_linter/src/rules/pylint/helpers.rs @@ -10,7 +10,7 @@ use crate::settings::LinterSettings; /// Returns the value of the `name` parameter to, e.g., a `TypeVar` constructor. pub(super) fn type_param_name(arguments: &Arguments) -> Option<&str> { // Handle both `TypeVar("T")` and `TypeVar(name="T")`. - let name_param = arguments.find_argument("name", 0)?; + let name_param = arguments.find_argument_value("name", 0)?; if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &name_param { Some(value.to_str()) } else { diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs index 00c8b42614..3b32895afd 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs @@ -107,7 +107,7 @@ fn is_open(func: &Expr, semantic: &SemanticModel) -> Option { /// Returns the mode argument, if present. fn extract_mode(call: &ast::ExprCall, kind: Kind) -> Option<&Expr> { match kind { - Kind::Builtin => call.arguments.find_argument("mode", 1), - Kind::Pathlib => call.arguments.find_argument("mode", 0), + Kind::Builtin => call.arguments.find_argument_value("mode", 1), + Kind::Pathlib => call.arguments.find_argument_value("mode", 0), } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_default.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_default.rs index 72c4f6df85..735e24cc29 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_default.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_default.rs @@ -63,7 +63,7 @@ pub(crate) fn invalid_envvar_default(checker: &mut Checker, call: &ast::ExprCall }) { // Find the `default` argument, if it exists. - let Some(expr) = call.arguments.find_argument("default", 1) else { + let Some(expr) = call.arguments.find_argument_value("default", 1) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_value.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_value.rs index 5ff818c600..8c735d95f7 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_value.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_envvar_value.rs @@ -47,7 +47,7 @@ pub(crate) fn invalid_envvar_value(checker: &mut Checker, call: &ast::ExprCall) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["os", "getenv"])) { // Find the `key` argument, if it exists. - let Some(expr) = call.arguments.find_argument("key", 0) else { + let Some(expr) = call.arguments.find_argument_value("key", 0) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index 94a1f80898..952ea2faac 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -152,15 +152,18 @@ fn enumerate_items<'a>( }; // If the `enumerate` call has a non-zero `start`, don't omit. - if !arguments.find_argument("start", 1).map_or(true, |expr| { - matches!( - expr, - Expr::NumberLiteral(ast::ExprNumberLiteral { - value: Number::Int(Int::ZERO), - .. - }) - ) - }) { + if !arguments + .find_argument_value("start", 1) + .map_or(true, |expr| { + matches!( + expr, + Expr::NumberLiteral(ast::ExprNumberLiteral { + value: Number::Int(Int::ZERO), + .. + }) + ) + }) + { return None; } diff --git a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs index 2873048893..1aa1ea934a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs @@ -203,19 +203,19 @@ fn is_violation(call: &ast::ExprCall, qualified_name: &Callee) -> bool { match qualified_name { Callee::Qualified(qualified_name) => match qualified_name.segments() { ["" | "codecs" | "_io", "open"] => { - if let Some(mode_arg) = call.arguments.find_argument("mode", 1) { + if let Some(mode_arg) = call.arguments.find_argument_value("mode", 1) { if is_binary_mode(mode_arg).unwrap_or(true) { // binary mode or unknown mode is no violation return false; } } // else mode not specified, defaults to text mode - call.arguments.find_argument("encoding", 3).is_none() + call.arguments.find_argument_value("encoding", 3).is_none() } ["tempfile", tempfile_class @ ("TemporaryFile" | "NamedTemporaryFile" | "SpooledTemporaryFile")] => { let mode_pos = usize::from(*tempfile_class == "SpooledTemporaryFile"); - if let Some(mode_arg) = call.arguments.find_argument("mode", mode_pos) { + if let Some(mode_arg) = call.arguments.find_argument_value("mode", mode_pos) { if is_binary_mode(mode_arg).unwrap_or(true) { // binary mode or unknown mode is no violation return false; @@ -225,27 +225,27 @@ fn is_violation(call: &ast::ExprCall, qualified_name: &Callee) -> bool { return false; } call.arguments - .find_argument("encoding", mode_pos + 2) + .find_argument_value("encoding", mode_pos + 2) .is_none() } ["io" | "_io", "TextIOWrapper"] => { - call.arguments.find_argument("encoding", 1).is_none() + call.arguments.find_argument_value("encoding", 1).is_none() } _ => false, }, Callee::Pathlib(attr) => match *attr { "open" => { - if let Some(mode_arg) = call.arguments.find_argument("mode", 0) { + if let Some(mode_arg) = call.arguments.find_argument_value("mode", 0) { if is_binary_mode(mode_arg).unwrap_or(true) { // binary mode or unknown mode is no violation return false; } } // else mode not specified, defaults to text mode - call.arguments.find_argument("encoding", 2).is_none() + call.arguments.find_argument_value("encoding", 2).is_none() } - "read_text" => call.arguments.find_argument("encoding", 0).is_none(), - "write_text" => call.arguments.find_argument("encoding", 1).is_none(), + "read_text" => call.arguments.find_argument_value("encoding", 0).is_none(), + "write_text" => call.arguments.find_argument_value("encoding", 1).is_none(), _ => false, }, } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs index 68738cd6a4..b08937a06e 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs @@ -71,7 +71,7 @@ pub(crate) fn redundant_open_modes(checker: &mut Checker, call: &ast::ExprCall) return; } - let Some(mode_param) = call.arguments.find_argument("mode", 1) else { + let Some(mode_param) = call.arguments.find_argument_value("mode", 1) else { return; }; let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &mode_param else { diff --git a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs index 9e3e890a2f..d6a0ee4355 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs @@ -178,7 +178,7 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF { // If the `start` argument is set to something other than the `range` default, // there's no clear fix. - let start = arguments.find_argument("start", 1); + let start = arguments.find_argument_value("start", 1); if start.map_or(true, |start| { matches!( start, diff --git a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs index 4c5ffe62cc..cb074efa8d 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs @@ -115,8 +115,8 @@ pub(crate) fn unnecessary_from_float(checker: &mut Checker, call: &ExprCall) { }; let Some(value) = (match method_name { - MethodName::FromFloat => call.arguments.find_argument("f", 0), - MethodName::FromDecimal => call.arguments.find_argument("dec", 0), + MethodName::FromFloat => call.arguments.find_argument_value("f", 0), + MethodName::FromDecimal => call.arguments.find_argument_value("dec", 0), }) else { return; }; diff --git a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs index bb1ff02446..e3d4833540 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs @@ -67,7 +67,7 @@ pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ast::Exp } // Decimal accepts arguments of the form: `Decimal(value='0', context=None)` - let Some(value) = call.arguments.find_argument("value", 0) else { + let Some(value) = call.arguments.find_argument_value("value", 0) else { return; }; diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 241877180b..280eb3f167 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -72,6 +72,7 @@ mod tests { #[test_case(Rule::UnnecessaryNestedLiteral, Path::new("RUF041.py"))] #[test_case(Rule::UnnecessaryNestedLiteral, Path::new("RUF041.pyi"))] #[test_case(Rule::IfKeyInDictDel, Path::new("RUF051.py"))] + #[test_case(Rule::FalsyDictGetFallback, Path::new("RUF056.py"))] #[test_case(Rule::UsedDummyVariable, Path::new("RUF052.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs b/crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs new file mode 100644 index 0000000000..18482224fe --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs @@ -0,0 +1,120 @@ +use crate::checkers::ast::Checker; +use crate::fix::edits::{remove_argument, Parentheses}; +use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Fix}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::{helpers::Truthiness, Expr, ExprAttribute, ExprName}; +use ruff_python_semantic::analyze::typing; +use ruff_python_semantic::SemanticModel; +use ruff_text_size::Ranged; + +/// ## What it does +/// Checks for `dict.get(key, falsy_value)` calls in boolean test positions. +/// +/// ## Why is this bad? +/// The default fallback `None` is already falsy. +/// +/// ## Example +/// +/// ```python +/// if dict.get(key, False): +/// ... +/// ``` +/// +/// Use instead: +/// +/// ```python +/// if dict.get(key): +/// ... +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the `dict.get()` call contains comments between arguments. +#[derive(ViolationMetadata)] +pub(crate) struct FalsyDictGetFallback; + +impl AlwaysFixableViolation for FalsyDictGetFallback { + #[derive_message_formats] + fn message(&self) -> String { + "Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy.".to_string() + } + + fn fix_title(&self) -> String { + "Remove falsy fallback from `dict.get()`".to_string() + } +} + +pub(crate) fn falsy_dict_get_fallback(checker: &mut Checker, expr: &Expr) { + let semantic = checker.semantic(); + + // Check if we are in a boolean test + if !semantic.in_boolean_test() { + return; + } + + // Check if the expression is a call + let Expr::Call(call) = expr else { + return; + }; + + // Check if the function being called is an attribute (e.g. `dict.get`) + let Expr::Attribute(ExprAttribute { value, attr, .. }) = &*call.func else { + return; + }; + + // Ensure the method called is `get` + if attr != "get" { + return; + } + + // Check if the object is a dictionary using the semantic model + if !value + .as_name_expr() + .is_some_and(|name| is_known_to_be_of_type_dict(semantic, name)) + { + return; + } + + // Get the fallback argument + let Some(fallback_arg) = call.arguments.find_argument("default", 1) else { + return; + }; + + // Check if the fallback is a falsy value + if Truthiness::from_expr(fallback_arg.value(), |id| semantic.has_builtin_binding(id)) + .into_bool() + != Some(false) + { + return; + } + + let mut diagnostic = Diagnostic::new(FalsyDictGetFallback, fallback_arg.range()); + + let comment_ranges = checker.comment_ranges(); + + // Determine applicability based on the presence of comments + let applicability = if comment_ranges.intersects(call.arguments.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.try_set_fix(|| { + remove_argument( + &fallback_arg, + &call.arguments, + Parentheses::Preserve, + checker.locator().contents(), + ) + .map(|edit| Fix::applicable_edit(edit, applicability)) + }); + + checker.diagnostics.push(diagnostic); +} + +fn is_known_to_be_of_type_dict(semantic: &SemanticModel, expr: &ExprName) -> bool { + let Some(binding) = semantic.only_binding(expr).map(|id| semantic.binding(id)) else { + return false; + }; + + typing::is_dict(binding, semantic) +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs b/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs index 2316e68e13..4d787d7873 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs @@ -103,7 +103,7 @@ fn is_dunder_version_split_dot(expr: &ast::Expr) -> bool { } let Some(ast::Expr::StringLiteral(ast::ExprStringLiteral { value, range: _ })) = - arguments.find_argument("sep", 0) + arguments.find_argument_value("sep", 0) else { return false; }; diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index e9e74d1a53..c612f86150 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -6,6 +6,7 @@ pub(crate) use collection_literal_concatenation::*; pub(crate) use decimal_from_float_literal::*; pub(crate) use default_factory_kwarg::*; pub(crate) use explicit_f_string_type_conversion::*; +pub(crate) use falsy_dict_get_fallback::*; pub(crate) use function_call_in_dataclass_default::*; pub(crate) use if_key_in_dict_del::*; pub(crate) use implicit_optional::*; @@ -54,6 +55,7 @@ mod confusables; mod decimal_from_float_literal; mod default_factory_kwarg; mod explicit_f_string_type_conversion; +mod falsy_dict_get_fallback; mod function_call_in_dataclass_default; mod helpers; mod if_key_in_dict_del; diff --git a/crates/ruff_linter/src/rules/ruff/rules/quadratic_list_summation.rs b/crates/ruff_linter/src/rules/ruff/rules/quadratic_list_summation.rs index d670f07366..84c904b7dc 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/quadratic_list_summation.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/quadratic_list_summation.rs @@ -128,7 +128,7 @@ fn convert_to_reduce(iterable: &Expr, call: &ast::ExprCall, checker: &Checker) - /// Returns `true` if the `start` argument to a `sum()` call is an empty list. fn start_is_empty_list(arguments: &Arguments, semantic: &SemanticModel) -> bool { - let Some(start_arg) = arguments.find_argument("start", 1) else { + let Some(start_arg) = arguments.find_argument_value("start", 1) else { return false; }; diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs index 2ab56d0692..7979bb14e7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs @@ -175,8 +175,8 @@ fn round_applicability(checker: &Checker, arguments: &Arguments) -> Option { diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs index f88afb8863..d4fe287a6f 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs @@ -173,14 +173,14 @@ impl<'a> ReFunc<'a> { // have already been filtered out from the `pattern` ("split", 2) => Some(ReFunc { kind: ReFuncKind::Split, - pattern: call.arguments.find_argument("pattern", 0)?, - string: call.arguments.find_argument("string", 1)?, + pattern: call.arguments.find_argument_value("pattern", 0)?, + string: call.arguments.find_argument_value("string", 1)?, }), // `sub` is only safe to fix if `repl` is a string. `re.sub` also // allows it to be a function, which will *not* work in the str // version ("sub", 3) => { - let repl = call.arguments.find_argument("repl", 1)?; + let repl = call.arguments.find_argument_value("repl", 1)?; let lit = resolve_string_literal(repl, semantic)?; let mut fixable = true; for (c, next) in lit.value.chars().tuple_windows() { @@ -207,24 +207,24 @@ impl<'a> ReFunc<'a> { kind: ReFuncKind::Sub { repl: fixable.then_some(repl), }, - pattern: call.arguments.find_argument("pattern", 0)?, - string: call.arguments.find_argument("string", 2)?, + pattern: call.arguments.find_argument_value("pattern", 0)?, + string: call.arguments.find_argument_value("string", 2)?, }) } ("match", 2) if in_if_context => Some(ReFunc { kind: ReFuncKind::Match, - pattern: call.arguments.find_argument("pattern", 0)?, - string: call.arguments.find_argument("string", 1)?, + pattern: call.arguments.find_argument_value("pattern", 0)?, + string: call.arguments.find_argument_value("string", 1)?, }), ("search", 2) if in_if_context => Some(ReFunc { kind: ReFuncKind::Search, - pattern: call.arguments.find_argument("pattern", 0)?, - string: call.arguments.find_argument("string", 1)?, + pattern: call.arguments.find_argument_value("pattern", 0)?, + string: call.arguments.find_argument_value("string", 1)?, }), ("fullmatch", 2) if in_if_context => Some(ReFunc { kind: ReFuncKind::Fullmatch, - pattern: call.arguments.find_argument("pattern", 0)?, - string: call.arguments.find_argument("string", 1)?, + pattern: call.arguments.find_argument_value("pattern", 0)?, + string: call.arguments.find_argument_value("string", 1)?, }), _ => None, } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap new file mode 100644 index 0000000000..0532ef08e6 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap @@ -0,0 +1,485 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +snapshot_kind: text +--- +RUF056.py:117:43: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +116 | # dict.get in ternary expression +117 | value = "not found" if my_dict.get("key", False) else "default" # [RUF056] + | ^^^^^ RUF056 +118 | +119 | # dict.get in an if statement + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +114 114 | # Invalid falsy fallbacks are when the call to dict.get is used in a boolean context +115 115 | +116 116 | # dict.get in ternary expression +117 |-value = "not found" if my_dict.get("key", False) else "default" # [RUF056] + 117 |+value = "not found" if my_dict.get("key") else "default" # [RUF056] +118 118 | +119 119 | # dict.get in an if statement +120 120 | if my_dict.get("key", False): # [RUF056] + +RUF056.py:120:23: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +119 | # dict.get in an if statement +120 | if my_dict.get("key", False): # [RUF056] + | ^^^^^ RUF056 +121 | pass + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +117 117 | value = "not found" if my_dict.get("key", False) else "default" # [RUF056] +118 118 | +119 119 | # dict.get in an if statement +120 |-if my_dict.get("key", False): # [RUF056] + 120 |+if my_dict.get("key"): # [RUF056] +121 121 | pass +122 122 | +123 123 | # dict.get in compound if statement + +RUF056.py:124:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +123 | # dict.get in compound if statement +124 | if True and my_dict.get("key", False): # [RUF056] + | ^^^^^ RUF056 +125 | pass + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +121 121 | pass +122 122 | +123 123 | # dict.get in compound if statement +124 |-if True and my_dict.get("key", False): # [RUF056] + 124 |+if True and my_dict.get("key"): # [RUF056] +125 125 | pass +126 126 | +127 127 | if my_dict.get("key", False) or True: # [RUF056] + +RUF056.py:127:23: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +125 | pass +126 | +127 | if my_dict.get("key", False) or True: # [RUF056] + | ^^^^^ RUF056 +128 | pass + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +124 124 | if True and my_dict.get("key", False): # [RUF056] +125 125 | pass +126 126 | +127 |-if my_dict.get("key", False) or True: # [RUF056] + 127 |+if my_dict.get("key") or True: # [RUF056] +128 128 | pass +129 129 | +130 130 | # dict.get in an assert statement + +RUF056.py:131:27: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +130 | # dict.get in an assert statement +131 | assert my_dict.get("key", False) # [RUF056] + | ^^^^^ RUF056 +132 | +133 | # dict.get in a while statement + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +128 128 | pass +129 129 | +130 130 | # dict.get in an assert statement +131 |-assert my_dict.get("key", False) # [RUF056] + 131 |+assert my_dict.get("key") # [RUF056] +132 132 | +133 133 | # dict.get in a while statement +134 134 | while my_dict.get("key", False): # [RUF056] + +RUF056.py:134:26: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +133 | # dict.get in a while statement +134 | while my_dict.get("key", False): # [RUF056] + | ^^^^^ RUF056 +135 | pass + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +131 131 | assert my_dict.get("key", False) # [RUF056] +132 132 | +133 133 | # dict.get in a while statement +134 |-while my_dict.get("key", False): # [RUF056] + 134 |+while my_dict.get("key"): # [RUF056] +135 135 | pass +136 136 | +137 137 | # dict.get in unary not expression + +RUF056.py:138:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +137 | # dict.get in unary not expression +138 | value = not my_dict.get("key", False) # [RUF056] + | ^^^^^ RUF056 +139 | +140 | # testing all falsy fallbacks + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +135 135 | pass +136 136 | +137 137 | # dict.get in unary not expression +138 |-value = not my_dict.get("key", False) # [RUF056] + 138 |+value = not my_dict.get("key") # [RUF056] +139 139 | +140 140 | # testing all falsy fallbacks +141 141 | value = not my_dict.get("key", False) # [RUF056] + +RUF056.py:141:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +140 | # testing all falsy fallbacks +141 | value = not my_dict.get("key", False) # [RUF056] + | ^^^^^ RUF056 +142 | value = not my_dict.get("key", []) # [RUF056] +143 | value = not my_dict.get("key", list()) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +138 138 | value = not my_dict.get("key", False) # [RUF056] +139 139 | +140 140 | # testing all falsy fallbacks +141 |-value = not my_dict.get("key", False) # [RUF056] + 141 |+value = not my_dict.get("key") # [RUF056] +142 142 | value = not my_dict.get("key", []) # [RUF056] +143 143 | value = not my_dict.get("key", list()) # [RUF056] +144 144 | value = not my_dict.get("key", {}) # [RUF056] + +RUF056.py:142:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +140 | # testing all falsy fallbacks +141 | value = not my_dict.get("key", False) # [RUF056] +142 | value = not my_dict.get("key", []) # [RUF056] + | ^^ RUF056 +143 | value = not my_dict.get("key", list()) # [RUF056] +144 | value = not my_dict.get("key", {}) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +139 139 | +140 140 | # testing all falsy fallbacks +141 141 | value = not my_dict.get("key", False) # [RUF056] +142 |-value = not my_dict.get("key", []) # [RUF056] + 142 |+value = not my_dict.get("key") # [RUF056] +143 143 | value = not my_dict.get("key", list()) # [RUF056] +144 144 | value = not my_dict.get("key", {}) # [RUF056] +145 145 | value = not my_dict.get("key", dict()) # [RUF056] + +RUF056.py:143:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^^^^ RUF056 +144 | value = not my_dict.get("key", {}) # [RUF056] +145 | value = not my_dict.get("key", dict()) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +140 140 | # testing all falsy fallbacks +141 141 | value = not my_dict.get("key", False) # [RUF056] +142 142 | value = not my_dict.get("key", []) # [RUF056] +143 |-value = not my_dict.get("key", list()) # [RUF056] + 143 |+value = not my_dict.get("key") # [RUF056] +144 144 | value = not my_dict.get("key", {}) # [RUF056] +145 145 | value = not my_dict.get("key", dict()) # [RUF056] +146 146 | value = not my_dict.get("key", set()) # [RUF056] + +RUF056.py:144:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^ RUF056 +145 | value = not my_dict.get("key", dict()) # [RUF056] +146 | value = not my_dict.get("key", set()) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +141 141 | value = not my_dict.get("key", False) # [RUF056] +142 142 | value = not my_dict.get("key", []) # [RUF056] +143 143 | value = not my_dict.get("key", list()) # [RUF056] +144 |-value = not my_dict.get("key", {}) # [RUF056] + 144 |+value = not my_dict.get("key") # [RUF056] +145 145 | value = not my_dict.get("key", dict()) # [RUF056] +146 146 | value = not my_dict.get("key", set()) # [RUF056] +147 147 | value = not my_dict.get("key", None) # [RUF056] + +RUF056.py:145:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^^^^ RUF056 +146 | value = not my_dict.get("key", set()) # [RUF056] +147 | value = not my_dict.get("key", None) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +142 142 | value = not my_dict.get("key", []) # [RUF056] +143 143 | value = not my_dict.get("key", list()) # [RUF056] +144 144 | value = not my_dict.get("key", {}) # [RUF056] +145 |-value = not my_dict.get("key", dict()) # [RUF056] + 145 |+value = not my_dict.get("key") # [RUF056] +146 146 | value = not my_dict.get("key", set()) # [RUF056] +147 147 | value = not my_dict.get("key", None) # [RUF056] +148 148 | value = not my_dict.get("key", 0) # [RUF056] + +RUF056.py:146:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^^^ RUF056 +147 | value = not my_dict.get("key", None) # [RUF056] +148 | value = not my_dict.get("key", 0) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +143 143 | value = not my_dict.get("key", list()) # [RUF056] +144 144 | value = not my_dict.get("key", {}) # [RUF056] +145 145 | value = not my_dict.get("key", dict()) # [RUF056] +146 |-value = not my_dict.get("key", set()) # [RUF056] + 146 |+value = not my_dict.get("key") # [RUF056] +147 147 | value = not my_dict.get("key", None) # [RUF056] +148 148 | value = not my_dict.get("key", 0) # [RUF056] +149 149 | value = not my_dict.get("key", 0.0) # [RUF056] + +RUF056.py:147:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^^ RUF056 +148 | value = not my_dict.get("key", 0) # [RUF056] +149 | value = not my_dict.get("key", 0.0) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +144 144 | value = not my_dict.get("key", {}) # [RUF056] +145 145 | value = not my_dict.get("key", dict()) # [RUF056] +146 146 | value = not my_dict.get("key", set()) # [RUF056] +147 |-value = not my_dict.get("key", None) # [RUF056] + 147 |+value = not my_dict.get("key") # [RUF056] +148 148 | value = not my_dict.get("key", 0) # [RUF056] +149 149 | value = not my_dict.get("key", 0.0) # [RUF056] +150 150 | value = not my_dict.get("key", "") # [RUF056] + +RUF056.py:148:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^ RUF056 +149 | value = not my_dict.get("key", 0.0) # [RUF056] +150 | value = not my_dict.get("key", "") # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +145 145 | value = not my_dict.get("key", dict()) # [RUF056] +146 146 | value = not my_dict.get("key", set()) # [RUF056] +147 147 | value = not my_dict.get("key", None) # [RUF056] +148 |-value = not my_dict.get("key", 0) # [RUF056] + 148 |+value = not my_dict.get("key") # [RUF056] +149 149 | value = not my_dict.get("key", 0.0) # [RUF056] +150 150 | value = not my_dict.get("key", "") # [RUF056] +151 151 | + +RUF056.py:149:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^ RUF056 +150 | value = not my_dict.get("key", "") # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +146 146 | value = not my_dict.get("key", set()) # [RUF056] +147 147 | value = not my_dict.get("key", None) # [RUF056] +148 148 | value = not my_dict.get("key", 0) # [RUF056] +149 |-value = not my_dict.get("key", 0.0) # [RUF056] + 149 |+value = not my_dict.get("key") # [RUF056] +150 150 | value = not my_dict.get("key", "") # [RUF056] +151 151 | +152 152 | # testing dict.get call using kwargs + +RUF056.py:150:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^ RUF056 +151 | +152 | # testing dict.get call using kwargs + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +147 147 | value = not my_dict.get("key", None) # [RUF056] +148 148 | value = not my_dict.get("key", 0) # [RUF056] +149 149 | value = not my_dict.get("key", 0.0) # [RUF056] +150 |-value = not my_dict.get("key", "") # [RUF056] + 150 |+value = not my_dict.get("key") # [RUF056] +151 151 | +152 152 | # testing dict.get call using kwargs +153 153 | value = not my_dict.get(key="key", default=False) # [RUF056] + +RUF056.py:153:36: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +152 | # testing dict.get call using kwargs +153 | value = not my_dict.get(key="key", default=False) # [RUF056] + | ^^^^^^^^^^^^^ RUF056 +154 | value = not my_dict.get(default=[], key="key") # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +150 150 | value = not my_dict.get("key", "") # [RUF056] +151 151 | +152 152 | # testing dict.get call using kwargs +153 |-value = not my_dict.get(key="key", default=False) # [RUF056] + 153 |+value = not my_dict.get(key="key") # [RUF056] +154 154 | value = not my_dict.get(default=[], key="key") # [RUF056] +155 155 | +156 156 | # testing invalid dict.get call with inline comment + +RUF056.py:154:25: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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] + | ^^^^^^^^^^ RUF056 +155 | +156 | # testing invalid dict.get call with inline comment + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +151 151 | +152 152 | # testing dict.get call using kwargs +153 153 | value = not my_dict.get(key="key", default=False) # [RUF056] +154 |-value = not my_dict.get(default=[], key="key") # [RUF056] + 154 |+value = not my_dict.get(key="key") # [RUF056] +155 155 | +156 156 | # testing invalid dict.get call with inline comment +157 157 | value = not my_dict.get("key", # comment1 + +RUF056.py:158:22: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +156 | # testing invalid dict.get call with inline comment +157 | value = not my_dict.get("key", # comment1 +158 | [] # comment2 + | ^^ RUF056 +159 | ) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Unsafe fix +154 154 | value = not my_dict.get(default=[], key="key") # [RUF056] +155 155 | +156 156 | # testing invalid dict.get call with inline comment +157 |-value = not my_dict.get("key", # comment1 +158 |- [] # comment2 + 157 |+value = not my_dict.get("key" # comment2 +159 158 | ) # [RUF056] +160 159 | +161 160 | # testing invalid dict.get call with kwargs and inline comment + +RUF056.py:163:25: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +161 | # testing invalid dict.get call with kwargs and inline comment +162 | value = not my_dict.get(key="key", # comment1 +163 | default=False # comment2 + | ^^^^^^^^^^^^^ RUF056 +164 | ) # [RUF056] +165 | value = not my_dict.get(default=[], # comment1 + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Unsafe fix +159 159 | ) # [RUF056] +160 160 | +161 161 | # testing invalid dict.get call with kwargs and inline comment +162 |-value = not my_dict.get(key="key", # comment1 +163 |- default=False # comment2 + 162 |+value = not my_dict.get(key="key" # comment2 +164 163 | ) # [RUF056] +165 164 | value = not my_dict.get(default=[], # comment1 +166 165 | key="key" # comment2 + +RUF056.py:165:25: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +163 | default=False # comment2 +164 | ) # [RUF056] +165 | value = not my_dict.get(default=[], # comment1 + | ^^^^^^^^^^ RUF056 +166 | key="key" # comment2 +167 | ) # [RUF056] + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Unsafe fix +162 162 | value = not my_dict.get(key="key", # comment1 +163 163 | default=False # comment2 +164 164 | ) # [RUF056] +165 |-value = not my_dict.get(default=[], # comment1 + 165 |+value = not my_dict.get(# comment1 +166 166 | key="key" # comment2 +167 167 | ) # [RUF056] +168 168 | + +RUF056.py:170:55: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +169 | # testing invalid dict.get calls +170 | value = not my_dict.get(key="key", other="something", default=False) + | ^^^^^^^^^^^^^ RUF056 +171 | value = not my_dict.get(default=False, other="something", key="test") + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +167 167 | ) # [RUF056] +168 168 | +169 169 | # testing invalid dict.get calls +170 |-value = not my_dict.get(key="key", other="something", default=False) + 170 |+value = not my_dict.get(key="key", other="something") +171 171 | value = not my_dict.get(default=False, other="something", key="test") + +RUF056.py:171:25: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +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") + | ^^^^^^^^^^^^^ RUF056 + | + = help: Remove falsy fallback from `dict.get()` + +ℹ Safe fix +168 168 | +169 169 | # testing invalid dict.get calls +170 170 | value = not my_dict.get(key="key", other="something", default=False) +171 |-value = not my_dict.get(default=False, other="something", key="test") + 171 |+value = not my_dict.get(other="something", key="test") diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 4904246eed..639f8d16dd 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -3780,6 +3780,15 @@ pub enum ArgOrKeyword<'a> { Keyword(&'a Keyword), } +impl<'a> ArgOrKeyword<'a> { + pub const fn value(&self) -> &'a Expr { + match self { + ArgOrKeyword::Arg(argument) => argument, + ArgOrKeyword::Keyword(keyword) => &keyword.value, + } + } +} + impl<'a> From<&'a Expr> for ArgOrKeyword<'a> { fn from(arg: &'a Expr) -> Self { Self::Arg(arg) @@ -3828,15 +3837,24 @@ impl Arguments { .nth(position) } - /// Return the argument with the given name or at the given position, or `None` if no such - /// argument exists. Used to retrieve arguments that can be provided _either_ as keyword or + /// Return the value for the argument with the given name or at the given position, or `None` if no such + /// argument exists. Used to retrieve argument values that can be provided _either_ as keyword or /// positional arguments. - pub fn find_argument(&self, name: &str, position: usize) -> Option<&Expr> { + pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> { self.find_keyword(name) .map(|keyword| &keyword.value) .or_else(|| self.find_positional(position)) } + /// Return the the argument with the given name or at the given position, or `None` if no such + /// argument exists. Used to retrieve arguments that can be provided _either_ as keyword or + /// positional arguments. + pub fn find_argument(&self, name: &str, position: usize) -> Option { + self.find_keyword(name) + .map(ArgOrKeyword::from) + .or_else(|| self.find_positional(position).map(ArgOrKeyword::from)) + } + /// Return the positional and keyword arguments in the order of declaration. /// /// Positional arguments are generally before keyword arguments, but star arguments are an diff --git a/ruff.schema.json b/ruff.schema.json index ca9e726de5..e92605f140 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3865,6 +3865,7 @@ "RUF051", "RUF052", "RUF055", + "RUF056", "RUF1", "RUF10", "RUF100",