Skip to content
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.

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

Add Implementation for Pylint E1132: Repeated Keyword #8706

Merged
merged 7 commits into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
def func(a=10, b=20, c=30):
pass

# Valid
func(a=11, b=21, c=31)
func(b=11, a=21, c=31)
func(c=11, a=21, b=31)
func(a=11, b=31, **{"c": 41})
func(a=11, **{"b": 21}, **{"c": 41})
func(a=11, **{"b": 21, "c": 41})
func(**{"b": 21, "c": 41})
func(**{"a": 11}, **{"b": 21}, **{"c": 41})
func(**{"a": 11, "b": 21, "c": 41})

# Invalid
func(a=11, c=31, **{"c": 41})
func(a=11, c=31, **{"c": 41, "a": 51})
func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
func(a=11, b=21, **{"c": 31}, **{"c": 32})
func(a=11, b=21, **{"c": 31, "c": 32})
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::NestedMinMax) {
pylint::rules::nested_min_max(checker, expr, func, args, keywords);
}
if checker.enabled(Rule::RepeatedKeywordArgument) {
pylint::rules::repeated_keyword_argument(checker, call);
}
if checker.enabled(Rule::PytestPatchWithLambda) {
if let Some(diagnostic) = flake8_pytest_style::rules::patch_with_lambda(call) {
checker.diagnostics.push(diagnostic);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject),
(Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat),
(Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise),
(Pylint, "E1132") => (RuleGroup::Preview, rules::pylint::rules::RepeatedKeywordArgument),
(Pylint, "E1142") => (RuleGroup::Stable, rules::pylint::rules::AwaitOutsideAsync),
(Pylint, "E1205") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooManyArgs),
(Pylint, "E1206") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooFewArgs),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ mod tests {
#[test_case(Rule::UnnecessaryLambda, Path::new("unnecessary_lambda.py"))]
#[test_case(Rule::NonAsciiImportName, Path::new("non_ascii_module_import.py"))]
#[test_case(Rule::NonAsciiName, Path::new("non_ascii_name.py"))]
#[test_case(
Rule::RepeatedKeywordArgument,
Path::new("repeated_keyword_argument.py")
)]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub(crate) use redefined_argument_from_local::*;
pub(crate) use redefined_loop_name::*;
pub(crate) use repeated_equality_comparison::*;
pub(crate) use repeated_isinstance_calls::*;
pub(crate) use repeated_keyword_argument::*;
pub(crate) use return_in_init::*;
pub(crate) use self_assigning_variable::*;
pub(crate) use single_string_slots::*;
Expand Down Expand Up @@ -116,6 +117,7 @@ mod redefined_argument_from_local;
mod redefined_loop_name;
mod repeated_equality_comparison;
mod repeated_isinstance_calls;
mod repeated_keyword_argument;
mod return_in_init;
mod self_assigning_variable;
mod single_string_slots;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
use std::hash::BuildHasherDefault;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Arguments, Expr, ExprCall, ExprDict, ExprStringLiteral};
use ruff_text_size::Ranged;
use rustc_hash::FxHashSet;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for repeated keyword arguments in function calls.
///
/// ## Why is this bad?
/// Python does not allow repeated keyword arguments in function calls. If a
/// function is called with the same keyword argument multiple times, the
/// interpreter will raise an exception.
///
/// ## Example
/// ```python
/// func(1, 2, c=3, **{"c": 4})
/// ```
///
/// ## References
/// - [Python documentation: Argument](https://docs.python.org/3/glossary.html#term-argument)
#[violation]
pub struct RepeatedKeywordArgument {
duplicate_keyword: String,
}

impl Violation for RepeatedKeywordArgument {
#[derive_message_formats]
fn message(&self) -> String {
let Self { duplicate_keyword } = self;
format!("Repeated keyword argument: `{duplicate_keyword}`")
}
}

pub(crate) fn repeated_keyword_argument(checker: &mut Checker, call: &ExprCall) {
let ExprCall {
arguments: Arguments { keywords, .. },
..
} = call;

let mut seen =
FxHashSet::with_capacity_and_hasher(keywords.len(), BuildHasherDefault::default());

for keyword in keywords {
if let Some(id) = &keyword.arg {
// Ex) `func(a=1, a=2)`
if !seen.insert(id.as_str()) {
checker.diagnostics.push(Diagnostic::new(
RepeatedKeywordArgument {
duplicate_keyword: id.to_string(),
},
keyword.range(),
));
}
} else if let Expr::Dict(ExprDict { keys, .. }) = &keyword.value {
// Ex) `func(**{"a": 1, "a": 2})`
for key in keys.iter().flatten() {
if let Expr::StringLiteral(ExprStringLiteral { value, .. }) = key {
if !seen.insert(value) {
checker.diagnostics.push(Diagnostic::new(
RepeatedKeywordArgument {
duplicate_keyword: value.to_string(),
},
key.range(),
));
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
repeated_keyword_argument.py:16:21: PLE1132 Repeated keyword argument: `c`
|
15 | # Invalid
16 | func(a=11, c=31, **{"c": 41})
| ^^^ PLE1132
17 | func(a=11, c=31, **{"c": 41, "a": 51})
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
|

repeated_keyword_argument.py:17:21: PLE1132 Repeated keyword argument: `c`
|
15 | # Invalid
16 | func(a=11, c=31, **{"c": 41})
17 | func(a=11, c=31, **{"c": 41, "a": 51})
| ^^^ PLE1132
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
|

repeated_keyword_argument.py:17:30: PLE1132 Repeated keyword argument: `a`
|
15 | # Invalid
16 | func(a=11, c=31, **{"c": 41})
17 | func(a=11, c=31, **{"c": 41, "a": 51})
| ^^^ PLE1132
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
|

repeated_keyword_argument.py:18:27: PLE1132 Repeated keyword argument: `b`
|
16 | func(a=11, c=31, **{"c": 41})
17 | func(a=11, c=31, **{"c": 41, "a": 51})
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
| ^^^ PLE1132
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
20 | func(a=11, b=21, **{"c": 31, "c": 32})
|

repeated_keyword_argument.py:18:36: PLE1132 Repeated keyword argument: `c`
|
16 | func(a=11, c=31, **{"c": 41})
17 | func(a=11, c=31, **{"c": 41, "a": 51})
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
| ^^^ PLE1132
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
20 | func(a=11, b=21, **{"c": 31, "c": 32})
|

repeated_keyword_argument.py:18:45: PLE1132 Repeated keyword argument: `a`
|
16 | func(a=11, c=31, **{"c": 41})
17 | func(a=11, c=31, **{"c": 41, "a": 51})
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
| ^^^ PLE1132
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
20 | func(a=11, b=21, **{"c": 31, "c": 32})
|

repeated_keyword_argument.py:19:34: PLE1132 Repeated keyword argument: `c`
|
17 | func(a=11, c=31, **{"c": 41, "a": 51})
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
| ^^^ PLE1132
20 | func(a=11, b=21, **{"c": 31, "c": 32})
|

repeated_keyword_argument.py:20:30: PLE1132 Repeated keyword argument: `c`
|
18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51})
19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})
20 | func(a=11, b=21, **{"c": 31, "c": 32})
| ^^^ PLE1132
|


2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading