|
| 1 | +use clippy_utils::diagnostics::span_lint_and_then; |
| 2 | +use rustc_errors::Applicability; |
| 3 | +use rustc_hir::def_id::{DefId, DefIdMap}; |
| 4 | +use rustc_hir::{GenericBound, Generics, PolyTraitRef, TraitBoundModifier, WherePredicate}; |
| 5 | +use rustc_lint::{LateContext, LateLintPass}; |
| 6 | +use rustc_middle::ty::{ClauseKind, PredicatePolarity}; |
| 7 | +use rustc_session::declare_lint_pass; |
| 8 | +use rustc_span::symbol::Ident; |
| 9 | + |
| 10 | +declare_clippy_lint! { |
| 11 | + /// ### What it does |
| 12 | + /// Lints `?Sized` bounds applied to type parameters that cannot be unsized |
| 13 | + /// |
| 14 | + /// ### Why is this bad? |
| 15 | + /// The `?Sized` bound is misleading because it cannot be satisfied by an |
| 16 | + /// unsized type |
| 17 | + /// |
| 18 | + /// ### Example |
| 19 | + /// ```rust |
| 20 | + /// // `T` cannot be unsized because `Clone` requires it to be `Sized` |
| 21 | + /// fn f<T: Clone + ?Sized>(t: &T) {} |
| 22 | + /// ``` |
| 23 | + /// Use instead: |
| 24 | + /// ```rust |
| 25 | + /// fn f<T: Clone>(t: &T) {} |
| 26 | + /// |
| 27 | + /// // or choose alternative bounds for `T` so that it can be unsized |
| 28 | + /// ``` |
| 29 | + #[clippy::version = "1.79.0"] |
| 30 | + pub NEEDLESS_MAYBE_SIZED, |
| 31 | + suspicious, |
| 32 | + "a `?Sized` bound that is unusable due to a `Sized` requirement" |
| 33 | +} |
| 34 | +declare_lint_pass!(NeedlessMaybeSized => [NEEDLESS_MAYBE_SIZED]); |
| 35 | + |
| 36 | +#[allow(clippy::struct_field_names)] |
| 37 | +struct Bound<'tcx> { |
| 38 | + /// The [`DefId`] of the type parameter the bound refers to |
| 39 | + param: DefId, |
| 40 | + ident: Ident, |
| 41 | + |
| 42 | + trait_bound: &'tcx PolyTraitRef<'tcx>, |
| 43 | + modifier: TraitBoundModifier, |
| 44 | + |
| 45 | + predicate_pos: usize, |
| 46 | + bound_pos: usize, |
| 47 | +} |
| 48 | + |
| 49 | +/// Finds all of the [`Bound`]s that refer to a type parameter and are not from a macro expansion |
| 50 | +fn type_param_bounds<'tcx>(generics: &'tcx Generics<'tcx>) -> impl Iterator<Item = Bound<'tcx>> { |
| 51 | + generics |
| 52 | + .predicates |
| 53 | + .iter() |
| 54 | + .enumerate() |
| 55 | + .filter_map(|(predicate_pos, predicate)| { |
| 56 | + let WherePredicate::BoundPredicate(bound_predicate) = predicate else { |
| 57 | + return None; |
| 58 | + }; |
| 59 | + |
| 60 | + let (param, ident) = bound_predicate.bounded_ty.as_generic_param()?; |
| 61 | + |
| 62 | + Some( |
| 63 | + bound_predicate |
| 64 | + .bounds |
| 65 | + .iter() |
| 66 | + .enumerate() |
| 67 | + .filter_map(move |(bound_pos, bound)| match bound { |
| 68 | + &GenericBound::Trait(ref trait_bound, modifier) => Some(Bound { |
| 69 | + param, |
| 70 | + ident, |
| 71 | + trait_bound, |
| 72 | + modifier, |
| 73 | + predicate_pos, |
| 74 | + bound_pos, |
| 75 | + }), |
| 76 | + GenericBound::Outlives(_) => None, |
| 77 | + }) |
| 78 | + .filter(|bound| !bound.trait_bound.span.from_expansion()), |
| 79 | + ) |
| 80 | + }) |
| 81 | + .flatten() |
| 82 | +} |
| 83 | + |
| 84 | +/// Searches the supertraits of the trait referred to by `trait_bound` recursively, returning the |
| 85 | +/// path taken to find a `Sized` bound if one is found |
| 86 | +fn path_to_sized_bound(cx: &LateContext<'_>, trait_bound: &PolyTraitRef<'_>) -> Option<Vec<DefId>> { |
| 87 | + fn search(cx: &LateContext<'_>, path: &mut Vec<DefId>) -> bool { |
| 88 | + let trait_def_id = *path.last().unwrap(); |
| 89 | + |
| 90 | + if Some(trait_def_id) == cx.tcx.lang_items().sized_trait() { |
| 91 | + return true; |
| 92 | + } |
| 93 | + |
| 94 | + for &(predicate, _) in cx.tcx.super_predicates_of(trait_def_id).predicates { |
| 95 | + if let ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder() |
| 96 | + && trait_predicate.polarity == PredicatePolarity::Positive |
| 97 | + && !path.contains(&trait_predicate.def_id()) |
| 98 | + { |
| 99 | + path.push(trait_predicate.def_id()); |
| 100 | + if search(cx, path) { |
| 101 | + return true; |
| 102 | + } |
| 103 | + path.pop(); |
| 104 | + } |
| 105 | + } |
| 106 | + |
| 107 | + false |
| 108 | + } |
| 109 | + |
| 110 | + let mut path = vec![trait_bound.trait_ref.trait_def_id()?]; |
| 111 | + search(cx, &mut path).then_some(path) |
| 112 | +} |
| 113 | + |
| 114 | +impl LateLintPass<'_> for NeedlessMaybeSized { |
| 115 | + fn check_generics(&mut self, cx: &LateContext<'_>, generics: &Generics<'_>) { |
| 116 | + let Some(sized_trait) = cx.tcx.lang_items().sized_trait() else { |
| 117 | + return; |
| 118 | + }; |
| 119 | + |
| 120 | + let maybe_sized_params: DefIdMap<_> = type_param_bounds(generics) |
| 121 | + .filter(|bound| { |
| 122 | + bound.trait_bound.trait_ref.trait_def_id() == Some(sized_trait) |
| 123 | + && bound.modifier == TraitBoundModifier::Maybe |
| 124 | + }) |
| 125 | + .map(|bound| (bound.param, bound)) |
| 126 | + .collect(); |
| 127 | + |
| 128 | + for bound in type_param_bounds(generics) { |
| 129 | + if bound.modifier == TraitBoundModifier::None |
| 130 | + && let Some(sized_bound) = maybe_sized_params.get(&bound.param) |
| 131 | + && let Some(path) = path_to_sized_bound(cx, bound.trait_bound) |
| 132 | + { |
| 133 | + span_lint_and_then( |
| 134 | + cx, |
| 135 | + NEEDLESS_MAYBE_SIZED, |
| 136 | + sized_bound.trait_bound.span, |
| 137 | + "`?Sized` bound is ignored because of a `Sized` requirement", |
| 138 | + |diag| { |
| 139 | + let ty_param = sized_bound.ident; |
| 140 | + diag.span_note( |
| 141 | + bound.trait_bound.span, |
| 142 | + format!("`{ty_param}` cannot be unsized because of the bound"), |
| 143 | + ); |
| 144 | + |
| 145 | + for &[current_id, next_id] in path.array_windows() { |
| 146 | + let current = cx.tcx.item_name(current_id); |
| 147 | + let next = cx.tcx.item_name(next_id); |
| 148 | + diag.note(format!("...because `{current}` has the bound `{next}`")); |
| 149 | + } |
| 150 | + |
| 151 | + diag.span_suggestion_verbose( |
| 152 | + generics.span_for_bound_removal(sized_bound.predicate_pos, sized_bound.bound_pos), |
| 153 | + "change the bounds that require `Sized`, or remove the `?Sized` bound", |
| 154 | + "", |
| 155 | + Applicability::MaybeIncorrect, |
| 156 | + ); |
| 157 | + }, |
| 158 | + ); |
| 159 | + |
| 160 | + return; |
| 161 | + } |
| 162 | + } |
| 163 | + } |
| 164 | +} |
0 commit comments