Skip to content

Commit

Permalink
Improve help for extra_unused_type_parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
@mkrasnitski
mkrasnitski committed Dec 21, 2022
1 parent e612c53 commit e1681e5
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 38 deletions.
103 changes: 80 additions & 23 deletions clippy_lints/src/extra_unused_type_parameters.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
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_hir::{
GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, LifetimeParamKind, 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, BytePos, Span};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -39,32 +42,77 @@ declare_lint_pass!(ExtraUnusedTypeParameters => [EXTRA_UNUSED_TYPE_PARAMETERS]);
struct TypeWalker<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
map: FxHashMap<DefId, Span>,
bound_map: FxHashMap<DefId, Span>,
generics: &'tcx Generics<'tcx>,
some_params_used: bool,
has_non_ty_params: 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 {
let mut has_non_ty_params = false;
let map = generics
.params
.iter()
.filter_map(|param| match &param.kind {
GenericParamKind::Type { .. } => Some((param.def_id.into(), param.span)),
GenericParamKind::Lifetime {
kind: LifetimeParamKind::Elided,
} => None,
_ => {
has_non_ty_params = true;
None
},
})
.collect();
Self {
cx,
map: generics
.params
.iter()
.filter_map(|param| match param.kind {
GenericParamKind::Type { .. } => Some((param.def_id.into(), param.span)),
_ => None,
})
.collect(),
map,
generics,
has_non_ty_params,
bound_map: FxHashMap::default(),
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 || self.has_non_ty_params {
MultiSpan::from_spans(
self.map
.iter()
.map(|(def_id, &(mut span))| {
if let Some(bound_span) = self.bound_map.get(def_id) {
span = span.with_hi(bound_span.hi());
}
span = source_map.span_extend_to_next_char(span, ',', false);
span = span.with_hi(BytePos(span.hi().0 + 1));

let max_hi = self.generics.span.hi();
if span.hi() >= max_hi {
span = span.with_hi(BytePos(max_hi.0 - 1));
}
span
})
.collect(),
)
} else {
self.generics.span.into()
};

span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, span, msg, None, help);
}
}

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

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
Expand All @@ -86,9 +136,16 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
}

fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
if let WherePredicate::BoundPredicate(where_bound_predicate) = predicate {
// Only check the right-hand side of where-bounds
for bound in where_bound_predicate.bounds {
if let WherePredicate::BoundPredicate(predicate) = predicate {
// Collect spans for bounds that appear in the list of generics (not in a where-clause)
// for use in forming the help message
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param()
&& predicate.span < self.generics.where_clause_span
{
self.bound_map.insert(def_id, predicate.span);
}
// Only walk the right-hand side of where-bounds
for bound in predicate.bounds {
walk_param_bound(self, bound);
}
}
Expand Down
12 changes: 11 additions & 1 deletion tests/ui/extra_unused_type_parameters.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ fn unused_ty<T>(x: u8) {}

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

fn unused_with_lt<'a, T>(x: &'a u8) {}

fn used_ty<T>(x: T, y: u8) {}

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

fn unused_with_bound<T: Default>(x: u8) {}
fn unused_bounded<T: Default, U>(x: U) {}

fn unused_where_clause<T, U>(x: U)
where
T: Default,
{
}

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()
Expand Down
53 changes: 39 additions & 14 deletions tests/ui/extra_unused_type_parameters.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,59 @@
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) {}
| ^
| ^^^^^^
|
= help: consider removing the parameters

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

error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:16:22
--> $DIR/extra_unused_type_parameters.rs:18:19
|
LL | fn unused_bounded<T: Default, U>(x: U) {}
| ^^^^^^^^^^^
|
LL | fn unused_with_bound<T: Default>(x: u8) {}
| ^
= help: consider removing the parameter

error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:39:23
--> $DIR/extra_unused_type_parameters.rs:20:24
|
LL | fn unused_where_clause<T, U>(x: U)
| ^^
|
= help: consider removing the parameter

error: type parameters go unused in function definition
--> $DIR/extra_unused_type_parameters.rs:26: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:49:22
|
LL | fn unused_ty_impl<T>(&self) {}
| ^
| ^^^
|
= help: consider removing the parameter

error: aborting due to 5 previous errors
error: aborting due to 7 previous errors

0 comments on commit e1681e5

Please sign in to comment.