Lint for `fold` closure that never moves the accumulator

[scottmcm]

What it does

Lint when a fold closure always returns the accumulator from the input, having only used it by reference. (As opposed to consuming the accumulator, or returning something else.)

Categories (optional)

• Kind: perf

The compiler currently cannot always optimize away passing along the accumulator every time (see rust-lang/rust#76725), so it's better to not do that if it's actually the same thing every time anyway.

Drawbacks

None.

(Well, it leaves something mut, but that's easily fixable with let v = v;.)

Example

    let word = word.to_lowercase();
    let char_count: HashMap<char, usize> = word.chars().fold(HashMap::new(), |mut chars, c| {
        *chars.entry(c).or_default() += 1;
        chars
    });

Could be written as:

    let word = word.to_lowercase();
    let mut char_count: HashMap<char, usize> = HashMap::new();
    word.chars().for_each(|c| *chars.entry(c).or_default() += 1);

[workingjubilee]

The somewhat odd

let x = {  let mut y = Vec::new();
    /* Do the mutation. */
    y
};

is also an option over binding and rebinding.

[dahlbaek]

I ran into this when working on an Advent of Code solution and was surprised that I could increase throughput for one of my solutions by 40% by just using a for_each instead of a fold 🤯 . However, looking at this with a data pipeline mindset (think apache spark), where I think Rust is gaining traction, isn't it worth considering the fact that a chain of iterator methods that end in a fold(...) can be continued directly with an iter(), whereas a chain that ends in a for_each cannot be continued? In data pipeline style code, that can be pretty bad for readability in my opinion.

For example, consider

collection
    .map(...)
    .fold(HashMap::new(), ...) // group_by/reduce_by_key operation
    .flat_map(...)
    .fold(HashMap::new(), ...) // group_by/reduce_by_key operation
    .map(...)
    .sum()

Switching this over to the for_each style will trash the flow of the code and thereby the readability (in my opinion).

Maybe it would be worth it to consider a name such as collect_with for what was called fold_mut in rust-lang/rust#76725, thereby somewhat giving it the role of a "custom builder" function rather than a niche fold variant? If such a collect_with was included in the std lib, I suppose one could put a direct suggestion in this lint to use collect_with instead of fold.

I note that the name collect_with was considered (and discarded) in rust-lang/rust#92982, but the collect_with that was considered there did not serve the purpose of an ad-hoc collect function (it didn't take a closure to specify how to build the collection).

Such a collect with could be implemented simply with

    fn collect_with<B, F>(self, mut init: B, mut f: F) -> B
    where
        Self: Sized,
        F: FnMut(&mut B, Self::Item),
    {
        self.for_each(|item| f(&mut init, item));
        init
    }

[jdupak]

@rustbot claim