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

New lint [redundant_at_rest_pattern] #11013

Merged
merged 3 commits into from
Jun 30, 2023

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jun 23, 2023

Closes #11011

It's always a great feeling when a new lint triggers on clippy itself 😄

changelog: New lint [redundant_at_rest_pattern]

@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 23, 2023
@matthiaskrgr
Copy link
Member

The first association when I read the title was "REST API" but I don't have any better ideas unfortunately. 😅

@matthiaskrgr
Copy link
Member

The book uses the term @ binding. https://doc.rust-lang.org/book/ch18-03-pattern-syntax.html#-bindings

Maybe redundant_pattern_at redundant_at_pattern or redundant_at_binding ?

@Centri3
Copy link
Member Author

Centri3 commented Jun 24, 2023

rest is for the pattern on the right, i.e., .., which is being extraneously used to get every element of the slice binded to the local on the left. The at operator in particular isn't really the issue

@giraffate
Copy link
Contributor

The rest pattern is reffered at https://doc.rust-lang.org/reference/patterns.html#rest-patterns. This lint detects the combination of @(at binding) and .. (rest pattern), so how about redundant_at_rest_pattern? (redundant_at_rest_pattern_binding is a little long?)

@Centri3
Copy link
Member Author

Centri3 commented Jun 27, 2023

I'm fine with redundant_at_rest_pattern

@Centri3 Centri3 changed the title New lint [redundant_rest_pattern] New lint [redundant_at_rest_pattern] Jun 27, 2023
@Centri3 Centri3 force-pushed the redundant_rest_pattern branch 2 times, most recently from 1ae3f2f to 4106124 Compare June 27, 2023 11:20
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, thanks! I made one comment.

Comment on lines +339 to +342
/// if let all = v {
/// // NOTE: Type is a `Vec` here
/// println!("all elements: {all:#?}");
/// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It warns irrefutable_let_patterns in this case, so can we add the case that simply print?

It would be better to detect irrefutable_let_patterns, but IMO this lint is still worth it as is.

Suggested change
/// if let all = v {
/// // NOTE: Type is a `Vec` here
/// println!("all elements: {all:#?}");
/// }
/// if let all = v {
/// // NOTE: Type is a `Vec` here
/// println!("all elements: {all:#?}");
/// }
/// // or
/// println!("all elements: {v:#?}");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. Many lints clippy or not will still lint even if it'll give a warning, and then that lint will hopefully be applied as well by the programmer. Though yeah I'm fine with adding just the println!

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jun 29, 2023

📌 Commit 3376c71 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 29, 2023

⌛ Testing commit 3376c71 with merge 3d4c536...

@bors
Copy link
Collaborator

bors commented Jun 30, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 3d4c536 to master...

@bors bors merged commit 3d4c536 into rust-lang:master Jun 30, 2023
8 checks passed
@Centri3 Centri3 deleted the redundant_rest_pattern branch June 30, 2023 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New lint to detect usage of [args @ ..]
5 participants