clippy_utils: make peel_*_ty_refs class of functions a bit more consistent

[ada4a]

I was looking for the function that peels refs from a type while also returning the number of refs peeled, and it turns out there were actually two of them?? I removed the by far less popular one, and made some other clean-up along the way

changelog: none

[rustbot]

r? @Alexendoo

rustbot has assigned @Alexendoo.
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

[ada4a]

Right. I don't really understand why rustfmt is complaining about the line -- it's even marked with #[rustfmt::skip]! Should I remove the attribute and format the line?

EDIT: ah, it's not quite that: what rustfmt actually tries to format is the import line (because of the moved peel_middle_ty_refs import), but then does a max width checks, which for some reason fails on the #[rustfmt::skip]-ed line.. Manually formatting just the import fixed the issue. This is probably a bug in rustfmt?

[ada4a]

Also it's a shame that lintcheck / base continues to run even though lintcheck / head already failed -- but there doesn't seem to be a good way to solve this

[samueltardieu]

Also it's a shame that lintcheck / base continues to run even though lintcheck / head already failed -- but there doesn't seem to be a good way to solve this

Why? If I remember correctly, the results from lintcheck/base are cached for a given base, so it will be reused if you git push -f after it has finished if you use the same base.

[ada4a]

Oh is it? That's nice. I guess the other direction (base fails, so head should as well) is also not a problem, because how would base end up in a broken state in the first place.. Well, good to know:)

[samueltardieu]

I like clean ups in general, as they reduce the technical debt.

However, when you use longer commit messages, could you use proper sentences and casing in them? It's ok for the short message to be concise, even though most of us still use proper casing, but longer messages should be both useful and easy to read.

[samueltardieu]

I think peel_and_count_refs would be a better name, we are in the ty module so I don't think it is useful to repeat "middle" here.

[ada4a]

I would agree if this were a method on middle::Ty, but it's a freestanding function which is often imported unqualified, so having some redundancy in the name could be helpful imo, WDYT?

The and_count part I do agree with though.

[samueltardieu]

This is the only part in Clippy where we use "middle_ty" as part of a function name, this is why it looks odd to me. And this is never used in the whole compiler as part of a function name either. In case of an ambiguity, it looks like "hir_ty" is used for rustc_hir::Ty, while "ty" is used for rustc_middle::ty::Ty.

[ada4a]

Fair enough. There's also peel_mid_ty_refs_is_mutable right beside it, and I'd like to give it a similarly structured name, but that's a bit hard given that that function does many things at once.

Maybe something like peel_ty_refs_with_count and peel_ty_refs_with_count_is_mutable?

[samueltardieu]

r? samueltardieu @rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.