Skip to content

Commit d69297a

Browse files
UrgauUrgau
committed
Suggest removing leading reference/pointer to make the impl local
1 parent 3852a5a commit d69297a

File tree

6 files changed

+75
-34
lines changed

6 files changed

+75
-34
lines changed

‎compiler/rustc_lint/messages.ftl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,11 +441,12 @@ lint_non_local_definitions_cargo_update = the {$macro_kind} `{$macro_name}` may
441441
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>
442442
443443
lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks should be written at the same level as their item
444-
.help =
444+
.move_help =
445445
move this `impl` block outside of the current {$body_kind_descr} {$depth ->
446446
[one] `{$body_name}`
447447
*[other] `{$body_name}` and up {$depth} bodies
448448
}
449+
.remove_help = remove `{$may_remove_part}` to make the `impl` local
449450
.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`
450451
.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`
451452
.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

‎compiler/rustc_lint/src/lints.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,7 @@ pub enum NonLocalDefinitionsDiag {
13421342
const_anon: Option<Option<Span>>,
13431343
move_help: Span,
13441344
may_move: Vec<Span>,
1345+
may_remove: Option<(Span, String)>,
13451346
has_trait: bool,
13461347
},
13471348
MacroRules {
@@ -1365,6 +1366,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13651366
const_anon,
13661367
move_help,
13671368
may_move,
1369+
may_remove,
13681370
has_trait,
13691371
} => {
13701372
diag.arg("depth", depth);
@@ -1382,7 +1384,17 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13821384
for sp in may_move {
13831385
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
13841386
}
1385-
diag.span_help(ms, fluent::lint_help);
1387+
diag.span_help(ms, fluent::lint_move_help);
1388+
1389+
if let Some((span, part)) = may_remove {
1390+
diag.arg("may_remove_part", part);
1391+
diag.span_suggestion(
1392+
span,
1393+
fluent::lint_remove_help,
1394+
"",
1395+
Applicability::MaybeIncorrect,
1396+
);
1397+
}
13861398

13871399
if let Some(cargo_update) = cargo_update {
13881400
diag.subdiagnostic(&diag.dcx, cargo_update);

‎compiler/rustc_lint/src/non_local_def.rs

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -136,35 +136,8 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
136136
};
137137

138138
// Part 1: Is the Self type local?
139-
let self_ty_has_local_parent = match impl_.self_ty.kind {
140-
TyKind::Path(QPath::Resolved(_, ty_path)) => {
141-
path_has_local_parent(ty_path, cx, parent, parent_parent)
142-
}
143-
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
144-
path_has_local_parent(
145-
principle_poly_trait_ref.trait_ref.path,
146-
cx,
147-
parent,
148-
parent_parent,
149-
)
150-
}
151-
TyKind::TraitObject([], _, _)
152-
| TyKind::InferDelegation(_, _)
153-
| TyKind::Slice(_)
154-
| TyKind::Array(_, _)
155-
| TyKind::Ptr(_)
156-
| TyKind::Ref(_, _)
157-
| TyKind::BareFn(_)
158-
| TyKind::Never
159-
| TyKind::Tup(_)
160-
| TyKind::Path(_)
161-
| TyKind::Pat(..)
162-
| TyKind::AnonAdt(_)
163-
| TyKind::OpaqueDef(_, _, _)
164-
| TyKind::Typeof(_)
165-
| TyKind::Infer
166-
| TyKind::Err(_) => false,
167-
};
139+
let self_ty_has_local_parent =
140+
ty_has_local_parent(&impl_.self_ty.kind, cx, parent, parent_parent);
168141

