Simplify key in dct and dct[key] to dct.get(key)

crates/ruff_linter/resources/test/fixtures/ruff/RUF019.py

@@ -0,0 +1,20 @@
+d = {}
+# RUF019
+if "k" in d and d["k"]:
+    pass
+
+k = "k"
+if k in d and d[k]:
+    pass
+
+if (k) in d and d[k]:
+    pass
+
+if k in d and d[(k)]:
+    pass
+
+# OK
+v = "k" in d and d["k"]
+
+if f() in d and d[f()]:
+    pass

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1412,6 +1412,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::RepeatedEqualityComparison) {
                 pylint::rules::repeated_equality_comparison(checker, bool_op);
             }
+            if checker.enabled(Rule::UnnecessaryKeyCheck) {
+                ruff::rules::unnecessary_key_check(checker, expr);
+            }
         }
         Expr::NamedExpr(..) => {
             if checker.enabled(Rule::AssignmentInAssert) {

crates/ruff_linter/src/codes.rs

@@ -866,6 +866,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         #[allow(deprecated)]
         (Ruff, "017") => (RuleGroup::Nursery, rules::ruff::rules::QuadraticListSummation),
         (Ruff, "018") => (RuleGroup::Preview, rules::ruff::rules::AssignmentInAssert),
+        (Ruff, "019") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryKeyCheck),
         (Ruff, "100") => (RuleGroup::Unspecified, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Unspecified, rules::ruff::rules::InvalidPyprojectToml),
 

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

@@ -43,6 +43,7 @@ mod tests {
     #[test_case(Rule::QuadraticListSummation, Path::new("RUF017_1.py"))]
     #[test_case(Rule::QuadraticListSummation, Path::new("RUF017_0.py"))]
     #[test_case(Rule::AssignmentInAssert, Path::new("RUF018.py"))]
+    #[test_case(Rule::UnnecessaryKeyCheck, Path::new("RUF019.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

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

@@ -12,6 +12,7 @@ pub(crate) use mutable_dataclass_default::*;
 pub(crate) use pairwise_over_zipped::*;
 pub(crate) use static_key_dict_comprehension::*;
 pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
+pub(crate) use unnecessary_key_check::*;
 #[cfg(feature = "unreachable-code")]
 pub(crate) use unreachable::*;
 pub(crate) use unused_noqa::*;
@@ -32,6 +33,7 @@ mod mutable_dataclass_default;
 mod pairwise_over_zipped;
 mod static_key_dict_comprehension;
 mod unnecessary_iterable_allocation_for_first_element;
+mod unnecessary_key_check;
 #[cfg(feature = "unreachable-code")]
 pub(crate) mod unreachable;
 mod unused_noqa;

crates/ruff_linter/src/rules/ruff/rules/unnecessary_key_check.rs

@@ -0,0 +1,131 @@
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr};
+
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::contains_effect;
+use ruff_python_ast::parenthesize::parenthesized_range;
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for unnecessary key checks prior to accessing a dictionary.
+///
+/// ## Why is this bad?
+/// When working with dictionaries, the `get` can be used to access a value
+/// without having to check if the dictionary contains the relevant key,
+/// returning `None` if the key is not present.
+///
+/// ## Examples
+/// ```python
+/// if "key" in dct and dct["key"]:
+///     ...
+/// ```
+///
+/// Use instead:
+/// ```python
+/// if dct.get("key"):
+///     ...
+/// ```
+#[violation]
+pub struct UnnecessaryKeyCheck;
+
+impl AlwaysFixableViolation for UnnecessaryKeyCheck {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Unnecessary key check before dictionary access")
+    }
+
+    fn fix_title(&self) -> String {
+        format!("Replace with `dict.get`")
+    }
+}
+
+/// RUF019
+pub(crate) fn unnecessary_key_check(checker: &mut Checker, expr: &Expr) {
+    if !checker.semantic().in_boolean_test() {
+        return;
+    }
+
+    let Expr::BoolOp(ast::ExprBoolOp {
+        op: BoolOp::And,
+        values,
+        ..
+    }) = expr
+    else {
+        return;
+    };
+
+    let [left, right] = values.as_slice() else {
+        return;
+    };
+
+    // Left should be, e.g., `key in dct`.
+    let Expr::Compare(ast::ExprCompare {
+        left: key_left,
+        ops,
+        comparators,
+        ..
+    }) = left
+    else {
+        return;
+    };
+
+    if !matches!(ops.as_slice(), [CmpOp::In]) {
+        return;
+    }
+
+    let [obj_left] = comparators.as_slice() else {
+        return;
+    };
+
+    // Right should be, e.g., `dct[key]`.
+    let Expr::Subscript(ast::ExprSubscript {
+        value: obj_right,
+        slice: key_right,
+        ..
+    }) = right
+    else {
+        return;
+    };
+
+    if ComparableExpr::from(obj_left) != ComparableExpr::from(obj_right)
+        || ComparableExpr::from(key_left) != ComparableExpr::from(key_right)
+    {
+        return;
+    }
+
+    if contains_effect(obj_left, |id| checker.semantic().is_builtin(id))
+        || contains_effect(key_left, |id| checker.semantic().is_builtin(id))
+    {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(UnnecessaryKeyCheck, expr.range());
+    diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+        format!(
+            "{}.get({})",
+            checker.locator().slice(
+                parenthesized_range(
+                    obj_right.into(),
+                    right.into(),
+                    checker.indexer().comment_ranges(),
+                    checker.locator().contents(),
+                )
+                .unwrap_or(obj_right.range())
+            ),
+            checker.locator().slice(
+                parenthesized_range(
+                    key_right.into(),
+                    right.into(),
+                    checker.indexer().comment_ranges(),
+                    checker.locator().contents(),
+                )
+                .unwrap_or(key_right.range())
+            ),
+        ),
+        expr.range(),
+    )));
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF019_RUF019.py.snap

