Add new lint inline_trait_bounds (#16486)

*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16486)*

This PR adds a new restriction lint, `inline_trait_bounds`, which
enforces the use of `where` clauses instead of inline trait bounds. The
lint is allow-by-default.

changelog: [`inline_trait_bounds`]: add new lint
fixes #16480

Author: samueltardieu

CHANGELOG.md

@@ -6805,6 +6805,7 @@ Released 2018-09-13
 [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax
 [`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body
 [`inline_modules`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_modules
+[`inline_trait_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_trait_bounds
 [`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each
 [`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one
 [`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic

clippy_lints/src/declared_lints.rs

@@ -232,6 +232,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY_INFO,
     crate::init_numbered_fields::INIT_NUMBERED_FIELDS_INFO,
     crate::inline_fn_without_body::INLINE_FN_WITHOUT_BODY_INFO,
+    crate::inline_trait_bounds::INLINE_TRAIT_BOUNDS_INFO,
     crate::int_plus_one::INT_PLUS_ONE_INFO,
     crate::item_name_repetitions::ENUM_VARIANT_NAMES_INFO,
     crate::item_name_repetitions::MODULE_INCEPTION_INFO,

clippy_lints/src/inline_trait_bounds.rs

@@ -0,0 +1,137 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::{HasSession, snippet};
+use rustc_ast::NodeId;
+use rustc_ast::ast::{Fn, FnRetTy, GenericParam, GenericParamKind};
+use rustc_ast::visit::{FnCtxt, FnKind};
+use rustc_errors::Applicability;
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Enforce that `where` bounds are used for all trait bounds.
+    ///
+    /// ### Why restrict this?
+    /// Enforce a single style throughout a codebase.
+    /// Avoid uncertainty about whether a bound should be inline
+    /// or out-of-line (i.e. a where bound).
+    /// Avoid complex inline bounds, which could make a function declaration more difficult to read.
+    ///
+    /// ### Known limitations
+    /// Only lints functions and method declararions. Bounds on structs, enums,
+    /// and impl blocks are not yet covered.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn foo<T: Clone>() {}
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// fn foo<T>() where T: Clone {}
+    /// ```
+    #[clippy::version = "1.97.0"]
+    pub INLINE_TRAIT_BOUNDS,
+    restriction,
+    "enforce that `where` bounds are used for all trait bounds"
+}
+
+declare_lint_pass!(InlineTraitBounds => [INLINE_TRAIT_BOUNDS]);
+
+impl EarlyLintPass for InlineTraitBounds {
+    fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: Span, _: NodeId) {
+        let FnKind::Fn(ctxt, _vis, f) = kind else {
+            return;
+        };
+
+        // Skip foreign functions (extern "C" etc.)
+        if !matches!(ctxt, FnCtxt::Free | FnCtxt::Assoc(..)) {
+            return;
+        }
+
+        if f.sig.span.in_external_macro(cx.sess().source_map()) {
+            return;
+        }
+
+        lint_fn(cx, f);
+    }
+}
+
+fn lint_fn(cx: &EarlyContext<'_>, f: &Fn) {
+    let generics = &f.generics;
+    let offenders: Vec<&GenericParam> = generics
+        .params
+        .iter()
+        .filter(|param| {
+            !param.bounds.is_empty() && matches!(param.kind, GenericParamKind::Lifetime | GenericParamKind::Type { .. })
+        })
+        .collect();
+    if offenders.is_empty() {
+        return;
+    }
+
+    let predicates = offenders
+        .iter()
+        .map(|param| build_predicate_text(cx, param))
+        .collect::<Vec<_>>();
+
+    let mut edits = Vec::new();
+
+    for param in offenders {
+        if let Some(colon) = param.colon_span {
+            let remove_span = colon.to(param.bounds.last().unwrap().span());
+
+            edits.push((remove_span, String::new()));
+        }
+    }
+
+    let predicate_text = predicates.join(", ");
+
+    let where_clause = &generics.where_clause;
+    if where_clause.has_where_token {
+        let (insert_at, suffix) = if let Some(last_pred) = where_clause.predicates.last() {
+            // existing `where` with predicates: append after last predicate
+            (last_pred.span.shrink_to_hi(), format!(", {predicate_text}"))
+        } else {
+            // `where` token present but empty predicate list
+            (where_clause.span.shrink_to_hi(), format!(" {predicate_text}"))
+        };
+
+        edits.push((insert_at, suffix));
+    } else {
+        let insert_at = match &f.sig.decl.output {
+            FnRetTy::Default(span) => span.shrink_to_lo(),
+            FnRetTy::Ty(ty) => ty.span.shrink_to_hi(),
+        };
+        edits.push((insert_at, format!(" where {predicate_text}")));
+    }
+
+    span_lint_and_then(
+        cx,
+        INLINE_TRAIT_BOUNDS,
+        generics.span,
+        "inline trait bounds used",
+        |diag| {
+            diag.multipart_suggestion(
+                "move bounds to a `where` clause",
+                edits,
+                Applicability::MachineApplicable,
+            );
+        },
+    );
+}
+
+fn build_predicate_text(cx: &EarlyContext<'_>, param: &GenericParam) -> String {
+    // bounds is guaranteed non-empty by the filter in `lint_fn`
+    let first = param.bounds.first().unwrap();
+    let last = param.bounds.last().unwrap();
+
+    let bounds_span = first.span().to(last.span());
+
+    let lhs = snippet(cx, param.ident.span, "..");
+
+    let rhs = snippet(cx, bounds_span, "..");
+
+    format!("{lhs}: {rhs}")
+}

clippy_lints/src/lib.rs

@@ -168,6 +168,7 @@ mod inherent_impl;
 mod inherent_to_string;
 mod init_numbered_fields;
 mod inline_fn_without_body;
+mod inline_trait_bounds;
 mod int_plus_one;
 mod item_name_repetitions;
 mod items_after_statements;
@@ -518,6 +519,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers)),
         Box::new(|| Box::new(cfg_not_test::CfgNotTest)),
         Box::new(|| Box::new(empty_line_after::EmptyLineAfter::new())),
