False positive on match_same_arms

[llogiq]

Example: Any kind of xor-like handling, e.g. let Foo be one variant of an enum:

match (x, y) {
    (Foo(_), Foo(_)) => None,
    (Foo(x), _) |
    (_, Foo(x)) => Some(x),
    _ => None
}

This will trigger the lint, because the position of the arms is not taken into account.

@regexident suggested to only lint adjacent same arms. Another option would be to check if those arms are disjunct and can be reordered without changing semantics, but that is obviously much much harder to do in the general case.

[Boddlnagg]

This lint also complained about this match, what I would consider to be a false positive:

match *self {
    SendError::InvalidData(msg) => msg,
    SendError::Other(msg) => msg
}

[mcarton]

Why?
This can be written

match *self {
    SendError::InvalidData(msg) | SendError::Other(msg) => msg
}

[Boddlnagg]

Oh, right. I've never used | before, so I totally didn't think about it that way. Maybe the documentation could mention it, in case there are other people out there who don't think about this ;-)

[mcarton]

The documentation does mention it 😄

[Boddlnagg]

Now this is really embarrassing, sorry. I wanted to report another clippy issue right now, but I'm going to sleep on it first.

[Manishearth]

Eh, report away, false positives aren't a big deal.

And in this case we should be suggesting the |. Documentation is fine for extended things, but suggestions should be prominent 😄

[Manishearth]

(when I say false positives I mean false positives when filing bugs, not clippy false positives )

[liamcurry]

Might also be worth mentioning matching on enums with named fields of the same name, e.g.:

pub enum Test {
    A { field: String },
    B { field: String },
}

impl Test {
    /// `match_same_arms` warning
    fn field_bad(&self) -> &str {
        match self {
            Self::A { field } => field,
            Self::B { field } => field,
        }
    }
    /// no warning
    fn field_good(&self) -> &str {
        match self {
            Self::A { field } | Self::B { field } => field,
        }
    }
}