Skip to content

Commit 363ebed

Browse files
mkrasnitskimkrasnitski
committed
Improve help for extra_unused_type_parameters 
1 parent 146ac51 commit 363ebed

File tree

3 files changed

+130
-38
lines changed

3 files changed

+130
-38
lines changed

‎clippy_lints/src/extra_unused_type_parameters.rs‎

Lines changed: 80 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1-
use clippy_utils::diagnostics::span_lint;
1+
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::trait_ref_of_method;
33
use rustc_data_structures::fx::FxHashMap;
4+
use rustc_errors::MultiSpan;
45
use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor};
5-
use rustc_hir::{GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, Ty, TyKind, WherePredicate};
6+
use rustc_hir::{
7+
GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, LifetimeParamKind, Ty, TyKind, WherePredicate,
8+
};
69
use rustc_lint::{LateContext, LateLintPass};
710
use rustc_middle::hir::nested_filter;
811
use rustc_session::{declare_lint_pass, declare_tool_lint};
9-
use rustc_span::{def_id::DefId, Span};
12+
use rustc_span::{def_id::DefId, BytePos, Span};
1013

1114
declare_clippy_lint! {
1215
/// ### What it does
@@ -39,32 +42,77 @@ declare_lint_pass!(ExtraUnusedTypeParameters => [EXTRA_UNUSED_TYPE_PARAMETERS]);
3942
struct TypeWalker<'cx, 'tcx> {
4043
cx: &'cx LateContext<'tcx>,
4144
map: FxHashMap<DefId, Span>,
45+
bound_map: FxHashMap<DefId, Span>,
46+
generics: &'tcx Generics<'tcx>,
47+
some_params_used: bool,
48+
has_non_ty_params: bool,
4249
}
4350

4451
impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
45-
fn new(cx: &'cx LateContext<'tcx>, generics: &Generics<'tcx>) -> Self {
52+
fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>) -> Self {
53+
let mut has_non_ty_params = false;
54+
let map = generics
55+
.params
56+
.iter()
57+
.filter_map(|param| match &param.kind {
58+
GenericParamKind::Type { .. } => Some((param.def_id.into(), param.span)),
59+
GenericParamKind::Lifetime {
60+
kind: LifetimeParamKind::Elided,
61+
} => None,
62+
_ => {
63+
has_non_ty_params = true;
64+
None
65+
},
66+
})
67+
.collect();
4668
Self {
4769
cx,
48-
map: generics
49-
.params
50-
.iter()
51-
.filter_map(|param| match param.kind {
52-
GenericParamKind::Type { .. } => Some((param.def_id.into(), param.span)),
53-
_ => None,
54-
})
55-
.collect(),
70+
map,
71+
generics,
72+
has_non_ty_params,
73+
bound_map: FxHashMap::default(),
74+
some_params_used: false,
5675
}
5776
}
5877

5978
fn emit_lint(&self) {
60-
for span in self.map.values() {
61-
span_lint(
62-
self.cx,
63-
EXTRA_UNUSED_TYPE_PARAMETERS,
64-
*span,
79+
let (msg, help) = match self.map.len() {
80+
0 => return,
81+
1 => (
6582
"type parameter goes unused in function definition",
66-
);
67-
}
83+
"consider removing the parameter",
84+
),
85+
_ => (
86+
"type parameters go unused in function definition",
87+
"consider removing the parameters",
88+
),
89+
};
90+
91+
let source_map = self.cx.tcx.sess.source_map();
92+
let span = if self.some_params_used || self.has_non_ty_params {
93+
MultiSpan::from_spans(
94+
self.map
95+
.iter()
96+
.map(|(def_id, &(mut span))| {
97+
if let Some(bound_span) = self.bound_map.get(def_id) {
98+
span = span.with_hi(bound_span.hi());
99+
}
100+
span = source_map.span_extend_to_next_char(span, ',', false);
101+
span = span.with_hi(BytePos(span.hi().0 + 1));
102+
103+
let max_hi = self.generics.span.hi();
104+
if span.hi() >= max_hi {
105+
span = span.with_hi(BytePos(max_hi.0 - 1));
106+
}
107+
span
108+
})
109+
.collect(),
110+
)
111+
} else {
112+
self.generics.span.into()
113+
};
114+
115+
span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, span, msg, None, help);
68116
}
69117
}
70118

@@ -73,7 +121,9 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
73121