169142
if self_ty_has_local_parent {
170143
return;
@@ -242,6 +215,18 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
242215
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
243216
.then_some(span_for_const_anon_suggestion);
244217

218+
let may_remove = match &impl_.self_ty.kind {
219+
TyKind::Ptr(mut_ty) | TyKind::Ref(_, mut_ty)
220+
if ty_has_local_parent(&mut_ty.ty.kind, cx, parent, parent_parent) =>
221+
{
222+
let type_ =
223+
if matches!(impl_.self_ty.kind, TyKind::Ptr(_)) { "*" } else { "&" };
224+
let part = format!("{}{}", type_, mut_ty.mutbl.prefix_str());
225+
Some((impl_.self_ty.span.shrink_to_lo().until(mut_ty.ty.span), part))
226+
}
227+
_ => None,
228+
};
229+
245230
cx.emit_span_lint(
246231
NON_LOCAL_DEFINITIONS,
247232
item.span.shrink_to_lo().to(impl_.self_ty.span),
@@ -255,6 +240,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
255240
cargo_update: cargo_update(),
256241
const_anon,
257242
may_move,
243+
may_remove,
258244
has_trait: impl_.of_trait.is_some(),
259245
},
260246
)
@@ -384,6 +370,42 @@ impl<'tcx> Visitor<'tcx> for PathCollector<'tcx> {
384370
}
385371
}
386372

373+
/// Given a `Ty` we check if the (outermost) type is local.
374+
fn ty_has_local_parent(
375+
ty_kind: &TyKind<'_>,
376+
cx: &LateContext<'_>,
377+
impl_parent: DefId,
378+
impl_parent_parent: Option<DefId>,
379+
) -> bool {
380+
match ty_kind {
381+
TyKind::Path(QPath::Resolved(_, ty_path)) => {
382+
path_has_local_parent(ty_path, cx, impl_parent, impl_parent_parent)
383+
}
384+
TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => path_has_local_parent(
385+
principle_poly_trait_ref.trait_ref.path,
386+
cx,
387+
impl_parent,
388+
impl_parent_parent,
389+
),
390+
TyKind::TraitObject([], _, _)
391+
| TyKind::InferDelegation(_, _)
392+
| TyKind::Slice(_)
393+
| TyKind::Array(_, _)
394+
| TyKind::Ptr(_)
395+
| TyKind::Ref(_, _)
396+
| TyKind::BareFn(_)
397+
| TyKind::Never
398+
| TyKind::Tup(_)
399+
| TyKind::Path(_)
400+
| TyKind::Pat(..)
401+
| TyKind::AnonAdt(_)
402+
| TyKind::OpaqueDef(_, _, _)
403+
| TyKind::Typeof(_)
404+
| TyKind::Infer
405+
| TyKind::Err(_) => false,
406+
}
407+
}
408+
387409
/// Given a path and a parent impl def id, this checks if the if parent resolution
388410
/// def id correspond to the def id of the parent impl definition.
389411
///

‎tests/ui/lint/non-local-defs/exhaustive.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
189189
--> $DIR/exhaustive.rs:58:5
190190
|
191191
LL | impl Trait for *mut InsideMain {}
192-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
192+
| ^^^^^^^^^^^^^^^-----^^^^^^^^^^
193+
| |
194+
| help: remove `*mut ` to make the `impl` local
193195
|
194196
= 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
195197
= 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`

‎tests/ui/lint/non-local-defs/from-local-for-global.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
4646
--> $DIR/from-local-for-global.rs:32:5
4747
|
4848
LL | impl StillNonLocal for &Foo {}
49-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
49+
| ^^^^^^^^^^^^^^^^^^^^^^^-^^^
50+
| |
51+
| help: remove `&` to make the `impl` local
5052
|
5153
= 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
5254
= 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`

‎tests/ui/lint/non-local-defs/trait-solver-overflow-123573.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
22
--> $DIR/trait-solver-overflow-123573.rs:12:5
33
|
44
LL | impl Test for &Local {}
5-
| ^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^-^^^^^
6+
| |
7+
| help: remove `&` to make the `impl` local
68
|
79
= 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
810
= 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`

0 commit comments

Comments
 (0)