Move some utils to clippy_utils::ty
#6907
Conversation
|
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
If all is well here, next I'll create |
|
☔ The latest upstream changes (presumably #6831) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
LGTM. Just the naming: flip1995::crate_name_repetition triggered: clippy_utils::ty_utils -> clippy_utils::ty.
Ultimately I would like to get rid of the re-export in clippy_lints/src/utils/mod.rs, so the split between use crate::utils::_ and use clippy_utils::ty::_ seems good to me.
(Great idea splitting up the utils, btw. I also thought about this before, but never got around to do it...)
| @@ -175,8 +176,8 @@ fn is_some_or_ok_call<'a>( | |||
| let outer_ty = cx.typeck_results().expr_ty(expr); | |||
|
|
|||
| // Check if outer and inner type are Option | |||
| let outer_is_some = utils::is_type_diagnostic_item(cx, outer_ty, sym::option_type); | |||
| let inner_is_some = utils::is_type_diagnostic_item(cx, inner_ty, sym::option_type); | |||
| let outer_is_some = is_type_diagnostic_item(cx, outer_ty, sym::option_type); | |||
There was a problem hiding this comment.
I'd like an internal lint checking for uses of utils::something(), but I'm not sure if that's too nitpicky.
There was a problem hiding this comment.
sounds reasonable
flip1995
added
S-waiting-on-author
Ha! I was on the fence about that but now that you say it, I agree. I will just have to put an alias on
My thoughts exactly. |
Yeah, we shouldn't import I'm unsure if we should just reexport everything from the |
I agree. It is just easier to wrap my head around all the utils when they are categorized. Also one step is to stop re-exporting |
Yeah, but I would like an extra PR for that, since that will be another big change. |
camsteffen
changed the title
clippy_utils::ty
|
|
||
| /// Peels off all references on the type. Returns the underlying type and the number of references | ||
| /// removed. | ||
| pub fn peel_mid_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) { |
There was a problem hiding this comment.
I guess it is also named "mid_ty" here for the same reason? Normally ty::Ty is referred to as ty_ty vs hir_ty.
There was a problem hiding this comment.
Yeah I think peel_ty_refs would have been fine here.
|
@bors r+ Thanks! |
|
📌 Commit eb7f8d6 has been approved by |
Move some utils to `clippy_utils::ty` changelog: none `clippy_utils::*` has become a giant junk drawer. This is one step to clean it up a bit. One motivation is that I believe the long import statements cause more merge conflicts.
|
@bors wake up! |
|
💥 Test timed out |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Move some utils to `clippy_utils::source` changelog: none Continues #6907
Don't re-export `clippy_utils::diagnostics::*` changelog: none Continues #6907
changelog: none
clippy_utils::*has become a giant junk drawer. This is one step to clean it up a bit. One motivation is that I believe the long import statements cause more merge conflicts.