languages/rust
languages/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust synced 2024-11-05 20:45:15 +00:00
Code Issues Projects Releases Packages Wiki Activity

non_local_defs: suggest removing leading ref/ptr to make the impl local

Browse source
This commit is contained in:
Urgau 2024-05-15 14:55:40 +02:00
parent ab23fd8dea
commit b71952904d
6 changed files with 75 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
compiler/rustc_lint/messages.ftl
View file

@ -543,11 +543,12 @@ lint_non_local_definitions_cargo_update = the {$macro_kind} `{$macro_name}` may
lint_non_local_definitions_deprecation = this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks should be written at the same level as their item
.help =
.move_help =
move this `impl` block outside of the current {$body_kind_descr} {$depth ->
[one] `{$body_name}`
*[other] `{$body_name}` and up {$depth} bodies
}
.remove_help = remove `{$may_remove_part}` to make the `impl` local
.without_trait = methods and associated constants are still usable outside the current expression, only `impl Local` and `impl dyn Local` can ever be private, and only if the type is nested in the same item as the `impl`
.with_trait = an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
.bounds = `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type

14
compiler/rustc_lint/src/lints.rs
View file

@ -1338,6 +1338,7 @@ pub enum NonLocalDefinitionsDiag {
const_anon: Option<Option<Span>>,
move_help: Span,
may_move: Vec<Span>,
may_remove: Option<(Span, String)>,
has_trait: bool,
},
MacroRules {
@ -1361,6 +1362,7 @@ fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, ()>) {
const_anon,
move_help,
may_move,
may_remove,
has_trait,
} => {
diag.primary_message(fluent::lint_non_local_definitions_impl);
@ -1379,7 +1381,17 @@ fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, ()>) {
for sp in may_move {
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
}
diag.span_help(ms, fluent::lint_help);
diag.span_help(ms, fluent::lint_move_help);
if let Some((span, part)) = may_remove {
diag.arg("may_remove_part", part);
diag.span_suggestion(
span,
fluent::lint_remove_help,
"",
Applicability::MaybeIncorrect,
);
}
if let Some(cargo_update) = cargo_update {
diag.subdiagnostic(&diag.dcx, cargo_update);

80
compiler/rustc_lint/src/non_local_def.rs
View file

@ -136,35 +136,8 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
};
// Part 1: Is the Self type local?
let self_ty_has_local_parent = match impl_.self_ty.kind {
TyKind::Path(QPath::Resolved(_, ty_path)) => {
path_has_local_parent(ty_path, cx, parent, parent_parent)
}
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
path_has_local_parent(
principle_poly_trait_ref.trait_ref.path,
cx,
parent,
parent_parent,
)
}
TyKind::TraitObject([], _, _)
| TyKind::InferDelegation(_, _)
| TyKind::Slice(_)
| TyKind::Array(_, _)
| TyKind::Ptr(_)
| TyKind::Ref(_, _)
| TyKind::BareFn(_)
| TyKind::Never
| TyKind::Tup(_)
| TyKind::Path(_)
| TyKind::Pat(..)
| TyKind::AnonAdt(_)
| TyKind::OpaqueDef(_, _, _)
| TyKind::Typeof(_)
| TyKind::Infer
| TyKind::Err(_) => false,
};
let self_ty_has_local_parent =
ty_has_local_parent(&impl_.self_ty.kind, cx, parent, parent_parent);
if self_ty_has_local_parent {
return;
@ -242,6 +215,18 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
.then_some(span_for_const_anon_suggestion);
let may_remove = match &impl_.self_ty.kind {
TyKind::Ptr(mut_ty) | TyKind::Ref(_, mut_ty)
if ty_has_local_parent(&mut_ty.ty.kind, cx, parent, parent_parent) =>
{
let type_ =
if matches!(impl_.self_ty.kind, TyKind::Ptr(_)) { "*" } else { "&" };
let part = format!("{}{}", type_, mut_ty.mutbl.prefix_str());
Some((impl_.self_ty.span.shrink_to_lo().until(mut_ty.ty.span), part))
}
_ => None,
};
cx.emit_span_lint(
NON_LOCAL_DEFINITIONS,
item.span.shrink_to_lo().to(impl_.self_ty.span),
@ -255,6 +240,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
cargo_update: cargo_update(),
const_anon,
may_move,
may_remove,
has_trait: impl_.of_trait.is_some(),
},
)
@ -384,6 +370,42 @@ fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) {
}
}
/// Given a `Ty` we check if the (outermost) type is local.
fn ty_has_local_parent(
ty_kind: &TyKind<'_>,
cx: &LateContext<'_>,
impl_parent: DefId,
impl_parent_parent: Option<DefId>,
) -> bool {
match ty_kind {
TyKind::Path(QPath::Resolved(_, ty_path)) => {
path_has_local_parent(ty_path, cx, impl_parent, impl_parent_parent)
}
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => path_has_local_parent(
principle_poly_trait_ref.trait_ref.path,
cx,
impl_parent,
impl_parent_parent,
),
TyKind::TraitObject([], _, _)
| TyKind::InferDelegation(_, _)
| TyKind::Slice(_)
| TyKind::Array(_, _)
| TyKind::Ptr(_)
| TyKind::Ref(_, _)
| TyKind::BareFn(_)
| TyKind::Never
| TyKind::Tup(_)
| TyKind::Path(_)
| TyKind::Pat(..)
| TyKind::AnonAdt(_)
| TyKind::OpaqueDef(_, _, _)
| TyKind::Typeof(_)
| TyKind::Infer
| TyKind::Err(_) => false,
}
}
/// Given a path and a parent impl def id, this checks if the if parent resolution
/// def id correspond to the def id of the parent impl definition.
///

4
tests/ui/lint/non-local-defs/exhaustive.stderr
View file

@ -189,7 +189,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
--> $DIR/exhaustive.rs:58:5
|
LL | impl Trait for *mut InsideMain {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^-----^^^^^^^^^^
| |
| help: remove `*mut ` to make the `impl` local
|
= note: `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`

4
tests/ui/lint/non-local-defs/from-local-for-global.stderr
View file

@ -46,7 +46,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
--> $DIR/from-local-for-global.rs:32:5
|
LL | impl StillNonLocal for &Foo {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^-^^^
| |
| help: remove `&` to make the `impl` local
|
= note: `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`

4
tests/ui/lint/non-local-defs/trait-solver-overflow-123573.stderr
View file

@ -2,7 +2,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
--> $DIR/trait-solver-overflow-123573.rs:12:5
|
LL | impl Test for &Local {}
| ^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^-^^^^^
| |
| help: remove `&` to make the `impl` local
|
= note: `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`