Improve diagnostic output of non_local_definitions lint

Cargo.lock

@@ -4062,6 +4062,7 @@ dependencies = [
  "rustc_feature",
  "rustc_fluent_macro",
  "rustc_hir",
+ "rustc_hir_pretty",
  "rustc_index",
  "rustc_infer",
  "rustc_macros",

compiler/rustc_lint/Cargo.toml

@@ -13,6 +13,7 @@ rustc_errors = { path = "../rustc_errors" }
 rustc_feature = { path = "../rustc_feature" }
 rustc_fluent_macro = { path = "../rustc_fluent_macro" }
 rustc_hir = { path = "../rustc_hir" }
+rustc_hir_pretty = { path = "../rustc_hir_pretty" }
 rustc_index = { path = "../rustc_index" }
 rustc_infer = { path = "../rustc_infer" }
 rustc_macros = { path = "../rustc_macros" }

compiler/rustc_lint/messages.ftl

@@ -542,17 +542,21 @@ 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, they should be avoided as they go against expectation
-    .help =
-        move this `impl` block outside the of the current {$body_kind_descr} {$depth ->
-            [one] `{$body_name}`
-           *[other] `{$body_name}` and up {$depth} bodies
-        }
-    .non_local = an `impl` definition is non-local if it is nested inside an item and may impact type checking outside of that item. This can be the case if neither the trait or the self type are at the same nesting level as the `impl`
-    .exception = one exception to the rule are anon-const (`const _: () = {"{"} ... {"}"}`) at top-level module and anon-const at the same nesting as the trait or type
+lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks should be written at the same level as their item
+    .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
+    .exception = items in an anonymous const item (`const _: () = {"{"} ... {"}"}`) are treated as in the same scope as the anonymous const's declaration
     .const_anon = use a const-anon item to suppress this lint
 
-lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, they should be avoided as they go against expectation
+lint_non_local_definitions_impl_move_help =
+    move the `impl` block outside of this {$body_kind_descr} {$depth ->
+        [one] `{$body_name}`
+       *[other] `{$body_name}` and up {$depth} bodies
+    }
+
+lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, `#[macro_export]` macro should be written at top level module
     .help =
         remove the `#[macro_export]` or move this `macro_rules!` outside the of the current {$body_kind_descr} {$depth ->
             [one] `{$body_name}`
@@ -561,7 +565,12 @@ lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, th
     .help_doctest =
         remove the `#[macro_export]` or make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}`
     .non_local = a `macro_rules!` definition is non-local if it is nested inside an item and has a `#[macro_export]` attribute
-    .exception = one exception to the rule are anon-const (`const _: () = {"{"} ... {"}"}`) at top-level module
+
+lint_non_local_definitions_may_move = may need to be moved as well
+
+lint_non_local_definitions_of_trait_not_local = `{$of_trait_str}` is not local
+
+lint_non_local_definitions_self_ty_not_local = `{$self_ty_str}` is not local
 
 lint_non_snake_case = {$sort} `{$name}` should have a snake case name
     .rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier

compiler/rustc_lint/src/lints.rs

@@ -6,7 +6,7 @@ use crate::errors::RequestedLevel;
 use crate::fluent_generated as fluent;
 use rustc_errors::{
     codes::*, Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString,
-    ElidedLifetimeInPathSubdiag, EmissionGuarantee, LintDiagnostic, SubdiagMessageOp,
+    ElidedLifetimeInPathSubdiag, EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp,
     Subdiagnostic, SuggestionStyle,
 };
 use rustc_hir::{def::Namespace, def_id::DefId};
@@ -1329,40 +1329,126 @@ pub struct SuspiciousDoubleRefCloneDiag<'a> {
 }
 
 // non_local_defs.rs
