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 recommending odd and broken (macro?) syntax in fix #7973

Closed
sigaloid opened this issue Nov 13, 2021 · 1 comment · Fixed by #7974
Closed

Clippy recommending odd and broken (macro?) syntax in fix #7973

sigaloid opened this issue Nov 13, 2021 · 1 comment · Fixed by #7974
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@sigaloid
Copy link

sigaloid commented Nov 13, 2021

Running cargo +nightly clippy on my fork.

fn pattern_to_vec(pattern: &str) -> Pattern {
        pattern
            .trim_matches('/')
            .split('/')
            .flat_map(|s| {
                if let Some(idx) = s.find('.') {
                     vec![s[..idx].to_string(), s[idx..].to_string()]
                 } else {
                     vec![s.to_string()]
                 }
            })
            .collect::<Vec<_>>()
    }

The fix that is given to me for this if let Some:

warning: use Option::map_or_else instead of an if let/else
   --> src/router.rs:85:17
    |
85  | /                 if let Some(idx) = s.find('.') {
86  | |                     vec![s[..idx].to_string(), s[idx..].to_string()]
87  | |                 } else {
88  | |                     vec![s.to_string()]
89  | |                 }
    | |_________________^ help: try: `s.find('.').map_or_else(|| <[_]>::into_vec(box [$($x),+]), |idx| <[_]>::into_vec(box [$($x),+]))`
    |

image

This fix doesn't compile at all, the syntax is just borked.

The original function:
image

The errors given when implementing the suggestion:
image

The right fix: image

You can test this out on the codebase:

git clone https://github.com/sigaloid/vial
cd vial/
git checkout dfc9b61cba9436baa87931bae389e114d342437e
cargo +nightly clippy

(it's the third warning from the bottom)

If I had to guess it would be something to do with the fact that it's an if statement inside of map_or_else, or maybe the vec macro?

Meta

Rust version (rustc -Vv):

rustc 1.56.1 (59eed8a2a 2021-11-01)
binary: rustc
commit-hash: 59eed8a2aac0230a8b53e89d4e99d55912ba6b35
commit-date: 2021-11-01
host: x86_64-unknown-linux-gnu
release: 1.56.1
LLVM version: 13.0.0

@rustbot label +I-suggestion-causes-error

clippy: clippy::option_if_let_else

note: the lint level is defined here
   --> src/lib.rs:214:5
    |
214 |     clippy::nursery,
    |     ^^^^^^^^^^^^^^^
    = note: `#[warn(clippy::option_if_let_else)]` implied by `#[warn(clippy::nursery)]`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
@sigaloid sigaloid added the C-bug Category: Clippy is not doing the correct thing label Nov 13, 2021
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Nov 13, 2021
@matthiaskrgr matthiaskrgr self-assigned this Nov 13, 2021
@matthiaskrgr
Copy link
Member

I think the problem is that the suggestion contains the expanded vec![] macro.

bors added a commit that referenced this issue Nov 15, 2021
…ugg_macro, r=flip1995

option_if_let_else: don't expand macros in suggestion

Fixes #7973

changelog: don't expand macros in suggestion of clippy::option_if_let_else
@bors bors closed this as completed in 3fe11e7 Nov 15, 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 I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants