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

"false positive" in if_same_then_else when the branches differ by a comment #5940

Closed
nagisa opened this issue Aug 22, 2020 · 3 comments
Closed
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@nagisa
Copy link
Member

nagisa commented Aug 22, 2020

I tried this code:

fn foo(a: bool, b: bool) -> Option<bool> {
    if a {
        Some(b)
    } else if b {
        // Some commentary about why this branch is important etc.
        None
    } else if !a {
        // Some more commentary about why this branch matters.
        None
    } else {
        Some(a)
    }
}

I expected to see this happen: no false positives!

Instead, this happened: if_same_then_else deny-by-default lint fired. While, yes, the branches are indeed "duplicate" at a logical level, they are not at a syntactic level. In this particular case the code ended up this way because it was important to document why we’re returning None if b and then None if !a, These comments could be rewritten to somehow combine them together, but there's probably nothing that’s clearer than just splitting code into a couple of the else if branches seen above.

Meta

  • cargo clippy -V: clippy 0.0.212 (de521cb 2020-08-21)
  • rustc -Vv: rustc 1.47.0-nightly (de521cb 2020-08-21)

(Knowing compilers, this is probably not super actionable, feel free to close if you aren’t too keen on adjusting behaviour here)

@nagisa nagisa added the C-bug Category: Clippy is not doing the correct thing label Aug 22, 2020
@nagisa
Copy link
Member Author

nagisa commented Aug 22, 2020

(note that the conditionals aren’t actually as trivial as portrayed, but this was a reduction I could quickly extract from the actual example)

@tnielens
Copy link
Contributor

This is not a false positive. The lint is designed so. If you consider having a separate branch more important here than applying the lint suggestion, you can annotate the fn definiction with #[allow(clippy::if_same_then_else)].

@nagisa
Copy link
Member Author

nagisa commented Aug 22, 2020

Duplicate of #3770.

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