diff options
Diffstat (limited to 'src/tools/clippy/clippy_lints/src/functions')
6 files changed, 101 insertions, 27 deletions
diff --git a/src/tools/clippy/clippy_lints/src/functions/impl_trait_in_params.rs b/src/tools/clippy/clippy_lints/src/functions/impl_trait_in_params.rs new file mode 100644 index 000000000..2811a73f6 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/functions/impl_trait_in_params.rs @@ -0,0 +1,50 @@ +use clippy_utils::{diagnostics::span_lint_and_then, is_in_test_function}; + +use rustc_hir::{intravisit::FnKind, Body, HirId}; +use rustc_lint::LateContext; +use rustc_span::Span; + +use super::IMPL_TRAIT_IN_PARAMS; + +pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: &'tcx Body<'_>, hir_id: HirId) { + if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public() && !is_in_test_function(cx.tcx, hir_id) + { + if let FnKind::ItemFn(ident, generics, _) = kind { + for param in generics.params { + if param.is_impl_trait() { + // No generics with nested generics, and no generics like FnMut(x) + span_lint_and_then( + cx, + IMPL_TRAIT_IN_PARAMS, + param.span, + "'`impl Trait` used as a function parameter'", + |diag| { + if let Some(gen_span) = generics.span_for_param_suggestion() { + diag.span_suggestion_with_style( + gen_span, + "add a type paremeter", + format!(", {{ /* Generic name */ }}: {}", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } else { + diag.span_suggestion_with_style( + Span::new( + body.params[0].span.lo() - rustc_span::BytePos(1), + ident.span.hi(), + ident.span.ctxt(), + ident.span.parent(), + ), + "add a type paremeter", + format!("<{{ /* Generic name */ }}: {}>", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } + }, + ); + } + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/functions/misnamed_getters.rs b/src/tools/clippy/clippy_lints/src/functions/misnamed_getters.rs index 27acad45c..8b53ee68e 100644 --- a/src/tools/clippy/clippy_lints/src/functions/misnamed_getters.rs +++ b/src/tools/clippy/clippy_lints/src/functions/misnamed_getters.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use rustc_errors::Applicability; -use rustc_hir::{intravisit::FnKind, Body, ExprKind, FnDecl, HirId, ImplicitSelfKind, Unsafety}; +use rustc_hir::{intravisit::FnKind, Body, ExprKind, FnDecl, ImplicitSelfKind, Unsafety}; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::Span; @@ -10,14 +10,7 @@ use std::iter; use super::MISNAMED_GETTERS; -pub fn check_fn( - cx: &LateContext<'_>, - kind: FnKind<'_>, - decl: &FnDecl<'_>, - body: &Body<'_>, - span: Span, - _hir_id: HirId, -) { +pub fn check_fn(cx: &LateContext<'_>, kind: FnKind<'_>, decl: &FnDecl<'_>, body: &Body<'_>, span: Span) { let FnKind::Method(ref ident, sig) = kind else { return; }; diff --git a/src/tools/clippy/clippy_lints/src/functions/mod.rs b/src/tools/clippy/clippy_lints/src/functions/mod.rs index 9dbce3f88..d2852b4ac 100644 --- a/src/tools/clippy/clippy_lints/src/functions/mod.rs +++ b/src/tools/clippy/clippy_lints/src/functions/mod.rs @@ -1,3 +1,4 @@ +mod impl_trait_in_params; mod misnamed_getters; mod must_use; mod not_unsafe_ptr_arg_deref; @@ -9,6 +10,7 @@ use rustc_hir as hir; use rustc_hir::intravisit; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::def_id::LocalDefId; use rustc_span::Span; declare_clippy_lint! { @@ -326,6 +328,32 @@ declare_clippy_lint! { "getter method returning the wrong field" } +declare_clippy_lint! { + /// ### What it does + /// Lints when `impl Trait` is being used in a function's paremeters. + /// ### Why is this bad? + /// Turbofish syntax (`::<>`) cannot be used when `impl Trait` is being used, making `impl Trait` less powerful. Readability may also be a factor. + /// + /// ### Example + /// ```rust + /// trait MyTrait {} + /// fn foo(a: impl MyTrait) { + /// // [...] + /// } + /// ``` + /// Use instead: + /// ```rust + /// trait MyTrait {} + /// fn foo<T: MyTrait>(a: T) { + /// // [...] + /// } + /// ``` + #[clippy::version = "1.68.0"] + pub IMPL_TRAIT_IN_PARAMS, + restriction, + "`impl Trait` is used in the function's parameters" +} + #[derive(Copy, Clone)] pub struct Functions { too_many_arguments_threshold: u64, @@ -353,6 +381,7 @@ impl_lint_pass!(Functions => [ RESULT_UNIT_ERR, RESULT_LARGE_ERR, MISNAMED_GETTERS, + IMPL_TRAIT_IN_PARAMS, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -363,12 +392,14 @@ impl<'tcx> LateLintPass<'tcx> for Functions { decl: &'tcx hir::FnDecl<'_>, body: &'tcx hir::Body<'_>, span: Span, - hir_id: hir::HirId, + def_id: LocalDefId, ) { + let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold); too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold); - not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, hir_id); - misnamed_getters::check_fn(cx, kind, decl, body, span, hir_id); + not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, def_id); + misnamed_getters::check_fn(cx, kind, decl, body, span); + impl_trait_in_params::check_fn(cx, &kind, body, hir_id); } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { diff --git a/src/tools/clippy/clippy_lints/src/functions/must_use.rs b/src/tools/clippy/clippy_lints/src/functions/must_use.rs index d22bede36..29bdc46b6 100644 --- a/src/tools/clippy/clippy_lints/src/functions/must_use.rs +++ b/src/tools/clippy/clippy_lints/src/functions/must_use.rs @@ -1,6 +1,6 @@ use rustc_ast::ast::Attribute; use rustc_errors::Applicability; -use rustc_hir::def_id::{DefIdSet, LocalDefId}; +use rustc_hir::def_id::DefIdSet; use rustc_hir::{self as hir, def::Res, QPath}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::{ @@ -27,14 +27,14 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_> let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id); let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.hir_id(), item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); } else if is_public && !is_proc_macro(cx.sess(), attrs) && !attrs.iter().any(|a| a.has_name(sym::no_mangle)) { check_must_use_candidate( cx, sig.decl, cx.tcx.hir().body(*body_id), item.span, - item.owner_id.def_id, + item.owner_id, item.span.with_hi(sig.decl.output.span().hi()), "this function could have a `#[must_use]` attribute", ); @@ -49,7 +49,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id.to_def_id(), sym::must_use); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.hir_id(), item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); } else if is_public && !is_proc_macro(cx.sess(), attrs) && trait_ref_of_method(cx, item.owner_id.def_id).is_none() @@ -59,7 +59,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp sig.decl, cx.tcx.hir().body(*body_id), item.span, - item.owner_id.def_id, + item.owner_id, item.span.with_hi(sig.decl.output.span().hi()), "this method could have a `#[must_use]` attribute", ); @@ -75,7 +75,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr let attrs = cx.tcx.hir().attrs(item.hir_id()); let attr = cx.tcx.get_attr(item.owner_id.to_def_id(), sym::must_use); if let Some(attr) = attr { - check_needless_must_use(cx, sig.decl, item.hir_id(), item.span, fn_header_span, attr); + check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr); } else if let hir::TraitFn::Provided(eid) = *eid { let body = cx.tcx.hir().body(eid); if attr.is_none() && is_public && !is_proc_macro(cx.sess(), attrs) { @@ -84,7 +84,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr sig.decl, body, item.span, - item.owner_id.def_id, + item.owner_id, item.span.with_hi(sig.decl.output.span().hi()), "this method could have a `#[must_use]` attribute", ); @@ -96,7 +96,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr fn check_needless_must_use( cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, - item_id: hir::HirId, + item_id: hir::OwnerId, item_span: Span, fn_header_span: Span, attr: &Attribute, @@ -131,7 +131,7 @@ fn check_must_use_candidate<'tcx>( decl: &'tcx hir::FnDecl<'_>, body: &'tcx hir::Body<'_>, item_span: Span, - item_id: LocalDefId, + item_id: hir::OwnerId, fn_span: Span, msg: &str, ) { @@ -139,8 +139,8 @@ fn check_must_use_candidate<'tcx>( || mutates_static(cx, body) || in_external_macro(cx.sess(), item_span) || returns_unit(decl) - || !cx.effective_visibilities.is_exported(item_id) - || is_must_use_ty(cx, return_ty(cx, cx.tcx.hir().local_def_id_to_hir_id(item_id))) + || !cx.effective_visibilities.is_exported(item_id.def_id) + || is_must_use_ty(cx, return_ty(cx, item_id)) { return; } diff --git a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 2c0bf551f..f2aa7b597 100644 --- a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -17,7 +17,7 @@ pub(super) fn check_fn<'tcx>( kind: intravisit::FnKind<'tcx>, decl: &'tcx hir::FnDecl<'tcx>, body: &'tcx hir::Body<'tcx>, - hir_id: hir::HirId, + def_id: LocalDefId, ) { let unsafety = match kind { intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }) => unsafety, @@ -25,7 +25,7 @@ pub(super) fn check_fn<'tcx>( intravisit::FnKind::Closure => return, }; - check_raw_ptr(cx, unsafety, decl, body, cx.tcx.hir().local_def_id(hir_id)); + check_raw_ptr(cx, unsafety, decl, body, def_id); } pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { @@ -58,7 +58,7 @@ fn check_raw_ptr<'tcx>( }, hir::ExprKind::MethodCall(_, recv, args, _) => { let def_id = typeck.type_dependent_def_id(e.hir_id).unwrap(); - if cx.tcx.fn_sig(def_id).skip_binder().unsafety == hir::Unsafety::Unsafe { + if cx.tcx.fn_sig(def_id).skip_binder().skip_binder().unsafety == hir::Unsafety::Unsafe { check_arg(cx, &raw_ptrs, recv); for arg in args { check_arg(cx, &raw_ptrs, arg); diff --git a/src/tools/clippy/clippy_lints/src/functions/result.rs b/src/tools/clippy/clippy_lints/src/functions/result.rs index 23da145d0..fa2a9b30c 100644 --- a/src/tools/clippy/clippy_lints/src/functions/result.rs +++ b/src/tools/clippy/clippy_lints/src/functions/result.rs @@ -21,7 +21,7 @@ fn result_err_ty<'tcx>( ) -> Option<(&'tcx hir::Ty<'tcx>, Ty<'tcx>)> { if !in_external_macro(cx.sess(), item_span) && let hir::FnRetTy::Return(hir_ty) = decl.output - && let ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).output()) + && let ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).subst_identity().output()) && is_type_diagnostic_item(cx, ty, sym::Result) && let ty::Adt(_, substs) = ty.kind() { |