74122
fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) {
75123
if let Some((def_id, _)) = t.peel_refs().as_generic_param() {
76-
self.map.remove(&def_id);
124+
if self.map.remove(&def_id).is_some() {
125+
self.some_params_used = true;
126+
}
77127
} else if let TyKind::OpaqueDef(id, _, _) = t.kind {
78128
// Explicitly walk OpaqueDef. Normally `walk_ty` would do the job, but it calls
79129
// `visit_nested_item`, which checks that `Self::NestedFilter::INTER` is set. We're
@@ -86,9 +136,16 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
86136
}
87137

88138
fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
89-
if let WherePredicate::BoundPredicate(where_bound_predicate) = predicate {
90-
// Only check the right-hand side of where-bounds
91-
for bound in where_bound_predicate.bounds {
139+
if let WherePredicate::BoundPredicate(predicate) = predicate {
140+
// Collect spans for bounds that appear in the list of generics (not in a where-clause)
141+
// for use in forming the help message
142+
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param()
143+
&& predicate.span < self.generics.where_clause_span
144+
{
145+
self.bound_map.insert(def_id, predicate.span);
146+
}
147+
// Only walk the right-hand side of where-bounds
148+
for bound in predicate.bounds {
92149
walk_param_bound(self, bound);
93150
}
94151
}

‎tests/ui/extra_unused_type_parameters.rs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ fn unused_ty<T>(x: u8) {}
55

66
fn unused_multi<T, U>(x: u8) {}
77

8+
fn unused_with_lt<'a, T>(x: &'a u8) {}
9+
810
fn used_ty<T>(x: T, y: u8) {}
911

1012
fn used_ref<'a, T>(x: &'a T) {}
@@ -13,7 +15,15 @@ fn used_ret<T: Default>(x: u8) -> T {
1315
T::default()
1416
}
1517

16-
fn unused_with_bound<T: Default>(x: u8) {}
18+
fn unused_bounded<T: Default, U>(x: U) {}
19+
20+
fn unused_where_clause<T, U>(x: U)
21+
where
22+
T: Default,
23+
{
24+
}
25+
26+
fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {}
1727

1828
fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize {
1929
iter.count()

‎tests/ui/extra_unused_type_parameters.stderr‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,59 @@
11
error: type parameter goes unused in function definition
2-
--> $DIR/extra_unused_type_parameters.rs:4:14
2+
--> $DIR/extra_unused_type_parameters.rs:4:13
33
|
44
LL | fn unused_ty<T>(x: u8) {}
5-
| ^
5+
| ^^^
66
|
7+
= help: consider removing the parameter
78
= note: `-D clippy::extra-unused-type-parameters` implied by `-D warnings`
89

9-
error: type parameter goes unused in function definition
10-
--> $DIR/extra_unused_type_parameters.rs:6:17
10+
error: type parameters go unused in function definition
11+
--> $DIR/extra_unused_type_parameters.rs:6:16
1112
|
1213
LL | fn unused_multi<T, U>(x: u8) {}
13-
| ^
14+
| ^^^^^^
15+
|
16+
= help: consider removing the parameters
1417

1518
error: type parameter goes unused in function definition
16-
--> $DIR/extra_unused_type_parameters.rs:6:20
19+
--> $DIR/extra_unused_type_parameters.rs:8:23
1720
|
18-
LL | fn unused_multi<T, U>(x: u8) {}
19-
| ^
21+
LL | fn unused_with_lt<'a, T>(x: &'a u8) {}
22+
| ^
23+
|
24+
= help: consider removing the parameter
2025

2126
error: type parameter goes unused in function definition
22-
--> $DIR/extra_unused_type_parameters.rs:16:22
27+
--> $DIR/extra_unused_type_parameters.rs:18:19
28+
|
29+
LL | fn unused_bounded<T: Default, U>(x: U) {}
30+
| ^^^^^^^^^^^
2331
|
24-
LL | fn unused_with_bound<T: Default>(x: u8) {}
25-
| ^
32+
= help: consider removing the parameter
2633

2734
error: type parameter goes unused in function definition
28-
--> $DIR/extra_unused_type_parameters.rs:39:23
35+
--> $DIR/extra_unused_type_parameters.rs:20:24
36+
|
37+
LL | fn unused_where_clause<T, U>(x: U)
38+
| ^^
39+
|
40+
= help: consider removing the parameter
41+
42+
error: type parameters go unused in function definition
43+
--> $DIR/extra_unused_type_parameters.rs:26:16
44+
|
45+
LL | fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {}
46+
| ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^
47+
|
48+
= help: consider removing the parameters
49+
50+
error: type parameter goes unused in function definition
51+
--> $DIR/extra_unused_type_parameters.rs:49:22
2952
|
3053
LL | fn unused_ty_impl<T>(&self) {}
31-
| ^
54+
| ^^^
55+
|
56+
= help: consider removing the parameter
3257

33-
error: aborting due to 5 previous errors
58+
error: aborting due to 7 previous errors
3459

0 commit comments

Comments
 (0)