-#[derive(LintDiagnostic)]
 pub enum NonLocalDefinitionsDiag {
-    #[diag(lint_non_local_definitions_impl)]
-    #[help]
-    #[note(lint_non_local)]
-    #[note(lint_exception)]
-    #[note(lint_non_local_definitions_deprecation)]
     Impl {
         depth: u32,
         body_kind_descr: &'static str,
         body_name: String,
-        #[subdiagnostic]
         cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
-        #[suggestion(lint_const_anon, code = "_", applicability = "machine-applicable")]
-        const_anon: Option<Span>,
+        const_anon: Option<Option<Span>>,
+        move_to: Option<(Span, Vec<Span>)>,
+        may_remove: Option<(Span, String)>,
+        has_trait: bool,
+        self_ty_str: String,
+        of_trait_str: Option<String>,
     },
-    #[diag(lint_non_local_definitions_macro_rules)]
     MacroRules {
         depth: u32,
         body_kind_descr: &'static str,
         body_name: String,
-        #[help]
         help: Option<()>,
-        #[help(lint_help_doctest)]
         doctest_help: Option<()>,
-        #[note(lint_non_local)]
-        #[note(lint_exception)]
-        #[note(lint_non_local_definitions_deprecation)]
-        notes: (),
-        #[subdiagnostic]
         cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
     },
 }
 
+impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
+    fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, ()>) {
+        match self {
+            NonLocalDefinitionsDiag::Impl {
+                depth,
+                body_kind_descr,
+                body_name,
+                cargo_update,
+                const_anon,
+                move_to,
+                may_remove,
+                has_trait,
+                self_ty_str,
+                of_trait_str,
+            } => {
+                diag.primary_message(fluent::lint_non_local_definitions_impl);
+                diag.arg("depth", depth);
+                diag.arg("body_kind_descr", body_kind_descr);
+                diag.arg("body_name", body_name);
+                diag.arg("self_ty_str", self_ty_str);
+                if let Some(of_trait_str) = of_trait_str {
+                    diag.arg("of_trait_str", of_trait_str);
+                }
+
+                if has_trait {
+                    diag.note(fluent::lint_bounds);
+                    diag.note(fluent::lint_with_trait);
+                } else {
+                    diag.note(fluent::lint_without_trait);
+                }
+
+                if let Some((move_help, may_move)) = move_to {
+                    let mut ms = MultiSpan::from_span(move_help);
+                    for sp in may_move {
+                        ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
+                    }
+                    diag.span_help(ms, fluent::lint_non_local_definitions_impl_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);
+                }
+                if let Some(const_anon) = const_anon {
+                    diag.note(fluent::lint_exception);
+                    if let Some(const_anon) = const_anon {
+                        diag.span_suggestion(
+                            const_anon,
+                            fluent::lint_const_anon,
+                            "_",
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                }
+
+                diag.note(fluent::lint_non_local_definitions_deprecation);
+            }
+            NonLocalDefinitionsDiag::MacroRules {
+                depth,
+                body_kind_descr,
+                body_name,
+                help,
+                doctest_help,
+                cargo_update,
+            } => {
+                diag.primary_message(fluent::lint_non_local_definitions_macro_rules);
+                diag.arg("depth", depth);
+                diag.arg("body_kind_descr", body_kind_descr);
+                diag.arg("body_name", body_name);
+
+                if let Some(()) = help {
+                    diag.help(fluent::lint_help);
+                }
+                if let Some(()) = doctest_help {
+                    diag.help(fluent::lint_help_doctest);
+                }
+
+                diag.note(fluent::lint_non_local);
+                diag.note(fluent::lint_non_local_definitions_deprecation);
+
+                if let Some(cargo_update) = cargo_update {
+                    diag.subdiagnostic(&diag.dcx, cargo_update);
+                }
+            }
+        }
+    }
+}
+
 #[derive(Subdiagnostic)]
 #[note(lint_non_local_definitions_cargo_update)]
 pub struct NonLocalDefinitionsCargoUpdateNote {