Skip to content

Commit a27bd22

Browse files
committed
clean-up
This mostly qualifies variable names with either `is_empty_` or `len_`, to make it easier to understand which of the two methods they relate to
1 parent 6885741 commit a27bd22

File tree

1 file changed

+79
-84
lines changed

1 file changed

+79
-84
lines changed

clippy_lints/src/len_zero.rs

Lines changed: 79 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
156156
&& let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id()
157157
&& let Some(local_id) = ty_id.as_local()
158158
&& let ty_hir_id = cx.tcx.local_def_id_to_hir_id(local_id)
159-
&& let Some(output) =
160-
parse_len_output(cx, cx.tcx.fn_sig(item.owner_id).instantiate_identity().skip_binder())
159+
&& let Some(output) = LenOutput::new(cx, cx.tcx.fn_sig(item.owner_id).instantiate_identity().skip_binder())
161160
{
162161
let (name, kind) = match cx.tcx.hir_node(ty_hir_id) {
163162
Node::ForeignItem(x) => (x.ident.name, "extern type"),
@@ -401,13 +400,6 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, ident: Iden
401400
}
402401
}
403402

404-
#[derive(Debug, Clone, Copy)]
405-
enum LenOutput {
406-
Integral,
407-
Option(DefId),
408-
Result(DefId),
409-
}
410-
411403
fn extract_future_output<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
412404
if let ty::Alias(_, alias_ty) = ty.kind()
413405
&& let Some(Node::OpaqueTy(opaque)) = cx.tcx.hir_get_if_local(alias_ty.def_id)
@@ -439,42 +431,49 @@ fn is_first_generic_integral<'tcx>(segment: &'tcx PathSegment<'tcx>) -> bool {
439431
}
440432
}
441433

442-
fn parse_len_output<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option<LenOutput> {
443-
if let Some(segment) = extract_future_output(cx, sig.output()) {
444-
let res = segment.res;
434+
#[derive(Debug, Clone, Copy)]
435+
enum LenOutput {
436+
Integral,
437+
Option(DefId),
438+
Result(DefId),
439+
}
445440

446-
if matches!(res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) {
447-
return Some(LenOutput::Integral);
448-
}
441+
impl LenOutput {
442+
fn new<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option<Self> {
443+
if let Some(segment) = extract_future_output(cx, sig.output()) {
444+
let res = segment.res;
449445

450-
if let Res::Def(_, def_id) = res
451-
&& let Some(res) = match cx.tcx.get_diagnostic_name(def_id) {
452-
Some(sym::Option) => Some(LenOutput::Option(def_id)),
453-
Some(sym::Result) => Some(LenOutput::Result(def_id)),
454-
_ => None,
446+
if matches!(res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) {
447+
return Some(Self::Integral);
455448
}
456-
&& is_first_generic_integral(segment)
457-
{
458-
return Some(res);
459-
}
460449

461-
return None;
462-
}
450+
if let Res::Def(_, def_id) = res
451+
&& let Some(res) = match cx.tcx.get_diagnostic_name(def_id) {
452+
Some(sym::Option) => Some(Self::Option(def_id)),
453+
Some(sym::Result) => Some(Self::Result(def_id)),
454+
_ => None,
455+
}
456+
&& is_first_generic_integral(segment)
457+
{
458+
return Some(res);
459+
}
460+
461+
return None;
462+
}
463463

464-
match *sig.output().kind() {
465-
ty::Int(_) | ty::Uint(_) => Some(LenOutput::Integral),
466-
ty::Adt(adt, subs) => match cx.tcx.get_diagnostic_name(adt.did()) {
467-
Some(sym::Option) => subs.type_at(0).is_integral().then(|| LenOutput::Option(adt.did())),
468-
Some(sym::Result) => subs.type_at(0).is_integral().then(|| LenOutput::Result(adt.did())),
464+
match *sig.output().kind() {
465+
ty::Int(_) | ty::Uint(_) => Some(Self::Integral),
466+
ty::Adt(adt, subs) => match cx.tcx.get_diagnostic_name(adt.did()) {
467+
Some(sym::Option) => subs.type_at(0).is_integral().then(|| Self::Option(adt.did())),
468+
Some(sym::Result) => subs.type_at(0).is_integral().then(|| Self::Result(adt.did())),
469+
_ => None,
470+
},
469471
_ => None,
470-
},
471-
_ => None,
472+
}
472473
}
473-
}
474474

475-
impl LenOutput {
476-
fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
477-
if let Some(segment) = extract_future_output(cx, ty) {
475+
fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, is_empty_output: Ty<'tcx>) -> bool {
476+
if let Some(segment) = extract_future_output(cx, is_empty_output) {
478477
return match (self, segment.res) {
479478
(_, Res::PrimTy(PrimTy::Bool)) => true,
480479
(Self::Option(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Option, def_id) => true,
@@ -483,58 +482,61 @@ impl LenOutput {
483482
};
484483
}
485484

486-
match (self, ty.kind()) {
485+
match (self, is_empty_output.kind()) {
487486
(_, &ty::Bool) => true,
488487
(Self::Option(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(),
489488
(Self::Result(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(),
490489
_ => false,
491490
}
492491
}
492+
}
493493

494-
fn expected_sig(self, self_kind: ImplicitSelfKind) -> String {
495-
let self_ref = match self_kind {
496-
ImplicitSelfKind::RefImm => "&",
497-
ImplicitSelfKind::RefMut => "&mut ",
498-
_ => "",
499-
};
500-
match self {
501-
Self::Integral => format!("expected signature: `({self_ref}self) -> bool`"),
502-
Self::Option(_) => {
503-
format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Option<bool>")
504-
},
505-
Self::Result(..) => {
506-
format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Result<bool>")
507-
},
508-
}
494+
/// The expected signature of `is_empty`, based on that of `len`
495+
fn expected_is_empty_sig(len_output: LenOutput, len_self_kind: ImplicitSelfKind) -> String {
496+
let self_ref = match len_self_kind {
497+
ImplicitSelfKind::RefImm => "&",
498+
ImplicitSelfKind::RefMut => "&mut ",
499+
_ => "",
500+
};
501+
match len_output {
502+
LenOutput::Integral => format!("expected signature: `({self_ref}self) -> bool`"),
503+
LenOutput::Option(_) => {
504+
format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Option<bool>")
505+
},
506+
LenOutput::Result(..) => {
507+
format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Result<bool>")
508+
},
509509
}
510510
}
511511

512512
/// Checks if the given signature matches the expectations for `is_empty`
513513
fn check_is_empty_sig<'tcx>(
514514
cx: &LateContext<'tcx>,
515-
sig: FnSig<'tcx>,
516-
self_kind: ImplicitSelfKind,
515+
is_empty_sig: FnSig<'tcx>,
516+
len_self_kind: ImplicitSelfKind,
517517
len_output: LenOutput,
518518
) -> bool {
519-
match &**sig.inputs_and_output {
520-
[arg, res] if len_output.matches_is_empty_output(cx, *res) => {
521-
matches!(
522-
(arg.kind(), self_kind),
523-
(ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm)
524-
| (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::RefMut)
525-
) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut))
526-
},
527-
_ => false,
519+
if let [is_empty_self_arg, is_empty_output] = &**is_empty_sig.inputs_and_output
520+
&& len_output.matches_is_empty_output(cx, *is_empty_output)
521+
{
522+
match (is_empty_self_arg.kind(), len_self_kind) {
523+
(ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm)
524+
| (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::RefMut) => true,
525+
(_, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut) if !is_empty_self_arg.is_ref() => true,
526+
_ => false,
527+
}
528+
} else {
529+
false
528530
}
529531
}
530532

531533
/// Checks if the given type has an `is_empty` method with the appropriate signature.
532534
#[expect(clippy::too_many_arguments)]
533535
fn check_for_is_empty(
534536
cx: &LateContext<'_>,
535-
span: Span,
536-
self_kind: ImplicitSelfKind,
537-
output: LenOutput,
537+
len_span: Span,
538+
len_self_kind: ImplicitSelfKind,
539+
len_output: LenOutput,
538540
impl_ty: DefId,
539541
item_name: Symbol,
540542
item_kind: &str,
@@ -556,20 +558,14 @@ fn check_for_is_empty(
556558
.flat_map(|&id| cx.tcx.associated_items(id).filter_by_name_unhygienic(sym::is_empty))
557559
.find(|item| item.is_fn());
558560

559-
let (msg, is_empty_span, self_kind) = match is_empty {
561+
let (msg, is_empty_span, is_empty_expected_sig) = match is_empty {
560562
None => (
561-
format!(
562-
"{item_kind} `{}` has a public `len` method, but no `is_empty` method",
563-
item_name.as_str(),
564-
),
563+
format!("{item_kind} `{item_name}` has a public `len` method, but no `is_empty` method"),
565564
None,
566565
None,
567566
),
568567
Some(is_empty) if !cx.effective_visibilities.is_exported(is_empty.def_id.expect_local()) => (
569-
format!(
570-
"{item_kind} `{}` has a public `len` method, but a private `is_empty` method",
571-
item_name.as_str(),
572-
),
568+
format!("{item_kind} `{item_name}` has a public `len` method, but a private `is_empty` method"),
573569
Some(cx.tcx.def_span(is_empty.def_id)),
574570
None,
575571
),
@@ -578,29 +574,28 @@ fn check_for_is_empty(
578574
&& check_is_empty_sig(
579575
cx,
580576
cx.tcx.fn_sig(is_empty.def_id).instantiate_identity().skip_binder(),
581-
self_kind,
582-
output,
577+
len_self_kind,
578+
len_output,
583579
)) =>
584580
{
585581
(
586582
format!(
587-
"{item_kind} `{}` has a public `len` method, but the `is_empty` method has an unexpected signature",
588-
item_name.as_str(),
583+
"{item_kind} `{item_name}` has a public `len` method, but the `is_empty` method has an unexpected signature",
589584
),
590585
Some(cx.tcx.def_span(is_empty.def_id)),
591-
Some(self_kind),
586+
Some(expected_is_empty_sig(len_output, len_self_kind)),
592587
)
593588
},
594589
Some(_) => return,
595590
};
596591

597592
if !fulfill_or_allowed(cx, LEN_WITHOUT_IS_EMPTY, [len_method_hir_id, ty_decl_hir_id]) {
598-
span_lint_and_then(cx, LEN_WITHOUT_IS_EMPTY, span, msg, |db| {
593+
span_lint_and_then(cx, LEN_WITHOUT_IS_EMPTY, len_span, msg, |db| {
599594
if let Some(span) = is_empty_span {
600595
db.span_note(span, "`is_empty` defined here");
601596
}
602-
if let Some(self_kind) = self_kind {
603-
db.note(output.expected_sig(self_kind));
597+
if let Some(expected_sig) = is_empty_expected_sig {
598+
db.note(expected_sig);
604599
}
605600
});
606601
}

0 commit comments

Comments
 (0)