Skip to content
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

manual_map recommends noncompilable code involving ? #6795

Closed
dtolnay opened this issue Feb 26, 2021 · 0 comments · Fixed by #6801
Closed

manual_map recommends noncompilable code involving ? #6795

dtolnay opened this issue Feb 26, 2021 · 0 comments · Fixed by #6801
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@dtolnay
Copy link
Member

dtolnay commented Feb 26, 2021

#![deny(clippy::manual_map)]

fn f(_x: &str) -> Result<bool, bool> {
    Ok(false)
}

pub fn g(x: Option<String>) -> Result<(), bool> {
    let _x = match x {
        Some(ref x) => Some(f(x)?),
        None => None,
    };
    Ok(())
}
$ cargo clippy

error: manual implementation of `Option::map`
  --> src/main.rs:8:14
   |
8  |       let _x = match x {
   |  ______________^
9  | |         Some(ref x) => Some(f(x)?),
10 | |         None => None,
11 | |     };
   | |_____^ help: try this: `x.as_ref().map(|x| f(x)?)`

Applying the suggestion made by clippy makes the code not compile:

error[E0277]: the `?` operator can only be used in a closure that returns `Result` or `Option` (or another type that implements `Try`)
 --> src/main.rs:8:33
  |
8 |     let _x = x.as_ref().map(|x| f(x)?);
  |                             ----^^^^^
  |                             |   |
  |                             |   cannot use the `?` operator in a closure that returns `bool`
  |                             this function should return `Result` or `Option` to accept `?`
  |
  = help: the trait `Try` is not implemented for `bool`
  = note: required by `from_error`

@Jarcho @llogiq

@dtolnay dtolnay 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 Feb 26, 2021
dtolnay added a commit to dtolnay/cargo-tally that referenced this issue Feb 26, 2021
dtolnay added a commit to dtolnay/cargo-tally that referenced this issue Feb 26, 2021
rust-lang/rust-clippy#6795

    error: manual implementation of `Option::map`
       --> src/tally.rs:410:19
        |
    410 |       let exclude = match args.exclude {
        |  ___________________^
    411 | |         Some(ref exclude) => Some(Regex::new(exclude)?),
    412 | |         None => None,
    413 | |     };
        | |_____^ help: try this: `args.exclude.as_ref().map(|exclude| Regex::new(exclude)?)`
        |
        = note: `-D clippy::manual-map` implied by `-D clippy::all`
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
dtolnay added a commit to dtolnay/syn that referenced this issue Feb 26, 2021
rust-lang/rust-clippy#6795

    error: manual implementation of `Option::map`
        --> src/item.rs:1194:22
         |
    1194 |               let ty = if let Some(eq_token) = input.parse()? {
         |  ______________________^
    1195 | |                 Some((eq_token, input.parse::<Type>()?))
    1196 | |             } else {
    1197 | |                 None
    1198 | |             };
         | |_____________^ help: try this: `input.parse()?.map(|eq_token| (eq_token, input.parse::<Type>()?))`
         |
         = note: `-D clippy::manual-map` implied by `-D clippy::all`
         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
@flip1995 flip1995 added the E-medium Call for participation: Medium difficulty level problem and requires some initial experience. label Feb 26, 2021
bors added a commit that referenced this issue Feb 26, 2021
Downgrade manual_map to nursery

I believe #6795 should be considered a blocker for this lint to be considered for enabling by default.

---

changelog: remove manual_map from default list of enabled lints
Thomasdezeeuw added a commit to Thomasdezeeuw/heph that referenced this issue Feb 27, 2021
It doesn't work (properly) with the try operator (?), see
rust-lang/rust-clippy#6795.
Thomasdezeeuw added a commit to Thomasdezeeuw/heph that referenced this issue Feb 27, 2021
It doesn't work (properly) with the try operator (?), see
rust-lang/rust-clippy#6795.
@bors bors closed this as completed in ece3543 Mar 2, 2021
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 E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants