[flake8-simplify] Implement dict-get-with-none-default (SIM910) (…

…#3874)

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)

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,

crates/ruff/src/codes.rs

@@ -356,6 +356,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Flake8Simplify, "223") => Rule::ExprAndFalse,
         (Flake8Simplify, "300") => Rule::YodaConditions,
         (Flake8Simplify, "401") => Rule::IfElseBlockInsteadOfDictGet,
+        (Flake8Simplify, "910") => Rule::DictGetWithNoneDefault,
 
         // pyupgrade
         (Pyupgrade, "001") => Rule::UselessMetaclassType,

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,

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<()> {

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);
+}

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,

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: ~
+