diff options
| -rw-r--r-- | clippy_lints/src/len_zero.rs | 78 | ||||
| -rw-r--r-- | tests/ui/len_without_is_empty.rs | 99 | ||||
| -rw-r--r-- | tests/ui/len_without_is_empty.stderr | 79 |
3 files changed, 231 insertions, 25 deletions
diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 717f2ea84f4..fb522be2f1a 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -10,9 +10,12 @@ use rustc_hir::{ ItemKind, Mutability, Node, TraitItemRef, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, AssocKind, FnSig}; +use rustc_middle::ty::{self, AssocKind, FnSig, Ty, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::{Span, Spanned, Symbol}; +use rustc_span::{ + source_map::{Span, Spanned, Symbol}, + symbol::sym, +}; declare_clippy_lint! { /// **What it does:** Checks for getting the length of something via `.len()` @@ -137,6 +140,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { if let Some(local_id) = ty_id.as_local(); let ty_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id); if !is_allowed(cx, LEN_WITHOUT_IS_EMPTY, ty_hir_id); + if let Some(output) = parse_len_output(cx, cx.tcx.fn_sig(item.def_id).skip_binder()); then { let (name, kind) = match cx.tcx.hir().find(ty_hir_id) { Some(Node::ForeignItem(x)) => (x.ident.name, "extern type"), @@ -148,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { } _ => return, }; - check_for_is_empty(cx, sig.span, sig.decl.implicit_self, ty_id, name, kind) + check_for_is_empty(cx, sig.span, sig.decl.implicit_self, output, ty_id, name, kind) } } } @@ -231,10 +235,62 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items } } +#[derive(Debug, Clone, Copy)] +enum LenOutput<'tcx> { + Integral, + Option(DefId), + Result(DefId, Ty<'tcx>), +} +fn parse_len_output(cx: &LateContext<'_>, sig: FnSig<'tcx>) -> Option<LenOutput<'tcx>> { + match *sig.output().kind() { + ty::Int(_) | ty::Uint(_) => Some(LenOutput::Integral), + ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::option_type, adt.did) => { + subs.type_at(0).is_integral().then(|| LenOutput::Option(adt.did)) + }, + ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::result_type, adt.did) => subs + .type_at(0) + .is_integral() + .then(|| LenOutput::Result(adt.did, subs.type_at(1))), + _ => None, + } +} + +impl LenOutput<'_> { + fn matches_is_empty_output(self, ty: Ty<'_>) -> bool { + match (self, ty.kind()) { + (_, &ty::Bool) => true, + (Self::Option(id), &ty::Adt(adt, subs)) if id == adt.did => subs.type_at(0).is_bool(), + (Self::Result(id, err_ty), &ty::Adt(adt, subs)) if id == adt.did => { + subs.type_at(0).is_bool() && TyS::same_type(subs.type_at(1), err_ty) + }, + _ => false, + } + } + + fn expected_sig(self, self_kind: ImplicitSelfKind) -> String { + let self_ref = match self_kind { + ImplicitSelfKind::ImmRef => "&", + ImplicitSelfKind::MutRef => "&mut ", + _ => "", + }; + match self { + Self::Integral => format!("expected signature: `({}self) -> bool`", self_ref), + Self::Option(_) => format!( + "expected signature: `({}self) -> bool` or `({}self) -> Option<bool>", + self_ref, self_ref + ), + Self::Result(..) => format!( + "expected signature: `({}self) -> bool` or `({}self) -> Result<bool>", + self_ref, self_ref + ), + } + } +} + /// Checks if the given signature matches the expectations for `is_empty` -fn check_is_empty_sig(cx: &LateContext<'_>, sig: FnSig<'_>, self_kind: ImplicitSelfKind) -> bool { +fn check_is_empty_sig(sig: FnSig<'_>, self_kind: ImplicitSelfKind, len_output: LenOutput<'_>) -> bool { match &**sig.inputs_and_output { - [arg, res] if *res == cx.tcx.types.bool => { + [arg, res] if len_output.matches_is_empty_output(res) => { matches!( (arg.kind(), self_kind), (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::ImmRef) @@ -250,6 +306,7 @@ fn check_for_is_empty( cx: &LateContext<'_>, span: Span, self_kind: ImplicitSelfKind, + output: LenOutput<'_>, impl_ty: DefId, item_name: Symbol, item_kind: &str, @@ -289,7 +346,7 @@ fn check_for_is_empty( }, Some(is_empty) if !(is_empty.fn_has_self_parameter - && check_is_empty_sig(cx, cx.tcx.fn_sig(is_empty.def_id).skip_binder(), self_kind)) => + && check_is_empty_sig(cx.tcx.fn_sig(is_empty.def_id).skip_binder(), self_kind, output)) => { ( format!( @@ -309,14 +366,7 @@ fn check_for_is_empty( db.span_note(span, "`is_empty` defined here"); } if let Some(self_kind) = self_kind { - db.note(&format!( - "expected signature: `({}self) -> bool`", - match self_kind { - ImplicitSelfKind::ImmRef => "&", - ImplicitSelfKind::MutRef => "&mut ", - _ => "", - } - )); + db.note(&output.expected_sig(self_kind)); } }); } diff --git a/tests/ui/len_without_is_empty.rs b/tests/ui/len_without_is_empty.rs index 6b3636a482e..b9d66347c27 100644 --- a/tests/ui/len_without_is_empty.rs +++ b/tests/ui/len_without_is_empty.rs @@ -1,3 +1,5 @@ +// edition:2018 + #![warn(clippy::len_without_is_empty)] #![allow(dead_code, unused)] @@ -172,9 +174,9 @@ pub trait DependsOnFoo: Foo { fn len(&mut self) -> usize; } +// issue #1562 pub struct MultipleImpls; -// issue #1562 impl MultipleImpls { pub fn len(&self) -> usize { 1 @@ -187,4 +189,99 @@ impl MultipleImpls { } } +// issue #6958 +pub struct OptionalLen; + +impl OptionalLen { + pub fn len(&self) -> Option<usize> { + Some(0) + } + + pub fn is_empty(&self) -> Option<bool> { + Some(true) + } +} + +pub struct OptionalLen2; +impl OptionalLen2 { + pub fn len(&self) -> Option<usize> { + Some(0) + } + + pub fn is_empty(&self) -> bool { + true + } +} + +pub struct OptionalLen3; +impl OptionalLen3 { + pub fn len(&self) -> usize { + 0 + } + + // should lint, len is not an option + pub fn is_empty(&self) -> Option<bool> { + None + } +} + +pub struct ResultLen; +impl ResultLen { + pub fn len(&self) -> Result<usize, ()> { + Ok(0) + } + + // Differing result types + pub fn is_empty(&self) -> Option<bool> { + Some(true) + } +} + +pub struct ResultLen2; +impl ResultLen2 { + pub fn len(&self) -> Result<usize, ()> { + Ok(0) + } + + pub fn is_empty(&self) -> Result<bool, ()> { + Ok(true) + } +} + +pub struct ResultLen3; +impl ResultLen3 { + pub fn len(&self) -> Result<usize, ()> { + Ok(0) + } + + // Non-fallible result is ok. + pub fn is_empty(&self) -> bool { + true + } +} + +pub struct OddLenSig; +impl OddLenSig { + // don't lint + pub fn len(&self) -> bool { + true + } +} + +// issue #6958 +pub struct AsyncLen; +impl AsyncLen { + async fn async_task(&self) -> bool { + true + } + + pub async fn len(&self) -> usize { + if self.async_task().await { 0 } else { 1 } + } + + pub async fn is_empty(&self) -> bool { + self.len().await == 0 + } +} + fn main() {} diff --git a/tests/ui/len_without_is_empty.stderr b/tests/ui/len_without_is_empty.stderr index f106506faf4..ff7f5562308 100644 --- a/tests/ui/len_without_is_empty.stderr +++ b/tests/ui/len_without_is_empty.stderr @@ -1,5 +1,5 @@ error: struct `PubOne` has a public `len` method, but no `is_empty` method - --> $DIR/len_without_is_empty.rs:7:5 + --> $DIR/len_without_is_empty.rs:9:5 | LL | pub fn len(&self) -> isize { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | pub fn len(&self) -> isize { = note: `-D clippy::len-without-is-empty` implied by `-D warnings` error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_empty` method - --> $DIR/len_without_is_empty.rs:55:1 + --> $DIR/len_without_is_empty.rs:57:1 | LL | / pub trait PubTraitsToo { LL | | fn len(&self) -> isize; @@ -15,50 +15,109 @@ LL | | } | |_^ error: struct `HasIsEmpty` has a public `len` method, but a private `is_empty` method - --> $DIR/len_without_is_empty.rs:68:5 + --> $DIR/len_without_is_empty.rs:70:5 | LL | pub fn len(&self) -> isize { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: `is_empty` defined here - --> $DIR/len_without_is_empty.rs:72:5 + --> $DIR/len_without_is_empty.rs:74:5 | LL | fn is_empty(&self) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: struct `HasWrongIsEmpty` has a public `len` method, but the `is_empty` method has an unexpected signature - --> $DIR/len_without_is_empty.rs:80:5 + --> $DIR/len_without_is_empty.rs:82:5 | LL | pub fn len(&self) -> isize { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: `is_empty` defined here - --> $DIR/len_without_is_empty.rs:84:5 + --> $DIR/len_without_is_empty.rs:86:5 | LL | pub fn is_empty(&self, x: u32) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: expected signature: `(&self) -> bool` error: struct `MismatchedSelf` has a public `len` method, but the `is_empty` method has an unexpected signature - --> $DIR/len_without_is_empty.rs:92:5 + --> $DIR/len_without_is_empty.rs:94:5 | LL | pub fn len(self) -> isize { | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: `is_empty` defined here - --> $DIR/len_without_is_empty.rs:96:5 + --> $DIR/len_without_is_empty.rs:98:5 | LL | pub fn is_empty(&self) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: expected signature: `(self) -> bool` error: trait `DependsOnFoo` has a `len` method but no (possibly inherited) `is_empty` method - --> $DIR/len_without_is_empty.rs:171:1 + --> $DIR/len_without_is_empty.rs:173:1 | LL | / pub trait DependsOnFoo: Foo { LL | | fn len(&mut self) -> usize; LL | | } | |_^ -error: aborting due to 6 previous errors +error: struct `OptionalLen3` has a public `len` method, but the `is_empty` method has an unexpected signature + --> $DIR/len_without_is_empty.rs:218:5 + | +LL | pub fn len(&self) -> usize { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `is_empty` defined here + --> $DIR/len_without_is_empty.rs:223:5 + | +LL | pub fn is_empty(&self) -> Option<bool> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: expected signature: `(&self) -> bool` + +error: struct `ResultLen` has a public `len` method, but the `is_empty` method has an unexpected signature + --> $DIR/len_without_is_empty.rs:230:5 + | +LL | pub fn len(&self) -> Result<usize, ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `is_empty` defined here + --> $DIR/len_without_is_empty.rs:235:5 + | +LL | pub fn is_empty(&self) -> Option<bool> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: expected signature: `(&self) -> bool` or `(&self) -> Result<bool> + +error: this returns a `Result<_, ()> + --> $DIR/len_without_is_empty.rs:230:5 + | +LL | pub fn len(&self) -> Result<usize, ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::result-unit-err` implied by `-D warnings` + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/len_without_is_empty.rs:242:5 + | +LL | pub fn len(&self) -> Result<usize, ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/len_without_is_empty.rs:246:5 + | +LL | pub fn is_empty(&self) -> Result<bool, ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/len_without_is_empty.rs:253:5 + | +LL | pub fn len(&self) -> Result<usize, ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: aborting due to 12 previous errors |