rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.44k stars 1.54k forks source link

Suggest replacing `split(...).collect().rev()` with `rsplit()` #11632

Open woodruffw opened 1 year ago

woodruffw commented 1 year ago

What it does

A user working with a string might forget that str::rsplit exists, and design an API like this:

     /// Returns this DNS name's labels, in reversed order
     /// (from top-level domain to most-specific subdomain).
     fn rlabels(&self) -> impl Iterator<Item = &'_ str> {
        self.as_str()
            .split('.')
            .collect::<Vec<_>>()
            .into_iter()
            .rev()
     }

which could be rewritten as:

    /// Returns this DNS name's labels, in reversed order
    /// (from top-level domain to most-specific subdomain).
    fn rlabels(&self) -> impl Iterator<Item = &'_ str> {
        self.as_str().rsplit('.')
    }

Clippy does not currently detect this.

Advantage

Drawbacks

None that I can think of.

Example

mystr.split('.').collect::<Vec<_>>.into_iter().rev()

Could be written as:

mystr.rsplit('.')
Jarcho commented 1 year ago

Note for anyone implementing this. rsplit doesn't have the same result as reversing the order of split, only some cases do. e.g. "x:::y".split("::") gives ["x", ":y"], but rsplit("::") gives ["y", "x:"].

woodruffw commented 1 year ago

Thanks for pointing that out, I didn't realize there were differences between the two. It sounds like a high-quality lint here will have to be opt in because of unreliability, then.

Edit: I misread the example as x::y, not x:::y. The difference for the latter makes sense.

woodruffw commented 1 year ago

@alex pointed out that there is actually a sound solution here: any needle of size 1 or that isn't a palindrome should be equivalent between the two for all inputs. Anything else may be equivalent for some inputs but not others.

Jarcho commented 1 year ago

A palindrome isn't the right condition, though they will always cause a problem. If a proper prefix of the pattern is also a suffix then the two will behave differently. e.g. splitting ab.ab on 1ab.ab.ab1 will work differently with split and rsplit.