Auto merge of #10632 - Alexendoo:needless-maybe-sized, r=Jarcho

Add `needless_maybe_sized` lint

changelog: new lint: [`needless_maybe_sized`]

Closes #10600

Author: bors

CHANGELOG.md

@@ -5577,6 +5577,7 @@ Released 2018-09-13
 [`needless_late_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init
 [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
 [`needless_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_match
+[`needless_maybe_sized`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_maybe_sized
 [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref
 [`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take
 [`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals

clippy_lints/src/declared_lints.rs

@@ -530,6 +530,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
     crate::needless_if::NEEDLESS_IF_INFO,
     crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
+    crate::needless_maybe_sized::NEEDLESS_MAYBE_SIZED_INFO,
     crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
     crate::needless_pass_by_ref_mut::NEEDLESS_PASS_BY_REF_MUT_INFO,
     crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,

clippy_lints/src/lib.rs

@@ -251,6 +251,7 @@ mod needless_else;
 mod needless_for_each;
 mod needless_if;
 mod needless_late_init;
+mod needless_maybe_sized;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_ref_mut;
 mod needless_pass_by_value;
@@ -1058,6 +1059,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
     store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));
     store.register_late_pass(|_| Box::new(missing_assert_message::MissingAssertMessage));
+    store.register_late_pass(|_| Box::new(needless_maybe_sized::NeedlessMaybeSized));
     store.register_late_pass(|_| Box::new(redundant_async_block::RedundantAsyncBlock));
     store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped));
     store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));

clippy_lints/src/needless_maybe_sized.rs

