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 https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616836099). It *also* makes sure that the expression is not part of a macro call (fixes https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616919639). ~~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)
This commit is contained in:
commit
701e77c87f
5 changed files with 72 additions and 13 deletions
|
@ -5,6 +5,7 @@ use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
|
||||||
use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, match_def_path, path_to_local, paths};
|
use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, match_def_path, path_to_local, paths};
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
|
use rustc_hir::def::DefKind;
|
||||||
use rustc_hir::def_id::DefId;
|
use rustc_hir::def_id::DefId;
|
||||||
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
|
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
|
||||||
use rustc_lint::{LateContext, LateLintPass};
|
use rustc_lint::{LateContext, LateLintPass};
|
||||||
|
@ -39,6 +40,7 @@ declare_clippy_lint! {
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct UselessConversion {
|
pub struct UselessConversion {
|
||||||
try_desugar_arm: Vec<HirId>,
|
try_desugar_arm: Vec<HirId>,
|
||||||
|
expn_depth: u32,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
|
impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
|
||||||
|
@ -105,6 +107,7 @@ fn into_iter_deep_call<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -
|
||||||
impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
|
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
|
||||||
if e.span.from_expansion() {
|
if e.span.from_expansion() {
|
||||||
|
self.expn_depth += 1;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -150,9 +153,14 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
||||||
{
|
{
|
||||||
if let Some(parent) = get_parent_expr(cx, e) {
|
if let Some(parent) = get_parent_expr(cx, e) {
|
||||||
let parent_fn = match parent.kind {
|
let parent_fn = match parent.kind {
|
||||||
ExprKind::Call(recv, args) if let ExprKind::Path(ref qpath) = recv.kind => {
|
ExprKind::Call(recv, args)
|
||||||
cx.qpath_res(qpath, recv.hir_id).opt_def_id()
|
if let ExprKind::Path(ref qpath) = recv.kind
|
||||||
.map(|did| (did, args, MethodOrFunction::Function))
|
&& let Some(did) = cx.qpath_res(qpath, recv.hir_id).opt_def_id()
|
||||||
|
// make sure that the path indeed points to a fn-like item, so that
|
||||||
|
// `fn_sig` does not ICE. (see #11065)
|
||||||
|
&& cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) =>
|
||||||
|
{
|
||||||
|
Some((did, args, MethodOrFunction::Function))
|
||||||
}
|
}
|
||||||
ExprKind::MethodCall(.., args, _) => {
|
ExprKind::MethodCall(.., args, _) => {
|
||||||
cx.typeck_results().type_dependent_def_id(parent.hir_id)
|
cx.typeck_results().type_dependent_def_id(parent.hir_id)
|
||||||
|
@ -168,6 +176,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
||||||
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
|
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
|
||||||
&& let ty::Param(param) = into_iter_param.kind()
|
&& let ty::Param(param) = into_iter_param.kind()
|
||||||
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
|
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
|
||||||
|
&& self.expn_depth == 0
|
||||||
{
|
{
|
||||||
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
|
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
|
||||||
// `foo.into_iter().into_iter()`
|
// `foo.into_iter().into_iter()`
|
||||||
|
@ -303,5 +312,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
|
||||||
if Some(&e.hir_id) == self.try_desugar_arm.last() {
|
if Some(&e.hir_id) == self.try_desugar_arm.last() {
|
||||||
self.try_desugar_arm.pop();
|
self.try_desugar_arm.pop();
|
||||||
}
|
}
|
||||||
|
if e.span.from_expansion() {
|
||||||
|
self.expn_depth -= 1;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
19
tests/ui/crashes/ice-11065.rs
Normal file
19
tests/ui/crashes/ice-11065.rs
Normal file
|
@ -0,0 +1,19 @@
|
||||||
|
#![warn(clippy::useless_conversion)]
|
||||||
|
|
||||||
|
use std::iter::FromIterator;
|
||||||
|
use std::option::IntoIter as OptionIter;
|
||||||
|
|
||||||
|
fn eq<T: Eq>(a: T, b: T) -> bool {
|
||||||
|
a == b
|
||||||
|
}
|
||||||
|
|
||||||
|
macro_rules! tests {
|
||||||
|
($($expr:expr, $ty:ty, ($($test:expr),*);)+) => (pub fn main() {$({
|
||||||
|
const C: $ty = $expr;
|
||||||
|
assert!(eq(C($($test),*), $expr($($test),*)));
|
||||||
|
})+})
|
||||||
|
}
|
||||||
|
|
||||||
|
tests! {
|
||||||
|
FromIterator::from_iter, fn(OptionIter<i32>) -> Vec<i32>, (Some(5).into_iter());
|
||||||
|
}
|
|
@ -153,6 +153,20 @@ fn main() {
|
||||||
let _ = vec![s4, s4, s4].into_iter();
|
let _ = vec![s4, s4, s4].into_iter();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[allow(dead_code)]
|
||||||
|
fn issue11065_fp() {
|
||||||
|
use std::option::IntoIter;
|
||||||
|
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
|
||||||
|
|
||||||
|
macro_rules! x {
|
||||||
|
($e:expr) => {
|
||||||
|
takes_into_iter($e);
|
||||||
|
let _: IntoIter<i32> = $e; // removing `.into_iter()` leads to a type error here
|
||||||
|
};
|
||||||
|
}
|
||||||
|
x!(Some(5).into_iter());
|
||||||
|
}
|
||||||
|
|
||||||
#[allow(dead_code)]
|
#[allow(dead_code)]
|
||||||
fn explicit_into_iter_fn_arg() {
|
fn explicit_into_iter_fn_arg() {
|
||||||
fn a<T>(_: T) {}
|
fn a<T>(_: T) {}
|
||||||
|
|
|
@ -153,6 +153,20 @@ fn main() {
|
||||||
let _ = vec![s4, s4, s4].into_iter().into_iter();
|
let _ = vec![s4, s4, s4].into_iter().into_iter();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[allow(dead_code)]
|
||||||
|
fn issue11065_fp() {
|
||||||
|
use std::option::IntoIter;
|
||||||
|
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
|
||||||
|
|
||||||
|
macro_rules! x {
|
||||||
|
($e:expr) => {
|
||||||
|
takes_into_iter($e);
|
||||||
|
let _: IntoIter<i32> = $e; // removing `.into_iter()` leads to a type error here
|
||||||
|
};
|
||||||
|
}
|
||||||
|
x!(Some(5).into_iter());
|
||||||
|
}
|
||||||
|
|
||||||
#[allow(dead_code)]
|
#[allow(dead_code)]
|
||||||
fn explicit_into_iter_fn_arg() {
|
fn explicit_into_iter_fn_arg() {
|
||||||
fn a<T>(_: T) {}
|
fn a<T>(_: T) {}
|
||||||
|
|
|
@ -119,61 +119,61 @@ LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
|
||||||
|
|
||||||
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
||||||
--> $DIR/useless_conversion.rs:169:7
|
--> $DIR/useless_conversion.rs:183:7
|
||||||
|
|
|
|
||||||
LL | b(vec![1, 2].into_iter());
|
LL | b(vec![1, 2].into_iter());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
||||||
|
|
|
|
||||||
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
||||||
--> $DIR/useless_conversion.rs:159:13
|
--> $DIR/useless_conversion.rs:173:13
|
||||||
|
|
|
|
||||||
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
||||||
--> $DIR/useless_conversion.rs:170:7
|
--> $DIR/useless_conversion.rs:184:7
|
||||||
|
|
|
|
||||||
LL | c(vec![1, 2].into_iter());
|
LL | c(vec![1, 2].into_iter());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
||||||
|
|
|
|
||||||
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
||||||
--> $DIR/useless_conversion.rs:160:18
|
--> $DIR/useless_conversion.rs:174:18
|
||||||
|
|
|
|
||||||
LL | fn c(_: impl IntoIterator<Item = i32>) {}
|
LL | fn c(_: impl IntoIterator<Item = i32>) {}
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
||||||
--> $DIR/useless_conversion.rs:171:7
|
--> $DIR/useless_conversion.rs:185:7
|
||||||
|
|
|
|
||||||
LL | d(vec![1, 2].into_iter());
|
LL | d(vec![1, 2].into_iter());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`: `vec![1, 2]`
|
||||||
|
|
|
|
||||||
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
||||||
--> $DIR/useless_conversion.rs:163:12
|
--> $DIR/useless_conversion.rs:177:12
|
||||||
|
|
|
|
||||||
LL | T: IntoIterator<Item = i32>,
|
LL | T: IntoIterator<Item = i32>,
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
||||||
--> $DIR/useless_conversion.rs:174:7
|
--> $DIR/useless_conversion.rs:188:7
|
||||||
|
|
|
|
||||||
LL | b(vec![1, 2].into_iter().into_iter());
|
LL | b(vec![1, 2].into_iter().into_iter());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
|
||||||
|
|
|
|
||||||
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
||||||
--> $DIR/useless_conversion.rs:159:13
|
--> $DIR/useless_conversion.rs:173:13
|
||||||
|
|
|
|
||||||
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
|
||||||
--> $DIR/useless_conversion.rs:175:7
|
--> $DIR/useless_conversion.rs:189:7
|
||||||
|
|
|
|
||||||
LL | b(vec![1, 2].into_iter().into_iter().into_iter());
|
LL | b(vec![1, 2].into_iter().into_iter().into_iter());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing the `.into_iter()`s: `vec![1, 2]`
|
||||||
|
|
|
|
||||||
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
|
||||||
--> $DIR/useless_conversion.rs:159:13
|
--> $DIR/useless_conversion.rs:173:13
|
||||||
|
|
|
|
||||||
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue