[flake8-simplify] Only trigger SIM401 on known dictionaries (SIM401) (#…

…15995)

## Summary

This change resolves #15814 to ensure that `SIM401` is only triggered on
known dictionary types. Before, the rule was getting triggered even on
types that _resemble_ a dictionary but are not actually a dictionary.

I did this using the `is_known_to_be_of_type_dict(...)` functionality.
The logic for this function was duplicated in a few spots, so I moved
the code to a central location, removed redundant definitions, and
updated existing calls to use the single definition of the function!

## Test Plan

Since this PR only modifies an existing rule, I made changes to the
existing test instead of adding new ones. I made sure that `SIM401` is
triggered on types that are clearly dictionaries and that it's not
triggered on a simple custom dictionary-like type (using a modified
version of [the code in the issue](#15814))

The additional changes to de-duplicate `is_known_to_be_of_type_dict`
don't break any existing tests -- I think this should be fine since the
logic remains the same (please let me know if you think otherwise, I'm
excited to get feedback and work towards a good fix 🙂).

---------

Co-authored-by: Junhson Jean-Baptiste <[email protected]>
Co-authored-by: Micha Reiser <[email protected]>

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

@@ -2,6 +2,8 @@
 # Positive cases
 ###
 
+a_dict = {}
+
 # SIM401 (pattern-1)
 if key in a_dict:
     var = a_dict[key]
@@ -26,6 +28,8 @@
 else:
     var = "default"
 
+dicts = {"key": a_dict}
+
 # SIM401 (complex expression in dict)
 if key in dicts[idx]:
     var = dicts[idx][key]
@@ -115,6 +119,28 @@
 else:
     vars[idx] = "default"
 
+class NotADictionary:
+    def __init__(self):
+        self._dict = {}
+
+    def __getitem__(self, key):
+        return self._dict[key]
+
+    def __setitem__(self, key, value):
+        self._dict[key] = value
+
+    def __iter__(self):
+        return self._dict.__iter__()
+
+not_dict = NotADictionary()
+not_dict["key"] = "value"
+
+# OK (type `NotADictionary` is not a known dictionary type)
+if "key" in not_dict:
+    value = not_dict["key"]
+else:
+    value = None
+
 ###
 # Positive cases (preview)
 ###

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

@@ -5,7 +5,9 @@ use ruff_python_ast::helpers::contains_effect;
 use ruff_python_ast::{
     self as ast, Arguments, CmpOp, ElifElseClause, Expr, ExprContext, Identifier, Stmt,
 };
-use ruff_python_semantic::analyze::typing::{is_sys_version_block, is_type_checking_block};
+use ruff_python_semantic::analyze::typing::{
+    is_known_to_be_of_type_dict, is_sys_version_block, is_type_checking_block,
+};
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
@@ -113,18 +115,27 @@ pub(crate) fn if_else_block_instead_of_dict_get(checker: &mut Checker, stmt_if:
     let [orelse_var] = orelse_var.as_slice() else {
         return;
     };
+
     let Expr::Compare(ast::ExprCompare {
         left: test_key,
         ops,
         comparators: test_dict,
         range: _,
-    }) = test.as_ref()
+    }) = &**test
     else {
         return;
     };
     let [test_dict] = &**test_dict else {
         return;
     };
+
+    if !test_dict
+        .as_name_expr()
+        .is_some_and(|dict_name| is_known_to_be_of_type_dict(checker.semantic(), dict_name))
+    {
+        return;
+    }
+
     let (expected_var, expected_value, default_var, default_value) = match ops[..] {
         [CmpOp::In] => (body_var, body_value, orelse_var, orelse_value.as_ref()),
         [CmpOp::NotIn] => (orelse_var, orelse_value, body_var, body_value.as_ref()),