diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c3fb84ad9f..f53143e564b6 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c8f8382dd9e4..4218d1fdc290 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -232,6 +232,7 @@ 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, diff --git a/clippy_lints/src/inline_trait_bounds.rs b/clippy_lints/src/inline_trait_bounds.rs new file mode 100644 index 000000000000..99716253c53e --- /dev/null +++ b/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() {} + /// ``` + /// + /// Use instead: + /// ```no_run + /// fn foo() 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::>(); + + 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}") +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0875982f3bbf..af25b1f7f1aa 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -168,6 +168,7 @@ 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); diff --git a/tests/ui/inline_trait_bounds.fixed b/tests/ui/inline_trait_bounds.fixed new file mode 100644 index 000000000000..f9cb6df4288b --- /dev/null +++ b/tests/ui/inline_trait_bounds.fixed @@ -0,0 +1,94 @@ +#![warn(clippy::inline_trait_bounds)] + +// Free functions + +fn inline_simple() where T: Clone {} +//~^ inline_trait_bounds + +fn inline_multiple() 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() +//~^ inline_trait_bounds +where + T: core::fmt::Debug, T: Clone, +{ +} + +fn inline_with_const() where T: Clone {} +//~^ inline_trait_bounds + +fn inline_with_return(val: T) -> T where T: Clone { + //~^ inline_trait_bounds + val +} + +// Trait methods + +trait MyTrait { + fn trait_method_inline(&self) where T: Clone; + //~^ inline_trait_bounds + + fn trait_method_default(&self) where T: Clone + Copy {} + //~^ inline_trait_bounds + + fn trait_method_where(&self) + where + T: Clone; +} + +// Impl methods + +struct MyStruct; + +impl MyStruct { + fn impl_method_inline(&self) where T: Clone {} + //~^ inline_trait_bounds + + fn impl_method_multiple(&self) where T: Clone, U: core::fmt::Debug {} + //~^ inline_trait_bounds +} + +impl MyTrait for MyStruct { + fn trait_method_inline(&self) where T: Clone {} + //~^ inline_trait_bounds + + fn trait_method_default(&self) where T: Clone + Copy {} + //~^ inline_trait_bounds + + fn trait_method_where(&self) + where + T: Clone, + { + } +} + +// Should NOT lint + +fn where_only() +where + T: Clone, +{ +} + +fn no_bounds() {} + +fn no_generics() {} + +struct InlineStruct(T); + +enum InlineEnum { + A(T), +} + +#[allow(invalid_type_param_default)] +//~v inline_trait_bounds +fn with_default_value(x: T) -> T where T: Clone { + x +} diff --git a/tests/ui/inline_trait_bounds.rs b/tests/ui/inline_trait_bounds.rs new file mode 100644 index 000000000000..86cf740b43f4 --- /dev/null +++ b/tests/ui/inline_trait_bounds.rs @@ -0,0 +1,94 @@ +#![warn(clippy::inline_trait_bounds)] + +// Free functions + +fn inline_simple() {} +//~^ inline_trait_bounds + +fn inline_multiple() {} +//~^ 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() +//~^ inline_trait_bounds +where + T: core::fmt::Debug, +{ +} + +fn inline_with_const() {} +//~^ inline_trait_bounds + +fn inline_with_return(val: T) -> T { + //~^ inline_trait_bounds + val +} + +// Trait methods + +trait MyTrait { + fn trait_method_inline(&self); + //~^ inline_trait_bounds + + fn trait_method_default(&self) {} + //~^ inline_trait_bounds + + fn trait_method_where(&self) + where + T: Clone; +} + +// Impl methods + +struct MyStruct; + +impl MyStruct { + fn impl_method_inline(&self) {} + //~^ inline_trait_bounds + + fn impl_method_multiple(&self) {} + //~^ inline_trait_bounds +} + +impl MyTrait for MyStruct { + fn trait_method_inline(&self) {} + //~^ inline_trait_bounds + + fn trait_method_default(&self) {} + //~^ inline_trait_bounds + + fn trait_method_where(&self) + where + T: Clone, + { + } +} + +// Should NOT lint + +fn where_only() +where + T: Clone, +{ +} + +fn no_bounds() {} + +fn no_generics() {} + +struct InlineStruct(T); + +enum InlineEnum { + A(T), +} + +#[allow(invalid_type_param_default)] +//~v inline_trait_bounds +fn with_default_value(x: T) -> T { + x +} diff --git a/tests/ui/inline_trait_bounds.stderr b/tests/ui/inline_trait_bounds.stderr new file mode 100644 index 000000000000..730e99b17754 --- /dev/null +++ b/tests/ui/inline_trait_bounds.stderr @@ -0,0 +1,162 @@ +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:5:17 + | +LL | fn inline_simple() {} + | ^^^^^^^^^^ + | + = note: `-D clippy::inline-trait-bounds` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::inline_trait_bounds)]` +help: move bounds to a `where` clause + | +LL - fn inline_simple() {} +LL + fn inline_simple() where T: Clone {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:8:19 + | +LL | fn inline_multiple() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn inline_multiple() {} +LL + fn inline_multiple() where T: Clone + Copy, U: core::fmt::Debug {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:11:19 + | +LL | fn inline_lifetime<'a: 'b, 'b>(x: &'a str, y: &'b str) -> &'b str { + | ^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn inline_lifetime<'a: 'b, 'b>(x: &'a str, y: &'b str) -> &'b str { +LL + fn inline_lifetime<'a, 'b>(x: &'a str, y: &'b str) -> &'b str where 'a: 'b { + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:17:21 + | +LL | fn inline_with_where() + | ^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL ~ fn inline_with_where() +LL | +LL | where +LL ~ T: core::fmt::Debug, T: Clone, + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:24:21 + | +LL | fn inline_with_const() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn inline_with_const() {} +LL + fn inline_with_const() where T: Clone {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:27:22 + | +LL | fn inline_with_return(val: T) -> T { + | ^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn inline_with_return(val: T) -> T { +LL + fn inline_with_return(val: T) -> T where T: Clone { + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:35:27 + | +LL | fn trait_method_inline(&self); + | ^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn trait_method_inline(&self); +LL + fn trait_method_inline(&self) where T: Clone; + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:38:28 + | +LL | fn trait_method_default(&self) {} + | ^^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn trait_method_default(&self) {} +LL + fn trait_method_default(&self) where T: Clone + Copy {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:51:26 + | +LL | fn impl_method_inline(&self) {} + | ^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn impl_method_inline(&self) {} +LL + fn impl_method_inline(&self) where T: Clone {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:54:28 + | +LL | fn impl_method_multiple(&self) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn impl_method_multiple(&self) {} +LL + fn impl_method_multiple(&self) where T: Clone, U: core::fmt::Debug {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:59:27 + | +LL | fn trait_method_inline(&self) {} + | ^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn trait_method_inline(&self) {} +LL + fn trait_method_inline(&self) where T: Clone {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:62:28 + | +LL | fn trait_method_default(&self) {} + | ^^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn trait_method_default(&self) {} +LL + fn trait_method_default(&self) where T: Clone + Copy {} + | + +error: inline trait bounds used + --> tests/ui/inline_trait_bounds.rs:92:22 + | +LL | fn with_default_value(x: T) -> T { + | ^^^^^^^^^^^^^^^^ + | +help: move bounds to a `where` clause + | +LL - fn with_default_value(x: T) -> T { +LL + fn with_default_value(x: T) -> T where T: Clone { + | + +error: aborting due to 13 previous errors +