Skip to content

Uplift clippy::for_loops_over_fallibles to rustc #99272

Closed
@WaffleLapkin

Description

@WaffleLapkin

@jyn514 showed an interesting bug:

for x in rx.recv().await {
    // ...
}

The sneaky bug is that rx.recv().await returns an Option and Option implements IntoIterator. So instead of calling rx.recv().await repeatedly until it returns None (intended usage), this calls it once and executes the loop body if it's Some(x).

This can easily be written by a newcomer or even an experienced user, it compiles and does an unexpected thing. This is especially easy to write with async code since we don't have async version of for loop and while let is commonly used instead. Option: IntoIterator is useful in generic code or with iterator combinators (e.g. .chain(opt)), but I think using it in a for loop is never intended.

Clippy has a lint for that: for_loops_over_fallibles which, in my opinion, should be uplifted to rustc.

Clippy currently only suggests using if let Some (i.e. preserving code behavior), but we could also suggest using while let Some (i.e. changing code behavior, in recv case to the good) and, in case of for x in i.next() we can suggest for x in i (iterator specific fix).


I'm willing to work on this, but want to first get a signoff that we indeed want to uplift the lint :)

@rustbot label +A-lint +T-compiler +D-newcomer-roadblock

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.D-newcomer-roadblockDiagnostics: Confusing error or lint; hard to understand for new users.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-langRelevant to the language team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions