Skip to content

Commit e186ed2

Browse files
committed
check return type of get and indexing
1 parent 46b3264 commit e186ed2

File tree

4 files changed

+94
-18
lines changed

4 files changed

+94
-18
lines changed

clippy_lints/src/indexing_slicing.rs

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
use clippy_utils::consts::{constant, Constant};
44
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
55
use clippy_utils::higher;
6-
use clippy_utils::ty::{adt_has_inherent_method, deref_chain};
6+
use clippy_utils::ty::{deref_chain, get_adt_inherent_method};
77
use rustc_ast::ast::RangeLimits;
88
use rustc_hir::{Expr, ExprKind};
99
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty;
10+
use rustc_middle::ty::{self, Ty};
1111
use rustc_session::impl_lint_pass;
12+
use rustc_span::sym;
1213

1314
declare_clippy_lint! {
1415
/// ### What it does
@@ -111,7 +112,7 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
111112
&& deref.any(|l| {
112113
l.peel_refs().is_slice()
113114
|| l.peel_refs().is_array()
114-
|| adt_has_inherent_method(cx, l.peel_refs(), sym!(get))
115+
|| ty_has_appliciable_get_function(cx, l.peel_refs(), expr_ty, expr)
115116
})
116117
{
117118
let note = "the suggestion might not be applicable in constant blocks";
@@ -240,3 +241,36 @@ fn to_const_range(cx: &LateContext<'_>, range: higher::Range<'_>, array_size: u1
240241

241242
(start, end)
242243
}
244+
245+
/// Checks if the output Ty of the `get` method on this Ty (if any) matches the Ty returned by the
246+
/// indexing operation (if any).
247+
fn ty_has_appliciable_get_function<'tcx>(
248+
cx: &LateContext<'tcx>,
249+
ty: Ty<'tcx>,
250+
array_ty: Ty<'tcx>,
251+
index_expr: &Expr<'_>,
252+
) -> bool {
253+
if let ty::Adt(_, array_args) = array_ty.kind()
254+
&& let Some(get_output_ty) = get_adt_inherent_method(cx, ty, sym!(get)).map(|m| {
255+
cx.tcx
256+
.fn_sig(m.def_id)
257+
.instantiate(cx.tcx, array_args)
258+
.output()
259+
.skip_binder()
260+
})
261+
&& let ty::Adt(def, args) = get_output_ty.kind()
262+
&& cx.tcx.is_diagnostic_item(sym::Option, def.0.did)
263+
&& let Some(option_generic_param) = args.get(0)
264+
&& let generic_ty = option_generic_param.expect_ty().peel_refs()
265+
&& let _ = println!(
266+
"{}, {}",
267+
cx.typeck_results().expr_ty(index_expr).peel_refs(),
268+
generic_ty.peel_refs()
269+
)
270+
&& cx.typeck_results().expr_ty(index_expr).peel_refs() == generic_ty.peel_refs()
271+
{
272+
true
273+
} else {
274+
false
275+
}
276+
}

clippy_lints/src/iter_without_into_iter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::get_parent_as_impl;
33
use clippy_utils::source::snippet;
4-
use clippy_utils::ty::{adt_has_inherent_method, deref_chain, implements_trait, make_normalized_projection};
4+
use clippy_utils::ty::{deref_chain, get_adt_inherent_method, implements_trait, make_normalized_projection};
55
use rustc_ast::Mutability;
66
use rustc_errors::Applicability;
77
use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind};
@@ -139,7 +139,7 @@ impl LateLintPass<'_> for IterWithoutIntoIter {
139139
}
140140
&& !deref_chain(cx, ty).any(|ty| {
141141
// We can't check inherent impls for slices, but we know that they have an `iter(_mut)` method
142-
ty.peel_refs().is_slice() || adt_has_inherent_method(cx, ty, expected_method_name)
142+
ty.peel_refs().is_slice() || get_adt_inherent_method(cx, ty, expected_method_name).is_some()
143143
})
144144
&& let Some(iter_assoc_span) = imp.items.iter().find_map(|item| {
145145
if item.ident.name == sym!(IntoIter) {

clippy_utils/src/ty.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ use rustc_middle::mir::ConstValue;
1717
use rustc_middle::traits::EvaluationResult;
1818
use rustc_middle::ty::layout::ValidityRequirement;
1919
use rustc_middle::ty::{
20-
self, AdtDef, AliasTy, AssocKind, Binder, BoundRegion, FnSig, GenericArg, GenericArgKind, GenericArgsRef,
21-
GenericParamDefKind, IntTy, ParamEnv, Region, RegionKind, TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
22-
TypeVisitableExt, TypeVisitor, UintTy, Upcast, VariantDef, VariantDiscr,
20+
self, AdtDef, AliasTy, AssocItem, AssocKind, Binder, BoundRegion, FnSig, GenericArg, GenericArgKind,
21+
GenericArgsRef, GenericParamDefKind, IntTy, List, ParamEnv, Region, RegionKind, ToPredicate, TraitRef, Ty, TyCtxt,
22+
TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, UintTy, VariantDef, VariantDiscr,
2323
};
2424
use rustc_span::symbol::Ident;
2525
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
@@ -1345,16 +1345,22 @@ pub fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl
13451345
/// Checks if a Ty<'_> has some inherent method Symbol.
13461346
/// This does not look for impls in the type's `Deref::Target` type.
13471347
/// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`.
1348-
pub fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
1348+
pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<&'a AssocItem> {
13491349
if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) {
1350-
cx.tcx.inherent_impls(ty_did).into_iter().flatten().any(|&did| {
1351-
cx.tcx
1352-
.associated_items(did)
1353-
.filter_by_name_unhygienic(method_name)
1354-
.next()
1355-
.is_some_and(|item| item.kind == ty::AssocKind::Fn)
1356-
})
1350+
cx.tcx
1351+
.inherent_impls(ty_did)
1352+
.into_iter()
1353+
.flatten()
1354+
.map(|&did| {
1355+
cx.tcx
1356+
.associated_items(did)
1357+
.filter_by_name_unhygienic(method_name)
1358+
.next()
1359+
.filter(|item| item.kind == ty::AssocKind::Fn)
1360+
})
1361+
.next()
1362+
.flatten()
13571363
} else {
1358-
false
1364+
None
13591365
}
13601366
}

tests/ui/indexing_slicing_slice.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@
22
// We also check the out_of_bounds_indexing lint here, because it lints similar things and
33
// we want to avoid false positives.
44
#![warn(clippy::out_of_bounds_indexing)]
5-
#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::useless_vec)]
5+
#![allow(
6+
clippy::no_effect,
7+
clippy::unnecessary_operation,
8+
clippy::useless_vec,
9+
unused_must_use,
10+
unused
11+
)]
612
#![warn(clippy::indexing_slicing)]
713

814
use std::ops::Index;
@@ -19,6 +25,28 @@ impl<T> Index<bool> for BoolMap<T> {
1925
}
2026
}
2127

28+
struct BoolMapWithGet<T> {
29+
false_value: T,
30+
true_value: T,
31+
}
32+
33+
impl<T> Index<bool> for BoolMapWithGet<T> {
34+
type Output = T;
35+
fn index(&self, index: bool) -> &Self::Output {
36+
if index { &self.true_value } else { &self.false_value }
37+
}
38+
}
39+
40+
impl<T> BoolMapWithGet<T> {
41+
fn get(&self, index: bool) -> Option<&T> {
42+
if index {
43+
Some(&self.true_value)
44+
} else {
45+
Some(&self.false_value)
46+
}
47+
}
48+
}
49+
2250
fn main() {
2351
let x = [1, 2, 3, 4];
2452
let index: usize = 1;
@@ -73,4 +101,12 @@ fn main() {
73101
};
74102

75103
map[true]; // Ok, because `get` does not exist (custom indexing)
104+
105+
let map_with_get = BoolMapWithGet {
106+
false_value: 2,
107+
true_value: 4,
108+
};
109+
110+
// Lint on this, because `get` does exist with same signature
111+
map_with_get[true];
76112
}

0 commit comments

Comments
 (0)