Skip to content

manual_split_once suggestion does something different than the original code #7674

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
jonasbb opened this issue Sep 15, 2021 · 1 comment
Closed
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@jonasbb
Copy link

jonasbb commented Sep 15, 2021

I tried this code:

    let domain = "foobar.de";
    
    // Original code
    let x = domain
        .rsplitn(2, '.')
        .next()
        .expect("The domain is never empty, thus one substring always exists.");
    dbg!(x); // => prints "de"

clippy correctly emits a warning for the manual_split_once lint, however, the suggestion is wrong even though the lint is marked as MachineApplicable.

clippy suggests this replacement:

domain
        .rsplitn(2, '.')
        .next()

// ==> 

Some(domain.rsplit_once('.').map_or(domain, |x| x.0))

The suggestion is wrong, and it should be x.1 at the end. The original code prints "de" while the new one prints "foobar", which you can see in this playground.

Meta

Rust version (rustc -Vv):

rustc 1.55.0 (c8dfcfe04 2021-09-06)
binary: rustc
commit-hash: c8dfcfe046a7680554bf4eb612bad840e7631c4b
commit-date: 2021-09-06
host: x86_64-unknown-linux-gnu
release: 1.55.0
LLVM version: 12.0.1
@jonasbb jonasbb added the C-bug Category: Clippy is not doing the correct thing label Sep 15, 2021
@Jarcho
Copy link
Contributor

Jarcho commented Sep 15, 2021

Was fixed in #7663

@jonasbb jonasbb closed this as completed Sep 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
Projects
None yet
Development

No branches or pull requests

2 participants