Skip to content

Commit 11f06e0

Browse files
authored
Detect SIM910 when using variadic keyword arguments, i.e., **kwargs (#13503)
Closes #13493
1 parent f27a8b8 commit 11f06e0

File tree

3 files changed

+75
-2
lines changed

3 files changed

+75
-2
lines changed

Diff for: crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM910.py

+16
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,19 @@
3333
# OK
3434
ages = ["Tom", "Maria", "Dog"]
3535
age = ages.get("Cat", None)
36+
37+
# SIM910
38+
def foo(**kwargs):
39+
a = kwargs.get('a', None)
40+
41+
# SIM910
42+
def foo(some_dict: dict):
43+
a = some_dict.get('a', None)
44+
45+
# OK
46+
def foo(some_other: object):
47+
a = some_other.get('a', None)
48+
49+
# OK
50+
def foo(some_other):
51+
a = some_other.get('a', None)

Diff for: crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM910_SIM910.py.snap

+40
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,44 @@ SIM910.py:31:7: SIM910 [*] Use `ages.get("Cat")` instead of `ages.get("Cat", Non
119119
33 33 | # OK
120120
34 34 | ages = ["Tom", "Maria", "Dog"]
121121

122+
SIM910.py:39:9: SIM910 [*] Use `kwargs.get('a')` instead of `kwargs.get('a', None)`
123+
|
124+
37 | # SIM910
125+
38 | def foo(**kwargs):
126+
39 | a = kwargs.get('a', None)
127+
| ^^^^^^^^^^^^^^^^^^^^^ SIM910
128+
40 |
129+
41 | # SIM910
130+
|
131+
= help: Replace `kwargs.get('a', None)` with `kwargs.get('a')`
132+
133+
Safe fix
134+
36 36 |
135+
37 37 | # SIM910
136+
38 38 | def foo(**kwargs):
137+
39 |- a = kwargs.get('a', None)
138+
39 |+ a = kwargs.get('a')
139+
40 40 |
140+
41 41 | # SIM910
141+
42 42 | def foo(some_dict: dict):
122142

143+
SIM910.py:43:9: SIM910 [*] Use `some_dict.get('a')` instead of `some_dict.get('a', None)`
144+
|
145+
41 | # SIM910
146+
42 | def foo(some_dict: dict):
147+
43 | a = some_dict.get('a', None)
148+
| ^^^^^^^^^^^^^^^^^^^^^^^^ SIM910
149+
44 |
150+
45 | # OK
151+
|
152+
= help: Replace `some_dict.get('a', None)` with `some_dict.get('a')`
153+
154+
Safe fix
155+
40 40 |
156+
41 41 | # SIM910
157+
42 42 | def foo(some_dict: dict):
158+
43 |- a = some_dict.get('a', None)
159+
43 |+ a = some_dict.get('a')
160+
44 44 |
161+
45 45 | # OK
162+
46 46 | def foo(some_other: object):

Diff for: crates/ruff_python_semantic/src/analyze/typing.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -738,9 +738,26 @@ pub fn is_list(binding: &Binding, semantic: &SemanticModel) -> bool {
738738

739739
/// Test whether the given binding can be considered a dictionary.
740740
///
741-
/// For this, we check what value might be associated with it through it's initialization and
742-
/// what annotation it has (we consider `dict` and `typing.Dict`).
741+
/// For this, we check what value might be associated with it through it's initialization,
742+
/// what annotation it has (we consider `dict` and `typing.Dict`), and if it is a variadic keyword
743+
/// argument parameter.
743744
pub fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool {
745+
// ```python
746+
// def foo(**kwargs):
747+
// ...
748+
// ```
749+
if matches!(binding.kind, BindingKind::Argument) {
750+
if let Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) =
751+
binding.statement(semantic)
752+
{
753+
if let Some(kwarg_parameter) = parameters.kwarg.as_deref() {
754+
if kwarg_parameter.name.range() == binding.range() {
755+
return true;
756+
}
757+
}
758+
}
759+
}
760+
744761
check_type::<DictChecker>(binding, semantic)
745762
}
746763

0 commit comments

Comments
 (0)