Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positives for extra_unused_type_parameters #10321

Merged
merged 1 commit into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions clippy_lints/src/extra_unused_type_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ 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, 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::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{def_id::DefId, Span};

Expand Down Expand Up @@ -55,10 +57,13 @@ struct TypeWalker<'cx, 'tcx> {
/// 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,
/// 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> {
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 ty_params = generics
.params
Expand All @@ -74,12 +79,18 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
}
})
.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 {
cx,
ty_params,
bounds: FxHashMap::default(),
generics,
all_params_unused,
fn_body_empty,
}
}

Expand All @@ -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 {
self.generics.span.into() // Remove the entire list of generics
} else {
Expand Down Expand Up @@ -139,12 +150,17 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {

fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate<'tcx>) {
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()
&& let PredicateOrigin::GenericParam = predicate.origin
{
self.bounds.insert(def_id, predicate.span);
// Collect spans for any bounds on type parameters. We only keep bounds that appear in
// the list of generics (not in a where-clause).
//
// 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);
}
}
// Only walk the right-hand side of where-bounds
for bound in predicate.bounds {
Expand All @@ -160,17 +176,22 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> {

impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Fn(_, generics, _) = item.kind {
let mut walker = TypeWalker::new(cx, generics);
if let ItemKind::Fn(_, generics, body_id) = item.kind
&& !in_external_macro(cx.sess(), item.span)
{
let mut walker = TypeWalker::new(cx, generics, body_id);
walk_item(&mut walker, item);
walker.emit_lint();
}
}

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(..) = item.kind && trait_ref_of_method(cx, item.owner_id.def_id).is_none() {
let mut walker = TypeWalker::new(cx, item.generics);
if let ImplItemKind::Fn(.., body_id) = item.kind
&& 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);
walker.emit_lint();
}
Expand Down
19 changes: 17 additions & 2 deletions tests/ui/extra_unused_type_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@ fn used_ret<T: Default>(x: u8) -> T {
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)
where
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 {
iter.count()
Expand Down Expand Up @@ -66,4 +71,14 @@ where
.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() {}
10 changes: 5 additions & 5 deletions tests/ui/extra_unused_type_parameters.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,29 @@ LL | fn unused_with_lt<'a, T>(x: &'a u8) {}
error: type parameter goes unused in function definition
--> $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

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)
| ^^
|
= help: consider removing the parameter

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

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) {}
| ^^^
Expand Down
1 change: 0 additions & 1 deletion tests/ui/type_repetition_in_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![deny(clippy::type_repetition_in_bounds)]
#![allow(clippy::extra_unused_type_parameters)]

use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/type_repetition_in_bounds.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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,
| ^^^^^^^^
Expand All @@ -12,23 +12,23 @@ LL | #![deny(clippy::type_repetition_in_bounds)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider combining the bounds: `Self: Clone + Copy + Default + Ord`

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,
| ^^^^^^^^
|
= help: consider combining the bounds: `T: ?Sized + Clone`

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,
| ^^^^^^^^^
Expand Down