Fix false positives for extra_unused_type_parameters

This commit is contained in:
Michael Krasnitski 2023-02-11 12:22:25 -05:00
parent 0f7558148c
commit 8789b37d06
5 changed files with 61 additions and 26 deletions

View file

@ -4,10 +4,12 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_errors::MultiSpan; use rustc_errors::MultiSpan;
use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor}; use rustc_hir::intravisit::{walk_impl_item, walk_item, walk_param_bound, walk_ty, Visitor};
use rustc_hir::{ use rustc_hir::{
GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, PredicateOrigin, Ty, TyKind, WherePredicate, BodyId, ExprKind, GenericParamKind, Generics, ImplItem, ImplItemKind, Item, ItemKind, PredicateOrigin, Ty, TyKind,
WherePredicate,
}; };
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::nested_filter; use rustc_middle::hir::nested_filter;
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{def_id::DefId, Span}; use rustc_span::{def_id::DefId, Span};
@ -55,10 +57,13 @@ struct TypeWalker<'cx, 'tcx> {
/// Otherwise, if any type parameters end up being used, or if any lifetime or const-generic /// 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`. /// parameters are present, this will be set to `false`.
all_params_unused: bool, all_params_unused: bool,
/// Whether or not the function has an empty body, in which case any bounded type parameters
/// will not be linted.
fn_body_empty: bool,
} }
impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> { impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>) -> Self { fn new(cx: &'cx LateContext<'tcx>, generics: &'tcx Generics<'tcx>, body_id: BodyId) -> Self {
let mut all_params_unused = true; let mut all_params_unused = true;
let ty_params = generics let ty_params = generics
.params .params
@ -74,12 +79,18 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
} }
}) })
.collect(); .collect();
let body = cx.tcx.hir().body(body_id).value;
let fn_body_empty =
matches!(&body.kind, ExprKind::Block(block, None) if block.stmts.is_empty() && block.expr.is_none());
Self { Self {
cx, cx,
ty_params, ty_params,
bounds: FxHashMap::default(), bounds: FxHashMap::default(),
generics, generics,
all_params_unused, all_params_unused,
fn_body_empty,
} }
} }
@ -96,7 +107,7 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
), ),
}; };
let source_map = self.cx.tcx.sess.source_map(); let source_map = self.cx.sess().source_map();
let span = if self.all_params_unused { let span = if self.all_params_unused {
self.generics.span.into() // Remove the entire list of generics self.generics.span.into() // Remove the entire list of generics
} else { } else {
@ -139,13 +150,18 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) { fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
if let WherePredicate::BoundPredicate(predicate) = predicate { if let WherePredicate::BoundPredicate(predicate) = predicate {
// Collect spans for bounds that appear in the list of generics (not in a where-clause) // Collect spans for any bounds on type parameters. We only keep bounds that appear in
// for use in forming the help message // the list of generics (not in a where-clause).
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param() //
&& let PredicateOrigin::GenericParam = predicate.origin // Also, if the function body is empty, we don't lint the corresponding type parameters
{ // (See https://github.com/rust-lang/rust-clippy/issues/10319).
if let Some((def_id, _)) = predicate.bounded_ty.peel_refs().as_generic_param() {
if self.fn_body_empty {
self.ty_params.remove(&def_id);
} else if let PredicateOrigin::GenericParam = predicate.origin {
self.bounds.insert(def_id, predicate.span); self.bounds.insert(def_id, predicate.span);
} }
}
// Only walk the right-hand side of where-bounds // Only walk the right-hand side of where-bounds
for bound in predicate.bounds { for bound in predicate.bounds {
walk_param_bound(self, bound); walk_param_bound(self, bound);
@ -160,8 +176,10 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {
impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters { impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Fn(_, generics, _) = item.kind { if let ItemKind::Fn(_, generics, body_id) = item.kind
let mut walker = TypeWalker::new(cx, generics); && !in_external_macro(cx.sess(), item.span)
{
let mut walker = TypeWalker::new(cx, generics, body_id);
walk_item(&mut walker, item); walk_item(&mut walker, item);
walker.emit_lint(); walker.emit_lint();
} }
@ -169,8 +187,11 @@ impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
// Only lint on inherent methods, not trait methods. // Only lint on inherent methods, not trait methods.
if let ImplItemKind::Fn(..) = item.kind && trait_ref_of_method(cx, item.owner_id.def_id).is_none() { if let ImplItemKind::Fn(.., body_id) = item.kind
let mut walker = TypeWalker::new(cx, item.generics); && trait_ref_of_method(cx, item.owner_id.def_id).is_none()
&& !in_external_macro(cx.sess(), item.span)
{
let mut walker = TypeWalker::new(cx, item.generics, body_id);
walk_impl_item(&mut walker, item); walk_impl_item(&mut walker, item);
walker.emit_lint(); walker.emit_lint();
} }

View file

@ -15,15 +15,20 @@ fn used_ret<T: Default>(x: u8) -> T {
T::default() T::default()
} }
fn unused_bounded<T: Default, U>(x: U) {} fn unused_bounded<T: Default, U>(x: U) {
unimplemented!();
}
fn unused_where_clause<T, U>(x: U) fn unused_where_clause<T, U>(x: U)
where where
T: Default, T: Default,
{ {
unimplemented!();
} }
fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {} fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {
unimplemented!();
}
fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize { fn used_opaque<A>(iter: impl Iterator<Item = A>) -> usize {
iter.count() iter.count()
@ -66,4 +71,14 @@ where
.filter_map(move |(i, a)| if i == index { None } else { Some(a) }) .filter_map(move |(i, a)| if i == index { None } else { Some(a) })
} }
mod issue10319 {
fn assert_send<T: Send>() {}
fn assert_send_where<T>()
where
T: Send,
{
}
}
fn main() {} fn main() {}

View file

@ -26,13 +26,13 @@ LL | fn unused_with_lt<'a, T>(x: &'a u8) {}
error: type parameter goes unused in function definition error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:18:19 --> $DIR/extra_unused_type_parameters.rs:18:19
| |
LL | fn unused_bounded<T: Default, U>(x: U) {} LL | fn unused_bounded<T: Default, U>(x: U) {
| ^^^^^^^^^^^ | ^^^^^^^^^^^
| |
= help: consider removing the parameter = help: consider removing the parameter
error: type parameter goes unused in function definition error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:20:24 --> $DIR/extra_unused_type_parameters.rs:22:24
| |
LL | fn unused_where_clause<T, U>(x: U) LL | fn unused_where_clause<T, U>(x: U)
| ^^ | ^^
@ -40,15 +40,15 @@ LL | fn unused_where_clause<T, U>(x: U)
= help: consider removing the parameter = help: consider removing the parameter
error: type parameters go unused in function definition error: type parameters go unused in function definition
--> $DIR/extra_unused_type_parameters.rs:26:16 --> $DIR/extra_unused_type_parameters.rs:29:16
| |
LL | fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {} LL | fn some_unused<A, B, C, D: Iterator<Item = (B, C)>, E>(b: B, c: C) {
| ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^ | ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^
| |
= help: consider removing the parameters = help: consider removing the parameters
error: type parameter goes unused in function definition error: type parameter goes unused in function definition
--> $DIR/extra_unused_type_parameters.rs:49:22 --> $DIR/extra_unused_type_parameters.rs:54:22
| |
LL | fn unused_ty_impl<T>(&self) {} LL | fn unused_ty_impl<T>(&self) {}
| ^^^ | ^^^

View file

@ -1,5 +1,4 @@
#![deny(clippy::type_repetition_in_bounds)] #![deny(clippy::type_repetition_in_bounds)]
#![allow(clippy::extra_unused_type_parameters)]
use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};

View file

@ -1,5 +1,5 @@
error: this type has already been used as a bound predicate error: this type has already been used as a bound predicate
--> $DIR/type_repetition_in_bounds.rs:9:5 --> $DIR/type_repetition_in_bounds.rs:8:5
| |
LL | T: Clone, LL | T: Clone,
| ^^^^^^^^ | ^^^^^^^^
@ -12,7 +12,7 @@ LL | #![deny(clippy::type_repetition_in_bounds)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this type has already been used as a bound predicate error: this type has already been used as a bound predicate
--> $DIR/type_repetition_in_bounds.rs:26:5 --> $DIR/type_repetition_in_bounds.rs:25:5
| |
LL | Self: Copy + Default + Ord, LL | Self: Copy + Default + Ord,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -20,7 +20,7 @@ LL | Self: Copy + Default + Ord,
= help: consider combining the bounds: `Self: Clone + Copy + Default + Ord` = help: consider combining the bounds: `Self: Clone + Copy + Default + Ord`
error: this type has already been used as a bound predicate error: this type has already been used as a bound predicate
--> $DIR/type_repetition_in_bounds.rs:86:5 --> $DIR/type_repetition_in_bounds.rs:85:5
| |
LL | T: Clone, LL | T: Clone,
| ^^^^^^^^ | ^^^^^^^^
@ -28,7 +28,7 @@ LL | T: Clone,
= help: consider combining the bounds: `T: ?Sized + Clone` = help: consider combining the bounds: `T: ?Sized + Clone`
error: this type has already been used as a bound predicate error: this type has already been used as a bound predicate
--> $DIR/type_repetition_in_bounds.rs:91:5 --> $DIR/type_repetition_in_bounds.rs:90:5
| |
LL | T: ?Sized, LL | T: ?Sized,
| ^^^^^^^^^ | ^^^^^^^^^