@@ -0,0 +1,82 @@
+---
+source: crates/ruff_linter/src/rules/ruff/mod.rs
+---
+RUF019.py:3:4: RUF019 [*] Unnecessary key check before dictionary access
+  |
+1 | d = {}
+2 | # RUF019
+3 | if "k" in d and d["k"]:
+  |    ^^^^^^^^^^^^^^^^^^^ RUF019
+4 |     pass
+  |
+  = help: Replace with `dict.get`
+
+ℹ Fix
+1 1 | d = {}
+2 2 | # RUF019
+3   |-if "k" in d and d["k"]:
+  3 |+if d.get("k"):
+4 4 |     pass
+5 5 | 
+6 6 | k = "k"
+
+RUF019.py:7:4: RUF019 [*] Unnecessary key check before dictionary access
+  |
+6 | k = "k"
+7 | if k in d and d[k]:
+  |    ^^^^^^^^^^^^^^^ RUF019
+8 |     pass
+  |
+  = help: Replace with `dict.get`
+
+ℹ Fix
+4 4 |     pass
+5 5 | 
+6 6 | k = "k"
+7   |-if k in d and d[k]:
+  7 |+if d.get(k):
+8 8 |     pass
+9 9 | 
+10 10 | if (k) in d and d[k]:
+
+RUF019.py:10:4: RUF019 [*] Unnecessary key check before dictionary access
+   |
+ 8 |     pass
+ 9 | 
+10 | if (k) in d and d[k]:
+   |    ^^^^^^^^^^^^^^^^^ RUF019
+11 |     pass
+   |
+   = help: Replace with `dict.get`
+
+ℹ Fix
+7  7  | if k in d and d[k]:
+8  8  |     pass
+9  9  | 
+10    |-if (k) in d and d[k]:
+   10 |+if d.get(k):
+11 11 |     pass
+12 12 | 
+13 13 | if k in d and d[(k)]:
+
+RUF019.py:13:4: RUF019 [*] Unnecessary key check before dictionary access
+   |
+11 |     pass
+12 | 
+13 | if k in d and d[(k)]:
+   |    ^^^^^^^^^^^^^^^^^ RUF019
+14 |     pass
+   |
+   = help: Replace with `dict.get`
+
+ℹ Fix
+10 10 | if (k) in d and d[k]:
+11 11 |     pass
+12 12 | 
+13    |-if k in d and d[(k)]:
+   13 |+if d.get((k)):
+14 14 |     pass
+15 15 | 
+16 16 | # OK
+
+