Skip to content

Commit

Permalink
Point to the Self type and Trait to give for context
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed May 15, 2024
1 parent d69297a commit abefe35
Show file tree
Hide file tree
Showing 15 changed files with 311 additions and 69 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4228,6 +4228,7 @@ dependencies = [
"rustc_feature",
"rustc_fluent_macro",
"rustc_hir",
"rustc_hir_pretty",
"rustc_index",
"rustc_infer",
"rustc_macros",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,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
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,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,
Expand All @@ -1368,10 +1370,16 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
may_move,
may_remove,
has_trait,
self_ty_str,
of_trait_str,
} => {
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);
Expand Down
83 changes: 76 additions & 7 deletions compiler/rustc_lint/src/non_local_def.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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};

Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand All @@ -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(),
Expand Down Expand Up @@ -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)),
}
}
3 changes: 3 additions & 0 deletions tests/ui/lint/non-local-defs/cargo-update.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
37 changes: 29 additions & 8 deletions tests/ui/lint/non-local-defs/consts.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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`
Expand All @@ -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`
Expand Down
30 changes: 24 additions & 6 deletions tests/ui/lint/non-local-defs/exhaustive-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand Down
Loading

0 comments on commit abefe35

Please sign in to comment.