Skip to content

return_and_then: suggests using ? operator on Option in function which returns Result #14781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ivan-aksamentov opened this issue May 11, 2025 · 1 comment · Fixed by #14783
Closed
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-bug Issue: The suggestion compiles but changes the code to behave in an unintended way

Comments

@ivan-aksamentov
Copy link

ivan-aksamentov commented May 11, 2025

Summary

Related: #14051
CC: @aaron-ang

It seems that return_and_then does not take into account the surrounding context when suggesting a transformation.

The transformation suggested, changes meaning of the code (early return) and is only correct inside blocks returning an Option. Anywhere else it produces code which does not compile, and if it did compile, it would behave differently.

I discovered this by running

cargo clippy --fix

So this fix is marked for autofix (or whatever it is called) but is not currently safe to use.

Really good lint I'd like to enable, but needs some more work.

I think 2 parts are missing here (the list is not exhaustive)

  • check that we are about to return from the parent block (my code does not return; it's a let binding)
  • check that the parent block returns an Option (my block does not; it returns Result)

(I don't know the terminology, but by "parent block" I mean the block that the suggested ? operator will return from)

Lint Name

return_and_then

Reproducer

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=ed73a58ce166b40d8c9567a5843aa773

I tried this code (a bit contrived, but derived from some real code):

#![warn(clippy::return_and_then)]
#![allow(dead_code, unused_variables)]

fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> {
  Ok((1, 1))
}

fn bug(_: Option<&str>) -> Result<(), ()> {
  let year: Option<&str> = None;
  let month: Option<&str> = None;
  let day: Option<&str> = None;

  let _day = if let (Some(year), Some(month)) = (year, month) {
    day.and_then(|day| foo(day, (1, 31)).ok())
  } else {
    None
  };

  Ok(())
}

fn main() {
    println!("Hello, world!");
}

I expected to see this happen:

No warning, or warning with the suggested code being correct.

Instead, this happened:

warning: use the question mark operator instead of an `and_then` call
  --> src/main.rs:14:5
   |
14 |     day.and_then(|day| foo(day, (1, 31)).ok())
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then
note: the lint level is defined here
  --> src/main.rs:1:9
   |
1  | #![warn(clippy::return_and_then)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^
help: try
   |
14 ~     let day = day?;
15 +     foo(day, (1, 31)).ok()
   |

After applying the transform, the code becomes:

#![warn(clippy::return_and_then)]
#![allow(dead_code, unused_variables)]

fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> {
  Ok((1, 1))
}

fn bug(_: Option<&str>) -> Result<(), ()> {
  let year: Option<&str> = None;
  let month: Option<&str> = None;
  let day: Option<&str> = None;

  let _day = if let (Some(year), Some(month)) = (year, month) {
    let day = day?;
    foo(day, (1, 31)).ok()
  } else {
    None
  };

  Ok(())
}

fn main() {
    println!("Hello, world!");
}

Not only it changes meaning (early return, which original code has no intention to do), but also triggering compiler error:

error[E0277]: the `?` operator can only be used on `Result`s, not `Option`s, in a function that returns `Result`
  --> src/main.rs:14:18
   |
8  | fn bug(_: Option<&str>) -> Result<(), ()> {
   | ----------------------------------------- this function returns a `Result`
...
14 |     let day = day?;
   |                  ^ use `.ok_or(...)?` to provide an error compatible with `Result<(), ()>`
   |
   = help: the trait `FromResidual<Option<Infallible>>` is not implemented for `Result<(), ()>`
   = help: the trait `FromResidual<Result<Infallible, E>>` is implemented for `Result<T, F>`

Version

rustc 1.86.0 (05f9846f8 2025-03-31)
binary: rustc
commit-hash: 05f9846f893b09a1be1fc8560e33fc3c815cfecb
commit-date: 2025-03-31
host: x86_64-unknown-linux-gnu
release: 1.86.0
LLVM version: 19.1.7

clippy 0.1.86 (05f9846f89 2025-03-31)

Additional Labels

No response

@ivan-aksamentov ivan-aksamentov added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels May 11, 2025
@samueltardieu
Copy link
Contributor

@rustbot label +I-suggestion-causes-bug
@rustbot claim

@rustbot rustbot added the I-suggestion-causes-bug Issue: The suggestion compiles but changes the code to behave in an unintended way label May 11, 2025
github-merge-queue bot pushed a commit that referenced this issue May 12, 2025
If an expression is not going to return from the current function of
closure, it should not get linted.

This also allows `return` expression to be linted, in addition to the
final expression. Those require blockification and proper indentation.

changelog: [`return_and_then`]: only lint returning expressions

Fixes #14781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-bug Issue: The suggestion compiles but changes the code to behave in an unintended way
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants