match_like_matches_macro lint could be improved a bit more

[GuillaumeGomez]

For example, this case doesn't warn:

match self {
    WorkMode::Normal => true,
    WorkMode::Sys => true,
    _ => false,
}

But it could be improved using the matches! macro:

matches!(self, WorkMode::Normal | WorkMode::Sys)

[ebroto]

This case will overlap with match_same_arms. Since match_like_matches_macro is warn-by-default, it should have priority, so the former should not trigger.

[alex-700]

Hi, I'd like to hack on this.

I think that every match, where every arm expression is a bool literal, is a good candidate for matches! (yeah, I remember about some special cases (where I need to suppress this), like redundant_pattern_matching. But at the same time there are additional questions with exhaustiveness, e.g. where match enumerate all enum variants, and matches! can't do this.

Now clippy suggests matches! only in the case when we have a wildcard pattern.

@ebroto what do you think about this trade-off?

[ebroto]

I think that every match, where every arm expression is a bool literal, is a good candidate for matches!

I think you're right. The main problem I can see with targeting more than two arms where one is wildcard is that we can have multiple if guards that could be difficult to "merge" in order to be used in matches!(). We probably should not lint there.

But at the same time there are additional questions with exhaustiveness, e.g. where match enumerate all enum variants, and matches! can't do this

Not sure I got you here. You mean that in some cases match should be preferred because of the exhaustiveness check?

[alex-700]

We probably should not lint there.

Yeap, patterns with if guards are evil for this lint.

You mean that in some cases match should be preferred because of the exhaustiveness check?

Yeap, you got it right!

I thought more and came to an idea that there are some issues about an order of patterns too. And I need a hard machinery from rustc (to check that I can reorder some patterns), it's hard to use from clippy.

So the plan is triggering this lint for the following.

match <...> {
  <pat_0> => b0,
  <pat_1> => b0,
  ...
  <pat_k> => b0,
  <pat_k+1> => b1,
  ...
  <pat_n> => b1,
  _ => b1,
}

where b0 and b1 are different bool literals, (and maybe even const bool 🤔).

And the question from my previous comment was about the last _ => b1. @ebroto what do you think: should we make it optional and lint even in the case, when there are no wildcard pattern?

[ebroto]

@alex-700 given that this is a warn-by-default lint, I think the safest approach for the moment would be a generalization of the initial comment in this issue:

match <...> {
    <pat_0> => b0,
    <pat_1> => b0,
    ...
    <pat_n> => b0,
    _ => b1,
}

What do you think?