New lint: Returning unit type when expecting Ord

[Uriopass]

I had a very weird bug today, I was trying to sort my list using a somewhat complicated key expression with sort_by_key, but no sorting was happening it was like getting a random order.

Turns out I had an extra semicolon at the end of my expression, making my key return () instead which implements Ord (always Equal).

Example:

vec.sort_by_key(|x| {
    some_function(some_expression(x));
                                  // ^---- extra semicolon here
});

[basil-cow]

Should extend this to all places that expect function Fn(...) -> K, K: Ord and that function is not a |...| ()?

[Uriopass]

Yes, that's pretty much what my titles says I think.

[basil-cow]

It's probably not a good first issue, you have to look at the bounds of the generic function and check if the last statement of the closure is an explicit (), I would say it's too confusing

[flip1995]

Yeah, to make it generally apply to Fn(...) -> K, K: Ord types might be a little bit harder. But only linting sort_by_key is just calling cx.tcx.expr_ty(last_expr_in_closure), and checking if it is ()-type.

[Aaron1011]

I think this could be generalized to any function that implicitly returns (). If we have code like this:

fn my_fn() -> SomeType { 
    ...
}

fn foo<T>() -> T where T: SomeBound() {
    ...
    my_fn();
}

If both SomeType and () are valid return types for foo (that is, they both satisfy all of the bounds), we could suggest making the intended value more explicit (return my_expr(); or return ()).

[rabisg0]

From what I've seen return () doesn't remain in the hir so linting based on that might be difficult.