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

clippy::map_flatten multiline suggestion is missing closing tokens #8506

Closed
CBenoit opened this issue Mar 4, 2022 · 3 comments · Fixed by #8520
Closed

clippy::map_flatten multiline suggestion is missing closing tokens #8506

CBenoit opened this issue Mar 4, 2022 · 3 comments · Fixed by #8520
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@CBenoit
Copy link
Contributor

CBenoit commented Mar 4, 2022

Summary

Suggestion is incorrect Rust code.

Reproducer

I tried this code:

Some(5)
    .map(|some_long_variable_name| {
        if some_long_variable_name <= 5 {
            Some(some_long_variable_name)
        } else {
            None
        }
    })
    .flatten();

I expected to see this happen:

warning: called `map(..).flatten()` on an `Option`
  --> picky-signtool/src/main.rs:14:12
   |
14 |       Some(5)
   |  ____________^
15 | |         .map(|some_long_variable_name| {
16 | |             if some_long_variable_name <= 5 {
17 | |                 Some(some_long_variable_name)
...  |
21 | |         })
22 | |         .flatten();
   | |__________________^
   |
   = note: `#[warn(clippy::map_flatten)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `and_then` instead
   |
14 ~     Some(5).and_then(|some_long_variable_name| {
15 +             if some_long_variable_name <= 5 {
16 +                 Some(some_long_variable_name)
17 +             } else {
18 +                 None
19 +             }
20 +     });
 ...

Instead, this happened:

warning: called `map(..).flatten()` on an `Option`
  --> picky-signtool/src/main.rs:14:12
   |
14 |       Some(5)
   |  ____________^
15 | |         .map(|some_long_variable_name| {
16 | |             if some_long_variable_name <= 5 {
17 | |                 Some(some_long_variable_name)
...  |
21 | |         })
22 | |         .flatten();
   | |__________________^
   |
   = note: `#[warn(clippy::map_flatten)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `and_then` instead
   |
14 ~     Some(5).and_then(|some_long_variable_name| {
15 +             if some_long_variable_name <= 5 {
16 +                 Some(some_long_variable_name)
17 +             } else {
18 +                 None
19 +             }
 ...

Version

rustc 1.59.0 (9d1b2106e 2022-02-23)
binary: rustc
commit-hash: 9d1b2106e23b1abd32fce1f17267604a5102f57a
commit-date: 2022-02-23
host: x86_64-unknown-linux-gnu
release: 1.59.0
LLVM version: 13.0.0

Additional Labels

No response

@CBenoit CBenoit added the C-bug Category: Clippy is not doing the correct thing label Mar 4, 2022
@xFrednet xFrednet added the L-suggestion Lint: Improving, adding or fixing lint suggestions label Mar 4, 2022
@J-ZhengLi
Copy link
Member

J-ZhengLi commented Mar 9, 2022

This reminds me of issue #8239, I'll see what I can do. @rustbot claim

@J-ZhengLi
Copy link
Member

J-ZhengLi commented Mar 10, 2022

@xFrednet Sorry for the pin, but I'm confused about how should I implement this. Since this seems to be a design flaw of rustc_errors::emitter, until someone has find better solution, should I try to hide the suggestion code while displaying it as part of help message using span_lint_and_then?

@xFrednet
Copy link
Member

xFrednet commented Mar 10, 2022

Hey @J-ZhengLi, no worries you're always welcome to ping me 🙃 This is a "design flaw" at one pint it might be good to add a new display option to rustc to say that the edges of the marked span are important. For now, we can check if the suggestion is larger than MAX_SUGGESTION_HIGHLIGHT_LINES and if so adjust the suggestion to only show the important snippets. In this case, you can create a suggestion like this:

warning: called `map(..).flatten()` on an `Option`
  --> picky-signtool/src/main.rs:14:12
   |
14 |       Some(5)
   |  ____________^
15 | |         .map(|some_long_variable_name| {
16 | |             if some_long_variable_name <= 5 {
17 | |                 Some(some_long_variable_name)
...  |
21 | |         })
22 | |         .flatten();
   | |__________________^
   |
   = note: `#[warn(clippy::map_flatten)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `and_then` instead
   |
14 ~     Some(5).and_then(|some_long_variable_name| {
15 +             if some_long_variable_name <= 5 {
16 +                 Some(some_long_variable_name)
   |
help: and remove the final `.flatten()`
   |
18 +                 None
19 +             }
20 ~         });
   |

You can also create a helper method that can be shared between lints if you want to be fancy. The suggested method of formatting will be a good solution in the meantime 🙃

Feel free to ping me if you want to implement either solution and want help or just a review 🙃

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 L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
3 participants