rust-lang / rust-lang/rust-clippy

Lint against return position `impl Trait` returning `()`

Open
#13,909 1 comment 11 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-lint
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

No files, tests, or entry points are named in the issue. Start by resolving the proposed lint's scope, especially nested return-position impl Trait and the debated local-variable case; done means suspicious accidental unit returns are detected while explicit () expressions are accepted.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.