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/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,29 @@ 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}
+
+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"]