Skip to content

[ruff] Validate arguments before offering a fix (RUF056) #18631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 13, 2025
Merged

[ruff] Validate arguments before offering a fix (RUF056) #18631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e553151
add regression tests
ntBre Jun 11, 2025
27be8ea
make the fix sometimes available, avoid fix for invalid args
ntBre Jun 11, 2025
a973920
add case without a key argument
ntBre Jun 11, 2025
57dbdef
regroup kwarg test cases
ntBre Jun 13, 2025
1626487
regroup a few more cases
ntBre Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF056.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
# 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)
Copy link
Member

@MichaReiser MichaReiser Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the tests on line 152 or change them, or group them?

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=...)
33 changes: 28 additions & 5 deletions crates/ruff_linter/src/rules/ruff/rules/falsy_dict_get_fallback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<String> {
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();

Expand Down Expand Up @@ -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
Expand Down
217 changes: 103 additions & 114 deletions ...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF056_RUF056.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
|
Expand All @@ -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()`

Expand All @@ -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()`
Loading