Skip to content

Commit efc9e11

Browse files
JarchoJarcho
committed
Consider generic arguments when linting trait functions in only_used_in_recursion 
1 parent efc305a commit efc9e11

File tree

3 files changed

+92
-20
lines changed

3 files changed

+92
-20
lines changed

‎clippy_lints/src/only_used_in_recursion.rs‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use rustc_hir::def_id::DefId;
66
use rustc_hir::hir_id::HirIdMap;
77
use rustc_hir::{Body, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Node, PatKind, TraitItem, TraitItemKind};
88
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
10+
use rustc_middle::ty::{self, ConstKind};
911
use rustc_session::{declare_tool_lint, impl_lint_pass};
1012
use rustc_span::Span;
1113

@@ -81,11 +83,21 @@ declare_clippy_lint! {
8183
}
8284
impl_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
8385

84-
#[derive(Debug)]
86+
#[derive(Clone, Copy)]
87+
enum FnKind {
88+
Fn,
89+
TraitFn,
90+
// This is a hack. Ideally we would store a `SubstsRef<'tcx>` type here, but a lint pass must be `'static`.
91+
// Substitutions are, however, interned. This allows us to store the pointer as a `usize` when comparing for
92+
// equality.
93+
ImplTraitFn(usize),
94+
}
95+
8596
struct Param {
86-
/// The function this is a parameter for
97+
/// The function this is a parameter for.
8798
fn_id: DefId,
88-
/// The index of this parameter
99+
fn_kind: FnKind,
100+
/// The index of this parameter.
89101
idx: u32,
90102
span: Span,
91103
/// Whether this parameter should be linted. Set by `Params::flag_for_linting`.
@@ -94,9 +106,10 @@ struct Param {
94106
uses: Vec<Usage>,
95107
}
96108
impl Param {
97-
fn new(fn_id: DefId, idx: u32, span: Span) -> Self {
109+
fn new(fn_id: DefId, fn_kind: FnKind, idx: u32, span: Span) -> Self {
98110
Self {
99111
fn_id,
112+
fn_kind,
100113
idx,
101114
span,
102115
block_lint: Cell::new(false),
@@ -211,14 +224,15 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
211224
}
212225
// `skip_params` is either `0` or `1` to skip the `self` parameter in trait functions.
213226
// It can't be renamed, and it can't be removed without removing it from multiple functions.
214-
let (fn_id, skip_params) = match get_parent_node(cx.tcx, body.value.hir_id) {
215-
Some(Node::Item(i)) => (i.def_id.to_def_id(), 0),
227+
let (fn_id, fn_kind, skip_params) = match get_parent_node(cx.tcx, body.value.hir_id) {
228+
Some(Node::Item(i)) => (i.def_id.to_def_id(), FnKind::Fn, 0),
216229
Some(Node::TraitItem(&TraitItem {
217230
kind: TraitItemKind::Fn(ref sig, _),
218231
def_id,
219232
..
220233
})) => (
221234
def_id.to_def_id(),
235+
FnKind::TraitFn,
222236
if sig.decl.implicit_self.has_implicit_self() {
223237
1
224238
} else {
@@ -230,17 +244,22 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
230244
def_id,
231245
..
232246
})) => {
233-
if let Some(trait_fn_id) = cx.tcx.associated_item(def_id).trait_item_def_id {
247+
#[allow(trivial_casts)]
248+
if let Some(Node::Item(item)) = get_parent_node(cx.tcx, cx.tcx.hir().local_def_id_to_hir_id(def_id))
249+
&& let Some(trait_ref) = cx.tcx.impl_trait_ref(item.def_id)
250+
&& let Some(trait_item_id) = cx.tcx.associated_item(def_id).trait_item_def_id
251+
{
234252
(
235-
trait_fn_id,
253+
trait_item_id,
254+
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize),
236255
if sig.decl.implicit_self.has_implicit_self() {
237256
1
238257
} else {
239258
0
240259
},
241260
)
242261
} else {
243-
(def_id.to_def_id(), 0)
262+
(def_id.to_def_id(), FnKind::Fn, 0)
244263
}
245264
},
246265
_ => return,
@@ -252,7 +271,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
252271
.filter_map(|(idx, p)| match p.pat.kind {
253272
#[allow(clippy::cast_possible_truncation)]
254273
PatKind::Binding(_, id, ident, None) if !ident.as_str().starts_with('_') => {
255-
Some((id, Param::new(fn_id, idx as u32, ident.span)))
274+
Some((id, Param::new(fn_id, fn_kind, idx as u32, ident.span)))
256275
},
257276
_ => None,
258277
})
@@ -275,7 +294,10 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
275294
Some((Node::Expr(parent), child_id)) => match parent.kind {
276295
// Recursive call. Track which index the parameter is used in.
277296
ExprKind::Call(callee, args)
278-
if path_def_id(cx, callee).map_or(false, |id| id == param.fn_id) =>
297+
if path_def_id(cx, callee).map_or(false, |id| {
298+
id == param.fn_id
299+
&& has_matching_substs(param.fn_kind, typeck.node_substs(callee.hir_id))
300+
}) =>
279301
{
280302
if let Some(idx) = args.iter().position(|arg| arg.hir_id == child_id) {
281303
#[allow(clippy::cast_possible_truncation)]
@@ -284,9 +306,10 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
284306
return;
285307
},
286308
ExprKind::MethodCall(_, args, _)
287-
if typeck
288-
.type_dependent_def_id(parent.hir_id)
289-
.map_or(false, |id| id == param.fn_id) =>
309+
if typeck.type_dependent_def_id(parent.hir_id).map_or(false, |id| {
310+
id == param.fn_id
311+
&& has_matching_substs(param.fn_kind, typeck.node_substs(parent.hir_id))
312+
}) =>
290313
{
291314
if let Some(idx) = args.iter().position(|arg| arg.hir_id == child_id) {
292315
#[allow(clippy::cast_possible_truncation)]
@@ -318,7 +341,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
318341
ExprKind::Field(..) | ExprKind::AddrOf(..) | ExprKind::Cast(..) => {
319342
e = parent;
320343
continue;
321-
}
344+
},
322345
_ => (),
323346
},
324347
_ => (),
@@ -347,3 +370,16 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
347370
}
348371
}
349372
}
373+
374+
fn has_matching_substs(kind: FnKind, substs: SubstsRef<'_>) -> bool {
375+
match kind {
376+
FnKind::Fn => true,
377+
FnKind::TraitFn => substs.iter().enumerate().all(|(idx, subst)| match subst.unpack() {
378+
GenericArgKind::Lifetime(_) => true,
379+
GenericArgKind::Type(ty) => matches!(*ty.kind(), ty::Param(ty) if ty.index as usize == idx),
380+
GenericArgKind::Const(c) => matches!(c.val(), ConstKind::Param(c) if c.index as usize == idx),
381+
}),
382+
#[allow(trivial_casts)]
383+
FnKind::ImplTraitFn(expected_substs) => substs as *const _ as usize == expected_substs,
384+
}
385+
}

‎tests/ui/only_used_in_recursion.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,26 @@ impl B for A {
7676
}
7777
}
7878

79+
impl B for () {
80+
fn method(flag: u32, a: usize) -> usize {
81+
if flag == 0 { 0 } else { a }
82+
}
83+
84+
fn method_self(&self, flag: u32, a: usize) -> usize {
85+
if flag == 0 { 0 } else { a }
86+
}
87+
}
88+
89+
impl B for u32 {
90+
fn method(flag: u32, a: usize) -> usize {
91+
if flag == 0 { 0 } else { <() as B>::method(flag, a) }
92+
}
93+
94+
fn method_self(&self, flag: u32, a: usize) -> usize {
95+
if flag == 0 { 0 } else { ().method_self(flag, a) }
96+
}
97+
}
98+
7999
trait C {
80100
fn method(flag: u32, a: usize) -> usize {
81101
if flag == 0 { 0 } else { Self::method(flag - 1, a) }
@@ -133,4 +153,20 @@ fn only_let(x: u32) {
133153
let _z = x * y;
134154
}
135155

156+
trait E<T: E<()>> {
157+
fn method(flag: u32, a: usize) -> usize {
158+
if flag == 0 {
159+
0
160+
} else {
161+
<T as E<()>>::method(flag - 1, a)
162+
}
163+
}
164+
}
165+
166+
impl E<()> for () {
167+
fn method(flag: u32, a: usize) -> usize {
168+
if flag == 0 { 0 } else { a }
169+
}
170+
}
171+
136172
fn main() {}

‎tests/ui/only_used_in_recursion.stderr‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,31 +85,31 @@ LL | fn method_self(&self, flag: u32, a: usize) -> usize {
8585
| ^
8686

8787
error: parameter is only used in recursion
88-
--> $DIR/only_used_in_recursion.rs:80:26
88+
--> $DIR/only_used_in_recursion.rs:100:26
8989
|
9090
LL | fn method(flag: u32, a: usize) -> usize {
9191
| ^
9292

9393
error: parameter is only used in recursion
94-
--> $DIR/only_used_in_recursion.rs:84:38
94+
--> $DIR/only_used_in_recursion.rs:104:38
9595
|
9696
LL | fn method_self(&self, flag: u32, a: usize) -> usize {
9797
| ^
9898

9999
error: parameter is only used in recursion
100-
--> $DIR/only_used_in_recursion.rs:93:35
100+
--> $DIR/only_used_in_recursion.rs:113:35
101101
|
102102
LL | fn _with_inner(flag: u32, a: u32, b: u32) -> usize {
103103
| ^
104104

105105
error: parameter is only used in recursion
106-
--> $DIR/only_used_in_recursion.rs:94:25
106+
--> $DIR/only_used_in_recursion.rs:114:25
107107
|
108108
LL | fn inner(flag: u32, a: u32) -> u32 {
109109
| ^
110110

111111
error: parameter is only used in recursion
112-
--> $DIR/only_used_in_recursion.rs:102:34
112+
--> $DIR/only_used_in_recursion.rs:122:34
113113
|
114114
LL | fn _with_closure(a: Option<u32>, b: u32, f: impl Fn(u32, u32) -> Option<u32>) -> u32 {
115115
| ^

0 commit comments

Comments
 (0)