Rust-clippy: or_fn_call lint suggests broken code

[DevinR528]

While working on a lint I got this warning in the dog food tests

error: use of `or_insert` followed by a function call
   --> clippy_lints/src/macro_use.rs:185:34
    |
185 | ...                   .or_insert(vec![])
    |                        ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(vec![])`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call

The suggestions also fails to compile vec![] is not a function error. So fixing that with or_insert_with(|| vec![]) gives

error: redundant closure found
   --> clippy_lints/src/macro_use.rs:194:49
    |
194 | ...                   .or_insert_with(|| vec![])
    |                                       ^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

[ebroto]

vec![] expands to ::alloc::vec::Vec::new() which is const fn, so I think the first case is a duplicate of #5658.

For the second case, it seems redundant_closure may have the same problem as or_fun_call, and should also ignore const fns.

[tnielens]

I think the redundant-closure lint is right here. It recommends to change || vec!() with Vec::new. A suggestion might help though.

As for the or_fun_call lint, a change was merged in #5658 that ignores nullary const fn. It works fine for map_or for example. Somehow or_insert is handled differently.

See following snippet as an illustration of both remarks (playground):

fn main() {
    let mut map = std::collections::HashMap::new();
    map.insert(1, vec!(1));
    map.entry(1).or_insert(vec!()); // or_fun_call

    let _v:Vec<i32> = Some(vec!(5)).map_or(vec!(), |i| i); // no lint
    let _v:Vec<i32> = Some(vec!(5)).map_or_else(|| vec!(), |i| i); // redundant-closure
    let _v:Vec<i32> = Some(vec!(5)).map_or_else(Vec::new, |i| i); // no lint
}

[ebroto]

Good catch! I've opened a PR to fix the cases that were not being ignored. TBH I'm not sure why the map_or case was working with vec![] since they expand to the same code :thinking:

As you said, the redundant_closure lint seems legit, so I think that we can close this issue after the PR is merged.