initial clippy::ref_patterns implementation

[NotAPenguin0]

This implements a new lint that discourages the use of the ref keyword as outlined in #9010. I think there are still some things to improve about this lint, but I need some feedback before I can implement those.

• Maybe it's desirable that a quick fix is listed too, instead of a generic avoid using the ref keyword lint.
• let ref x = y already has a lint (clippy::toplevel_ref_arg). This implementation will report this too, so you get two lints for the same issue, which is not great. I don't really know a way around this though.
• The dogfood test is currently failing locally, though I ran cargo clippy -- -D clippy::all -D clippy::pedantic and got no output, so I'm not sure why this is.

Any help with these would be greatly appreciated.

fixes #9010
changelog: [ref_patterns]: newly added lint

[rustbot]

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

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[bors]

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

[Alexendoo]

• The dogfood test is currently failing locally, though I ran cargo clippy -- -D clippy::all -D clippy::pedantic and got no output, so I'm not sure why this is.

The clippy packages aren't in a cargo workspace due to being part of the rust-lang/rust repo workspace so that would only be linting the root crate, you can run the dogfood test with cargo test --features internal --test dogfood

[NotAPenguin0]

I see, thanks. I'll run that and see what I can fix.

[Alexendoo]

Maybe it's desirable that a quick fix is listed too, instead of a generic avoid using the ref keyword lint.

It's hard to produce a specific suggestion since it's often not as simple as putting an & somewhere in the expression, e.g.

let x = (String::new(), String::new());
let (a, ref b) = x;

let ref x = y already has a lint (clippy::toplevel_ref_arg). This implementation will report this too, so you get two lints for the same issue, which is not great. I don't really know a way around this though.

You could check where toplevel_ref_arg is emitted if this lint isn't enabled with is_lint_allowed(cx, REF_PATTERN, pat.hir_id)

[NotAPenguin0]

@rustbot review

[NotAPenguin0]

Should I do anything on my end to squash and rebase?

[Alexendoo]

Yeah though actually one last thing also, could you change the lint + filenames to be ref_patterns (plural)

For the rebase/squash it would be something along the lines of

git fetch upstream
git rebase -i upstream/master

Then changing the picks of all but your first commit to be fixup (or f for short), saving + exiting and resolving any conflicts that arise

[NotAPenguin0]

Err, that doesn't include the rename commit. I'll fix that first

[Alexendoo]

Great, thanks!

@bors r+

[bors]

📌 Commit 56e8e1a has been approved by Alexendoo

It is now in the queue for this repository.

[bors]

⌛ Testing commit 56e8e1a with merge 8798c66...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 8798c66 to master...