diff options
| author | Guillaume Gomez <guillaume1.gomez@gmail.com> | 2024-01-06 19:32:52 +0100 |
|---|---|---|
| committer | Guillaume Gomez <guillaume1.gomez@gmail.com> | 2024-01-09 14:09:00 +0100 |
| commit | 4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258 (patch) | |
| tree | 6b1b1feacac44a544dd85f08703c1a5065cb5e85 | |
| parent | 3b8323d79024efb4ed4664faa4f29a1a7117177a (diff) | |
| download | rust-4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258.tar.gz rust-4ab6924bca42b5d6c5753dd3d3a9e5a2b9eae258.zip | |
Extend `useless_asref` lint on `map(clone)`
| -rw-r--r-- | clippy_lints/src/methods/useless_asref.rs | 135 |
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, + ); +} |
