Lint against return position impl Trait returning ()

[kpreid]

What it does

If Trait is implemented for (), then a function returning impl Trait can accidentally return () as that implementor simply by a misplaced semicolon. Clippy could have a lint against

• any return position impl Trait
• whose concrete type is (),
• but where that () value did not come from a literal () expression in tail position or return.

This lint would fit into the suspicious category.

Advantage

Users will be able to find such bugs quicker, without needing to first test the behavior of the returned opaquely-typed value and find that it is wrong.

See also prior discussion on IRLO.

Drawbacks

An author might be intentionally passing through () by calling another function that currently returns () but will change in the future, and not want to satisfy the lint by adding ; () because that would disconnect the relationship between the two.

Example

The lint should fire on this code, because it returns () even though it looks like it returns something more meaningful:

fn key() -> impl Eq {
    [1, 10, 2, 0].sort()
}

The lint should not fire on this code:

fn key() -> impl Eq {
    [1, 10, 2, 0].sort(); // pretend I wrote something more useful here
    ()
}

It should also lint any part of the return value that is impl Trait; for example, the impl Trait might be wrapped in Result (or a tuple, or a Vec, or many other things):

fn try_get_key() -> Result<impl Eq, Error> {
    Ok([1, 10, 2, 0].sort()) // should fire
}

fn is_on_purpose_really() -> Result<impl Eq, Error> {
    Ok(()) // should not fire
}

It is debatable whether it should fire on this code:

fn hmm() -> impl Eq {
    let x = ();
    x
}

but the let_unit_value lint fires in that case anyway.

[jplatte]

To add a(nother) real-world example, in axum we had at least one user confused when they wrote something like

fn my_route_handler() -> impl IntoResponse {
    foo.into_response();
}

(discussion)

We were fortunately able to turn this into a warning in axum-core 0.4.4 by adding #[must_use] to IntoResponse::into_response, but it would have been less bad before had this clippy lint existed.