about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGuillaume Gomez <guillaume1.gomez@gmail.com>2024-01-06 19:32:52 +0100
committerGuillaume Gomez <guillaume1.gomez@gmail.com>2024-01-09 14:09:00 +0100
commit4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258 (patch)
tree6b1b1feacac44a544dd85f08703c1a5065cb5e85
parent3b8323d79024efb4ed4664faa4f29a1a7117177a (diff)
downloadrust-4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258.tar.gz
rust-4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258.zip
Extend `useless_asref` lint on `map(clone)`
-rw-r--r--clippy_lints/src/methods/useless_asref.rs135
1 files changed, 132 insertions, 3 deletions
diff --git a/clippy_lints/src/methods/useless_asref.rs b/clippy_lints/src/methods/useless_asref.rs
index 84ee64e88a6..f3aa8858250 100644
--- a/clippy_lints/src/methods/useless_asref.rs
+++ b/clippy_lints/src/methods/useless_asref.rs
@@ -1,19 +1,52 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::walk_ptrs_ty_depth;
-use clippy_utils::{get_parent_expr, is_trait_method};
+use clippy_utils::{get_parent_expr, is_diag_trait_item, match_def_path, paths, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
-use rustc_span::sym;
+use rustc_middle::ty::adjustment::Adjust;
+use rustc_middle::ty::{Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
+use rustc_span::{sym, Span};
+
+use core::ops::ControlFlow;
 
 use super::USELESS_ASREF;
 
+/// Returns the first type inside the `Option`/`Result` type passed as argument.
+fn get_enum_ty(enum_ty: Ty<'_>) -> Option<Ty<'_>> {
+    struct ContainsTyVisitor {
+        level: usize,
+    }
+
+    impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ContainsTyVisitor {
+        type BreakTy = Ty<'tcx>;
+
+        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+            self.level += 1;
+            if self.level == 1 {
+                t.super_visit_with(self)
+            } else {
+                ControlFlow::Break(t)
+            }
+        }
+    }
+
+    match enum_ty.visit_with(&mut ContainsTyVisitor { level: 0 }) {
+        ControlFlow::Break(ty) => Some(ty),
+        ControlFlow::Continue(()) => None,
+    }
+}
+
 /// Checks for the `USELESS_ASREF` lint.
 pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str, recvr: &hir::Expr<'_>) {
     // when we get here, we've already checked that the call name is "as_ref" or "as_mut"
     // check if the call is to the actual `AsRef` or `AsMut` trait
-    if is_trait_method(cx, expr, sym::AsRef) || is_trait_method(cx, expr, sym::AsMut) {
+    let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else {
+        return;
+    };
+
+    if is_diag_trait_item(cx, def_id, sym::AsRef) || is_diag_trait_item(cx, def_id, sym::AsMut) {
         // check if the type after `as_ref` or `as_mut` is the same as before
         let rcv_ty = cx.typeck_results().expr_ty(recvr);
         let res_ty = cx.typeck_results().expr_ty(expr);
@@ -39,5 +72,101 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str,
                 applicability,
             );
         }
+    } else if match_def_path(cx, def_id, &["core", "option", "Option", call_name])
+        || match_def_path(cx, def_id, &["core", "result", "Result", call_name])
+    {
+        let rcv_ty = cx.typeck_results().expr_ty(recvr).peel_refs();
+        let res_ty = cx.typeck_results().expr_ty(expr).peel_refs();
+
+        if let Some(rcv_ty) = get_enum_ty(rcv_ty)
+            && let Some(res_ty) = get_enum_ty(res_ty)
+            // If the only thing the `as_mut`/`as_ref` call is doing is adding references and not
+            // changing the type, then we can move forward.
+            && rcv_ty.peel_refs() == res_ty.peel_refs()
+            && let Some(parent) = get_parent_expr(cx, expr)
+            && let hir::ExprKind::MethodCall(segment, _, args, _) = parent.kind
+            && segment.ident.span != expr.span
+            // We check that the called method name is `map`.
+            && segment.ident.name == sym::map
+            // And that it only has one argument.
+            && let [arg] = args
+        {
+            match arg.kind {
+                hir::ExprKind::Closure(&hir::Closure { body, .. }) => {
+                    // If it's a closure, we need to check what is called.
+                    let closure_body = cx.tcx.hir().body(body);
+                    let closure_expr = peel_blocks(closure_body.value);
+                    match closure_expr.kind {
+                        hir::ExprKind::MethodCall(method, obj, [], _) => {
+                            if method.ident.name == sym::clone
+                                && let Some(fn_id) = cx.typeck_results().type_dependent_def_id(closure_expr.hir_id)
+                                && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
+                                // We check it's the `Clone` trait.
+                                && cx.tcx.lang_items().clone_trait().map_or(false, |id| id == trait_id)
+                                // no autoderefs
+                                && !cx.typeck_results().expr_adjustments(obj).iter()
+                                    .any(|a| matches!(a.kind, Adjust::Deref(Some(..))))
+                            {
+                                lint_as_ref_clone(cx, expr.span.with_hi(parent.span.hi()), recvr, call_name);
+                            }
+                        },
+                        hir::ExprKind::Call(call, [_]) => {
+                            if let hir::ExprKind::Path(qpath) = call.kind {
+                                check_qpath(
+                                    cx,
+                                    expr.span.with_hi(parent.span.hi()),
+                                    recvr,
+                                    call_name,
+                                    qpath,
+                                    call.hir_id,
+                                );
+                            }
+                        },
+                        _ => {},
+                    }
+                },
+                hir::ExprKind::Path(qpath) => check_qpath(
+                    cx,
+                    expr.span.with_hi(parent.span.hi()),
+                    recvr,
+                    call_name,
+                    qpath,
+                    arg.hir_id,
+                ),
+                _ => {},
+            }
+        }
     }
 }
+
+fn check_qpath(
+    cx: &LateContext<'_>,
+    span: Span,
+    recvr: &hir::Expr<'_>,
+    call_name: &str,
+    qpath: hir::QPath<'_>,
+    hir_id: hir::HirId,
+) {
+    // We check it's calling the `clone` method of the `Clone` trait.
+    if let Some(path_def_id) = cx.qpath_res(&qpath, hir_id).opt_def_id()
+        && match_def_path(cx, path_def_id, &paths::CLONE_TRAIT_METHOD)
+    {
+        lint_as_ref_clone(cx, span, recvr, call_name);
+    }
+}
+
+fn lint_as_ref_clone(cx: &LateContext<'_>, span: Span, recvr: &hir::Expr<'_>, call_name: &str) {
+    let mut applicability = Applicability::MachineApplicable;
+    span_lint_and_sugg(
+        cx,
+        USELESS_ASREF,
+        span,
+        &format!("this call to `{call_name}.map(...)` does nothing"),
+        "try",
+        format!(
+            "{}.clone()",
+            snippet_with_applicability(cx, recvr.span, "..", &mut applicability)
+        ),
+        applicability,
+    );
+}