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 24, 2024
1 parent 03503f7 commit 60b87df
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 6 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):
24 changes: 18 additions & 6 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,13 +536,22 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
},

BindingKind::Argument => match binding.statement(semantic) {
// ```python
// def foo(x: annotation):
// ...
// ```
//
// We trust the annotation and see if the type checker matches the annotation.
Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) => {
// ```python
// def foo(**kwargs):
// ...
// ```
if let Some(kwarg_parameter) = parameters.kwarg.as_deref() {
if kwarg_parameter.name.range() == binding.range() {
return true;
}
}
// ```python
// def foo(x: annotation):
// ...
// ```
//
// We trust the annotation and see if the type checker matches the annotation.
let Some(parameter) = find_parameter(parameters.as_ref(), binding) else {
return false;
};
Expand All @@ -564,6 +573,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
T::match_annotation(annotation.as_ref(), semantic)
}

_ => false,
},

Expand Down Expand Up @@ -774,6 +784,8 @@ pub fn is_io_base_expr(expr: &Expr, semantic: &SemanticModel) -> bool {
}

/// Find the [`ParameterWithDefault`] corresponding to the given [`Binding`].
///
/// Excludes varaiadic paramters.
#[inline]
fn find_parameter<'a>(
parameters: &'a Parameters,
Expand Down

0 comments on commit 60b87df

Please sign in to comment.