@@ -0,0 +1,164 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use rustc_errors::Applicability;
+use rustc_hir::def_id::{DefId, DefIdMap};
+use rustc_hir::{GenericBound, Generics, PolyTraitRef, TraitBoundModifier, WherePredicate};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{ClauseKind, PredicatePolarity};
+use rustc_session::declare_lint_pass;
+use rustc_span::symbol::Ident;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Lints `?Sized` bounds applied to type parameters that cannot be unsized
+    ///
+    /// ### Why is this bad?
+    /// The `?Sized` bound is misleading because it cannot be satisfied by an
+    /// unsized type
+    ///
+    /// ### Example
+    /// ```rust
+    /// // `T` cannot be unsized because `Clone` requires it to be `Sized`
+    /// fn f<T: Clone + ?Sized>(t: &T) {}
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn f<T: Clone>(t: &T) {}
+    ///
+    /// // or choose alternative bounds for `T` so that it can be unsized
+    /// ```
+    #[clippy::version = "1.79.0"]
+    pub NEEDLESS_MAYBE_SIZED,
+    suspicious,
+    "a `?Sized` bound that is unusable due to a `Sized` requirement"
+}
+declare_lint_pass!(NeedlessMaybeSized => [NEEDLESS_MAYBE_SIZED]);
+
+#[allow(clippy::struct_field_names)]
+struct Bound<'tcx> {
+    /// The [`DefId`] of the type parameter the bound refers to
+    param: DefId,
+    ident: Ident,
+
+    trait_bound: &'tcx PolyTraitRef<'tcx>,
+    modifier: TraitBoundModifier,
+
+    predicate_pos: usize,
+    bound_pos: usize,
+}
+
+/// Finds all of the [`Bound`]s that refer to a type parameter and are not from a macro expansion
+fn type_param_bounds<'tcx>(generics: &'tcx Generics<'tcx>) -> impl Iterator<Item = Bound<'tcx>> {
+    generics
+        .predicates
+        .iter()
+        .enumerate()
+        .filter_map(|(predicate_pos, predicate)| {
+            let WherePredicate::BoundPredicate(bound_predicate) = predicate else {
+                return None;
+            };
+
+            let (param, ident) = bound_predicate.bounded_ty.as_generic_param()?;
+
+            Some(
+                bound_predicate
+                    .bounds
+                    .iter()
+                    .enumerate()
+                    .filter_map(move |(bound_pos, bound)| match bound {
+                        &GenericBound::Trait(ref trait_bound, modifier) => Some(Bound {
+                            param,
+                            ident,
+                            trait_bound,
+                            modifier,
+                            predicate_pos,
+                            bound_pos,
+                        }),
+                        GenericBound::Outlives(_) => None,
+                    })
+                    .filter(|bound| !bound.trait_bound.span.from_expansion()),
+            )
+        })
+        .flatten()
+}
+
+/// Searches the supertraits of the trait referred to by `trait_bound` recursively, returning the
+/// path taken to find a `Sized` bound if one is found
+fn path_to_sized_bound(cx: &LateContext<'_>, trait_bound: &PolyTraitRef<'_>) -> Option<Vec<DefId>> {
+    fn search(cx: &LateContext<'_>, path: &mut Vec<DefId>) -> bool {
+        let trait_def_id = *path.last().unwrap();
+
+        if Some(trait_def_id) == cx.tcx.lang_items().sized_trait() {
+            return true;
+        }
+
+        for &(predicate, _) in cx.tcx.super_predicates_of(trait_def_id).predicates {
+            if let ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
+                && trait_predicate.polarity == PredicatePolarity::Positive
+                && !path.contains(&trait_predicate.def_id())
+            {
+                path.push(trait_predicate.def_id());
+                if search(cx, path) {
+                    return true;
+                }
+                path.pop();
+            }
+        }
+
+        false
+    }
+
+    let mut path = vec![trait_bound.trait_ref.trait_def_id()?];
+    search(cx, &mut path).then_some(path)
+}
+
+impl LateLintPass<'_> for NeedlessMaybeSized {
+    fn check_generics(&mut self, cx: &LateContext<'_>, generics: &Generics<'_>) {
+        let Some(sized_trait) = cx.tcx.lang_items().sized_trait() else {
+            return;
+        };
+
+        let maybe_sized_params: DefIdMap<_> = type_param_bounds(generics)
+            .filter(|bound| {
+                bound.trait_bound.trait_ref.trait_def_id() == Some(sized_trait)
+                    && bound.modifier == TraitBoundModifier::Maybe
+            })
+            .map(|bound| (bound.param, bound))
+            .collect();
+
+        for bound in type_param_bounds(generics) {
+            if bound.modifier == TraitBoundModifier::None
+                && let Some(sized_bound) = maybe_sized_params.get(&bound.param)
+                && let Some(path) = path_to_sized_bound(cx, bound.trait_bound)
+            {
+                span_lint_and_then(
+                    cx,
+                    NEEDLESS_MAYBE_SIZED,
+                    sized_bound.trait_bound.span,
+                    "`?Sized` bound is ignored because of a `Sized` requirement",
+                    |diag| {
+                        let ty_param = sized_bound.ident;
+                        diag.span_note(
+                            bound.trait_bound.span,
+                            format!("`{ty_param}` cannot be unsized because of the bound"),
+                        );
+
+                        for &[current_id, next_id] in path.array_windows() {
+                            let current = cx.tcx.item_name(current_id);
+                            let next = cx.tcx.item_name(next_id);
+                            diag.note(format!("...because `{current}` has the bound `{next}`"));
+                        }
+
+                        diag.span_suggestion_verbose(
+                            generics.span_for_bound_removal(sized_bound.predicate_pos, sized_bound.bound_pos),
+                            "change the bounds that require `Sized`, or remove the `?Sized` bound",
+                            "",
+                            Applicability::MaybeIncorrect,
+                        );
+                    },
+                );
+
+                return;
+            }
+        }
+    }
+}

tests/ui-toml/type_repetition_in_bounds/main.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::needless_maybe_sized)]
 #![warn(clippy::type_repetition_in_bounds)]
 
 fn f<T>()

tests/ui-toml/type_repetition_in_bounds/main.stderr

@@ -1,5 +1,5 @@
 error: this type has already been used as a bound predicate
-  --> tests/ui-toml/type_repetition_in_bounds/main.rs:13:5
+  --> tests/ui-toml/type_repetition_in_bounds/main.rs:14:5
    |
 LL |     T: Unpin + PartialEq,
    |     ^^^^^^^^^^^^^^^^^^^^

tests/ui/auxiliary/proc_macros.rs

@@ -58,7 +58,7 @@ fn group_with_span(delimiter: Delimiter, stream: TokenStream, span: Span) -> Gro
 const ESCAPE_CHAR: char = '$';
 
 /// Takes a single token followed by a sequence of tokens. Returns the sequence of tokens with their
