Add let_else_ok_or lint#17207
Conversation
|
rustbot has assigned @samueltardieu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Lintcheck changes for b086a00
This comment will be updated if you push new changes |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
The lint will suggest code that doesn't compile in const contexts, such as:
const fn f(a: Option<u32>) -> Result<u32, ()> {
let Some(a) = a else {
return Err(());
};
Ok(a)
}You should check that:
- either you are not in a
constcontext - or the three features
const_try,const_trait_implandconst_option_opsare enabled (you can do the test once and store the result in theLetElseOkOrstruct, and useimpl_lint_pass!instead ofdeclare_lint_pass!)
|
Reminder, once the PR becomes ready for a review, use |
c849920 to
87b80a6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
05c9949 to
4d49b6c
Compare
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
|
Lintcheck - 5 hits on default set (27 crates), 22 hits on extended set (499 crates), no ICEs. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
New `style` lint that detects `let...else` statements binding the payload
of an `Option` whose `else` branch only does `return Err(<expr>)`, and
suggests the more concise `Option::ok_or`/`ok_or_else` with the `?`
operator:
let Some(value) = opt else {
return Err("missing");
};
// ->
let value = opt.ok_or("missing")?;
`ok_or_else` is suggested when the error value is non-trivial, to preserve
the laziness of the original `else` branch. The lint bails on `ref`
bindings, comments or `#[cfg]` in the `else` branch, and `return Err(..)`
expressions produced by a macro expansion (e.g. `anyhow::bail!`), where
the error value cannot be rendered into a sound suggestion.
`ok_or`/`ok_or_else` move the error value unconditionally, whereas the
`else` branch only runs on `None`. When the error value moves a non-`Copy`
local that is still used on the `Some` path (or could be moved again across
a loop iteration), the rewrite would reference a moved value and not
compile, so the suggestion is downgraded to non-machine-applicable in that
case (values that are merely borrowed, e.g. `Err(format!("{x}"))`, stay
machine-applicable). The error snippet is rendered through the statement's
context so an error value that is itself a macro call (`format!(..)`) is
shown as written instead of leaking its expansion internals.
b086a00 to
e5ec071
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
☔ The latest upstream changes (possibly #17385) made this pull request unmergeable. Please resolve the merge conflicts. |
New
stylelint that detectslet...elsestatements binding the payload of anOptionwhoseelsebranch only doesreturn Err(<expr>), and suggests the more conciseOption::ok_or/ok_or_elsewith the?operator:ok_or_elseis suggested when the error value is non-trivial, to preserve the laziness of the originalelsebranch. The lint bails onrefbindings, comments or#[cfg]in theelsebranch, andreturn Err(..)expressions produced by a macro expansion (e.g.anyhow::bail!), where the error value cannot be rendered into a sound suggestion.changelog: Add
let_else_ok_orlint