Auto merge of #12488 - Jacherr:issue-11525, r=llogiq

Disable `indexing_slicing` for custom Index impls

Fixes https://github.com/rust-lang/rust-clippy/issues/11525

Disables `indexing_slicing` for custom Index impls, specifically any implementations that also do not have a `get` method anywhere along the deref chain (so, for example, it still lints on Vec, which has its `get` method as part of the deref chain).

Thanks `@y21` for pointing me in the right direction with a couple of handy util functions for deref chain and inherent methods, saved a headache there!

changelog: FP: Disable `indexing_slicing` for custom Index impls
This commit is contained in:
bors 2024-05-31 16:42:50 +00:00
commit 28e887fe71
5 changed files with 232 additions and 54 deletions

View file

@ -3,11 +3,13 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::higher;
use clippy_utils::ty::{deref_chain, get_adt_inherent_method};
use rustc_ast::ast::RangeLimits;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty};
use rustc_session::impl_lint_pass;
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
@ -104,7 +106,15 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
return;
}
if let ExprKind::Index(array, index, _) = &expr.kind {
if let ExprKind::Index(array, index, _) = &expr.kind
&& let expr_ty = cx.typeck_results().expr_ty(array)
&& let mut deref = deref_chain(cx, expr_ty)
&& deref.any(|l| {
l.peel_refs().is_slice()
|| l.peel_refs().is_array()
|| ty_has_applicable_get_function(cx, l.peel_refs(), expr_ty, expr)
})
{
let note = "the suggestion might not be applicable in constant blocks";
let ty = cx.typeck_results().expr_ty(array).peel_refs();
if let Some(range) = higher::Range::hir(index) {
@ -231,3 +241,33 @@ fn to_const_range(cx: &LateContext<'_>, range: higher::Range<'_>, array_size: u1
(start, end)
}
/// Checks if the output Ty of the `get` method on this Ty (if any) matches the Ty returned by the
/// indexing operation (if any).
fn ty_has_applicable_get_function<'tcx>(
cx: &LateContext<'tcx>,
ty: Ty<'tcx>,
array_ty: Ty<'tcx>,
index_expr: &Expr<'_>,
) -> bool {
if let ty::Adt(_, _) = array_ty.kind()
&& let Some(get_output_ty) = get_adt_inherent_method(cx, ty, sym!(get)).map(|m| {
cx.tcx
.fn_sig(m.def_id)
.skip_binder()
.output()
.skip_binder()
})
&& let ty::Adt(def, args) = get_output_ty.kind()
&& cx.tcx.is_diagnostic_item(sym::Option, def.0.did)
&& let Some(option_generic_param) = args.first()
&& let generic_ty = option_generic_param.expect_ty().peel_refs()
// FIXME: ideally this would handle type params and projections properly, for now just assume it's the same type
&& (cx.typeck_results().expr_ty(index_expr).peel_refs() == generic_ty.peel_refs()
|| matches!(generic_ty.peel_refs().kind(), ty::Param(_) | ty::Alias(_, _)))
{
true
} else {
false
}
}

View file

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::get_parent_as_impl;
use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, make_normalized_projection};
use clippy_utils::ty::{deref_chain, get_adt_inherent_method, implements_trait, make_normalized_projection};
use rustc_ast::Mutability;
use rustc_errors::Applicability;
use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind};
@ -9,8 +9,7 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, Ty};
use rustc_session::declare_lint_pass;
use rustc_span::{sym, Symbol};
use std::iter;
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
@ -124,33 +123,6 @@ fn is_ty_exported(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
.is_some_and(|did| cx.effective_visibilities.is_exported(did))
}
/// Returns the deref chain of a type, starting with the type itself.
fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl Iterator<Item = Ty<'tcx>> + 'cx {
iter::successors(Some(ty), |&ty| {
if let Some(deref_did) = cx.tcx.lang_items().deref_trait()
&& implements_trait(cx, ty, deref_did, &[])
{
make_normalized_projection(cx.tcx, cx.param_env, deref_did, sym::Target, [ty])
} else {
None
}
})
}
fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) {
cx.tcx.inherent_impls(ty_did).into_iter().flatten().any(|&did| {
cx.tcx
.associated_items(did)
.filter_by_name_unhygienic(method_name)
.next()
.is_some_and(|item| item.kind == ty::AssocKind::Fn)
})
} else {
false
}
}
impl LateLintPass<'_> for IterWithoutIntoIter {
fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) {
if !in_external_macro(cx.sess(), item.span)
@ -167,7 +139,7 @@ impl LateLintPass<'_> for IterWithoutIntoIter {
}
&& !deref_chain(cx, ty).any(|ty| {
// We can't check inherent impls for slices, but we know that they have an `iter(_mut)` method
ty.peel_refs().is_slice() || adt_has_inherent_method(cx, ty, expected_method_name)
ty.peel_refs().is_slice() || get_adt_inherent_method(cx, ty, expected_method_name).is_some()
})
&& let Some(iter_assoc_span) = imp.items.iter().find_map(|item| {
if item.ident.name == sym!(IntoIter) {