Skip to content

Commit 2562f84

Browse files
borsflip1995
bors
authored and
flip1995
committed
Auto merge of #11070 - y21:issue11065, r=flip1995
[`useless_conversion`]: only lint on paths to fn items and fix FP in macro Fixes #11065 (which is actually two issues: an ICE and a false positive) It now makes sure that the function call path points to a function-like item (and not e.g. a `const` like in the linked issue), so that calling `TyCtxt::fn_sig` later in the lint does not ICE (fixes rust-lang/rust-clippy#11065 (comment)). It *also* makes sure that the expression is not part of a macro call (fixes rust-lang/rust-clippy#11065 (comment)). ~~I'm not sure if there's a better way to check this other than to walk the parent expr chain and see if any of them are expansions.~~ (edit: it doesn't do this anymore) changelog: [`useless_conversion`]: fix ICE when call receiver is a non-fn item changelog: [`useless_conversion`]: don't lint if argument is a macro argument (fixes a FP) r? `@llogiq` (reviewed #10814, which introduced these issues)
1 parent 3a86898 commit 2562f84

File tree

5 files changed

+72
-13
lines changed

5 files changed

+72
-13
lines changed

‎src/tools/clippy/clippy_lints/src/useless_conversion.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
66
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, path_to_local, paths};
77
use if_chain::if_chain;
88
use rustc_errors::Applicability;
9+
use rustc_hir::def::DefKind;
910
use rustc_hir::def_id::DefId;
1011
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
1112
use rustc_lint::{LateContext, LateLintPass};
@@ -40,6 +41,7 @@ declare_clippy_lint! {
4041
#[derive(Default)]
4142
pub struct UselessConversion {
4243
try_desugar_arm: Vec<HirId>,
44+
expn_depth: u32,
4345
}
4446

4547
impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
@@ -106,6 +108,7 @@ fn into_iter_deep_call<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -
106108
impl<'tcx> LateLintPass<'tcx> for UselessConversion {
107109
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
108110
if e.span.from_expansion() {
111+
self.expn_depth += 1;
109112
return;
110113
}
111114

@@ -151,9 +154,14 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
151154
{
152155
if let Some(parent) = get_parent_expr(cx, e) {
153156
let parent_fn = match parent.kind {
154-
ExprKind::Call(recv, args) if let ExprKind::Path(ref qpath) = recv.kind => {
155-
cx.qpath_res(qpath, recv.hir_id).opt_def_id()
156-
.map(|did| (did, args, MethodOrFunction::Function))
157+
ExprKind::Call(recv, args)
158+
if let ExprKind::Path(ref qpath) = recv.kind
159+
&& let Some(did) = cx.qpath_res(qpath, recv.hir_id).opt_def_id()
160+
// make sure that the path indeed points to a fn-like item, so that
161+
// `fn_sig` does not ICE. (see #11065)
162+
&& cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) =>
163+
{
164+
Some((did, args, MethodOrFunction::Function))
157165
}
158166
ExprKind::MethodCall(.., args, _) => {
159167
cx.typeck_results().type_dependent_def_id(parent.hir_id)
@@ -169,6 +177,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
169177
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
170178
&& let ty::Param(param) = into_iter_param.kind()
171179
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
180+
&& self.expn_depth == 0
172181
{
173182
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
174183
// `foo.into_iter().into_iter()`
@@ -304,5 +313,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
304313
if Some(&e.hir_id) == self.try_desugar_arm.last() {
305314
self.try_desugar_arm.pop();
306315
}
316+
if e.span.from_expansion() {
317+
self.expn_depth -= 1;
318+
}
307319
}
308320
}

‎src/tools/clippy/tests/ui/crashes/ice-11065.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#![warn(clippy::useless_conversion)]
2+
3+
use std::iter::FromIterator;
4+
use std::option::IntoIter as OptionIter;
5+
6+
fn eq<T: Eq>(a: T, b: T) -> bool {
7+
a == b
8+
}
9+
10+
macro_rules! tests {
11+
($($expr:expr, $ty:ty, ($($test:expr),*);)+) => (pub fn main() {$({
12+
const C: $ty = $expr;
13+
assert!(eq(C($($test),*), $expr($($test),*)));
14+
})+})
15+
}
16+
17+
tests! {
18+
FromIterator::from_iter, fn(OptionIter<i32>) -> Vec<i32>, (Some(5).into_iter());
19+
}

‎src/tools/clippy/tests/ui/useless_conversion.fixed

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,20 @@ fn main() {
155155
let _ = vec![s4, s4, s4].into_iter();
156156
}
157157

158+
#[allow(dead_code)]
159+
fn issue11065_fp() {
160+
use std::option::IntoIter;
161+
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
162+
163+
macro_rules! x {
164+
($e:expr) => {
165+
takes_into_iter($e);
166+
let _: IntoIter<i32> = $e; // removing `.into_iter()` leads to a type error here
167+
};
168+
}
169+
x!(Some(5).into_iter());
170+
}
171+
158172
#[allow(dead_code)]
159173
fn explicit_into_iter_fn_arg() {
160174
fn a<T>(_: T) {}

‎src/tools/clippy/tests/ui/useless_conversion.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,20 @@ fn main() {
155155
let _ = vec![s4, s4, s4].into_iter().into_iter();
156156
}
157157

158+
#[allow(dead_code)]
159+
fn issue11065_fp() {
160+
use std::option::IntoIter;
161+
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
162+
163+
macro_rules! x {
164+
($e:expr) => {
165+
takes_into_iter($e);
166+
let _: IntoIter<i32> = $e; // removing `.into_iter()` leads to a type error here
167+
};
168+
}
169+
x!(Some(5).into_iter());
170+
}
171+
158172
#[allow(dead_code)]
159173
fn explicit_into_iter_fn_arg() {
160174
fn a<T>(_: T) {}

‎src/tools/clippy/tests/ui/useless_conversion.stderr

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,61 +119,61 @@ LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
119119
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
120120

121121
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
122-
--> $DIR/useless_conversion.rs:171:7
122+
--> $DIR/useless_conversion.rs:185:7
123123
|
124124
LL | b(vec![1, 2].into_iter());
125125
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
126126
|
127127
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
128-
--> $DIR/useless_conversion.rs:161:13
128+
--> $DIR/useless_conversion.rs:175:13
129129
|
130130
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
131131
| ^^^^^^^^^^^^^^^^^^^^^^^^
132132

133133
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
134-
--> $DIR/useless_conversion.rs:172:7
134+
--> $DIR/useless_conversion.rs:186:7
135135
|
136136
LL | c(vec![1, 2].into_iter());
137137
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
138138
|
139139
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
140-
--> $DIR/useless_conversion.rs:162:18
140+
--> $DIR/useless_conversion.rs:176:18
141141
|
142142
LL | fn c(_: impl IntoIterator<Item = i32>) {}
143143
| ^^^^^^^^^^^^^^^^^^^^^^^^
144144

145145
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
146-
--> $DIR/useless_conversion.rs:173:7
146+
--> $DIR/useless_conversion.rs:187:7
147147
|
148148
LL | d(vec![1, 2].into_iter());
149149
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
150150
|
151151
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
152-
--> $DIR/useless_conversion.rs:165:12
152+
--> $DIR/useless_conversion.rs:179:12
153153
|
154154
LL | T: IntoIterator<Item = i32>,
155155
| ^^^^^^^^^^^^^^^^^^^^^^^^
156156

157157
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
158-
--> $DIR/useless_conversion.rs:176:7
158+
--> $DIR/useless_conversion.rs:190:7
159159
|
160160
LL | b(vec![1, 2].into_iter().into_iter());
161161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
162162
|
163163
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
164-
--> $DIR/useless_conversion.rs:161:13
164+
--> $DIR/useless_conversion.rs:175:13
165165
|
166166
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
167167
| ^^^^^^^^^^^^^^^^^^^^^^^^
168168

169169
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
170-
--> $DIR/useless_conversion.rs:177:7
170+
--> $DIR/useless_conversion.rs:191:7
171171
|
172172
LL | b(vec![1, 2].into_iter().into_iter().into_iter());
173173
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
174174
|
175175
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
176-
--> $DIR/useless_conversion.rs:161:13
176+
--> $DIR/useless_conversion.rs:175:13
177177
|
178178
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
179179
| ^^^^^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)