Improve help for extra_unused_type_parameters

Author: mkrasnitski

clippy_lints/src/extra_unused_type_parameters.rs

@@ -1,12 +1,13 @@
-use clippy_utils::diagnostics::span_lint;
+use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::trait_ref_of_method;
 use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::MultiSpan;
 use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor};
 use rustc_hir::{GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, Ty, TyKind, WherePredicate};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::{def_id::DefId, Span};
+use rustc_span::{def_id::DefId, source_map::SourceMap, BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -39,10 +40,12 @@ declare_lint_pass!(ExtraUnusedTypeParameters => [EXTRA_UNUSED_TYPE_PARAMETERS]);
 struct TypeWalker<'cx, 'tcx> {
     cx: &'cx LateContext<'tcx>,
     map: FxHashMap<DefId, Span>,
+    generics: &'tcx Generics<'tcx>,
+    some_params_used: bool,
 }
 
 impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
-    fn new(cx: &'cx LateContext<'tcx>, generics: &Generics<'tcx>) -> Self {
+    fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>) -> Self {
         Self {
             cx,
             map: generics
@@ -53,27 +56,78 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
                     _ => None,
                 })
                 .collect(),
+            generics,
+            some_params_used: false,
         }
     }
 
     fn emit_lint(&self) {
-        for span in self.map.values() {
-            span_lint(
-                self.cx,
-                EXTRA_UNUSED_TYPE_PARAMETERS,
-                *span,
+        let (msg, help) = match self.map.len() {
+            0 => return,
+            1 => (
                 "type parameter goes unused in function definition",
-            );
+                "consider removing the parameter",
+            ),
+            _ => (
+                "type parameters go unused in function definition",
+                "consider removing the parameters",
+            ),
+        };
+
+        let source_map = self.cx.tcx.sess.source_map();
+        let span = if self.some_params_used {
+            MultiSpan::from_spans(
+                self.map
+                    .values()
+                    .into_iter()
+                    .map(|&span| {
+                        let span = span_extend_through_nested_brackets(&source_map, span);
+                        let span = span_extend_through_next_char(&source_map, span, ',', false);
+                        let max_hi = self.generics.span.hi();
+                        if span.hi() >= max_hi {
+                            span.with_hi(BytePos(max_hi.0 - 1))
+                        } else {
+                            span
+                        }
+                    })
+                    .collect(),
+            )
+        } else {
+            self.generics.span.into()
+        };
+
+        span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, span, msg, None, help);
+    }
+}
+
+fn span_extend_through_next_char(source_map: &SourceMap, span: Span, c: char, accept_commas: bool) -> Span {
+    if let Ok(next_source) = source_map.span_to_next_source(span) {
+        let next_source = next_source.split(c).next().unwrap_or("");
+        if !next_source.contains('\n') && (accept_commas || !next_source.contains(',')) {
+            return span.with_hi(BytePos(span.hi().0 + next_source.len() as u32 + 1));
         }
     }
+
+    span
+}
+
+fn span_extend_through_nested_brackets(source_map: &SourceMap, span: Span) -> Span {
+    let mut new_span = span_extend_through_next_char(source_map, span, '<', false);
+    if new_span.hi() == span.hi() {
+        return span;
+    }
+    new_span = span_extend_through_nested_brackets(source_map, new_span);
+    span_extend_through_next_char(source_map, new_span, '>', true)
 }
 
 impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
     type NestedFilter = nested_filter::OnlyBodies;
 
     fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) {
         if let Some((def_id, _)) = t.peel_refs().as_generic_param() {
-            self.map.remove(&def_id);
+            if self.map.remove(&def_id).is_some() {
+                self.some_params_used = true;
+            }
         } else if let TyKind::OpaqueDef(id, _, _) = t.kind {
             // Explicitly walk OpaqueDef. Normally `walk_ty` would do the job, but it calls
             // `visit_nested_item`, which checks that `Self::NestedFilter::INTER` is set. We're

tests/ui/extra_unused_type_parameters.rs

@@ -15,6 +15,8 @@ fn used_ret<T: Default>(x: u8) -> T {
 
 fn unused_with_bound<T: Default>(x: u8) {}
 
+fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {}
+
 fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize {
     iter.count()
 }

tests/ui/extra_unused_type_parameters.stderr

@@ -1,34 +1,43 @@
 error: type parameter goes unused in function definition
-  --> $DIR/extra_unused_type_parameters.rs:4:14
+  --> $DIR/extra_unused_type_parameters.rs:4:13
    |
 LL | fn unused_ty<T>(x: u8) {}
-   |              ^
+   |             ^^^
    |
+   = help: consider removing the parameter
    = note: `-D clippy::extra-unused-type-parameters` implied by `-D warnings`
 
-error: type parameter goes unused in function definition
-  --> $DIR/extra_unused_type_parameters.rs:6:17
+error: type parameters go unused in function definition
+  --> $DIR/extra_unused_type_parameters.rs:6:16
    |
 LL | fn unused_multi<T, U>(x: u8) {}
-   |                 ^
-
-error: type parameter goes unused in function definition
-  --> $DIR/extra_unused_type_parameters.rs:6:20
+   |                ^^^^^^
    |
-LL | fn unused_multi<T, U>(x: u8) {}
-   |                    ^
+   = help: consider removing the parameters
 
 error: type parameter goes unused in function definition
-  --> $DIR/extra_unused_type_parameters.rs:16:22
+  --> $DIR/extra_unused_type_parameters.rs:16:21
    |
 LL | fn unused_with_bound<T: Default>(x: u8) {}
-   |                      ^
+   |                     ^^^^^^^^^^^^
+   |
+   = help: consider removing the parameter
+
+error: type parameters go unused in function definition
+  --> $DIR/extra_unused_type_parameters.rs:18:16
+   |
+LL | fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {}
+   |                ^^       ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^
+   |
+   = help: consider removing the parameters
 
 error: type parameter goes unused in function definition
-  --> $DIR/extra_unused_type_parameters.rs:39:23
+  --> $DIR/extra_unused_type_parameters.rs:41:22
    |
 LL |     fn unused_ty_impl<T>(&self) {}
-   |                       ^
+   |                      ^^^
+   |
+   = help: consider removing the parameter
 
 error: aborting due to 5 previous errors