Skip to content

Commit 8a98609

Browse files
committed
Auto merge of #10028 - mkrasnitski:extra_unused_type_parameters, r=flip1995
Add `extra_unused_type_parameters` lint Closes #9240. ~~Keeping this as draft for now, because of a bug that I don't know how to fix.~~ It seems that opaque return types are not walked properly, for some unknown reason. As in, the following: ```rust fn used_ret_opaque<A>() -> impl Iterator<Item = A> { std::iter::empty() } ``` This triggers the lint even though it shouldn't. Discussion on Zulip didn't illuminate any possible reasons why, so PR-ing this now to increase visibility. --- changelog: new lint: [`extra_unused_type_parameters`] [#10028](#10028) <!-- changelog_checked -->
2 parents 006a4cc + fba16e2 commit 8a98609

19 files changed

+384
-66
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4383,6 +4383,7 @@ Released 2018-09-13
43834383
[`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice
43844384
[`extend_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_with_drain
43854385
[`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes
4386+
[`extra_unused_type_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_type_parameters
43864387
[`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
43874388
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
43884389
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are over 550 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are over 600 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
1111
You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
A collection of lints to catch common mistakes and improve your
77
[Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are over 550 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are over 600 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
Lints are divided into categories, each with a default [lint
1212
level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
156156
crate::exhaustive_items::EXHAUSTIVE_STRUCTS_INFO,
157157
crate::exit::EXIT_INFO,
158158
crate::explicit_write::EXPLICIT_WRITE_INFO,
159+
crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
159160
crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,
160161
crate::float_literal::EXCESSIVE_PRECISION_INFO,
161162
crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::trait_ref_of_method;
3+
use rustc_data_structures::fx::FxHashMap;
4+
use rustc_errors::MultiSpan;
5+
use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor};
6+
use rustc_hir::{
7+
GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, PredicateOrigin, Ty, TyKind, WherePredicate,
8+
};
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_middle::hir::nested_filter;
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
use rustc_span::{def_id::DefId, Span};
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// Checks for type parameters in generics that are never used anywhere else.
17+
///
18+
/// ### Why is this bad?
19+
/// Functions cannot infer the value of unused type parameters; therefore, calling them
20+
/// requires using a turbofish, which serves no purpose but to satisfy the compiler.
21+
///
22+
/// ### Example
23+
/// ```rust
24+
/// // unused type parameters
25+
/// fn unused_ty<T>(x: u8) {
26+
/// // ..
27+
/// }
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// fn no_unused_ty(x: u8) {
32+
/// // ..
33+
/// }
34+
/// ```
35+
#[clippy::version = "1.69.0"]
36+
pub EXTRA_UNUSED_TYPE_PARAMETERS,
37+
complexity,
38+
"unused type parameters in function definitions"
39+
}
40+
declare_lint_pass!(ExtraUnusedTypeParameters => [EXTRA_UNUSED_TYPE_PARAMETERS]);
41+
42+
/// A visitor struct that walks a given function and gathers generic type parameters, plus any
43+
/// trait bounds those parameters have.
44+
struct TypeWalker<'cx, 'tcx> {
45+
cx: &'cx LateContext<'tcx>,
46+
/// Collection of all the type parameters and their spans.
47+
ty_params: FxHashMap<DefId, Span>,
48+
/// Collection of any (inline) trait bounds corresponding to each type parameter.
49+
bounds: FxHashMap<DefId, Span>,
50+
/// The entire `Generics` object of the function, useful for querying purposes.
51+
generics: &'tcx Generics<'tcx>,
52+
/// The value of this will remain `true` if *every* parameter:
53+
/// 1. Is a type parameter, and
54+
/// 2. Goes unused in the function.
55+
/// Otherwise, if any type parameters end up being used, or if any lifetime or const-generic
56+
/// parameters are present, this will be set to `false`.
57+
all_params_unused: bool,
58+
}
59+
60+
impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
61+
fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>) -> Self {
62+
let mut all_params_unused = true;
63+
let ty_params = generics
64+
.params
65+
.iter()
66+
.filter_map(|param| {
67+
if let GenericParamKind::Type { .. } = param.kind {
68+
Some((param.def_id.into(), param.span))
69+
} else {
70+
if !param.is_elided_lifetime() {
71+
all_params_unused = false;
72+
}
73+
None
74+
}
75+
})
76+
.collect();
77+
Self {
78+
cx,
79+
ty_params,
80+
bounds: FxHashMap::default(),
81+
generics,
82+
all_params_unused,
83+
}
84+
}
85+
86+
fn emit_lint(&self) {
87+
let (msg, help) = match self.ty_params.len() {
88+
0 => return,
89+
1 => (
90+
"type parameter goes unused in function definition",
91+
"consider removing the parameter",
92+
),
93+
_ => (
94+
"type parameters go unused in function definition",
95+
"consider removing the parameters",
96+
),
97+
};
98+
99+
let source_map = self.cx.tcx.sess.source_map();
100+
let span = if self.all_params_unused {
101+
self.generics.span.into() // Remove the entire list of generics
102+
} else {
103+
MultiSpan::from_spans(
104+
self.ty_params
105+
.iter()
106+
.map(|(def_id, &span)| {
107+
// Extend the span past any trait bounds, and include the comma at the end.
108+
let span_to_extend = self.bounds.get(def_id).copied().map_or(span, Span::shrink_to_hi);
109+
let comma_range = source_map.span_extend_to_next_char(span_to_extend, '>', false);
110+
let comma_span = source_map.span_through_char(comma_range, ',');
111+
span.with_hi(comma_span.hi())
112+
})
113+
.collect(),
114+
)
115+
};
116+
117+
span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, span, msg, None, help);
118+
}
119+
}
120+
121+
impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
122+
type NestedFilter = nested_filter::OnlyBodies;
123+
124+
fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) {
125+
if let Some((def_id, _)) = t.peel_refs().as_generic_param() {
126+
if self.ty_params.remove(&def_id).is_some() {
127+
self.all_params_unused = false;
128+
}
129+
} else if let TyKind::OpaqueDef(id, _, _) = t.kind {
130+
// Explicitly walk OpaqueDef. Normally `walk_ty` would do the job, but it calls
131+
// `visit_nested_item`, which checks that `Self::NestedFilter::INTER` is set. We're
132+
// using `OnlyBodies`, so the check ends up failing and the type isn't fully walked.
133+
let item = self.nested_visit_map().item(id);
134+
walk_item(self, item);
135+
} else {
136+
walk_ty(self, t);
137+
}
138+
}
139+
140+
fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
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+
&& let PredicateOrigin::GenericParam = predicate.origin
146+
{
147+
self.bounds.insert(def_id, predicate.span);
148+
}
149+
// Only walk the right-hand side of where-bounds
150+
for bound in predicate.bounds {
151+
walk_param_bound(self, bound);
152+
}
153+
}
154+
}
155+
156+
fn nested_visit_map(&mut self) -> Self::Map {
157+
self.cx.tcx.hir()
158+
}
159+
}
160+
161+
impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
162+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
163+
if let ItemKind::Fn(_, generics, _) = item.kind {
164+
let mut walker = TypeWalker::new(cx, generics);
165+
walk_item(&mut walker, item);
166+
walker.emit_lint();
167+
}
168+
}
169+
170+
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
171+
// Only lint on inherent methods, not trait methods.
172+
if let ImplItemKind::Fn(..) = item.kind && trait_ref_of_method(cx, item.owner_id.def_id).is_none() {
173+
let mut walker = TypeWalker::new(cx, item.generics);
174+
walk_impl_item(&mut walker, item);
175+
walker.emit_lint();
176+
}
177+
}
178+
}

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ mod excessive_bools;
122122
mod exhaustive_items;
123123
mod exit;
124124
mod explicit_write;
125+
mod extra_unused_type_parameters;
125126
mod fallible_impl_from;
126127
mod float_literal;
127128
mod floating_point_arithmetic;
@@ -910,6 +911,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
910911
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
911912
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
912913
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
914+
store.register_late_pass(|_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters));
913915
// add lints here, do not remove this comment, it's used in `new_lint`
914916
}
915917

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
#![allow(unused, clippy::needless_lifetimes)]
2+
#![warn(clippy::extra_unused_type_parameters)]
3+
4+
fn unused_ty<T>(x: u8) {}
5+
6+
fn unused_multi<T, U>(x: u8) {}
7+
8+
fn unused_with_lt<'a, T>(x: &'a u8) {}
9+
10+
fn used_ty<T>(x: T, y: u8) {}
11+
12+
fn used_ref<'a, T>(x: &'a T) {}
13+
14+
fn used_ret<T: Default>(x: u8) -> T {
15+
T::default()
16+
}
17+
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) {}
27+
28+
fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize {
29+
iter.count()
30+
}
31+
32+
fn used_ret_opaque<A>() -> impl Iterator<Item = A> {
33+
std::iter::empty()
34+
}
35+
36+
fn used_vec_box<T>(x: Vec<Box<T>>) {}
37+
38+
fn used_body<T: Default + ToString>() -> String {
39+
T::default().to_string()
40+
}
41+
42+
fn used_closure<T: Default + ToString>() -> impl Fn() {
43+
|| println!("{}", T::default().to_string())
44+
}
45+
46+
struct S;
47+
48+
impl S {
49+
fn unused_ty_impl<T>(&self) {}
50+
}
51+
52+
// Don't lint on trait methods
53+
trait Foo {
54+
fn bar<T>(&self);
55+
}
56+
57+
impl Foo for S {
58+
fn bar<T>(&self) {}
59+
}
60+
61+
fn skip_index<A, Iter>(iter: Iter, index: usize) -> impl Iterator<Item = A>
62+
where
63+
Iter: Iterator<Item = A>,
64+
{
65+
iter.enumerate()
66+
.filter_map(move |(i, a)| if i == index { None } else { Some(a) })
67+
}
68+
69+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
error: type parameter goes unused in function definition
2+
--> $DIR/extra_unused_type_parameters.rs:4:13
3+
|
4+
LL | fn unused_ty<T>(x: u8) {}
5+
| ^^^
6+
|
7+
= help: consider removing the parameter
8+
= note: `-D clippy::extra-unused-type-parameters` implied by `-D warnings`
9+
10+
error: type parameters go unused in function definition
11+
--> $DIR/extra_unused_type_parameters.rs:6:16
12+
|
13+
LL | fn unused_multi<T, U>(x: u8) {}
14+
| ^^^^^^
15+
|
16+
= help: consider removing the parameters
17+
18+
error: type parameter goes unused in function definition
19+
--> $DIR/extra_unused_type_parameters.rs:8:23
20+
|
21+
LL | fn unused_with_lt<'a, T>(x: &'a u8) {}
22+
| ^
23+
|
24+
= help: consider removing the parameter
25+
26+
error: type parameter goes unused in function definition
27+
--> $DIR/extra_unused_type_parameters.rs:18:19
28+
|
29+
LL | fn unused_bounded<T: Default, U>(x: U) {}
30+
| ^^^^^^^^^^^
31+
|
32+
= help: consider removing the parameter
33+
34+
error: type parameter goes unused in function definition
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
52+
|
53+
LL | fn unused_ty_impl<T>(&self) {}
54+
| ^^^
55+
|
56+
= help: consider removing the parameter
57+
58+
error: aborting due to 7 previous errors
59+

tests/ui/needless_lifetimes.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#![allow(
66
unused,
77
clippy::boxed_local,
8+
clippy::extra_unused_type_parameters,
89
clippy::needless_pass_by_value,
910
clippy::unnecessary_wraps,
1011
dyn_drop,

tests/ui/needless_lifetimes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#![allow(
66
unused,
77
clippy::boxed_local,
8+
clippy::extra_unused_type_parameters,
89
clippy::needless_pass_by_value,
910
clippy::unnecessary_wraps,
1011
dyn_drop,

0 commit comments

Comments
 (0)