Proposed changes to or_fun_call/unwrap_or_else_default

[smoelius]

Description

I propose the following changes to or_fun_call and unwrap_or_else_default (which I would implement):

1. Implement the missing functionality from Suggest Entry::or_default for Entry::or_insert(Default::default()) #9342, e.g., handle or_insert_with(Default::default).
2. Merge unwrap_or_else_default into or_fun_call. At present, there is an ambiguity as to where the just mentioned new functionality should go. However, I agree with @giraffate when he wrote here that things like unwrap_or_else(Vec::new) should be handled by or_fun_call. Merging unwrap_or_else_default into or_fun_call would resolve the ambiguity.
3. Move or_fun_call to complexity. If I understand correctly, the "false positives" described in Make it clear that or_fun_call can be a false-positive #9829 are because the suggestions don't improve performance, not that the suggestions don't apply. However, even when the suggestions don't improve performance, they still improve "code that does something simple but in a complex way."

Note that 2 and 3 are intertwined. If I have misunderstood the reasoning in #9829, it might make sense to keep unwrap_or_else_default as a separate style lint. (cc: @hrxi @llogiq)

Thoughts?

Version

No response

Additional Labels

No response

[hrxi]

3. because the suggestions don't improve performance, not that the suggestions don't apply.

The suggestions apply in the sense that they produce equivalent code.

However, changing from .unwrap_or(Vec::new()) to .unwrap_or_default() doesn't seem to increase code readability. It even loses information.

[smoelius]

@hrxi Would you say your point also applies to warnings like this?

rust-clippy/tests/ui/unwrap_or_else_default.stderr

Lines 27 to 31 in be15e60

	error: use of `.unwrap_or_else(..)` to construct default value
	--> $DIR/unwrap_or_else_default.rs:71:5
	|
	LL | with_default_type.unwrap_or_else(Vec::new);
	| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()`

If so, that would seem like an additional reason to merge unwrap_or_else_default into or_fun_call (regardless of its category).

[hrxi]

Yes, I'd say my point also applies to this example.

If so, that would seem like an additional reason to merge unwrap_or_else_default into or_fun_call (regardless of its category).

Mhh, I'm not sure. unwrap_or_else(Default::default)can surely always be replaced byunwrap_or_default(). unwrap_or_else(expensive_function)` cannot. Do you want to merge them because one has to be aware of the other to not issues double warning?

[smoelius]

Do you want to merge them because one has to be aware of the other to not issues double warning?

Essentially, yes.

1. Right now, .unwrap_or(Vec::new()) and .unwrap_or_else(Vec::new) are handled by two different lints. I would argue they should be handled by the same lint. (This is essentially the ambiguity I was referring to above.)

2. If both cases are handled by unwrap_or_else_default, then or_fun_call needs a special case to ignore .unwrap_or(Vec::new()). I would argue this additional complexity isn't worth it.

1 is more significant than 2, though, IMHO.