Skip to content

Commit f3faaeb

Browse files
mkrasnitskimkrasnitski
committed
Improve help for extra_unused_type_parameters 
1 parent addc520 commit f3faaeb

File tree

3 files changed

+132
-38
lines changed

3 files changed

+132
-38
lines changed

‎clippy_lints/src/extra_unused_type_parameters.rs

Lines changed: 82 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,79 @@ 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+
non_ty => {
60+
if let GenericParamKind::Lifetime { kind } = non_ty {
61+
match kind {
62+
LifetimeParamKind::Elided => (),
63+
_ => has_non_ty_params = true,
64+
}
65+
}
66+
None
67+
},
68+
})
69+
.collect();
4670
Self {
4771
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(),
72+
map,
73+
generics,
74+
has_non_ty_params,
75+
bound_map: FxHashMap::default(),
76+
some_params_used: false,
5677
}
5778
}
5879

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

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

74124
fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) {
75125
if let Some((def_id, _)) = t.peel_refs().as_generic_param() {
76-
self.map.remove(&def_id);
126+
if self.map.remove(&def_id).is_some() {
127+
self.some_params_used = true;
128+
}
77129
} else if let TyKind::OpaqueDef(id, _, _) = t.kind {
78130
// Explicitly walk OpaqueDef. Normally `walk_ty` would do the job, but it calls
79131
// `visit_nested_item`, which checks that `Self::NestedFilter::INTER` is set. We're
@@ -86,9 +138,16 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
86138
}
87139

88140
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 {
141+
if let WherePredicate::BoundPredicate(predicate) = predicate {
142+
// Collect spans for bounds that appear in the list of generics (not in a where-clause)
143+
// for use in forming the help message
144+
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param()
145+
&& predicate.span < self.generics.where_clause_span
146+
{
147+
self.bound_map.insert(def_id, predicate.span);
148+
}
149+
// Only walk the right-hand side of where-bounds
150+
for bound in predicate.bounds {
92151
walk_param_bound(self, bound);
93152
}
94153
}

‎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)