-/// span set to that of the first token. Tokens may be escaped with either `#ident` or `#(tokens)`.
+/// span set to that of the first token. Tokens may be escaped with either `$ident` or `$(tokens)`.
 #[proc_macro]
 pub fn with_span(input: TokenStream) -> TokenStream {
     let mut iter = input.into_iter();
@@ -72,7 +72,7 @@ pub fn with_span(input: TokenStream) -> TokenStream {
 }
 
 /// Takes a sequence of tokens and return the tokens with the span set such that they appear to be
-/// from an external macro. Tokens may be escaped with either `#ident` or `#(tokens)`.
+/// from an external macro. Tokens may be escaped with either `$ident` or `$(tokens)`.
 #[proc_macro]
 pub fn external(input: TokenStream) -> TokenStream {
     let mut res = TokenStream::new();
@@ -84,7 +84,7 @@ pub fn external(input: TokenStream) -> TokenStream {
 }
 
 /// Copies all the tokens, replacing all their spans with the given span. Tokens can be escaped
-/// either by `#ident` or `#(tokens)`.
+/// either by `$ident` or `$(tokens)`.
 fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Result<()> {
     while let Some(tt) = input.next() {
         match tt {

tests/ui/needless_maybe_sized.fixed

@@ -0,0 +1,116 @@
+//@aux-build:proc_macros.rs
+
+#![allow(unused, clippy::multiple_bound_locations)]
+#![warn(clippy::needless_maybe_sized)]
+
+extern crate proc_macros;
+use proc_macros::external;
+
+fn directly<T: Sized>(t: &T) {}
+
+trait A: Sized {}
+trait B: A {}
+
+fn depth_1<T: A>(t: &T) {}
+fn depth_2<T: B>(t: &T) {}
+
+// We only need to show one
+fn multiple_paths<T: A + B>(t: &T) {}
+
+fn in_where<T>(t: &T)
+where
+    T: Sized,
+{
+}
+
+fn mixed_1<T: Sized>(t: &T)
+{
+}
+
+fn mixed_2<T>(t: &T)
+where
+    T: Sized,
+{
+}
+
+fn mixed_3<T>(t: &T)
+where
+    T: Sized,
+{
+}
+
+struct Struct<T: Sized>(T);
+
+impl<T: Sized> Struct<T> {
+    fn method<U: Sized>(&self) {}
+}
+
+enum Enum<T: Sized + 'static> {
+    Variant(&'static T),
+}
+
+union Union<'a, T: Sized> {
+    a: &'a T,
+}
+
+trait Trait<T: Sized> {
+    fn trait_method<U: Sized>() {}
+
+    type GAT<U: Sized>;
+
+    type Assoc: Sized + ?Sized; // False negative
+}
+
+trait SecondInTrait: Send + Sized {}
+fn second_in_trait<T: SecondInTrait>() {}
+
+fn impl_trait(_: &(impl Sized)) {}
+
+trait GenericTrait<T>: Sized {}
+fn in_generic_trait<T: GenericTrait<U>, U>() {}
+
+mod larger_graph {
+    // C1  C2  Sized
+    //  \  /\  /
+    //   B1  B2
+    //    \  /
+    //     A1
+
+    trait C1 {}
+    trait C2 {}
+    trait B1: C1 + C2 {}
+    trait B2: C2 + Sized {}
+    trait A1: B1 + B2 {}
+
+    fn larger_graph<T: A1>() {}
+}
+
+// Should not lint
+
+fn sized<T: Sized>() {}
+fn maybe_sized<T: ?Sized>() {}
+
+struct SeparateBounds<T: ?Sized>(T);
+impl<T: Sized> SeparateBounds<T> {}
+
+trait P {}
+trait Q: P {}
+
+fn ok_depth_1<T: P + ?Sized>() {}
+fn ok_depth_2<T: Q + ?Sized>() {}
+
+external! {
+    fn in_macro<T: Clone + ?Sized>(t: &T) {}
+
+    fn with_local_clone<T: $Clone + ?Sized>(t: &T) {}
+}
+
+#[derive(Clone)]
+struct InDerive<T: ?Sized> {
+    t: T,
+}
+
+struct Refined<T: ?Sized>(T);
+impl<T: Sized> Refined<T> {}
+
+fn main() {}