non_local_defs: point to Self and Trait to give more context

Author: Urgau

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

@@ -567,6 +567,10 @@ lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, `#
 
 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
     .cannot_convert_note = `{$sc}` cannot be used as a raw identifier

compiler/rustc_lint/src/lints.rs

@@ -1340,6 +1340,8 @@ pub enum NonLocalDefinitionsDiag {
         may_move: Vec<Span>,
         may_remove: Option<(Span, String)>,
         has_trait: bool,
+        self_ty_str: String,
+        of_trait_str: Option<String>,
     },
     MacroRules {
         depth: u32,
@@ -1364,11 +1366,17 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
                 may_move,
                 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);

compiler/rustc_lint/src/non_local_def.rs

@@ -1,3 +1,4 @@
+use rustc_errors::MultiSpan;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::HirId;
 use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, TyKind};
@@ -9,12 +10,13 @@ use rustc_middle::ty::{EarlyBinder, TraitRef, TypeSuperFoldable};
 use rustc_session::{declare_lint, impl_lint_pass};
 use rustc_span::def_id::{DefId, LOCAL_CRATE};
 use rustc_span::Span;
-use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind};
+use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind, Symbol};
 use rustc_trait_selection::infer::TyCtxtInferExt;
 use rustc_trait_selection::traits::error_reporting::ambiguity::{
     compute_applicable_impls_for_diagnostics, CandidateSource,
 };
 
+use crate::fluent_generated as fluent;
 use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
 use crate::{LateContext, LateLintPass, LintContext};
 
@@ -201,11 +203,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
                     .into_iter()
                     .filter_map(|path| {
                         if path_has_local_parent(&path, cx, parent, parent_parent) {
-                            if let Some(args) = &path.segments.last().unwrap().args {
-                                Some(path.span.until(args.span_ext))
-                            } else {
-                                Some(path.span)
-                            }
+                            Some(path_span_without_args(&path))
                         } else {
                             None
                         }
@@ -227,9 +225,29 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
                     _ => None,
                 };
 
+                let impl_span = item.span.shrink_to_lo().to(impl_.self_ty.span);
+                let mut ms = MultiSpan::from_span(impl_span);
+
+                let (self_ty_span, self_ty_str) =
+                    self_ty_kind_for_diagnostic(&impl_.self_ty, cx.tcx);
+
+                ms.push_span_label(
+                    self_ty_span,
+                    fluent::lint_non_local_definitions_self_ty_not_local,
+                );
+                let of_trait_str = if let Some(of_trait) = &impl_.of_trait {
+                    ms.push_span_label(
+                        path_span_without_args(&of_trait.path),
+                        fluent::lint_non_local_definitions_of_trait_not_local,
+                    );
+                    Some(path_name_to_string(&of_trait.path))
+                } else {
+                    None
+                };
+
                 cx.emit_span_lint(
                     NON_LOCAL_DEFINITIONS,
-                    item.span.shrink_to_lo().to(impl_.self_ty.span),
+                    ms,
                     NonLocalDefinitionsDiag::Impl {
                         depth: self.body_depth,
                         move_help: item.span,
@@ -239,6 +257,8 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
                             .unwrap_or_else(|| "<unnameable>".to_string()),
                         cargo_update: cargo_update(),
                         const_anon,
+                        self_ty_str,
+                        of_trait_str,
                         may_move,
                         may_remove,
                         has_trait: impl_.of_trait.is_some(),
@@ -447,3 +467,52 @@ fn did_has_local_parent(
             false
         }
 }
+
+/// Return for a given `Path` the span until the last args
+fn path_span_without_args(path: &Path<'_>) -> Span {
+    if let Some(args) = &path.segments.last().unwrap().args {
+        path.span.until(args.span_ext)
+    } else {
+        path.span
+    }
+}
+
+/// Return a "error message-able" ident for the last segment of the `Path`
+fn path_name_to_string(path: &Path<'_>) -> String {
+    path.segments.last().unwrap().ident.name.to_ident_string()
+}
+
+/// Compute the `Span` and visual representation for the `Self` we want to point at;
+/// It follows part of the actual logic of non-local, and if possible return the least
+/// amount possible for the span and representation.
+fn self_ty_kind_for_diagnostic(ty: &rustc_hir::Ty<'_>, tcx: TyCtxt<'_>) -> (Span, String) {
+    match ty.kind {
+        TyKind::Path(QPath::Resolved(_, ty_path)) => (
+            path_span_without_args(ty_path),
+            ty_path
+                .res
+                .opt_def_id()
+                .map(|did| tcx.opt_item_name(did))
+                .flatten()
+                .as_ref()
+                .map(|s| Symbol::as_str(s))
+                .unwrap_or("<unnameable>")
+                .to_string(),
+        ),
+        TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => {
+            let path = &principle_poly_trait_ref.trait_ref.path;
+            (
+                path_span_without_args(path),
+                path.res
+                    .opt_def_id()
+                    .map(|did| tcx.opt_item_name(did))
+                    .flatten()
+                    .as_ref()
+                    .map(|s| Symbol::as_str(s))
+                    .unwrap_or("<unnameable>")
+                    .to_string(),
+            )
+        }
+        _ => (ty.span, rustc_hir_pretty::ty_to_string(&tcx, ty)),
+    }
+}

tests/ui/lint/non-local-defs/cargo-update.stderr

@@ -3,6 +3,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
    |
 LL | non_local_macro::non_local_impl!(LocalStruct);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   | |
+   | `LocalStruct` is not local
+   | `Debug` is not 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`

