Add new unnecessary_get_then_check clippy lint

Author: GuillaumeGomez

CHANGELOG.md

@@ -5732,6 +5732,7 @@ Released 2018-09-13
 [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
 [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map
 [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold
+[`unnecessary_get_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check
 [`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join
 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
 [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap

clippy_lints/src/declared_lints.rs

@@ -455,6 +455,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::UNNECESSARY_FILTER_MAP_INFO,
     crate::methods::UNNECESSARY_FIND_MAP_INFO,
     crate::methods::UNNECESSARY_FOLD_INFO,
+    crate::methods::UNNECESSARY_GET_THEN_CHECK_INFO,
     crate::methods::UNNECESSARY_JOIN_INFO,
     crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
     crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,

clippy_lints/src/methods/mod.rs

@@ -110,6 +110,7 @@ mod unit_hash;
 mod unnecessary_fallible_conversions;
 mod unnecessary_filter_map;
 mod unnecessary_fold;
+mod unnecessary_get_then_check;
 mod unnecessary_iter_cloned;
 mod unnecessary_join;
 mod unnecessary_lazy_eval;
@@ -4011,6 +4012,35 @@ declare_clippy_lint! {
     r#"creating a `CStr` through functions when `c""` literals can be used"#
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks the usage of `.get().is_some()` or `.get().is_none()` on std map types.
+    ///
+    /// ### Why is this bad?
+    /// It can be done in one call with `.contains()`/`.contains_keys()`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # use std::collections::HashSet;
+    /// let s: HashSet<String> = HashSet::new();
+    /// if s.get("a").is_some() {
+    ///     // code
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # use std::collections::HashSet;
+    /// let s: HashSet<String> = HashSet::new();
+    /// if s.contains("a") {
+    ///     // code
+    /// }
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub UNNECESSARY_GET_THEN_CHECK,
+    suspicious,
+    "calling `.get().is_some()` or `.get().is_none()` instead of `.contains()` or `.contains_key()`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4171,6 +4201,7 @@ impl_lint_pass!(Methods => [
     OPTION_AS_REF_CLONED,
     UNNECESSARY_RESULT_MAP_OR_ELSE,
     MANUAL_C_STR_LITERALS,
+    UNNECESSARY_GET_THEN_CHECK,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4587,8 +4618,8 @@ impl Methods {
                 },
                 ("is_file", []) => filetype_is_file::check(cx, expr, recv),
                 ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, &self.msrv),
-                ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
-                ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
+                ("is_none", []) => check_is_some_is_none(cx, expr, recv, call_span, false),
+                ("is_some", []) => check_is_some_is_none(cx, expr, recv, call_span, true),
                 ("iter" | "iter_mut" | "into_iter", []) => {
                     iter_on_single_or_empty_collections::check(cx, expr, name, recv);
                 },
@@ -4899,9 +4930,15 @@ impl Methods {
     }
 }
 
-fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, is_some: bool) {
-    if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span, _)) = method_call(recv) {
-        search_is_some::check(cx, expr, name, is_some, f_recv, arg, recv, span);
+fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, call_span: Span, is_some: bool) {
+    match method_call(recv) {
+        Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span, _)) => {
+            search_is_some::check(cx, expr, name, is_some, f_recv, arg, recv, span);
+        },
+        Some(("get", f_recv, [arg], _, _)) => {
+            unnecessary_get_then_check::check(cx, call_span, recv, f_recv, arg, is_some);
+        },
+        _ => {},
     }
 }
 

clippy_lints/src/methods/unnecessary_get_then_check.rs

@@ -0,0 +1,85 @@
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
+use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::is_type_diagnostic_item;
+
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::Ty;
+use rustc_span::{sym, Span};
+
+use super::UNNECESSARY_GET_THEN_CHECK;
+
+fn is_a_std_set_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    is_type_diagnostic_item(cx, ty, sym::HashSet) || is_type_diagnostic_item(cx, ty, sym::BTreeSet)
+}
+
+fn is_a_std_map_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    is_type_diagnostic_item(cx, ty, sym::HashMap) || is_type_diagnostic_item(cx, ty, sym::BTreeMap)
+}
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    call_span: Span,
+    get_call: &Expr<'_>,
+    get_caller: &Expr<'_>,
+    arg: &Expr<'_>,
+    is_some: bool,
+) {
+    let caller_ty = cx.typeck_results().expr_ty(get_caller);
+
+    let is_set = is_a_std_set_type(cx, caller_ty);
+    let is_map = is_a_std_map_type(cx, caller_ty);
+
+    if !is_set && !is_map {
+        return;
+    }
+    let ExprKind::MethodCall(path, _, _, get_call_span) = get_call.kind else {
+        return;
+    };
+    let both_calls_span = get_call_span.with_hi(call_span.hi());
+    if let Some(snippet) = snippet_opt(cx, both_calls_span)
+        && let Some(arg_snippet) = snippet_opt(cx, arg.span)
+    {
+        let generics_snippet = if let Some(generics) = path.args
+            && let Some(generics_snippet) = snippet_opt(cx, generics.span_ext)
+        {
+            format!("::{generics_snippet}")
+        } else {
+            String::new()
+        };
+        let suggestion = if is_set {
+            format!("contains{generics_snippet}({arg_snippet})")
+        } else {
+            format!("contains_key{generics_snippet}({arg_snippet})")
+        };
+        if is_some {
+            span_lint_and_sugg(
+                cx,
+                UNNECESSARY_GET_THEN_CHECK,
+                both_calls_span,
+                &format!("unnecessary use of `{snippet}`"),
+                "replace it with",
+                suggestion,
+                Applicability::MaybeIncorrect,
+            );
+        } else if let Some(caller_snippet) = snippet_opt(cx, get_caller.span) {
+            let full_span = get_caller.span.with_hi(call_span.hi());
+
+            span_lint_and_then(
+                cx,
+                UNNECESSARY_GET_THEN_CHECK,
+                both_calls_span,
+                &format!("unnecessary use of `{snippet}`"),
+                |diag| {
+                    diag.span_suggestion(
+                        full_span,
+                        "replace it with",
+                        format!("{}{caller_snippet}.{suggestion}", if is_some { "" } else { "!" }),
+                        Applicability::MaybeIncorrect,
+                    );
+                },
+            );
+        }
+    }
+}