From 2493be2196f6ba16eaea25915c25d93f1c2664b5 Mon Sep 17 00:00:00 2001 From: bluthej Date: Sun, 26 Mar 2023 18:25:15 +0200 Subject: [PATCH] Improve `is_range_full` implementation Make this function work with signed integer types by extracting the underlying type and finding the min and max values. Change the signature to make it more consistent: - The range is now given as an `Expr` in order to extract the type - The container's path is now passed, and only as an `Option` so that the function can be called in the general case without a container --- clippy_lints/src/methods/clear_with_drain.rs | 7 +- clippy_lints/src/methods/iter_with_drain.rs | 7 +- clippy_utils/src/lib.rs | 71 ++++++++++++++++---- 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/methods/clear_with_drain.rs b/clippy_lints/src/methods/clear_with_drain.rs index bf7e7d12405..24496bd4689 100644 --- a/clippy_lints/src/methods/clear_with_drain.rs +++ b/clippy_lints/src/methods/clear_with_drain.rs @@ -1,9 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher::Range; use clippy_utils::is_range_full; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; -use rustc_hir::Expr; +use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::LateContext; use rustc_span::symbol::sym; use rustc_span::Span; @@ -12,7 +11,9 @@ use super::CLEAR_WITH_DRAIN; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: &Expr<'_>) { let ty = cx.typeck_results().expr_ty(recv); - if is_type_diagnostic_item(cx, ty, sym::Vec) && let Some(range) = Range::hir(arg) && is_range_full(cx, recv, range) + if is_type_diagnostic_item(cx, ty, sym::Vec) + && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind + && is_range_full(cx, arg, Some(container_path)) { span_lint_and_sugg( cx, diff --git a/clippy_lints/src/methods/iter_with_drain.rs b/clippy_lints/src/methods/iter_with_drain.rs index ea92e3a549f..f6772c5c6b3 100644 --- a/clippy_lints/src/methods/iter_with_drain.rs +++ b/clippy_lints/src/methods/iter_with_drain.rs @@ -1,8 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher::Range; use clippy_utils::is_range_full; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::LateContext; use rustc_span::symbol::sym; use rustc_span::Span; @@ -14,8 +13,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span && let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def() && let Some(ty_name) = cx.tcx.get_diagnostic_name(adt.did()) && matches!(ty_name, sym::Vec | sym::VecDeque) - && let Some(range) = Range::hir(arg) - && is_range_full(cx, recv, range) + && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind + && is_range_full(cx, arg, Some(container_path)) { span_lint_and_sugg( cx, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index e8225feb33d..2e839fdf472 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -96,6 +96,7 @@ use rustc_hir::{ use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::place::PlaceBase; +use rustc_middle::mir::ConstantKind; use rustc_middle::ty as rustc_ty; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; use rustc_middle::ty::binding::BindingMode; @@ -114,7 +115,7 @@ use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; use rustc_target::abi::Integer; -use crate::consts::{constant, Constant}; +use crate::consts::{constant, miri_to_const, Constant}; use crate::higher::Range; use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param}; use crate::visitors::for_each_expr; @@ -1492,22 +1493,66 @@ pub fn is_else_clause(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool { } } -/// Checks whether the given `Range` is equivalent to a `RangeFull`. -/// Inclusive ranges are not considered because they already constitute a lint. -pub fn is_range_full(cx: &LateContext<'_>, container: &Expr<'_>, range: Range<'_>) -> bool { - range.start.map_or(true, |e| is_integer_const(cx, e, 0)) - && range.end.map_or(true, |e| { - if range.limits == RangeLimits::HalfOpen - && let ExprKind::Path(QPath::Resolved(None, container_path)) = container.kind - && let ExprKind::MethodCall(name, self_arg, [], _) = e.kind - && name.ident.name == sym::len - && let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind +/// Checks whether the given `Expr` is a range equivalent to a `RangeFull`. +/// For the lower bound, this means that: +/// - either there is none +/// - or it is the smallest value that can be represented by the range's integer type +/// For the upper bound, this means that: +/// - either there is none +/// - or it is the largest value that can be represented by the range's integer type and is +/// inclusive +/// - or it is a call to some container's `len` method and is exclusive, and the range is passed to +/// a method call on that same container (e.g. `v.drain(..v.len())`) +/// If the given `Expr` is not some kind of range, the function returns `false`. +pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Option<&Path<'_>>) -> bool { + let ty = cx.typeck_results().expr_ty(expr); + if let Some(Range { start, end, limits }) = Range::hir(expr) { + let start_is_none_or_min = start.map_or(true, |start| { + if let rustc_ty::Adt(_, subst) = ty.kind() + && let bnd_ty = subst.type_at(0) + && let Some(min_val) = bnd_ty.numeric_min_val(cx.tcx) + && let const_val = cx.tcx.valtree_to_const_val((bnd_ty, min_val.to_valtree())) + && let min_const_kind = ConstantKind::from_value(const_val, bnd_ty) + && let Some(min_const) = miri_to_const(cx.tcx, min_const_kind) + && let Some((start_const, _)) = constant(cx, cx.typeck_results(), start) { - container_path.res == path.res + start_const == min_const } else { false } - }) + }); + let end_is_none_or_max = end.map_or(true, |end| { + match limits { + RangeLimits::Closed => { + if let rustc_ty::Adt(_, subst) = ty.kind() + && let bnd_ty = subst.type_at(0) + && let Some(max_val) = bnd_ty.numeric_max_val(cx.tcx) + && let const_val = cx.tcx.valtree_to_const_val((bnd_ty, max_val.to_valtree())) + && let max_const_kind = ConstantKind::from_value(const_val, bnd_ty) + && let Some(max_const) = miri_to_const(cx.tcx, max_const_kind) + && let Some((end_const, _)) = constant(cx, cx.typeck_results(), end) + { + end_const == max_const + } else { + false + } + }, + RangeLimits::HalfOpen => { + if let Some(container_path) = container_path + && let ExprKind::MethodCall(name, self_arg, [], _) = end.kind + && name.ident.name == sym::len + && let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind + { + container_path.res == path.res + } else { + false + } + }, + } + }); + return start_is_none_or_min && end_is_none_or_max; + } + false } /// Checks whether the given expression is a constant integer of the given value.