mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add suggestions to extra_unused_type_parameters

Browse Source
This commit is contained in:
Michael Krasnitski 2023-03-22 22:34:53 -04:00
parent 1d1e72308e
commit 50d92d0b60
8 changed files with 323 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

179
clippy_lints/src/extra_unused_type_parameters.rs
View File

@ -1,10 +1,10 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::trait_ref_of_method;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::MultiSpan;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor};
use rustc_hir::{
BodyId, ExprKind, GenericBound, GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind,
BodyId, ExprKind, GenericBound, GenericParam, GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind,
PredicateOrigin, Ty, TyKind, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
@ -53,13 +53,19 @@ pub fn new(avoid_breaking_exported_api: bool) -> Self {
}
}
/// Don't lint external macros or functions with empty bodies. Also, don't lint public items if
/// the `avoid_breaking_exported_api` config option is set.
fn check_false_positive(&self, cx: &LateContext<'_>, span: Span, def_id: LocalDefId, body_id: BodyId) -> bool {
/// Don't lint external macros or functions with empty bodies. Also, don't lint exported items
/// if the `avoid_breaking_exported_api` config option is set.
fn is_empty_exported_or_macro(
&self,
cx: &LateContext<'_>,
span: Span,
def_id: LocalDefId,
body_id: BodyId,
) -> bool {
let body = cx.tcx.hir().body(body_id).value;
let fn_empty = matches!(&body.kind, ExprKind::Block(blk, None) if blk.stmts.is_empty() && blk.expr.is_none());
let is_exported = cx.effective_visibilities.is_exported(def_id);
in_external_macro(cx.sess(), span) || (self.avoid_breaking_exported_api && is_exported) || fn_empty
in_external_macro(cx.sess(), span) || fn_empty || (is_exported && self.avoid_breaking_exported_api)
}
}
@ -69,85 +75,129 @@ fn check_false_positive(&self, cx: &LateContext<'_>, span: Span, def_id: LocalDe
/// trait bounds those parameters have.
struct TypeWalker<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
/// Collection of all the function's type parameters.
/// Collection of the function's type parameters. Once the function has been walked, this will
/// contain only unused type parameters.
ty_params: FxHashMap<DefId, Span>,
/// Collection of any (inline) trait bounds corresponding to each type parameter.
bounds: FxHashMap<DefId, Span>,
/// Collection of any inline trait bounds corresponding to each type parameter.
inline_bounds: FxHashMap<DefId, Span>,
/// Collection of any type parameters with trait bounds that appear in a where clause.
where_bounds: FxHashSet<DefId>,
/// The entire `Generics` object of the function, useful for querying purposes.
generics: &'tcx Generics<'tcx>,
/// The value of this will remain `true` if *every* parameter:
/// 1. Is a type parameter, and
/// 2. Goes unused in the function.
/// Otherwise, if any type parameters end up being used, or if any lifetime or const-generic
/// parameters are present, this will be set to `false`.
all_params_unused: bool,
}
impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>) -> Self {
let mut all_params_unused = true;
let ty_params = generics
.params
.iter()
.filter_map(|param| {
if let GenericParamKind::Type { synthetic, .. } = param.kind {
(!synthetic).then_some((param.def_id.into(), param.span))
} else {
if !param.is_elided_lifetime() {
all_params_unused = false;
}
None
}
.filter_map(|param| match param.kind {
GenericParamKind::Type { synthetic, .. } if !synthetic => Some((param.def_id.into(), param.span)),
_ => None,
})
.collect();
Self {
cx,
ty_params,
bounds: FxHashMap::default(),
inline_bounds: FxHashMap::default(),
where_bounds: FxHashSet::default(),
generics,
all_params_unused,
}
}
fn mark_param_used(&mut self, def_id: DefId) {
if self.ty_params.remove(&def_id).is_some() {
self.all_params_unused = false;
}
fn get_bound_span(&self, param: &'tcx GenericParam<'tcx>) -> Span {
self.inline_bounds
.get(&param.def_id.to_def_id())
.map_or(param.span, |bound_span| param.span.with_hi(bound_span.hi()))
}
fn emit_help(&self, spans: Vec<Span>, msg: &str, help: &'static str) {
span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, spans, msg, None, help);
}
fn emit_sugg(&self, spans: Vec<Span>, msg: &str, help: &'static str) {
let suggestions: Vec<(Span, String)> = spans.iter().copied().zip(std::iter::repeat(String::new())).collect();
span_lint_and_then(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, spans, msg, |diag| {
diag.multipart_suggestion(help, suggestions, Applicability::MachineApplicable);
});
}
fn emit_lint(&self) {
let (msg, help) = match self.ty_params.len() {
let explicit_params = self
.generics
.params
.iter()
.filter(|param| !param.is_elided_lifetime() && !param.is_impl_trait())
.collect::<Vec<_>>();
let extra_params = explicit_params
.iter()
.enumerate()
.filter(|(_, param)| self.ty_params.contains_key(&param.def_id.to_def_id()))
.collect::<Vec<_>>();
let (msg, help) = match extra_params.len() {
0 => return,
1 => (
"type parameter goes unused in function definition",
format!(
"type parameter `{}` goes unused in function definition",
extra_params[0].1.name.ident()
),
"consider removing the parameter",
),
_ => (
"type parameters go unused in function definition",
format!(
"type parameters go unused in function definition: {}",
extra_params
.iter()
.map(|(_, param)| param.name.ident().to_string())
.collect::<Vec<_>>()
.join(", ")
),
"consider removing the parameters",
),
};
let source_map = self.cx.sess().source_map();
let span = if self.all_params_unused {
self.generics.span.into() // Remove the entire list of generics
// If any parameters are bounded in where clauses, don't try to form a suggestion.
// Otherwise, the leftover where bound would produce code that wouldn't compile.
if extra_params
.iter()
.any(|(_, param)| self.where_bounds.contains(&param.def_id.to_def_id()))
{
let spans = extra_params
.iter()
.map(|(_, param)| self.get_bound_span(param))
.collect::<Vec<_>>();
self.emit_help(spans, &msg, help);
} else {
MultiSpan::from_spans(
self.ty_params
let spans = if explicit_params.len() == extra_params.len() {
vec![self.generics.span] // Remove the entire list of generics
} else {
let mut end: Option<LocalDefId> = None;
extra_params
.iter()
.map(|(def_id, &span)| {
// Extend the span past any trait bounds, and include the comma at the end.
let span_to_extend = self.bounds.get(def_id).copied().map_or(span, Span::shrink_to_hi);
let comma_range = source_map.span_extend_to_next_char(span_to_extend, '>', false);
let comma_span = source_map.span_through_char(comma_range, ',');
span.with_hi(comma_span.hi())
})
.collect(),
)
};
.rev()
.map(|(idx, param)| {
if let Some(next) = explicit_params.get(idx + 1) && end != Some(next.def_id) {
// Extend the current span forward, up until the next param in the list.
param.span.until(next.span)
} else {
// Extend the current span back to include the comma following the previous
// param. If the span of the next param in the list has already been
// extended, we continue the chain. This is why we're iterating in reverse.
end = Some(param.def_id);
span_lint_and_help(self.cx, EXTRA_UNUSED_TYPE_PARAMETERS, span, msg, None, help);
// idx will never be 0, else we'd be removing the entire list of generics
let prev = explicit_params[idx - 1];
let prev_span = self.get_bound_span(prev);
self.get_bound_span(param).with_lo(prev_span.hi())
}
})
.collect()
};
self.emit_sugg(spans, &msg, help);
};
}
}
@ -162,7 +212,7 @@ 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.mark_param_used(def_id);
self.ty_params.remove(&def_id);
} 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
@ -176,9 +226,18 @@ fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) {
fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
if let WherePredicate::BoundPredicate(predicate) = predicate {
// Collect spans for any bounds on type parameters. We only keep bounds that appear in
// the list of generics (not in a where-clause).
// Collect spans for any bounds on type parameters.
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param() {
match predicate.origin {
PredicateOrigin::GenericParam => {
self.inline_bounds.insert(def_id, predicate.span);
},
PredicateOrigin::WhereClause => {
self.where_bounds.insert(def_id);
},
PredicateOrigin::ImplTrait => (),
}
// If the bound contains non-public traits, err on the safe side and don't lint the
// corresponding parameter.
if !predicate
@ -187,12 +246,10 @@ fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
.filter_map(bound_to_trait_def_id)
.all(|id| self.cx.effective_visibilities.is_exported(id))
{
self.mark_param_used(def_id);
} else if let PredicateOrigin::GenericParam = predicate.origin {
self.bounds.insert(def_id, predicate.span);
self.ty_params.remove(&def_id);
}
}
// Only walk the right-hand side of where-bounds
// Only walk the right-hand side of where bounds
for bound in predicate.bounds {
walk_param_bound(self, bound);
}
@ -207,7 +264,7 @@ fn nested_visit_map(&mut self) -> Self::Map {
impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Fn(_, generics, body_id) = item.kind
&& !self.check_false_positive(cx, item.span, item.owner_id.def_id, body_id)
&& !self.is_empty_exported_or_macro(cx, item.span, item.owner_id.def_id, body_id)
{
let mut walker = TypeWalker::new(cx, generics);
walk_item(&mut walker, item);
@ -219,7 +276,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>
// Only lint on inherent methods, not trait methods.
if let ImplItemKind::Fn(.., body_id) = item.kind
&& trait_ref_of_method(cx, item.owner_id.def_id).is_none()
&& !self.check_false_positive(cx, item.span, item.owner_id.def_id, body_id)
&& !self.is_empty_exported_or_macro(cx, item.span, item.owner_id.def_id, body_id)
{
let mut walker = TypeWalker::new(cx, item.generics);
walk_impl_item(&mut walker, item);

1
tests/ui-toml/extra_unused_type_parameters/clippy.toml Normal file
View File

@ -0,0 +1 @@
avoid-breaking-exported-api = true

9
tests/ui-toml/extra_unused_type_parameters/extra_unused_type_parameters.rs Normal file
View File

@ -0,0 +1,9 @@
pub struct S;
impl S {
pub fn exported_fn<T>() {
unimplemented!();
}
}
fn main() {}

105
tests/ui/extra_unused_type_parameters.fixed Normal file
View File

@ -0,0 +1,105 @@
// run-rustfix
#![allow(unused, clippy::needless_lifetimes)]
#![warn(clippy::extra_unused_type_parameters)]
fn unused_ty(x: u8) {
unimplemented!()
}
fn unused_multi(x: u8) {
unimplemented!()
}
fn unused_with_lt<'a>(x: &'a u8) {
unimplemented!()
}
fn used_ty<T>(x: T, y: u8) {}
fn used_ref<'a, T>(x: &'a T) {}
fn used_ret<T: Default>(x: u8) -> T {
T::default()
}
fn unused_bounded<U>(x: U) {
unimplemented!();
}
fn some_unused<B, C>(b: B, c: C) {
unimplemented!();
}
fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize {
iter.count()
}
fn used_ret_opaque<A>() -> impl Iterator<Item = A> {
std::iter::empty()
}
fn used_vec_box<T>(x: Vec<Box<T>>) {}
fn used_body<T: Default + ToString>() -> String {
T::default().to_string()
}
fn used_closure<T: Default + ToString>() -> impl Fn() {
|| println!("{}", T::default().to_string())
}
struct S;
impl S {
fn unused_ty_impl(&self) {
unimplemented!()
}
}
// Don't lint on trait methods
trait Foo {
fn bar<T>(&self);
}
impl Foo for S {
fn bar<T>(&self) {}
}
fn skip_index<A, Iter>(iter: Iter, index: usize) -> impl Iterator<Item = A>
where
Iter: Iterator<Item = A>,
{
iter.enumerate()
.filter_map(move |(i, a)| if i == index { None } else { Some(a) })
}
fn unused_opaque(dummy: impl Default) {
unimplemented!()
}
mod unexported_trait_bounds {
mod private {
pub trait Private {}
}
fn priv_trait_bound<T: private::Private>() {
unimplemented!();
}
fn unused_with_priv_trait_bound<T: private::Private>() {
unimplemented!();
}
}
mod issue10319 {
fn assert_send<T: Send>() {}
fn assert_send_where<T>()
where
T: Send,
{
}
}
fn main() {}

11
tests/ui/extra_unused_type_parameters.rs
View File

@ -1,3 +1,5 @@
// run-rustfix
#![allow(unused, clippy::needless_lifetimes)]
#![warn(clippy::extra_unused_type_parameters)]
@ -21,14 +23,7 @@ fn used_ret<T: Default>(x: u8) -> T {
T::default()
}
fn unused_bounded<T: Default, U>(x: U) {
unimplemented!();
}
fn unused_where_clause<T, U>(x: U)
where
T: Default,
{
fn unused_bounded<T: Default, U, V: Default>(x: U) {
unimplemented!();
}

83
tests/ui/extra_unused_type_parameters.stderr
View File

@ -1,75 +1,64 @@
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:4:13
error: type parameter `T` goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:6:13
|
LL | fn unused_ty<T>(x: u8) {
| ^^^
| ^^^ help: consider removing the parameter
|
= help: consider removing the parameter
= note: `-D clippy::extra-unused-type-parameters` implied by `-D warnings`
error: type parameters go unused in function definition
--> $DIR/extra_unused_type_parameters.rs:8:16
error: type parameters go unused in function definition: T, U
--> $DIR/extra_unused_type_parameters.rs:10:16
|
LL | fn unused_multi<T, U>(x: u8) {
| ^^^^^^
|
= help: consider removing the parameters
| ^^^^^^ help: consider removing the parameters
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:12:23
error: type parameter `T` goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:14:21
|
LL | fn unused_with_lt<'a, T>(x: &'a u8) {
| ^
|
= help: consider removing the parameter
| ^^^ help: consider removing the parameter
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:24:19
error: type parameters go unused in function definition: T, V
--> $DIR/extra_unused_type_parameters.rs:26:19
|
LL | fn unused_bounded<T: Default, U>(x: U) {
| ^^^^^^^^^^^
LL | fn unused_bounded<T: Default, U, V: Default>(x: U) {
| ^^^^^^^^^^^^ ^^^^^^^^^^^^
|
help: consider removing the parameters
|
LL - fn unused_bounded<T: Default, U, V: Default>(x: U) {
LL + fn unused_bounded<U>(x: U) {
|
= help: consider removing the parameter
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:28: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:35:16
error: type parameters go unused in function definition: A, D, E
--> $DIR/extra_unused_type_parameters.rs:30:16
|
LL | fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {
| ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^
| ^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider removing the parameters
|
LL - fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {
LL + fn some_unused<B, C>(b: B, c: C) {
|
= help: consider removing the parameters
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:60:22
error: type parameter `T` goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:55:22
|
LL | fn unused_ty_impl<T>(&self) {
| ^^^
|
= help: consider removing the parameter
| ^^^ help: consider removing the parameter
error: type parameters go unused in function definition
--> $DIR/extra_unused_type_parameters.rs:82:17
error: type parameters go unused in function definition: A, B
--> $DIR/extra_unused_type_parameters.rs:77:17
|
LL | fn unused_opaque<A, B>(dummy: impl Default) {
| ^^^^^^
|
= help: consider removing the parameters
| ^^^^^^ help: consider removing the parameters
error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:95:58
error: type parameter `U` goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:90:56
|
LL | fn unused_with_priv_trait_bound<T: private::Private, U>() {
| ^
|
= help: consider removing the parameter
| ^^^ help: consider removing the parameter
error: aborting due to 9 previous errors
error: aborting due to 8 previous errors

24
tests/ui/extra_unused_type_parameters_unfixable.rs Normal file
View File

@ -0,0 +1,24 @@
#![warn(clippy::extra_unused_type_parameters)]
fn unused_where_clause<T, U>(x: U)
where
T: Default,
{
unimplemented!();
}
fn unused_multi_where_clause<T, U, V: Default>(x: U)
where
T: Default,
{
unimplemented!();
}
fn unused_all_where_clause<T, U: Default, V: Default>()
where
T: Default,
{
unimplemented!();
}
fn main() {}

27
tests/ui/extra_unused_type_parameters_unfixable.stderr Normal file
View File

@ -0,0 +1,27 @@
error: type parameter `T` goes unused in function definition
--> $DIR/extra_unused_type_parameters_unfixable.rs:3:24
|
LL | fn unused_where_clause<T, U>(x: U)
| ^
|
= help: consider removing the parameter
= note: `-D clippy::extra-unused-type-parameters` implied by `-D warnings`
error: type parameters go unused in function definition: T, V
--> $DIR/extra_unused_type_parameters_unfixable.rs:10:30
|
LL | fn unused_multi_where_clause<T, U, V: Default>(x: U)
| ^ ^^^^^^^^^^
|
= help: consider removing the parameters
error: type parameters go unused in function definition: T, U, V
--> $DIR/extra_unused_type_parameters_unfixable.rs:17:28
|
LL | fn unused_all_where_clause<T, U: Default, V: Default>()
| ^ ^^^^^^^^^^ ^^^^^^^^^^
|
= help: consider removing the parameters
error: aborting due to 3 previous errors