diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py index 831ec4bcead15..0755092b8bfdb 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py @@ -149,23 +149,39 @@ def inner(): 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 +# regression tests for https://github.com/astral-sh/ruff/issues/18628 +# we should avoid fixes when there are "unknown" arguments present, including +# extra positional arguments, either of the positional-only arguments passed as +# a keyword, or completely unknown keywords. + +# extra positional +not my_dict.get("key", False, "?!") + +# `default` is positional-only, so these are invalid +not my_dict.get("key", default=False) +not my_dict.get(key="key", default=False) +not my_dict.get(default=[], key="key") +not my_dict.get(default=False) +not my_dict.get(key="key", other="something", default=False) +not my_dict.get(default=False, other="something", key="test") + +# comments don't really matter here because of the kwargs but include them for +# completeness +not my_dict.get( + key="key", # comment1 + default=False, # comment2 +) # comment 3 +not my_dict.get( + default=[], # comment1 + key="key", # comment2 +) # comment 3 + +# the fix is arguably okay here because the same `takes no keyword arguments` +# TypeError is raised at runtime before and after the fix, but we still bail +# out for having an unrecognized number of arguments +not my_dict.get("key", False, foo=...) 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 index 64b0bcdc71768..c7064b6a2c2ba 100644 --- 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 @@ -5,7 +5,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::fix::edits::{Parentheses, remove_argument}; -use crate::{AlwaysFixableViolation, Applicability, Fix}; +use crate::{Applicability, Fix, FixAvailability, Violation}; /// ## What it does /// Checks for `dict.get(key, falsy_value)` calls in boolean test positions. @@ -28,21 +28,34 @@ use crate::{AlwaysFixableViolation, Applicability, Fix}; /// ``` /// /// ## Fix safety -/// This rule's fix is marked as safe, unless the `dict.get()` call contains comments between arguments. +/// +/// This rule's fix is marked as safe, unless the `dict.get()` call contains comments between +/// arguments that will be deleted. +/// +/// ## Fix availability +/// +/// This rule's fix is unavailable in cases where invalid arguments are provided to `dict.get`. As +/// shown in the [documentation], `dict.get` takes two positional-only arguments, so invalid cases +/// are identified by the presence of more than two arguments or any keyword arguments. +/// +/// [documentation]: https://docs.python.org/3.13/library/stdtypes.html#dict.get #[derive(ViolationMetadata)] pub(crate) struct FalsyDictGetFallback; -impl AlwaysFixableViolation for FalsyDictGetFallback { +impl Violation for FalsyDictGetFallback { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[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() + fn fix_title(&self) -> Option { + Some("Remove falsy fallback from `dict.get()`".to_string()) } } +/// RUF056 pub(crate) fn falsy_dict_get_fallback(checker: &Checker, expr: &Expr) { let semantic = checker.semantic(); @@ -89,6 +102,16 @@ pub(crate) fn falsy_dict_get_fallback(checker: &Checker, expr: &Expr) { let mut diagnostic = checker.report_diagnostic(FalsyDictGetFallback, fallback_arg.range()); + // All arguments to `dict.get` are positional-only. + if !call.arguments.keywords.is_empty() { + return; + } + + // And there are only two of them, at most. + if call.arguments.args.len() > 2 { + return; + } + let comment_ranges = checker.comment_ranges(); // Determine applicability based on the presence of comments 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 index c269084fef655..044f2db0fe9b8 100644 --- 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 @@ -323,7 +323,7 @@ RUF056.py:149:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in 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 +152 152 | # testing invalid dict.get call with inline comment RUF056.py:150:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. | @@ -332,7 +332,7 @@ RUF056.py:150:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in 150 | value = not my_dict.get("key", "") # [RUF056] | ^^ RUF056 151 | -152 | # testing dict.get call using kwargs +152 | # testing invalid dict.get call with inline comment | = help: Remove falsy fallback from `dict.get()` @@ -343,142 +343,131 @@ RUF056.py:150:32: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in 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] +152 152 | # testing invalid dict.get call with inline comment +153 153 | value = not my_dict.get("key", # comment1 -RUF056.py:153:36: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. +RUF056.py:154:22: 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] +152 | # testing invalid dict.get call with inline comment +153 | value = not my_dict.get("key", # comment1 +154 | [] # comment2 + | ^^ RUF056 +155 | ) # [RUF056] | = help: Remove falsy fallback from `dict.get()` -ℹ Safe fix +ℹ Unsafe 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 +152 152 | # testing invalid dict.get call with inline comment +153 |-value = not my_dict.get("key", # comment1 +154 |- [] # comment2 + 153 |+value = not my_dict.get("key" # comment2 +155 154 | ) # [RUF056] +156 155 | +157 156 | # regression tests for https://github.com/astral-sh/ruff/issues/18628 + +RUF056.py:163:24: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +162 | # extra positional +163 | not my_dict.get("key", False, "?!") + | ^^^^^ RUF056 +164 | +165 | # `default` is positional-only, so these are invalid | = 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] +RUF056.py:166:24: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +165 | # `default` is positional-only, so these are invalid +166 | not my_dict.get("key", default=False) + | ^^^^^^^^^^^^^ RUF056 +167 | not my_dict.get(key="key", default=False) +168 | not my_dict.get(default=[], key="key") | = 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 +RUF056.py:167:28: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +165 | # `default` is positional-only, so these are invalid +166 | not my_dict.get("key", default=False) +167 | not my_dict.get(key="key", default=False) + | ^^^^^^^^^^^^^ RUF056 +168 | not my_dict.get(default=[], key="key") +169 | not my_dict.get(default=False) | = 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] +RUF056.py:168:17: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +166 | not my_dict.get("key", default=False) +167 | not my_dict.get(key="key", default=False) +168 | not my_dict.get(default=[], key="key") + | ^^^^^^^^^^ RUF056 +169 | not my_dict.get(default=False) +170 | not my_dict.get(key="key", other="something", default=False) | = 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") +RUF056.py:169:17: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +167 | not my_dict.get(key="key", default=False) +168 | not my_dict.get(default=[], key="key") +169 | not my_dict.get(default=False) + | ^^^^^^^^^^^^^ RUF056 +170 | not my_dict.get(key="key", other="something", default=False) +171 | 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:170:47: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +168 | not my_dict.get(default=[], key="key") +169 | not my_dict.get(default=False) +170 | not my_dict.get(key="key", other="something", default=False) + | ^^^^^^^^^^^^^ RUF056 +171 | not my_dict.get(default=False, other="something", key="test") + | + = help: Remove falsy fallback from `dict.get()` -RUF056.py:171:25: RUF056 [*] Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. +RUF056.py:171:17: 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 +169 | not my_dict.get(default=False) +170 | not my_dict.get(key="key", other="something", default=False) +171 | not my_dict.get(default=False, other="something", key="test") + | ^^^^^^^^^^^^^ RUF056 +172 | +173 | # comments don't really matter here because of the kwargs but include them for | = 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") +RUF056.py:177:5: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +175 | not my_dict.get( +176 | key="key", # comment1 +177 | default=False, # comment2 + | ^^^^^^^^^^^^^ RUF056 +178 | ) # comment 3 +179 | not my_dict.get( + | + = help: Remove falsy fallback from `dict.get()` + +RUF056.py:180:5: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +178 | ) # comment 3 +179 | not my_dict.get( +180 | default=[], # comment1 + | ^^^^^^^^^^ RUF056 +181 | key="key", # comment2 +182 | ) # comment 3 + | + = help: Remove falsy fallback from `dict.get()` + +RUF056.py:187:24: RUF056 Avoid providing a falsy fallback to `dict.get()` in boolean test positions. The default fallback `None` is already falsy. + | +185 | # TypeError is raised at runtime before and after the fix, but we still bail +186 | # out for having an unrecognized number of arguments +187 | not my_dict.get("key", False, foo=...) + | ^^^^^ RUF056 + | + = help: Remove falsy fallback from `dict.get()`