Skip to content

Commit

Permalink
Detect SIM910 when using variadic keyword arguments, i.e., **kwargs
Browse files Browse the repository at this point in the history
Closes #13493
  • Loading branch information
zanieb committed Sep 25, 2024
1 parent 03503f7 commit 9da25d5
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,19 @@
# OK
ages = ["Tom", "Maria", "Dog"]
age = ages.get("Cat", None)

# SIM910
def foo(**kwargs):
a = kwargs.get('a', None)

# SIM910
def foo(some_dict: dict):
a = some_dict.get('a', None)

# OK
def foo(some_other: object):
a = some_other.get('a', None)

# OK
def foo(some_other):
a = some_other.get('a', None)
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,44 @@ SIM910.py:31:7: SIM910 [*] Use `ages.get("Cat")` instead of `ages.get("Cat", Non
33 33 | # OK
34 34 | ages = ["Tom", "Maria", "Dog"]

SIM910.py:39:9: SIM910 [*] Use `kwargs.get('a')` instead of `kwargs.get('a', None)`
|
37 | # SIM910
38 | def foo(**kwargs):
39 | a = kwargs.get('a', None)
| ^^^^^^^^^^^^^^^^^^^^^ SIM910
40 |
41 | # SIM910
|
= help: Replace `kwargs.get('a', None)` with `kwargs.get('a')`

Safe fix
36 36 |
37 37 | # SIM910
38 38 | def foo(**kwargs):
39 |- a = kwargs.get('a', None)
39 |+ a = kwargs.get('a')
40 40 |
41 41 | # SIM910
42 42 | def foo(some_dict: dict):

SIM910.py:43:9: SIM910 [*] Use `some_dict.get('a')` instead of `some_dict.get('a', None)`
|
41 | # SIM910
42 | def foo(some_dict: dict):
43 | a = some_dict.get('a', None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ SIM910
44 |
45 | # OK
|
= help: Replace `some_dict.get('a', None)` with `some_dict.get('a')`

Safe fix
40 40 |
41 41 | # SIM910
42 42 | def foo(some_dict: dict):
43 |- a = some_dict.get('a', None)
43 |+ a = some_dict.get('a')
44 44 |
45 45 | # OK
46 46 | def foo(some_other: object):
21 changes: 19 additions & 2 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,9 +738,26 @@ pub fn is_list(binding: &Binding, semantic: &SemanticModel) -> bool {

/// Test whether the given binding can be considered a dictionary.
///
/// For this, we check what value might be associated with it through it's initialization and
/// what annotation it has (we consider `dict` and `typing.Dict`).
/// For this, we check what value might be associated with it through it's initialization,
/// what annotation it has (we consider `dict` and `typing.Dict`), and if it is a variadic keyword
/// argument parameter.
pub fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool {
// ```python
// def foo(**kwargs):
// ...
// ```
if matches!(binding.kind, BindingKind::Argument) {
if let Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) =
binding.statement(semantic)
{
if let Some(kwarg_parameter) = parameters.kwarg.as_deref() {
if kwarg_parameter.name.range() == binding.range() {
return true;
}
}
}
}

check_type::<DictChecker>(binding, semantic)
}

Expand Down

0 comments on commit 9da25d5

Please sign in to comment.