Skip to content

Commit 8ff0cf8

Browse files
Fix len_zero FP on unstable methods (#15894)
Closes #15890 changelog: [`len_zero`] fix FP on unstable methods
2 parents 09c3237 + 9eba2cc commit 8ff0cf8

File tree

9 files changed

+169
-96
lines changed

9 files changed

+169
-96
lines changed

book/src/lint_configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
859859
* [`io_other_error`](https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error)
860860
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
861861
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
862+
* [`len_zero`](https://rust-lang.github.io/rust-clippy/master/index.html#len_zero)
862863
* [`lines_filter_map_ok`](https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok)
863864
* [`manual_abs_diff`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff)
864865
* [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)

clippy_config/src/conf.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,7 @@ define_Conf! {
748748
io_other_error,
749749
iter_kv_map,
750750
legacy_numeric_constants,
751+
len_zero,
751752
lines_filter_map_ok,
752753
manual_abs_diff,
753754
manual_bits,

clippy_lints/src/len_zero.rs

Lines changed: 133 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
use clippy_config::Conf;
12
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
3+
use clippy_utils::msrvs::Msrv;
24
use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
35
use clippy_utils::source::{SpanRangeExt, snippet_with_context};
46
use clippy_utils::sugg::{Sugg, has_enclosing_paren};
@@ -10,12 +12,12 @@ use rustc_hir::def::Res;
1012
use rustc_hir::def_id::{DefId, DefIdSet};
1113
use rustc_hir::{
1214
BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, HirId, ImplItem, ImplItemKind, ImplicitSelfKind,
13-
Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatExprKind, PatKind, PathSegment, PrimTy, QPath, TraitItemId,
14-
TyKind,
15+
Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatExprKind, PatKind, PathSegment, PrimTy, QPath, RustcVersion,
16+
StabilityLevel, StableSince, TraitItemId, TyKind,
1517
};
1618
use rustc_lint::{LateContext, LateLintPass};
1719
use rustc_middle::ty::{self, FnSig, Ty};
18-
use rustc_session::declare_lint_pass;
20+
use rustc_session::impl_lint_pass;
1921
use rustc_span::source_map::Spanned;
2022
use rustc_span::symbol::kw;
2123
use rustc_span::{Ident, Span, Symbol};
@@ -120,7 +122,17 @@ declare_clippy_lint! {
120122
"checking `x == \"\"` or `x == []` (or similar) when `.is_empty()` could be used instead"
121123
}
122124

123-
declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]);
125+
pub struct LenZero {
126+
msrv: Msrv,
127+
}
128+
129+
impl_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]);
130+
131+
impl LenZero {
132+
pub fn new(conf: &'static Conf) -> Self {
133+
Self { msrv: conf.msrv }
134+
}
135+
}
124136

125137
impl<'tcx> LateLintPass<'tcx> for LenZero {
126138
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
@@ -184,7 +196,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
184196
_ => false,
185197
}
186198
&& !expr.span.from_expansion()
187-
&& has_is_empty(cx, lt.init)
199+
&& has_is_empty(cx, lt.init, self.msrv)
188200
{
189201
let mut applicability = Applicability::MachineApplicable;
190202

@@ -206,7 +218,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
206218
&& cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::PartialEq)
207219
&& !expr.span.from_expansion()
208220
{
209-
check_empty_expr(
221+
self.check_empty_expr(
210222
cx,
211223
expr.span,
212224
lhs_expr,
@@ -226,29 +238,110 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
226238
let actual_span = span_without_enclosing_paren(cx, expr.span);
227239
match cmp {
228240
BinOpKind::Eq => {
229-
check_cmp(cx, actual_span, left, right, "", 0); // len == 0
230-
check_cmp(cx, actual_span, right, left, "", 0); // 0 == len
241+
self.check_cmp(cx, actual_span, left, right, "", 0); // len == 0
242+
self.check_cmp(cx, actual_span, right, left, "", 0); // 0 == len
231243
},
232244
BinOpKind::Ne => {
233-
check_cmp(cx, actual_span, left, right, "!", 0); // len != 0
234-
check_cmp(cx, actual_span, right, left, "!", 0); // 0 != len
245+
self.check_cmp(cx, actual_span, left, right, "!", 0); // len != 0
246+
self.check_cmp(cx, actual_span, right, left, "!", 0); // 0 != len
235247
},
236248
BinOpKind::Gt => {
237-
check_cmp(cx, actual_span, left, right, "!", 0); // len > 0
238-
check_cmp(cx, actual_span, right, left, "", 1); // 1 > len
249+
self.check_cmp(cx, actual_span, left, right, "!", 0); // len > 0
250+
self.check_cmp(cx, actual_span, right, left, "", 1); // 1 > len
239251
},
240252
BinOpKind::Lt => {
241-
check_cmp(cx, actual_span, left, right, "", 1); // len < 1
242-
check_cmp(cx, actual_span, right, left, "!", 0); // 0 < len
253+
self.check_cmp(cx, actual_span, left, right, "", 1); // len < 1
254+
self.check_cmp(cx, actual_span, right, left, "!", 0); // 0 < len
243255
},
244-
BinOpKind::Ge => check_cmp(cx, actual_span, left, right, "!", 1), // len >= 1
245-
BinOpKind::Le => check_cmp(cx, actual_span, right, left, "!", 1), // 1 <= len
256+
BinOpKind::Ge => self.check_cmp(cx, actual_span, left, right, "!", 1), // len >= 1
257+
BinOpKind::Le => self.check_cmp(cx, actual_span, right, left, "!", 1), // 1 <= len
246258
_ => (),
247259
}
248260
}
249261
}
250262
}
251263

264+
impl LenZero {
265+
fn check_cmp(
266+
&self,
267+
cx: &LateContext<'_>,
268+
span: Span,
269+
method: &Expr<'_>,
270+
lit: &Expr<'_>,
271+
op: &str,
272+
compare_to: u32,
273+
) {
274+
if method.span.from_expansion() {
275+
return;
276+
}
277+
278+
if let (&ExprKind::MethodCall(method_path, receiver, [], _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
279+
// check if we are in an is_empty() method
280+
if parent_item_name(cx, method) == Some(sym::is_empty) {
281+
return;
282+
}
283+
284+
self.check_len(cx, span, method_path.ident.name, receiver, &lit.node, op, compare_to);
285+
} else {
286+
self.check_empty_expr(cx, span, method, lit, op);
287+
}
288+
}
289+
290+
#[expect(clippy::too_many_arguments)]
291+
fn check_len(
292+
&self,
293+
cx: &LateContext<'_>,
294+
span: Span,
295+
method_name: Symbol,
296+
receiver: &Expr<'_>,
297+
lit: &LitKind,
298+
op: &str,
299+
compare_to: u32,
300+
) {
301+
if let LitKind::Int(lit, _) = *lit {
302+
// check if length is compared to the specified number
303+
if lit != u128::from(compare_to) {
304+
return;
305+
}
306+
307+
if method_name == sym::len && has_is_empty(cx, receiver, self.msrv) {
308+
let mut applicability = Applicability::MachineApplicable;
309+
span_lint_and_sugg(
310+
cx,
311+
LEN_ZERO,
312+
span,
313+
format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
314+
format!("using `{op}is_empty` is clearer and more explicit"),
315+
format!(
316+
"{op}{}.is_empty()",
317+
snippet_with_context(cx, receiver.span, span.ctxt(), "_", &mut applicability).0,
318+
),
319+
applicability,
320+
);
321+
}
322+
}
323+
}
324+
325+
fn check_empty_expr(&self, cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
326+
if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1, self.msrv) {
327+
let mut applicability = Applicability::MachineApplicable;
328+
329+
let lit1 = peel_ref_operators(cx, lit1);
330+
let lit_str = Sugg::hir_with_context(cx, lit1, span.ctxt(), "_", &mut applicability).maybe_paren();
331+
332+
span_lint_and_sugg(
333+
cx,
334+
COMPARISON_TO_EMPTY,
335+
span,
336+
"comparison to empty slice",
337+
format!("using `{op}is_empty` is clearer and more explicit"),
338+
format!("{op}{lit_str}.is_empty()"),
339+
applicability,
340+
);
341+
}
342+
}
343+
}
344+
252345
fn span_without_enclosing_paren(cx: &LateContext<'_>, span: Span) -> Span {
253346
let Some(snippet) = span.get_source_text(cx) else {
254347
return span;
@@ -513,75 +606,6 @@ fn check_for_is_empty(
513606
}
514607
}
515608

516-
fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {
517-
if method.span.from_expansion() {
518-
return;
519-
}
520-
521-
if let (&ExprKind::MethodCall(method_path, receiver, [], _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
522-
// check if we are in an is_empty() method
523-
if parent_item_name(cx, method) == Some(sym::is_empty) {
524-
return;
525-
}
526-
527-
check_len(cx, span, method_path.ident.name, receiver, &lit.node, op, compare_to);
528-
} else {
529-
check_empty_expr(cx, span, method, lit, op);
530-
}
531-
}
532-
533-
fn check_len(
534-
cx: &LateContext<'_>,
535-
span: Span,
536-
method_name: Symbol,
537-
receiver: &Expr<'_>,
538-
lit: &LitKind,
539-
op: &str,
540-
compare_to: u32,
541-
) {
542-
if let LitKind::Int(lit, _) = *lit {
543-
// check if length is compared to the specified number
544-
if lit != u128::from(compare_to) {
545-
return;
546-
}
547-
548-
if method_name == sym::len && has_is_empty(cx, receiver) {
549-
let mut applicability = Applicability::MachineApplicable;
550-
span_lint_and_sugg(
551-
cx,
552-
LEN_ZERO,
553-
span,
554-
format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
555-
format!("using `{op}is_empty` is clearer and more explicit"),
556-
format!(
557-
"{op}{}.is_empty()",
558-
snippet_with_context(cx, receiver.span, span.ctxt(), "_", &mut applicability).0,
559-
),
560-
applicability,
561-
);
562-
}
563-
}
564-
}
565-
566-
fn check_empty_expr(cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
567-
if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) {
568-
let mut applicability = Applicability::MachineApplicable;
569-
570-
let lit1 = peel_ref_operators(cx, lit1);
571-
let lit_str = Sugg::hir_with_context(cx, lit1, span.ctxt(), "_", &mut applicability).maybe_paren();
572-
573-
span_lint_and_sugg(
574-
cx,
575-
COMPARISON_TO_EMPTY,
576-
span,
577-
"comparison to empty slice",
578-
format!("using `{op}is_empty` is clearer and more explicit"),
579-
format!("{op}{lit_str}.is_empty()"),
580-
applicability,
581-
);
582-
}
583-
}
584-
585609
fn is_empty_string(expr: &Expr<'_>) -> bool {
586610
if let ExprKind::Lit(lit) = expr.kind
587611
&& let LitKind::Str(lit, _) = lit.node
@@ -600,51 +624,65 @@ fn is_empty_array(expr: &Expr<'_>) -> bool {
600624
}
601625

602626
/// Checks if this type has an `is_empty` method.
603-
fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
627+
fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: Msrv) -> bool {
604628
/// Gets an `AssocItem` and return true if it matches `is_empty(self)`.
605-
fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool {
629+
fn is_is_empty_and_stable(cx: &LateContext<'_>, item: &ty::AssocItem, msrv: Msrv) -> bool {
606630
if item.is_fn() {
607631
let sig = cx.tcx.fn_sig(item.def_id).skip_binder();
608632
let ty = sig.skip_binder();
609633
ty.inputs().len() == 1
634+
&& cx.tcx.lookup_stability(item.def_id).is_none_or(|stability| {
635+
if let StabilityLevel::Stable { since, .. } = stability.level {
636+
let version = match since {
637+
StableSince::Version(version) => version,
638+
StableSince::Current => RustcVersion::CURRENT,
639+
StableSince::Err(_) => return false,
640+
};
641+
642+
msrv.meets(cx, version)
643+
} else {
644+
// Unstable fn, check if the feature is enabled.
645+
cx.tcx.features().enabled(stability.feature) && msrv.current(cx).is_none()
646+
}
647+
})
610648
} else {
611649
false
612650
}
613651
}
614652

615653
/// Checks the inherent impl's items for an `is_empty(self)` method.
616-
fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId) -> bool {
654+
fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId, msrv: Msrv) -> bool {
617655
cx.tcx.inherent_impls(id).iter().any(|imp| {
618656
cx.tcx
619657
.associated_items(*imp)
620658
.filter_by_name_unhygienic(sym::is_empty)
621-
.any(|item| is_is_empty(cx, item))
659+
.any(|item| is_is_empty_and_stable(cx, item, msrv))
622660
})
623661
}
624662

625-
fn ty_has_is_empty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, depth: usize) -> bool {
663+
fn ty_has_is_empty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, depth: usize, msrv: Msrv) -> bool {
626664
match ty.kind() {
627665
ty::Dynamic(tt, ..) => tt.principal().is_some_and(|principal| {
628666
cx.tcx
629667
.associated_items(principal.def_id())
630668
.filter_by_name_unhygienic(sym::is_empty)
631-
.any(|item| is_is_empty(cx, item))
669+
.any(|item| is_is_empty_and_stable(cx, item, msrv))
632670
}),
633-
ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id),
671+
ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id, msrv),
634672
ty::Adt(id, _) => {
635-
has_is_empty_impl(cx, id.did())
673+
has_is_empty_impl(cx, id.did(), msrv)
636674
|| (cx.tcx.recursion_limit().value_within_limit(depth)
637675
&& cx.tcx.get_diagnostic_item(sym::Deref).is_some_and(|deref_id| {
638676
implements_trait(cx, ty, deref_id, &[])
639677
&& cx
640678
.get_associated_type(ty, deref_id, sym::Target)
641-
.is_some_and(|deref_ty| ty_has_is_empty(cx, deref_ty, depth + 1))
679+
.is_some_and(|deref_ty| ty_has_is_empty(cx, deref_ty, depth + 1, msrv))
642680
}))
643681
},
644682
ty::Array(..) | ty::Slice(..) | ty::Str => true,
645683
_ => false,
646684
}
647685
}
648686

