rust-clippy
rust-clippy copied to clipboard
rust-lang
`len_without_is_empty` triggers when both `len` and `is_empty` have non-standard signatures
Summary
I have a proxy type (a symbol from a string interner) which has len and is_empty method which:
- are fallible
- take the interner as a method parameter
len_without_is_empty triggers with these methods, which is unexpected since neither method matches the method signature the lint looks for.
Lint Name
len_without_is_empty
Reproducer
I tried this code:
#[warn(clippy::len_without_is_empty)]
pub trait Intern {
type Symbol: Copy;
fn lookup_symbol(&self, sym: Self::Symbol) -> Result<Option<&[u8]>, String>;
}
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct Symbol(u32);
impl Symbol {
#[inline]
#[must_use]
pub fn is_empty<T, U>(self, interner: &T) -> bool
where
T: Intern<Symbol = U>,
U: Copy + From<Symbol>,
{
if let Ok(Some(bytes)) = interner.lookup_symbol(self.into()) {
bytes.is_empty()
} else {
true
}
}
#[inline]
#[must_use]
// #[allow(clippy::len_without_is_empty)]
pub fn len<T, U>(self, interner: &T) -> usize
where
T: Intern<Symbol = U>,
U: Copy + From<Symbol>,
{
if let Ok(Some(bytes)) = interner.lookup_symbol(self.into()) {
bytes.len()
} else {
0_usize
}
}
}
I saw this happen:
Checking playground v0.0.1 (/playground)
warning: struct `Symbol` has a public `len` method, but the `is_empty` method has an unexpected signature
--> src/lib.rs:31:5
|
31 | / pub fn len<T, U>(self, interner: &T) -> usize
32 | | where
33 | | T: Intern<Symbol = U>,
34 | | U: Copy + From<Symbol>,
| |_______________________________^
|
= note: `#[warn(clippy::len_without_is_empty)]` on by default
note: `is_empty` defined here
--> src/lib.rs:16:5
|
16 | / pub fn is_empty<T, U>(self, interner: &T) -> bool
17 | | where
18 | | T: Intern<Symbol = U>,
19 | | U: Copy + From<Symbol>,
| |_______________________________^
= note: expected signature: `(self) -> bool`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
warning: `playground` (lib) generated 1 warning
Finished dev [unoptimized + debuginfo] target(s) in 0.48s
I expected to see this happen: No error because neither len nor is_empty has the expected signature.
Version
rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: x86_64-apple-darwin
release: 1.64.0
LLVM version: 14.0.6
Additional Labels
No response
@lopopolo @xFrednet So something interesting is happening here. While checking the len method clippy only looks at the return value type so if its an signed or unsigned int it triggers the len_without_is_empty lint. It does not realise that the method has an extra argument of a different type. But then if you implement the is_empty method as you did above it checks the complete method signature and realises that it is incorrect and should be (self) -> bool. So I think clippy is not checking the complete method signature for len somehow. This is my initial observation. Will update after some more investigation
So I think my above hunch was right. We don't check the len function inputs. My solution is to add a check where we see if the len function has any other arguments besides the self. If it does we don't trigger the lint. This is off the top of my head. Please suggest alternatives or improvements.
Hey @akshay1992kalbhor, thank you for investigating this issue further. Your suggestion to fix the bug sounds like the correct one, so go for it! :upside_down_face:
Yes, one PR should cover all related changes. In clippy this can often look like a lot of changes due to the test files, but that's totally fine :).