diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM910.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM910.py new file mode 100644 index 0000000000..83b103d760 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM910.py @@ -0,0 +1,27 @@ +# SIM910 +{}.get(key, None) + +# SIM910 +{}.get("key", None) + +# OK +{}.get(key) + +# OK +{}.get("key") + +# OK +{}.get(key, False) + +# OK +{}.get("key", False) + +# SIM910 +if a := {}.get(key, None): + pass + +# SIM910 +a = {}.get(key, None) + +# SIM910 +({}).get(key, None) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 1042c643d3..811a1d9057 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2918,6 +2918,10 @@ where flake8_simplify::rules::open_file_with_context_handler(self, func); } + if self.settings.rules.enabled(Rule::DictGetWithNoneDefault) { + flake8_simplify::rules::dict_get_with_none_default(self, expr); + } + // flake8-use-pathlib if self.settings.rules.any_enabled(&[ Rule::OsPathAbspath, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 6404c80949..6de83693ce 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -356,6 +356,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Simplify, "223") => Rule::ExprAndFalse, (Flake8Simplify, "300") => Rule::YodaConditions, (Flake8Simplify, "401") => Rule::IfElseBlockInsteadOfDictGet, + (Flake8Simplify, "910") => Rule::DictGetWithNoneDefault, // pyupgrade (Pyupgrade, "001") => Rule::UselessMetaclassType, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 73b846372e..268ff5d873 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -322,6 +322,7 @@ ruff_macros::register_rules!( rules::flake8_simplify::rules::ExprAndFalse, rules::flake8_simplify::rules::YodaConditions, rules::flake8_simplify::rules::IfElseBlockInsteadOfDictGet, + rules::flake8_simplify::rules::DictGetWithNoneDefault, // pyupgrade rules::pyupgrade::rules::UselessMetaclassType, rules::pyupgrade::rules::TypeOfPrimitive, diff --git a/crates/ruff/src/rules/flake8_simplify/mod.rs b/crates/ruff/src/rules/flake8_simplify/mod.rs index 4c915be63c..90566d33bf 100644 --- a/crates/ruff/src/rules/flake8_simplify/mod.rs +++ b/crates/ruff/src/rules/flake8_simplify/mod.rs @@ -38,6 +38,7 @@ mod tests { #[test_case(Rule::ExprAndFalse, Path::new("SIM223.py"); "SIM223")] #[test_case(Rule::YodaConditions, Path::new("SIM300.py"); "SIM300")] #[test_case(Rule::IfElseBlockInsteadOfDictGet, Path::new("SIM401.py"); "SIM401")] + #[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"); "SIM910")] #[test_case(Rule::IfElseBlockInsteadOfDictLookup, Path::new("SIM116.py"); "SIM116")] #[test_case(Rule::IfWithSameArms, Path::new("SIM114.py"); "SIM114")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs index 9ad9546aba..7ae934d59b 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs @@ -27,6 +27,25 @@ impl AlwaysAutofixableViolation for UncapitalizedEnvironmentVariables { } } +#[violation] +pub struct DictGetWithNoneDefault { + pub expected: String, + pub original: String, +} + +impl AlwaysAutofixableViolation for DictGetWithNoneDefault { + #[derive_message_formats] + fn message(&self) -> String { + let DictGetWithNoneDefault { expected, original } = self; + format!("Use `{expected}` instead of `{original}`") + } + + fn autofix_title(&self) -> String { + let DictGetWithNoneDefault { expected, original } = self; + format!("Replace `{original}` with `{expected}`") + } +} + /// SIM112 pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) { // check `os.environ['foo']` @@ -123,3 +142,64 @@ fn check_os_environ_subscript(checker: &mut Checker, expr: &Expr) { } checker.diagnostics.push(diagnostic); } + +/// SIM910 +pub fn dict_get_with_none_default(checker: &mut Checker, expr: &Expr) { + let ExprKind::Call { func, args, keywords } = &expr.node else { + return; + }; + if !keywords.is_empty() { + return; + } + let ExprKind::Attribute { value, attr, .. } = &func.node else { + return; + }; + if !matches!(value.node, ExprKind::Dict { .. }) { + return; + } + if attr != "get" { + return; + } + let Some(key) = args.get(0) else { + return; + }; + if !matches!(key.node, ExprKind::Constant { .. } | ExprKind::Name { .. }) { + return; + } + let Some(default) = args.get(1) else { + return; + }; + if !matches!( + default.node, + ExprKind::Constant { + value: Constant::None, + .. + } + ) { + return; + }; + + let expected = format!( + "{}({})", + checker.locator.slice(func), + checker.locator.slice(key) + ); + let original = checker.locator.slice(expr).to_string(); + + let mut diagnostic = Diagnostic::new( + DictGetWithNoneDefault { + expected: expected.clone(), + original, + }, + Range::from(expr), + ); + + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(Edit::replacement( + expected, + expr.location, + expr.end_location.unwrap(), + )); + } + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff/src/rules/flake8_simplify/rules/mod.rs b/crates/ruff/src/rules/flake8_simplify/rules/mod.rs index 750297f544..5cd8424536 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/mod.rs @@ -3,7 +3,10 @@ pub use ast_bool_op::{ expr_or_not_expr, expr_or_true, CompareWithTuple, DuplicateIsinstanceCall, ExprAndFalse, ExprAndNotExpr, ExprOrNotExpr, ExprOrTrue, }; -pub use ast_expr::{use_capital_environment_variables, UncapitalizedEnvironmentVariables}; +pub use ast_expr::{ + dict_get_with_none_default, use_capital_environment_variables, DictGetWithNoneDefault, + UncapitalizedEnvironmentVariables, +}; pub use ast_if::{ if_with_same_arms, manual_dict_lookup, needless_bool, nested_if_statements, use_dict_get_with_default, use_ternary_operator, CollapsibleIf, IfElseBlockInsteadOfDictGet, diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM910_SIM910.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM910_SIM910.py.snap new file mode 100644 index 0000000000..0f1ce5c3cf --- /dev/null +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM910_SIM910.py.snap @@ -0,0 +1,110 @@ +--- +source: crates/ruff/src/rules/flake8_simplify/mod.rs +expression: diagnostics +--- +- kind: + name: DictGetWithNoneDefault + body: "Use `{}.get(key)` instead of `{}.get(key, None)`" + suggestion: "Replace `{}.get(key, None)` with `{}.get(key)`" + fixable: true + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 17 + fix: + edits: + - content: "{}.get(key)" + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 17 + parent: ~ +- kind: + name: DictGetWithNoneDefault + body: "Use `{}.get(\"key\")` instead of `{}.get(\"key\", None)`" + suggestion: "Replace `{}.get(\"key\", None)` with `{}.get(\"key\")`" + fixable: true + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 19 + fix: + edits: + - content: "{}.get(\"key\")" + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 19 + parent: ~ +- kind: + name: DictGetWithNoneDefault + body: "Use `{}.get(key)` instead of `{}.get(key, None)`" + suggestion: "Replace `{}.get(key, None)` with `{}.get(key)`" + fixable: true + location: + row: 20 + column: 8 + end_location: + row: 20 + column: 25 + fix: + edits: + - content: "{}.get(key)" + location: + row: 20 + column: 8 + end_location: + row: 20 + column: 25 + parent: ~ +- kind: + name: DictGetWithNoneDefault + body: "Use `{}.get(key)` instead of `{}.get(key, None)`" + suggestion: "Replace `{}.get(key, None)` with `{}.get(key)`" + fixable: true + location: + row: 24 + column: 4 + end_location: + row: 24 + column: 21 + fix: + edits: + - content: "{}.get(key)" + location: + row: 24 + column: 4 + end_location: + row: 24 + column: 21 + parent: ~ +- kind: + name: DictGetWithNoneDefault + body: "Use `({}).get(key)` instead of `({}).get(key, None)`" + suggestion: "Replace `({}).get(key, None)` with `({}).get(key)`" + fixable: true + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 19 + fix: + edits: + - content: "({}).get(key)" + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 19 + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index d7edf1b58e..7ac29a7c25 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2168,6 +2168,9 @@ "SIM4", "SIM40", "SIM401", + "SIM9", + "SIM91", + "SIM910", "SLF", "SLF0", "SLF00",