Add non_binding_let_patterns lint

[uhiovk]

changelog: [non_binding_let_patterns]: add new lint

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 7 candidates
• 7 candidates expanded to 7 candidates
• Random selection from Jarcho, dswij, llogiq

[github-actions]

Lintcheck changes for 7c9cc56

Lint	Added	Removed	Changed
clippy::non_binding_let_patterns	220	0	0

This comment will be updated if you push new changes

[y21]

My first impression of this is that I can maybe agree that doing this on let-else statements is strange (like in the first example) and I could maybe see myself enable this lint on the basis of that alone, but on the other hand, something like if let Some(10..=20) = ... seems perfectly reasonable IMHO and I've definitely written these kinds of patterns before.

I understand that it's intended as a restriction lint, but I guess I am just curious how useful this would really be covering all of these at the same time. I personally don't think that if let Ok(None) = ... is confusing (compared to if matches!() {}) or that it misuses let statements as the motivation says. I guess I also don't really see much of a semantic difference between a single-arm match (which this lint presumably "prefers") and an if let, and clippy also has a lint for turning the former into the latter.

But, it's always a bit hard to judge restriction lints that I wouldn't enable myself and perhaps others would.
Is there prior discussion on this anywhere where this opinion is shared?

View changes since the review

[uhiovk]

Thank you for the feedback.

To answer your question: no, I don't know if there was any prior discussion on this. I just came up with this idea after writing a strange let-else statement myself.

I originally included if let and while let because they are mechanically similar, but now I agree with your point. Patterns like if let Some(10..=20) = ... are indeed common, which is why I initially put this in the restriction category.

Based on your opinion, I think a better approach would be to:

1. Remove the checks for if let and while let (or maybe split them to a separate restriction lint, if there's demand for it).
2. Keep the lint exclusive for let-else and rename it.
3. Move the lint from restriction to the pedantic (or style) category.

How does this sound to you?

[y21]

Yeah, IMO restricting it to just let-else seems more useful to me and might be applicable to more people, and I could then also see it somewhere in style/pedantic (exact category can be discussed later -- just wanted to first discuss the scope of the lint). Although I would also be curious to hear what @Jarcho thinks before changing anything to get another maintainer's opinion 🙂

[rustbot]

☔ The latest upstream changes (possibly #17124) made this pull request unmergeable. Please resolve the merge conflicts.