r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Lintcheck changes for de1028a

This comment will be updated if you push new changes

☔ The latest upstream changes (possibly e1130b6) made this pull request unmergeable. Please resolve the merge conflicts.

This is not a good way to fix unnecessary_lazy_evaluations. The eager_or_lazy check is used by a lot a lints that will have a regression.

This is not saying there's nothing to fix here. unnecessary_lazy_evaluations should be far less aggressive in general, but it needs to be something targeted at that lint in particular.

@Jarcho Could you provide an example for me to consider? At least as far as I see this change, it only updates an Eager decision to NoChange, which should only accept more styles than previous. The related lints would also rightfully be updated to consider that a developer may intend for them to be lazily evaluated

The only change that I would expect to arguably be breaking would be recommendations, which as shown in the tests, now may suggest to use a closure for a simple function call. This would actually be more conservative as I see it

Something like:

if foo { Some(x.len()) }
else { None }

This should be suggesting foo.then_some(x.len()).

In such a case x.len() isn't done lazily because you wanted it to be lazy, but because it's the most obvious way to write the if. When switching to then or then_some the simpler choice is to use then_some without the closure. Most uses of eager_of_lazy are for this kind of decision.

unnecessary_lazy_evaluations is unique in that a closure has already been chosen as the form to use. Special handling for this and it's inverse are well justified since an explicit decision on which to use was already made.