Skip to content

Commit b719529

Browse files
UrgauUrgau
committed
non_local_defs: suggest removing leading ref/ptr to make the impl local
1 parent ab23fd8 commit b719529

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
@@ -543,11 +543,12 @@ lint_non_local_definitions_cargo_update = the {$macro_kind} `{$macro_name}` may
543543
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>
544544
545545
lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks should be written at the same level as their item
546-
.help =
546+
.move_help =
547547
move this `impl` block outside of the current {$body_kind_descr} {$depth ->
548548
[one] `{$body_name}`
549549
*[other] `{$body_name}` and up {$depth} bodies
550550
}
551+
.remove_help = remove `{$may_remove_part}` to make the `impl` local
551552
.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`
552553
.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`
553554
.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
@@ -1338,6 +1338,7 @@ pub enum NonLocalDefinitionsDiag {
13381338
const_anon: Option<Option<Span>>,
13391339
move_help: Span,
13401340
may_move: Vec<Span>,
1341+
may_remove: Option<(Span, String)>,
13411342
has_trait: bool,
13421343
},
13431344
MacroRules {
@@ -1361,6 +1362,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13611362
const_anon,
13621363
move_help,
13631364
may_move,
1365+
may_remove,
13641366
has_trait,
13651367
} => {
13661368
diag.primary_message(fluent::lint_non_local_definitions_impl);
@@ -1379,7 +1381,17 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13791381
for sp in may_move {
13801382
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
13811383
}
1382-
diag.span_help(ms, fluent::lint_help);
1384+
diag.span_help(ms, fluent::lint_move_help);
1385+
1386+
if let Some((span, part)) = may_remove {
1387+
diag.arg("may_remove_part", part);
1388+
diag.span_suggestion(
1389+
span,
1390+
fluent::lint_remove_help,
1391+
"",
1392+
Applicability::MaybeIncorrect,
1393+
);
1394+
}
13831395

13841396
if let Some(cargo_update) = cargo_update {
13851397
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)