tests/ui/lint/non-local-defs/consts.stderr

@@ -5,7 +5,10 @@ LL | const Z: () = {
    |       - help: use a const-anon item to suppress this lint: `_`
 ...
 LL |     impl Uto for &Test {}
-   |     ^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---^^^^^-----
+   |          |       |
+   |          |       `&'_ Test` is not local
+   |          `Uto` is not 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`
@@ -22,7 +25,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:24:5
    |
 LL |     impl Uto2 for Test {}
-   |     ^^^^^^^^^^^^^^^^^^
+   |     ^^^^^----^^^^^----
+   |          |        |
+   |          |        `Test` is not local
+   |          `Uto2` is not 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`
@@ -38,7 +44,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:32:5
    |
 LL |     impl Uto3 for Test {}
-   |     ^^^^^^^^^^^^^^^^^^
+   |     ^^^^^----^^^^^----
+   |          |        |
+   |          |        `Test` is not local
+   |          `Uto3` is not 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`
@@ -54,7 +63,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:43:5
    |
 LL |     impl Test {
-   |     ^^^^^^^^^
+   |     ^^^^^----
+   |          |
+   |          `Test` is not local
    |
    = note: 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`
 help: move this `impl` block outside of the current function `main`
@@ -71,7 +82,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:50:9
    |
 LL |         impl Test {
-   |         ^^^^^^^^^
+   |         ^^^^^----
+   |              |
+   |              `Test` is not local
    |
    = note: 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`
 help: move this `impl` block outside of the current inline constant `<unnameable>` and up 2 bodies
@@ -88,7 +101,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:59:9
    |
 LL |         impl Test {
-   |         ^^^^^^^^^
+   |         ^^^^^----
+   |              |
+   |              `Test` is not local
    |
    = note: 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`
 help: move this `impl` block outside of the current constant `_` and up 2 bodies
@@ -106,7 +121,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:72:9
    |
 LL |         impl Uto9 for Test {}
-   |         ^^^^^^^^^^^^^^^^^^
+   |         ^^^^^----^^^^^----
+   |              |        |
+   |              |        `Test` is not local
+   |              `Uto9` is not 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`
@@ -121,7 +139,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/consts.rs:79:9
    |
 LL |         impl Uto10 for Test {}
-   |         ^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^-----^^^^^----
+   |              |         |
+   |              |         `Test` is not local
+   |              `Uto10` is not 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`

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

@@ -2,7 +2,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:7:5
    |
 LL |     impl PartialEq<()> for Dog {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^---
+   |          |                 |
+   |          |                 `Dog` is not local
+   |          `PartialEq` is not 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`
@@ -23,7 +26,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:14:5
    |
 LL |     impl PartialEq<()> for &Dog {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^----
+   |          |                 |
+   |          |                 `&'_ Dog` is not local
+   |          `PartialEq` is not 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`
@@ -43,7 +49,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:21:5
    |
 LL |     impl PartialEq<Dog> for () {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^^--
+   |          |                  |
+   |          |                  `()` is not local
+   |          `PartialEq` is not 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`
@@ -63,7 +72,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:28:5
    |
 LL |     impl PartialEq<&Dog> for () {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^^^--
+   |          |                   |
+   |          |                   `()` is not local
+   |          `PartialEq` is not 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`
@@ -83,7 +95,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:35:5
    |
 LL |     impl PartialEq<Dog> for &Dog {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^^----
+   |          |                  |
+   |          |                  `&'_ Dog` is not local
+   |          `PartialEq` is not 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`
@@ -103,7 +118,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam
   --> $DIR/exhaustive-trait.rs:42:5
    |
 LL |     impl PartialEq<&Dog> for &Dog {
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^---------^^^^^^^^^^^----
+   |          |                   |
+   |          |                   `&'_ Dog` is not local
+   |          `PartialEq` is not 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`