Extend dict-get-with-none-default (SIM910) to non-literals

crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM910.py

@@ -25,3 +25,11 @@
 
 # SIM910
 ({}).get(key, None)
+
+# SIM910
+ages = {"Tom": 23, "Maria": 23, "Dog": 11}
+age = ages.get("Cat", None)
+
+# OK
+ages = ["Tom", "Maria", "Dog"]
+age = ages.get("Cat", None)

crates/ruff_linter/src/rules/flake8_simplify/mod.rs

@@ -57,6 +57,7 @@ mod tests {
 
     #[test_case(Rule::InDictKeys, Path::new("SIM118.py"))]
     #[test_case(Rule::IfElseBlockInsteadOfDictGet, Path::new("SIM401.py"))]
+    #[test_case(Rule::DictGetWithNoneDefault, Path::new("SIM910.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",

crates/ruff_linter/src/rules/flake8_simplify/rules/ast_expr.rs

@@ -4,6 +4,7 @@ use ruff_text_size::Ranged;
 use crate::fix::snippet::SourceCodeSnippet;
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_semantic::analyze::typing::is_dict;
 
 use crate::checkers::ast::Checker;
 
@@ -62,26 +63,32 @@ impl Violation for UncapitalizedEnvironmentVariables {
 }
 
 /// ## What it does
-/// Check for `dict.get()` calls that pass `None` as the default value.
+/// Checks for `dict.get()` calls that pass `None` as the default value.
 ///
 /// ## Why is this bad?
 /// `None` is the default value for `dict.get()`, so it is redundant to pass it
 /// explicitly.
 ///
+/// In [preview], this rule applies to variables that are inferred to be
+/// dictionaries; in stable, it's limited to dictionary literals (e.g.,
+/// `{"foo": 1}.get("foo", None)`).
+///
 /// ## Example
 /// ```python
 /// ages = {"Tom": 23, "Maria": 23, "Dog": 11}
-/// age = ages.get("Cat", None)  # None
+/// age = ages.get("Cat", None)
 /// ```
 ///
 /// Use instead:
 /// ```python
 /// ages = {"Tom": 23, "Maria": 23, "Dog": 11}
-/// age = ages.get("Cat")  # None
+/// age = ages.get("Cat")
 /// ```
 ///
 /// ## References
 /// - [Python documentation: `dict.get`](https://docs.python.org/3/library/stdtypes.html#dict.get)
+///
+/// [preview]: https://docs.astral.sh/ruff/preview/
 #[violation]
 pub struct DictGetWithNoneDefault {
     expected: SourceCodeSnippet,
@@ -244,9 +251,6 @@ pub(crate) fn dict_get_with_none_default(checker: &mut Checker, expr: &Expr) {
     let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
         return;
     };
-    if !value.is_dict_expr() {
-        return;
-    }
     if attr != "get" {
         return;
     }
@@ -263,6 +267,28 @@ pub(crate) fn dict_get_with_none_default(checker: &mut Checker, expr: &Expr) {
         return;
     }
 
+    // Check if the value is a dictionary.
+    match value.as_ref() {
+        Expr::Dict(_) | Expr::DictComp(_) => {}
+        Expr::Name(name) => {
+            if checker.settings.preview.is_disabled() {
+                return;
+            }
+
+            let Some(binding) = checker
+                .semantic()
+                .only_binding(name)
+                .map(|id| checker.semantic().binding(id))
+            else {
+                return;
+            };
+            if !is_dict(binding, checker.semantic()) {
+                return;
+            }
+        }
+        _ => return,
+    }
+
     let expected = format!(
         "{}({})",
         checker.locator().slice(func.as_ref()),
@@ -277,7 +303,6 @@ pub(crate) fn dict_get_with_none_default(checker: &mut Checker, expr: &Expr) {
         },
         expr.range(),
     );
-
     diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
         expected,
         expr.range(),

crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM910_SIM910.py.snap

@@ -83,6 +83,8 @@ SIM910.py:27:1: SIM910 [*] Use `({}).get(key)` instead of `({}).get(key, None)`
 26 | # SIM910
 27 | ({}).get(key, None)
    | ^^^^^^^^^^^^^^^^^^^ SIM910
+28 | 
+29 | # SIM910
    |
    = help: Replace `({}).get(key, None)` with `({}).get(key)`
 
@@ -92,5 +94,8 @@ SIM910.py:27:1: SIM910 [*] Use `({}).get(key)` instead of `({}).get(key, None)`
 26 26 | # SIM910
 27    |-({}).get(key, None)
    27 |+({}).get(key)
+28 28 | 
+29 29 | # SIM910
+30 30 | ages = {"Tom": 23, "Maria": 23, "Dog": 11}
 
 

crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM910_SIM910.py.snap

@@ -0,0 +1,122 @@
+---
+source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
+---
+SIM910.py:2:1: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
+  |
+1 | # SIM910
+2 | {}.get(key, None)
+  | ^^^^^^^^^^^^^^^^^ SIM910
+3 | 
+4 | # SIM910
+  |
+  = help: Replace `{}.get(key, None)` with `{}.get(key)`
+
+ℹ Safe fix
+1 1 | # SIM910
+2   |-{}.get(key, None)
+  2 |+{}.get(key)
+3 3 | 
+4 4 | # SIM910
+5 5 | {}.get("key", None)
+
+SIM910.py:5:1: SIM910 [*] Use `{}.get("key")` instead of `{}.get("key", None)`
+  |
+4 | # SIM910
+5 | {}.get("key", None)
+  | ^^^^^^^^^^^^^^^^^^^ SIM910
+6 | 
+7 | # OK
+  |
+  = help: Replace `{}.get("key", None)` with `{}.get("key")`
+
+ℹ Safe fix
+2 2 | {}.get(key, None)
+3 3 | 
+4 4 | # SIM910
+5   |-{}.get("key", None)
+  5 |+{}.get("key")
+6 6 | 
+7 7 | # OK
+8 8 | {}.get(key)
+
+SIM910.py:20:9: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
+   |
+19 | # SIM910
+20 | if a := {}.get(key, None):
+   |         ^^^^^^^^^^^^^^^^^ SIM910
+21 |     pass
+   |
+   = help: Replace `{}.get(key, None)` with `{}.get(key)`
+
+ℹ Safe fix
+17 17 | {}.get("key", False)
+18 18 | 
+19 19 | # SIM910
+20    |-if a := {}.get(key, None):
+   20 |+if a := {}.get(key):
+21 21 |     pass
+22 22 | 
+23 23 | # SIM910
+
+SIM910.py:24:5: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
+   |
+23 | # SIM910
+24 | a = {}.get(key, None)
+   |     ^^^^^^^^^^^^^^^^^ SIM910
+25 | 
+26 | # SIM910
+   |
+   = help: Replace `{}.get(key, None)` with `{}.get(key)`
+
+ℹ Safe fix
+21 21 |     pass
+22 22 | 
+23 23 | # SIM910
+24    |-a = {}.get(key, None)
+   24 |+a = {}.get(key)
+25 25 | 
+26 26 | # SIM910
+27 27 | ({}).get(key, None)
+
+SIM910.py:27:1: SIM910 [*] Use `({}).get(key)` instead of `({}).get(key, None)`
+   |
+26 | # SIM910
+27 | ({}).get(key, None)
+   | ^^^^^^^^^^^^^^^^^^^ SIM910
+28 | 
+29 | # SIM910
+   |
+   = help: Replace `({}).get(key, None)` with `({}).get(key)`
+
+ℹ Safe fix
+24 24 | a = {}.get(key, None)
+25 25 | 
+26 26 | # SIM910
+27    |-({}).get(key, None)
+   27 |+({}).get(key)
+28 28 | 
+29 29 | # SIM910
+30 30 | ages = {"Tom": 23, "Maria": 23, "Dog": 11}
+
+SIM910.py:31:7: SIM910 [*] Use `ages.get("Cat")` instead of `ages.get("Cat", None)`
+   |
+29 | # SIM910
+30 | ages = {"Tom": 23, "Maria": 23, "Dog": 11}
+31 | age = ages.get("Cat", None)
+   |       ^^^^^^^^^^^^^^^^^^^^^ SIM910
+32 | 
+33 | # OK
+   |
+   = help: Replace `ages.get("Cat", None)` with `ages.get("Cat")`
+
+ℹ Safe fix
+28 28 | 
+29 29 | # SIM910
+30 30 | ages = {"Tom": 23, "Maria": 23, "Dog": 11}
+31    |-age = ages.get("Cat", None)
+   31 |+age = ages.get("Cat")
+32 32 | 
+33 33 | # OK
+34 34 | ages = ["Tom", "Maria", "Dog"]
+
+