single_match: Don't lint non-exhaustive matches; support tuples

[jubnzv]

single_match lint:

• Don't lint exhaustive enum patterns without a wild.
  Rationale: The definition of the enum could be changed, so the user can get non-exhaustive match after applying the suggested lint (see single_match should not lint exhaustive match #8282 (comment) for context).
• Lint match constructions with tuples (as suggested at single_match should not lint exhaustive match #8282 (comment))

Closes #8282

---

changelog: [single_match]: Don't lint exhaustive enum patterns without a wild.
changelog: [single_match]: Lint match constructions with tuples

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon.

Please see the contribution instructions for more information.

[jubnzv]

This is not finished yet, because I need some time to restore logic that calls match_types in the condition. This is required to handle cases, when the user has aliases named as standard structures (Err, Ok, etc.), I'd like to write tests for this.

I'm publishing this PR now to discuss overall design of changes, because there was some discussion in #8282. I'm open to ideas if there are better suggestions to implementing this.

[llogiq]

I commented where I found shorter ways or was befuddled with the implementation. In general, this should be mostly merge-worthy already apart from the nitpick comments I added.

[llogiq]

This puzzles me. Why don't we know at which argument #no is the ..? And why do we need the span for this? It seems like a very roundabout way to implement this.

[jubnzv]

The point is that we don't know the actual number of the patterns replaced by ...

For example:

match (Some(E::V), Some(E::V), Some(E::V), Some(E::V)) {
  (Some(E::V), .., None) => todo!(), // `..` replaces [1, 2]
  (.., None) => todo!(), // `..` replaces [0, 1, 2]
}

We want to iterate to the both arms at the same time, to make sure that the elements with the same index form the exhaustive match. So, the logic implemented here basically evaluates the maximum possible length of the patterns and traverses both arms, considering entries in .. as wildcards (_).

Probably, the most simple solution would be to run the lint iff the second arm contains only wildcards. But this will make the lint a bit less accurate, because we won't be able to generate warnings in some cases.

[llogiq]

In that case shouldn't, position- and rpositioning the .. pattern in the subpatterns be enough? Also "span" has a different meaning in most lints, so the naming could be improved (if needed at all).

[jubnzv]

I fixed the naming to make it differ from rustc's Spans.

I'm not sure if we want to remove these variables at all, because this would complicate the loop in places when we need to evaluate the relative offset from the current pattern to the .. position.

[camsteffen]

Just a note that the term "exhaustive match" doesn't really make sense like I thought it did when I posted the issue. All matches are exhaustive by necessity. The problem is more specific to enums, like "exhaustive enum patterns without a wild".

[llogiq]

Thank you! @bors r+

[bors]

📌 Commit a8fdf5c has been approved by llogiq

[bors]

⌛ Testing commit a8fdf5c with merge 7b89fa9...

[bors]

💔 Test failed - checks-action_test

[xFrednet]

The changelog entries were missing in the PR description, I added them now based on the current description 🙃

@bors retry

[bors]

⌛ Testing commit a8fdf5c with merge 0ed8ca4...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 0ed8ca4 to master...

[camelid]

Btw, does this work with tuple structs (S(_, _)) too?

[jubnzv]

Btw, does this work with tuple structs (S(_, _)) too?

No, this PR doesn't implement any additional logic with structs.

I think, we could process them just like tuples, i.e.:

// lint: S(_, _) forms exhaustive match, so it could be removed
match s {
  S(42, a) => dummy(a),
  S(_, _) => {},
}

// lint: S(..) forms exhaustive match, so it could be removed
match s {
  S(42, a) => dummy(a),
  S(..) => {},
}

Am I right?

[camsteffen]

Yes. It should generally work with all PatKind variants.

[jubnzv]

I added support for tuple structs in #8395.