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
This commit is contained in:
bluthej 2023-03-26 18:25:15 +02:00
parent ee0de538d4
commit 2493be2196
3 changed files with 65 additions and 20 deletions

View file

@ -1,9 +1,8 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher::Range;
use clippy_utils::is_range_full; use clippy_utils::is_range_full;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::Expr; use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_span::symbol::sym; use rustc_span::symbol::sym;
use rustc_span::Span; 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<'_>) { pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: &Expr<'_>) {
let ty = cx.typeck_results().expr_ty(recv); 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( span_lint_and_sugg(
cx, cx,

View file

@ -1,8 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher::Range;
use clippy_utils::is_range_full; use clippy_utils::is_range_full;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind}; use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_span::symbol::sym; use rustc_span::symbol::sym;
use rustc_span::Span; 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(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
&& let Some(ty_name) = cx.tcx.get_diagnostic_name(adt.did()) && let Some(ty_name) = cx.tcx.get_diagnostic_name(adt.did())
&& matches!(ty_name, sym::Vec | sym::VecDeque) && matches!(ty_name, sym::Vec | sym::VecDeque)
&& let Some(range) = Range::hir(arg) && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind
&& is_range_full(cx, recv, range) && is_range_full(cx, arg, Some(container_path))
{ {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,

View file

@ -96,6 +96,7 @@ use rustc_hir::{
use rustc_lexer::{tokenize, TokenKind}; use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::place::PlaceBase; use rustc_middle::hir::place::PlaceBase;
use rustc_middle::mir::ConstantKind;
use rustc_middle::ty as rustc_ty; use rustc_middle::ty as rustc_ty;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
use rustc_middle::ty::binding::BindingMode; use rustc_middle::ty::binding::BindingMode;
@ -114,7 +115,7 @@ use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::Span; use rustc_span::Span;
use rustc_target::abi::Integer; use rustc_target::abi::Integer;
use crate::consts::{constant, Constant}; use crate::consts::{constant, miri_to_const, Constant};
use crate::higher::Range; 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::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param};
use crate::visitors::for_each_expr; use crate::visitors::for_each_expr;
@ -1492,14 +1493,53 @@ pub fn is_else_clause(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
} }
} }
/// Checks whether the given `Range` is equivalent to a `RangeFull`. /// Checks whether the given `Expr` is a range equivalent to a `RangeFull`.
/// Inclusive ranges are not considered because they already constitute a lint. /// For the lower bound, this means that:
pub fn is_range_full(cx: &LateContext<'_>, container: &Expr<'_>, range: Range<'_>) -> bool { /// - either there is none
range.start.map_or(true, |e| is_integer_const(cx, e, 0)) /// - or it is the smallest value that can be represented by the range's integer type
&& range.end.map_or(true, |e| { /// For the upper bound, this means that:
if range.limits == RangeLimits::HalfOpen /// - either there is none
&& let ExprKind::Path(QPath::Resolved(None, container_path)) = container.kind /// - or it is the largest value that can be represented by the range's integer type and is
&& let ExprKind::MethodCall(name, self_arg, [], _) = e.kind /// 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)
{
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 && name.ident.name == sym::len
&& let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind && let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind
{ {
@ -1507,7 +1547,12 @@ pub fn is_range_full(cx: &LateContext<'_>, container: &Expr<'_>, range: Range<'_
} else { } else {
false 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. /// Checks whether the given expression is a constant integer of the given value.