diff --git a/CHANGELOG.md b/CHANGELOG.md index 9138a4fb9c5..5ed61f99780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4430,6 +4430,7 @@ Released 2018-09-13 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond +[`impl_trait_in_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_in_params [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2577cdc8c66..d05f22c231c 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -179,6 +179,7 @@ crate::from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR_INFO, crate::from_str_radix_10::FROM_STR_RADIX_10_INFO, crate::functions::DOUBLE_MUST_USE_INFO, + crate::functions::IMPL_TRAIT_IN_PARAMS_INFO, crate::functions::MISNAMED_GETTERS_INFO, crate::functions::MUST_USE_CANDIDATE_INFO, crate::functions::MUST_USE_UNIT_INFO, diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs new file mode 100644 index 00000000000..2811a73f6c1 --- /dev/null +++ b/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/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index 4399c68e130..d2852b4acad 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/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; @@ -327,6 +328,32 @@ "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(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, @@ -354,6 +381,7 @@ pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, lar RESULT_UNIT_ERR, RESULT_LARGE_ERR, MISNAMED_GETTERS, + IMPL_TRAIT_IN_PARAMS, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -371,6 +399,7 @@ fn check_fn( 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, 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/tests/ui/impl_trait_in_params.rs b/tests/ui/impl_trait_in_params.rs new file mode 100644 index 00000000000..07560101a41 --- /dev/null +++ b/tests/ui/impl_trait_in_params.rs @@ -0,0 +1,17 @@ +#![allow(unused)] +#![warn(clippy::impl_trait_in_params)] + +pub trait Trait {} +pub trait AnotherTrait {} + +// Should warn +pub fn a(_: impl Trait) {} +pub fn c(_: C, _: impl Trait) {} +fn d(_: impl AnotherTrait) {} + +// Shouldn't warn + +pub fn b(_: B) {} +fn e>(_: T) {} + +fn main() {} diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr new file mode 100644 index 00000000000..acfcc21445e --- /dev/null +++ b/tests/ui/impl_trait_in_params.stderr @@ -0,0 +1,25 @@ +error: '`impl Trait` used as a function parameter' + --> $DIR/impl_trait_in_params.rs:8:13 + | +LL | pub fn a(_: impl Trait) {} + | ^^^^^^^^^^ + | + = note: `-D clippy::impl-trait-in-params` implied by `-D warnings` +help: add a type paremeter + | +LL | pub fn a<{ /* Generic name */ }: Trait>(_: impl Trait) {} + | +++++++++++++++++++++++++++++++ + +error: '`impl Trait` used as a function parameter' + --> $DIR/impl_trait_in_params.rs:9:29 + | +LL | pub fn c(_: C, _: impl Trait) {} + | ^^^^^^^^^^ + | +help: add a type paremeter + | +LL | pub fn c(_: C, _: impl Trait) {} + | +++++++++++++++++++++++++++++++ + +error: aborting due to 2 previous errors +