649-
ty_has_is_empty(cx, cx.typeck_results().expr_ty(expr).peel_refs(), 0)
687+
ty_has_is_empty(cx, cx.typeck_results().expr_ty(expr).peel_refs(), 0, msrv)
650688
}

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
480480
store.register_late_pass(|_| Box::new(mut_mut::MutMut::default()));
481481
store.register_late_pass(|_| Box::new(unnecessary_mut_passed::UnnecessaryMutPassed));
482482
store.register_late_pass(|_| Box::<significant_drop_tightening::SignificantDropTightening<'_>>::default());
483-
store.register_late_pass(|_| Box::new(len_zero::LenZero));
483+
store.register_late_pass(move |_| Box::new(len_zero::LenZero::new(conf)));
484484
store.register_late_pass(move |_| Box::new(attrs::Attributes::new(conf)));
485485
store.register_late_pass(|_| Box::new(blocks_in_conditions::BlocksInConditions));
486486
store.register_late_pass(|_| Box::new(unicode::Unicode));

tests/ui/len_zero.fixed

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,7 @@ fn no_infinite_recursion() -> bool {
275275
// Do not crash while checking if S implements `.is_empty()`
276276
S == ""
277277
}
278+
279+
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
280+
vertices.len() == 0
281+
}

tests/ui/len_zero.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,7 @@ fn no_infinite_recursion() -> bool {
275275
// Do not crash while checking if S implements `.is_empty()`
276276
S == ""
277277
}
278+
279+
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
280+
vertices.len() == 0
281+
}

tests/ui/len_zero_unstable.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![warn(clippy::len_zero)]
2+
#![feature(exact_size_is_empty)]
3+
4+
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
5+
vertices.is_empty()
6+
//~^ len_zero
7+
}

tests/ui/len_zero_unstable.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![warn(clippy::len_zero)]
2+
#![feature(exact_size_is_empty)]
3+
4+
fn issue15890(vertices: &mut dyn ExactSizeIterator<Item = u8>) -> bool {
5+
vertices.len() == 0
6+
//~^ len_zero
7+
}

0 commit comments

Comments
 (0)