Auto merge of rust-lang#10197 - blyxyas:impl_trait_param, r=Jarcho

Add `impl_trait_in_params` lint

As this is a lint about style, and using `impl Trait` is purely cosmetical (even with downsides), a lot of unrelated files needed to allow this lint.

---

Resolves rust-lang#10030

changelog: New lint: [`impl_trait_in_params`]
[10197](rust-lang/rust-clippy#10197)
<!-- changelog_checked -->

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

clippy_lints/src/declared_lints.rs

@@ -179,6 +179,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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,

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 */ }}: {}", &param.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 */ }}: {}>", &param.name.ident().as_str()[5..]),
+                                    rustc_errors::Applicability::HasPlaceholders,
+                                    rustc_errors::SuggestionStyle::ShowAlways,
+                                );
+                            }
+                        },
+                    );
+                }
+            }
+        }
+    }
+}

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 @@ 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,
@@ -354,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 {
@@ -371,6 +399,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
         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<'_>) {

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<T> {}
+
+// Should warn
+pub fn a(_: impl Trait) {}
+pub fn c<C: Trait>(_: C, _: impl Trait) {}
+fn d(_: impl AnotherTrait<u32>) {}
+
+// Shouldn't warn
+
+pub fn b<B: Trait>(_: B) {}
+fn e<T: AnotherTrait<u32>>(_: T) {}
+
+fn main() {}

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: Trait>(_: C, _: impl Trait) {}
+   |                             ^^^^^^^^^^
+   |
+help: add a type paremeter
+   |
+LL | pub fn c<C: Trait, { /* Generic name */ }: Trait>(_: C, _: impl Trait) {}
+   |                  +++++++++++++++++++++++++++++++
+
+error: aborting due to 2 previous errors
+