+        Box::new(|| Box::new(inline_trait_bounds::InlineTraitBounds)),
         // add early passes here, used by `cargo dev new_lint`
     ];
     store.early_passes.extend(early_lints);

tests/ui/inline_trait_bounds.fixed

@@ -0,0 +1,94 @@
+#![warn(clippy::inline_trait_bounds)]
+
+// Free functions
+
+fn inline_simple<T>() where T: Clone {}
+//~^ inline_trait_bounds
+
+fn inline_multiple<T, U>() where T: Clone + Copy, U: core::fmt::Debug {}
+//~^ inline_trait_bounds
+
+fn inline_lifetime<'a, 'b>(x: &'a str, y: &'b str) -> &'b str where 'a: 'b {
+    //~^ inline_trait_bounds
+    y
+}
+
+#[allow(clippy::multiple_bound_locations)]
+fn inline_with_where<T>()
+//~^ inline_trait_bounds
+where
+    T: core::fmt::Debug, T: Clone,
+{
+}
+
+fn inline_with_const<T, const N: usize>() where T: Clone {}
+//~^ inline_trait_bounds
+
+fn inline_with_return<T>(val: T) -> T where T: Clone {
+    //~^ inline_trait_bounds
+    val
+}
+
+//  Trait methods
+
+trait MyTrait {
+    fn trait_method_inline<T>(&self) where T: Clone;
+    //~^ inline_trait_bounds
+
+    fn trait_method_default<T>(&self) where T: Clone + Copy {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_where<T>(&self)
+    where
+        T: Clone;
+}
+
+//   Impl methods
+
+struct MyStruct;
+
+impl MyStruct {
+    fn impl_method_inline<T>(&self) where T: Clone {}
+    //~^ inline_trait_bounds
+
+    fn impl_method_multiple<T, U>(&self) where T: Clone, U: core::fmt::Debug {}
+    //~^ inline_trait_bounds
+}
+
+impl MyTrait for MyStruct {
+    fn trait_method_inline<T>(&self) where T: Clone {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_default<T>(&self) where T: Clone + Copy {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_where<T>(&self)
+    where
+        T: Clone,
+    {
+    }
+}
+
+//  Should NOT lint
+
+fn where_only<T>()
+where
+    T: Clone,
+{
+}
+
+fn no_bounds<T, U>() {}
+
+fn no_generics() {}
+
+struct InlineStruct<T: Clone>(T);
+
+enum InlineEnum<T: Clone> {
+    A(T),
+}
+
+#[allow(invalid_type_param_default)]
+//~v inline_trait_bounds
+fn with_default_value<T = u32>(x: T) -> T where T: Clone {
+    x
+}

tests/ui/inline_trait_bounds.rs

@@ -0,0 +1,94 @@
+#![warn(clippy::inline_trait_bounds)]
+
+// Free functions
+
+fn inline_simple<T: Clone>() {}
+//~^ inline_trait_bounds
+
+fn inline_multiple<T: Clone + Copy, U: core::fmt::Debug>() {}
+//~^ inline_trait_bounds
+
+fn inline_lifetime<'a: 'b, 'b>(x: &'a str, y: &'b str) -> &'b str {
+    //~^ inline_trait_bounds
+    y
+}
+
+#[allow(clippy::multiple_bound_locations)]
+fn inline_with_where<T: Clone>()
+//~^ inline_trait_bounds
+where
+    T: core::fmt::Debug,
+{
+}
+
+fn inline_with_const<T: Clone, const N: usize>() {}
+//~^ inline_trait_bounds
+
+fn inline_with_return<T: Clone>(val: T) -> T {
+    //~^ inline_trait_bounds
+    val
+}
+
+//  Trait methods
+
+trait MyTrait {
+    fn trait_method_inline<T: Clone>(&self);
+    //~^ inline_trait_bounds
+
+    fn trait_method_default<T: Clone + Copy>(&self) {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_where<T>(&self)
+    where
+        T: Clone;
+}
+
+//   Impl methods
+
+struct MyStruct;
+
+impl MyStruct {
+    fn impl_method_inline<T: Clone>(&self) {}
+    //~^ inline_trait_bounds
+
+    fn impl_method_multiple<T: Clone, U: core::fmt::Debug>(&self) {}
+    //~^ inline_trait_bounds
+}
+
+impl MyTrait for MyStruct {
+    fn trait_method_inline<T: Clone>(&self) {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_default<T: Clone + Copy>(&self) {}
+    //~^ inline_trait_bounds
+
+    fn trait_method_where<T>(&self)
+    where
+        T: Clone,
+    {
+    }
+}
+
+//  Should NOT lint
+
+fn where_only<T>()
+where
+    T: Clone,
+{
+}
+
+fn no_bounds<T, U>() {}
+
+fn no_generics() {}
+
+struct InlineStruct<T: Clone>(T);
+
+enum InlineEnum<T: Clone> {
+    A(T),
+}
+
+#[allow(invalid_type_param_default)]
+//~v inline_trait_bounds
+fn with_default_value<T: Clone = u32>(x: T) -> T {
+    x
+}