X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fneedless_pass_by_value.rs;h=060037ed496910f714e114d130bf25b9f0af1f93;hb=93cc616a3eba891dc917243466c28647edfeff97;hp=532c0266946b3f71b183c5361733e812fd588440;hpb=3567ea546f9bdfb71ed107cff176df08b2da985f;p=rust.git diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 532c0266946..060037ed496 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -1,18 +1,23 @@ -use crate::utils::ptr::get_spans; -use crate::utils::{ - get_trait_def_id, implements_trait, is_copy, is_self, is_type_diagnostic_item, multispan_sugg, paths, snippet, - snippet_opt, span_lint_and_then, -}; +use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::ptr::get_spans; +use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::ty::{implements_trait, is_copy, is_type_diagnostic_item}; +use clippy_utils::{get_trait_def_id, is_self, paths}; use if_chain::if_chain; use rustc_ast::ast::Attribute; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_data_structures::fx::FxHashSet; +use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::intravisit::FnKind; -use rustc_hir::{BindingAnnotation, Body, FnDecl, GenericArg, HirId, ItemKind, Node, PatKind, QPath, TyKind}; +use rustc_hir::{ + BindingAnnotation, Body, FnDecl, GenericArg, HirId, Impl, ItemKind, Mutability, Node, PatKind, QPath, TyKind, +}; +use rustc_hir::{HirIdMap, HirIdSet}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, TypeFoldable}; +use rustc_middle::mir::FakeReadCause; +use rustc_middle::ty::{self, TypeVisitable}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::kw; use rustc_span::{sym, Span}; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; @@ -21,20 +26,22 @@ use std::borrow::Cow; declare_clippy_lint! { - /// **What it does:** Checks for functions taking arguments by value, but not + /// ### What it does + /// Checks for functions taking arguments by value, but not /// consuming them in its /// body. /// - /// **Why is this bad?** Taking arguments by reference is more flexible and can + /// ### Why is this bad? + /// Taking arguments by reference is more flexible and can /// sometimes avoid /// unnecessary allocations. /// - /// **Known problems:** + /// ### Known problems /// * This lint suggests taking an argument by reference, /// however sometimes it is better to let users decide the argument type /// (by using `Borrow` trait, for example), depending on how the function is used. /// - /// **Example:** + /// ### Example /// ```rust /// fn foo(v: Vec) { /// assert_eq!(v.len(), 42); @@ -46,6 +53,7 @@ /// assert_eq!(v.len(), 42); /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub NEEDLESS_PASS_BY_VALUE, pedantic, "functions taking arguments by value, but not consuming them in its body" @@ -64,7 +72,7 @@ macro_rules! need { } impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { - #[allow(clippy::too_many_lines)] + #[expect(clippy::too_many_lines)] fn check_fn( &mut self, cx: &LateContext<'tcx>, @@ -79,26 +87,27 @@ fn check_fn( } match kind { - FnKind::ItemFn(.., header, _, attrs) => { + FnKind::ItemFn(.., header) => { + let attrs = cx.tcx.hir().attrs(hir_id); if header.abi != Abi::Rust || requires_exact_signature(attrs) { return; } }, FnKind::Method(..) => (), - FnKind::Closure(..) => return, + FnKind::Closure => return, } // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { + if matches!( + item.kind, + ItemKind::Impl(Impl { of_trait: Some(_), .. }) | ItemKind::Trait(..) + ) { return; } } // Allow `Borrow` or functions to be taken by value - let borrow_trait = need!(get_trait_def_id(cx, &paths::BORROW_TRAIT)); let allowed_traits = [ need!(cx.tcx.lang_items().fn_trait()), need!(cx.tcx.lang_items().fn_once_trait()), @@ -114,13 +123,9 @@ fn check_fn( .filter(|p| !p.is_global()) .filter_map(|obligation| { // Note that we do not want to deal with qualified predicates here. - if let ty::PredicateKind::Atom(ty::PredicateAtom::Trait(pred, _)) = obligation.predicate.kind() { - if pred.def_id() == sized_trait { - return None; - } - Some(pred) - } else { - None + match obligation.predicate.kind().no_bound_vars() { + Some(ty::PredicateKind::Trait(pred)) if pred.def_id() != sized_trait => Some(pred), + _ => None, } }) .collect::>(); @@ -152,7 +157,7 @@ fn check_fn( // Ignore `self`s. if idx == 0 { if let PatKind::Binding(.., ident, _) = arg.pat.kind { - if ident.as_str() == "self" { + if ident.name == kw::SelfLower { continue; } } @@ -166,9 +171,9 @@ fn check_fn( let preds = preds.iter().filter(|t| t.self_ty() == ty).collect::>(); ( - preds.iter().any(|t| t.def_id() == borrow_trait), + preds.iter().any(|t| cx.tcx.is_diagnostic_item(sym::Borrow, t.def_id())), !preds.is_empty() && { - let ty_empty_region = cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_root_empty, ty); + let ty_empty_region = cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_erased, ty); preds.iter().all(|t| { let ty_params = t.trait_ref.substs.iter().skip(1).collect::>(); implements_trait(cx, ty_empty_region, t.def_id(), &ty_params) @@ -185,18 +190,19 @@ fn check_fn( if !implements_borrow_trait; if !all_borrowable_trait; - if let PatKind::Binding(mode, canonical_id, ..) = arg.pat.kind; + if let PatKind::Binding(BindingAnnotation(_, Mutability::Not), canonical_id, ..) = arg.pat.kind; if !moved_vars.contains(&canonical_id); then { - if mode == BindingAnnotation::Mutable || mode == BindingAnnotation::RefMut { - continue; - } - // Dereference suggestion - let sugg = |diag: &mut DiagnosticBuilder<'_>| { + let sugg = |diag: &mut Diagnostic| { if let ty::Adt(def, ..) = ty.kind() { - if let Some(span) = cx.tcx.hir().span_if_local(def.did) { - if can_type_implement_copy(cx.tcx, cx.param_env, ty).is_ok() { + if let Some(span) = cx.tcx.hir().span_if_local(def.did()) { + if can_type_implement_copy( + cx.tcx, + cx.param_env, + ty, + traits::ObligationCause::dummy_with_span(span), + ).is_ok() { diag.span_help(span, "consider marking this type as `Copy`"); } } @@ -204,10 +210,10 @@ fn check_fn( let deref_span = spans_need_deref.get(&canonical_id); if_chain! { - if is_type_diagnostic_item(cx, ty, sym::vec_type); + if is_type_diagnostic_item(cx, ty, sym::Vec); if let Some(clone_spans) = get_spans(cx, Some(body.id()), idx, &[("clone", ".to_owned()")]); - if let TyKind::Path(QPath::Resolved(_, ref path)) = input.kind; + if let TyKind::Path(QPath::Resolved(_, path)) = input.kind; if let Some(elem_ty) = path.segments.iter() .find(|seg| seg.ident.name == sym::Vec) .and_then(|ps| ps.args.as_ref()) @@ -227,12 +233,13 @@ fn check_fn( for (span, suggestion) in clone_spans { diag.span_suggestion( span, - &snippet_opt(cx, span) + snippet_opt(cx, span) .map_or( "change the call to".into(), |x| Cow::from(format!("change `{}` to", x)), - ), - suggestion.into(), + ) + .as_ref(), + suggestion, Applicability::Unspecified, ); } @@ -243,25 +250,26 @@ fn check_fn( } } - if is_type_diagnostic_item(cx, ty, sym::string_type) { + if is_type_diagnostic_item(cx, ty, sym::String) { if let Some(clone_spans) = get_spans(cx, Some(body.id()), idx, &[("clone", ".to_string()"), ("as_str", "")]) { diag.span_suggestion( input.span, "consider changing the type to", - "&str".to_string(), + "&str", Applicability::Unspecified, ); for (span, suggestion) in clone_spans { diag.span_suggestion( span, - &snippet_opt(cx, span) + snippet_opt(cx, span) .map_or( "change the call to".into(), |x| Cow::from(format!("change `{}` to", x)) - ), - suggestion.into(), + ) + .as_ref(), + suggestion, Applicability::Unspecified, ); } @@ -278,7 +286,7 @@ fn check_fn( spans.extend( deref_span .iter() - .cloned() + .copied() .map(|span| (span, format!("*{}", snippet(cx, span, "")))), ); spans.sort_by_key(|&(span, _)| span); @@ -310,10 +318,10 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool { #[derive(Default)] struct MovedVariablesCtxt { - moved_vars: FxHashSet, + moved_vars: HirIdSet, /// Spans which need to be prefixed with `*` for dereferencing the /// suggested additional reference. - spans_need_deref: FxHashMap>, + spans_need_deref: HirIdMap>, } impl MovedVariablesCtxt { @@ -325,13 +333,13 @@ fn move_common(&mut self, cmt: &euv::PlaceWithHirId<'_>) { } impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { - fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId, mode: euv::ConsumeMode) { - if let euv::ConsumeMode::Move = mode { - self.move_common(cmt); - } + fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId) { + self.move_common(cmt); } fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId) {} + + fn fake_read(&mut self, _: &rustc_typeck::expr_use_visitor::PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} }