fix: len_zero FP on unstable methods
This commit is contained in:
parent
d9fb15c4b1
commit
9eba2cccd8
9 changed files with 169 additions and 96 deletions
|
|
@ -859,6 +859,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
|
|||
* [`io_other_error`](https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error)
|
||||
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
|
||||
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
|
||||
* [`len_zero`](https://rust-lang.github.io/rust-clippy/master/index.html#len_zero)
|
||||
* [`lines_filter_map_ok`](https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok)
|
||||
* [`manual_abs_diff`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff)
|
||||
* [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)
|
||||
|
|
|
|||
|
|
@ -748,6 +748,7 @@ define_Conf! {
|
|||
io_other_error,
|
||||
iter_kv_map,
|
||||
legacy_numeric_constants,
|
||||
len_zero,
|
||||
lines_filter_map_ok,
|
||||
manual_abs_diff,
|
||||
manual_bits,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
use clippy_config::Conf;
|
||||
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
|
||||
use clippy_utils::msrvs::Msrv;
|
||||
use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
|
||||
use clippy_utils::source::{SpanRangeExt, snippet_with_context};
|
||||
use clippy_utils::sugg::{Sugg, has_enclosing_paren};
|
||||
|
|
@ -10,12 +12,12 @@ use rustc_hir::def::Res;
|
|||
use rustc_hir::def_id::{DefId, DefIdSet};
|
||||
use rustc_hir::{
|
||||
BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, HirId, ImplItem, ImplItemKind, ImplicitSelfKind,
|
||||
Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatExprKind, PatKind, PathSegment, PrimTy, QPath, TraitItemId,
|
||||
TyKind,
|
||||
Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatExprKind, PatKind, PathSegment, PrimTy, QPath, RustcVersion,
|
||||
StabilityLevel, StableSince, TraitItemId, TyKind,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::{self, FnSig, Ty};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_session::impl_lint_pass;
|
||||
use rustc_span::source_map::Spanned;
|
||||
use rustc_span::symbol::kw;
|
||||
use rustc_span::{Ident, Span, Symbol};
|
||||
|
|
@ -120,7 +122,17 @@ declare_clippy_lint! {
|
|||
"checking `x == \"\"` or `x == []` (or similar) when `.is_empty()` could be used instead"
|
||||
}
|
||||
|
||||
declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]);
|
||||
pub struct LenZero {
|
||||
msrv: Msrv,
|
||||
}
|
||||
|
||||
impl_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]);
|
||||
|
||||
impl LenZero {
|
||||
pub fn new(conf: &'static Conf) -> Self {
|
||||
Self { msrv: conf.msrv }
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for LenZero {
|
||||
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
|
||||
|
|
@ -184,7 +196,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
|
|||
_ => false,
|
||||
}
|
||||
&& !expr.span.from_expansion()
|
||||
&& has_is_empty(cx, lt.init)
|
||||
&& has_is_empty(cx, lt.init, self.msrv)
|
||||
{
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
|
||||
|
|
@ -206,7 +218,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
|
|||
&& cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::PartialEq)
|
||||
&& !expr.span.from_expansion()
|
||||
{
|
||||
check_empty_expr(
|
||||
self.check_empty_expr(
|
||||
cx,
|
||||
expr.span,
|
||||
lhs_expr,
|
||||
|
|
@ -226,29 +238,110 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
|
|||
let actual_span = span_without_enclosing_paren(cx, expr.span);
|
||||
match cmp {
|
||||
BinOpKind::Eq => {
|
||||
check_cmp(cx, actual_span, left, right, "", 0); // len == 0
|
||||
check_cmp(cx, actual_span, right, left, "", 0); // 0 == len
|
||||
self.check_cmp(cx, actual_span, left, right, "", 0); // len == 0
|
||||
self.check_cmp(cx, actual_span, right, left, "", 0); // 0 == len
|
||||
},
|
||||
BinOpKind::Ne => {
|
||||
check_cmp(cx, actual_span, left, right, "!", 0); // len != 0
|
||||
check_cmp(cx, actual_span, right, left, "!", 0); // 0 != len
|
||||
self.check_cmp(cx, actual_span, left, right, "!", 0); // len != 0
|
||||
self.check_cmp(cx, actual_span, right, left, "!", 0); // 0 != len
|
||||
},
|
||||
BinOpKind::Gt => {
|
||||
check_cmp(cx, actual_span, left, right, "!", 0); // len > 0
|
||||
check_cmp(cx, actual_span, right, left, "", 1); // 1 > len
|
||||
self.check_cmp(cx, actual_span, left, right, "!", 0); // len > 0
|
||||
self.check_cmp(cx, actual_span, right, left, "", 1); // 1 > len
|
||||
},
|
||||
BinOpKind::Lt => {
|
||||
check_cmp(cx, actual_span, left, right, "", 1); // len < 1
|
||||
check_cmp(cx, actual_span, right, left, "!", 0); // 0 < len
|
||||
self.check_cmp(cx, actual_span, left, right, "", 1); // len < 1
|
||||
self.check_cmp(cx, actual_span, right, left, "!", 0); // 0 < len
|
||||
},
|
||||
BinOpKind::Ge => check_cmp(cx, actual_span, left, right, "!", 1), // len >= 1
|
||||
BinOpKind::Le => check_cmp(cx, actual_span, right, left, "!", 1), // 1 <= len
|
||||
BinOpKind::Ge => self.check_cmp(cx, actual_span, left, right, "!", 1), // len >= 1
|
||||
BinOpKind::Le => self.check_cmp(cx, actual_span, right, left, "!", 1), // 1 <= len
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl LenZero {
|
||||
fn check_cmp(
|
||||
&self,
|
||||
cx: &LateContext<'_>,
|
||||
span: Span,
|
||||
method: &Expr<'_>,
|
||||
lit: &Expr<'_>,
|
||||
op: &str,
|
||||
compare_to: u32,
|
||||
) {
|
||||
if method.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
|
||||
if let (&ExprKind::MethodCall(method_path, receiver, [], _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
|
||||
// check if we are in an is_empty() method
|
||||
if parent_item_name(cx, method) == Some(sym::is_empty) {
|
||||
return;
|
||||
}
|
||||
|
||||
self.check_len(cx, span, method_path.ident.name, receiver, &lit.node, op, compare_to);
|
||||
} else {
|
||||
self.check_empty_expr(cx, span, method, lit, op);
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(clippy::too_many_arguments)]
|
||||
fn check_len(
|
||||
&self,
|
||||
cx: &LateContext<'_>,
|
||||
span: Span,
|
||||
method_name: Symbol,
|
||||
receiver: &Expr<'_>,
|
||||
lit: &LitKind,
|
||||
op: &str,
|
||||
compare_to: u32,
|
||||
) {
|
||||
if let LitKind::Int(lit, _) = *lit {
|
||||
// check if length is compared to the specified number
|
||||
if lit != u128::from(compare_to) {
|
||||
return;
|
||||
}
|
||||
|
||||
if method_name == sym::len && has_is_empty(cx, receiver, self.msrv) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
LEN_ZERO,
|
||||
span,
|
||||
format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
|
||||
format!("using `{op}is_empty` is clearer and more explicit"),
|
||||
format!(
|
||||
"{op}{}.is_empty()",
|
||||
snippet_with_context(cx, receiver.span, span.ctxt(), "_", &mut applicability).0,
|
||||
),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_empty_expr(&self, cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
|
||||
if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1, self.msrv) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
|
||||
let lit1 = peel_ref_operators(cx, lit1);
|
||||
let lit_str = Sugg::hir_with_context(cx, lit1, span.ctxt(), "_", &mut applicability).maybe_paren();
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
COMPARISON_TO_EMPTY,
|
||||
span,
|
||||
"comparison to empty slice",
|
||||
format!("using `{op}is_empty` is clearer and more explicit"),
|
||||
format!("{op}{lit_str}.is_empty()"),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn span_without_enclosing_paren(cx: &LateContext<'_>, span: Span) -> Span {
|
||||
let Some(snippet) = span.get_source_text(cx) else {
|
||||
return span;
|
||||
|
|
@ -513,75 +606,6 @@ fn check_for_is_empty(
|
|||
}
|
||||
}
|
||||
|
||||
fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {
|
||||
if method.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
|
||||
if let (&ExprKind::MethodCall(method_path, receiver, [], _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
|
||||
// check if we are in an is_empty() method
|
||||
if parent_item_name(cx, method) == Some(sym::is_empty) {
|
||||
return;
|
||||
}
|
||||
|
||||
check_len(cx, span, method_path.ident.name, receiver, &lit.node, op, compare_to);
|
||||
} else {
|
||||
check_empty_expr(cx, span, method, lit, op);
|
||||
}
|
||||
}
|
||||
|
||||
fn check_len(
|
||||
cx: &LateContext<'_>,
|
||||
span: Span,
|
||||
method_name: Symbol,
|
||||
receiver: &Expr<'_>,
|
||||
lit: &LitKind,
|
||||
op: &str,
|
||||
compare_to: u32,
|
||||
) {
|
||||
if let LitKind::Int(lit, _) = *lit {
|
||||
// check if length is compared to the specified number
|
||||
if lit != u128::from(compare_to) {
|
||||
return;
|
||||
}
|
||||
|
||||
if method_name == sym::len && has_is_empty(cx, receiver) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
LEN_ZERO,
|
||||
span,
|
||||
format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
|
||||
format!("using `{op}is_empty` is clearer and more explicit"),
|
||||
format!(
|
||||
"{op}{}.is_empty()",
|
||||
snippet_with_context(cx, receiver.span, span.ctxt(), "_", &mut applicability).0,
|
||||
),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_empty_expr(cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
|
||||
if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
|
||||
let lit1 = peel_ref_operators(cx, lit1);
|
||||
let lit_str = Sugg::hir_with_context(cx, lit1, span.ctxt(), "_", &mut applicability).maybe_paren();
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
COMPARISON_TO_EMPTY,
|
||||
span,
|
||||
"comparison to empty slice",
|
||||
format!("using `{op}is_empty` is clearer and more explicit"),
|
||||
format!("{op}{lit_str}.is_empty()"),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn is_empty_string(expr: &Expr<'_>) -> bool {
|
||||
if let ExprKind::Lit(lit) = expr.kind
|
||||
&& let LitKind::Str(lit, _) = lit.node
|
||||
|
|
@ -600,45 +624,59 @@ fn is_empty_array(expr: &Expr<'_>) -> bool {
|
|||
}
|
||||
|
||||
/// Checks if this type has an `is_empty` method.
|
||||
fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: Msrv) -> bool {
|
||||
/// Gets an `AssocItem` and return true if it matches `is_empty(self)`.
|
||||
fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool {
|
||||
fn is_is_empty_and_stable(cx: &LateContext<'_>, item: &ty::AssocItem, msrv: Msrv) -> bool {
|
||||
if item.is_fn() {
|
||||
let sig = cx.tcx.fn_sig(item.def_id).skip_binder();
|
||||
let ty = sig.skip_binder();
|
||||
ty.inputs().len() == 1
|
||||
&& cx.tcx.lookup_stability(item.def_id).is_none_or(|stability| {
|
||||
if let StabilityLevel::Stable { since, .. } = stability.level {
|
||||
let version = match since {
|
||||
StableSince::Version(version) => version,
|
||||
StableSince::Current => RustcVersion::CURRENT,
|
||||
StableSince::Err(_) => return false,
|
||||
};
|
||||
|
||||
msrv.meets(cx, version)
|
||||
} else {
|
||||
// Unstable fn, check if the feature is enabled.
|
||||
cx.tcx.features().enabled(stability.feature) && msrv.current(cx).is_none()
|
||||
}
|
||||
})
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks the inherent impl's items for an `is_empty(self)` method.
|
||||
fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId) -> bool {
|
||||
fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId, msrv: Msrv) -> bool {
|
||||
cx.tcx.inherent_impls(id).iter().any(|imp| {
|
||||
cx.tcx
|
||||
.associated_items(*imp)
|
||||
.filter_by_name_unhygienic(sym::is_empty)
|
||||
.any(|item| is_is_empty(cx, item))
|
||||
.any(|item| is_is_empty_and_stable(cx, item, msrv))
|
||||
})
|
||||
}
|
||||
|
||||
fn ty_has_is_empty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, depth: usize) -> bool {
|
||||
fn ty_has_is_empty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, depth: usize, msrv: Msrv) -> bool {
|
||||
match ty.kind() {
|
||||
ty::Dynamic(tt, ..) => tt.principal().is_some_and(|principal| {
|
||||
cx.tcx
|
||||
.associated_items(principal.def_id())
|
||||
.filter_by_name_unhygienic(sym::is_empty)
|
||||
.any(|item| is_is_empty(cx, item))
|
||||
.any(|item| is_is_empty_and_stable(cx, item, msrv))
|
||||
}),
|
||||
ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id),
|
||||
ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id, msrv),
|
||||
ty::Adt(id, _) => {
|
||||
has_is_empty_impl(cx, id.did())
|
||||
has_is_empty_impl(cx, id.did(), msrv)
|
||||
|| (cx.tcx.recursion_limit().value_within_limit(depth)
|
||||
&& cx.tcx.get_diagnostic_item(sym::Deref).is_some_and(|deref_id| {
|
||||
implements_trait(cx, ty, deref_id, &[])
|
||||
&& cx
|
||||
.get_associated_type(ty, deref_id, sym::Target)
|
||||
.is_some_and(|deref_ty| ty_has_is_empty(cx, deref_ty, depth + 1))
|
||||
.is_some_and(|deref_ty| ty_has_is_empty(cx, deref_ty, depth + 1, msrv))
|
||||
}))
|
||||
},
|
||||
ty::Array(..) | ty::Slice(..) | ty::Str => true,
|
||||
|
|
@ -646,5 +684,5 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
ty_has_is_empty(cx, cx.typeck_results().expr_ty(expr).peel_refs(), 0)
|
||||
ty_has_is_empty(cx, cx.typeck_results().expr_ty(expr).peel_refs(), 0, msrv)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -481,7 +481,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
|
|||
store.register_late_pass(|_| Box::new(mut_mut::MutMut::default()));
|
||||
store.register_late_pass(|_| Box::new(unnecessary_mut_passed::UnnecessaryMutPassed));
|
||||
store.register_late_pass(|_| Box::<significant_drop_tightening::SignificantDropTightening<'_>>::default());
|
||||
store.register_late_pass(|_| Box::new(len_zero::LenZero));
|
||||
store.register_late_pass(move |_| Box::new(len_zero::LenZero::new(conf)));
|
||||
store.register_late_pass(move |_| Box::new(attrs::Attributes::new(conf)));
|
||||
store.register_late_pass(|_| Box::new(blocks_in_conditions::BlocksInConditions));
|
||||
store.register_late_pass(|_| Box::new(unicode::Unicode));
|
||||
|
|
|
|||
|
|
@ -275,3 +275,7 @@ fn no_infinite_recursion() -> bool {
|
|||
// Do not crash while checking if S implements `.is_empty()`
|
||||
S == ""
|
||||
}
|
||||
|
||||
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
|
||||
vertices.len() == 0
|
||||
}
|
||||
|
|
|
|||
|
|
@ -275,3 +275,7 @@ fn no_infinite_recursion() -> bool {
|
|||
// Do not crash while checking if S implements `.is_empty()`
|
||||
S == ""
|
||||
}
|
||||
|
||||
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
|
||||
vertices.len() == 0
|
||||
}
|
||||
|
|
|
|||
7
tests/ui/len_zero_unstable.fixed
Normal file
7
tests/ui/len_zero_unstable.fixed
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
#![warn(clippy::len_zero)]
|
||||
#![feature(exact_size_is_empty)]
|
||||
|
||||
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
|
||||
vertices.is_empty()
|
||||
//~^ len_zero
|
||||
}
|
||||
7
tests/ui/len_zero_unstable.rs
Normal file
7
tests/ui/len_zero_unstable.rs
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
#![warn(clippy::len_zero)]
|
||||
#![feature(exact_size_is_empty)]
|
||||
|
||||
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
|
||||
vertices.len() == 0
|
||||
//~^ len_zero
|
||||
}
|
||||
11
tests/ui/len_zero_unstable.stderr
Normal file
11
tests/ui/len_zero_unstable.stderr
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
error: length comparison to zero
|
||||
--> tests/ui/len_zero_unstable.rs:5:5
|
||||
|
|
||||
LL | vertices.len() == 0
|
||||
| ^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `vertices.is_empty()`
|
||||
|
|
||||
= note: `-D clippy::len-zero` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::len_zero)]`
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue