diff options
| -rw-r--r-- | compiler/rustc_lint/messages.ftl | 4 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/lints.rs | 8 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/non_local_def.rs | 72 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/consts.stderr | 6 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/exhaustive-trait.stderr | 32 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/exhaustive.stderr | 42 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/from-local-for-global.stderr | 6 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/generics.stderr | 5 | ||||
| -rw-r--r-- | tests/ui/lint/non-local-defs/weird-exprs.stderr | 25 |
9 files changed, 76 insertions, 124 deletions
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index e17cb75d026..d329deff2ba 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -616,10 +616,6 @@ lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, `# 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 -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 diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 1f54505f873..dc53349cdf2 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1377,8 +1377,6 @@ pub(crate) enum NonLocalDefinitionsDiag { const_anon: Option<Option<Span>>, doctest: bool, has_trait: bool, - self_ty_str: String, - of_trait_str: Option<String>, macro_to_change: Option<(String, &'static str)>, }, MacroRules { @@ -1401,18 +1399,12 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag { const_anon, doctest, has_trait, - self_ty_str, - of_trait_str, macro_to_change, } => { 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 let Some((macro_to_change, macro_kind)) = macro_to_change { diag.arg("macro_to_change", macro_to_change); diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 5fe39e8be46..48246598452 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -1,12 +1,12 @@ use rustc_errors::MultiSpan; -use rustc_hir::def::DefKind; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{Body, HirId, Item, ItemKind, Node, Path, QPath, TyKind}; +use rustc_hir::{Body, HirId, Item, ItemKind, Node, Path, TyKind}; use rustc_middle::ty::TyCtxt; use rustc_session::{declare_lint, impl_lint_pass}; use rustc_span::def_id::{DefId, LOCAL_CRATE}; use rustc_span::symbol::kw; -use rustc_span::{ExpnKind, MacroKind, Span, Symbol, sym, sym}; +use rustc_span::{ExpnKind, MacroKind, Span, sym}; use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag}; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; @@ -142,6 +142,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { collector.visit_trait_ref(of_trait); } + // 1.5. Remove any path that doesn't resolve to a `DefId` or if it resolve to a + // type-param (e.g. `T`). + collector.paths.retain( + |p| matches!(p.res, Res::Def(def_kind, _) if def_kind != DefKind::TyParam), + ); + // 2. We check if any of path reference a "local" parent and if that the case // we bail out as asked by T-lang, even though this isn't correct from a // type-system point of view, as inference exists and could still leak the impl. @@ -174,23 +180,16 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { 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 { + for path in &collector.paths { + // FIXME: While a translatable diagnostic message can have an argument + // we (currently) have no way to set different args per diag msg with + // `MultiSpan::push_span_label`. + #[allow(rustc::untranslatable_diagnostic)] ms.push_span_label( - path_span_without_args(&of_trait.path), - fluent::lint_non_local_definitions_of_trait_not_local, + path_span_without_args(path), + format!("`{}` is not local", path_name_to_string(path)), ); - Some(path_name_to_string(&of_trait.path)) - } else { - None - }; + } let doctest = is_at_toplevel_doctest(); @@ -216,8 +215,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { .unwrap_or_else(|| "<unnameable>".to_string()), cargo_update: cargo_update(), const_anon, - self_ty_str, - of_trait_str, doctest, has_trait: impl_.of_trait.is_some(), macro_to_change, @@ -312,38 +309,3 @@ fn path_span_without_args(path: &Path<'_>) -> Span { 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.0.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)), - } -} diff --git a/tests/ui/lint/non-local-defs/consts.stderr b/tests/ui/lint/non-local-defs/consts.stderr index 9f70119e0f8..2b70d1e9f32 100644 --- a/tests/ui/lint/non-local-defs/consts.stderr +++ b/tests/ui/lint/non-local-defs/consts.stderr @@ -8,9 +8,9 @@ LL | const Z: () = { | move the `impl` block outside of this constant `Z` ... LL | impl Uto for &Test {} - | ^^^^^---^^^^^----- - | | | - | | `&'_ Test` is not local + | ^^^^^---^^^^^^---- + | | | + | | `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 diff --git a/tests/ui/lint/non-local-defs/exhaustive-trait.stderr b/tests/ui/lint/non-local-defs/exhaustive-trait.stderr index 67df0e31d5b..3b198f9b52a 100644 --- a/tests/ui/lint/non-local-defs/exhaustive-trait.stderr +++ b/tests/ui/lint/non-local-defs/exhaustive-trait.stderr @@ -21,9 +21,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl PartialEq<()> for &Dog { - | ^^^^^---------^^^^^^^^^---- - | | | - | | `&'_ Dog` is not local + | ^^^^^---------^^^^^^^^^^--- + | | | + | | `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 @@ -37,9 +37,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl PartialEq<Dog> for () { - | ^^^^^---------^^^^^^^^^^-- - | | | - | | `()` is not local + | ^^^^^---------^---^^^^^^^^ + | | | + | | `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 @@ -53,9 +53,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl PartialEq<&Dog> for () { - | ^^^^^---------^^^^^^^^^^^-- - | | | - | | `()` is not local + | ^^^^^---------^^---^^^^^^^^ + | | | + | | `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 @@ -69,9 +69,10 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl PartialEq<Dog> for &Dog { - | ^^^^^---------^^^^^^^^^^---- - | | | - | | `&'_ Dog` is not local + | ^^^^^---------^---^^^^^^^--- + | | | | + | | | `Dog` is not local + | | `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 @@ -85,9 +86,10 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl PartialEq<&Dog> for &Dog { - | ^^^^^---------^^^^^^^^^^^---- - | | | - | | `&'_ Dog` is not local + | ^^^^^---------^^---^^^^^^^--- + | | | | + | | | `Dog` is not local + | | `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 diff --git a/tests/ui/lint/non-local-defs/exhaustive.stderr b/tests/ui/lint/non-local-defs/exhaustive.stderr index 6dff48e9a36..c545d975014 100644 --- a/tests/ui/lint/non-local-defs/exhaustive.stderr +++ b/tests/ui/lint/non-local-defs/exhaustive.stderr @@ -65,9 +65,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for &dyn Trait {} - | ^^^^^-----^^^^^---------- - | | | - | | `&'_ dyn Trait` is not local + | ^^^^^-----^^^^^^^^^^----- + | | | + | | `Trait` is not local | `Trait` 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 @@ -81,9 +81,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for *mut Test {} - | ^^^^^-----^^^^^--------- - | | | - | | `*mut Test` is not local + | ^^^^^-----^^^^^^^^^^---- + | | | + | | `Test` is not local | `Trait` 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 @@ -97,9 +97,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for *mut [Test] {} - | ^^^^^-----^^^^^----------- - | | | - | | `*mut [Test]` is not local + | ^^^^^-----^^^^^^^^^^^----^ + | | | + | | `Test` is not local | `Trait` 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 @@ -113,9 +113,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for [Test; 8] {} - | ^^^^^-----^^^^^--------- - | | | - | | `[Test; 8]` is not local + | ^^^^^-----^^^^^^----^^^^ + | | | + | | `Test` is not local | `Trait` 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 @@ -129,9 +129,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for (Test,) {} - | ^^^^^-----^^^^^------- - | | | - | | `(Test,)` is not local + | ^^^^^-----^^^^^^----^^ + | | | + | | `Test` is not local | `Trait` 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 @@ -145,9 +145,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for fn(Test) -> () {} - | ^^^^^-----^^^^^-------------- - | | | - | | `fn(: Test) -> ()` is not local + | ^^^^^-----^^^^^^^^----^^^^^^^ + | | | + | | `Test` is not local | `Trait` 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 @@ -161,9 +161,9 @@ LL | fn main() { | --------- move the `impl` block outside of this function `main` ... LL | impl Trait for fn() -> Test {} - | ^^^^^-----^^^^^------------ - | | | - | | `fn() -> Test` is not local + | ^^^^^-----^^^^^^^^^^^^^---- + | | | + | | `Test` is not local | `Trait` 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 diff --git a/tests/ui/lint/non-local-defs/from-local-for-global.stderr b/tests/ui/lint/non-local-defs/from-local-for-global.stderr index 0d6e29d43cd..355cd6fbdcc 100644 --- a/tests/ui/lint/non-local-defs/from-local-for-global.stderr +++ b/tests/ui/lint/non-local-defs/from-local-for-global.stderr @@ -4,9 +4,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam LL | fn main() { | --------- move the `impl` block outside of this function `main` LL | impl From<Cat> for () { - | ^^^^^----^^^^^^^^^^-- - | | | - | | `()` is not local + | ^^^^^----^---^^^^^^^^ + | | | + | | `Cat` is not local | `From` 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 diff --git a/tests/ui/lint/non-local-defs/generics.stderr b/tests/ui/lint/non-local-defs/generics.stderr index 32324c3ab16..a4ceae4ed31 100644 --- a/tests/ui/lint/non-local-defs/generics.stderr +++ b/tests/ui/lint/non-local-defs/generics.stderr @@ -38,9 +38,8 @@ LL | fn bad() { | -------- move the `impl` block outside of this function `bad` ... LL | impl<T> Uto8 for T {} - | ^^^^^^^^----^^^^^- - | | | - | | `T` is not local + | ^^^^^^^^----^^^^^^ + | | | `Uto8` 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 diff --git a/tests/ui/lint/non-local-defs/weird-exprs.stderr b/tests/ui/lint/non-local-defs/weird-exprs.stderr index cd414d636d3..2441ca41895 100644 --- a/tests/ui/lint/non-local-defs/weird-exprs.stderr +++ b/tests/ui/lint/non-local-defs/weird-exprs.stderr @@ -4,9 +4,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam LL | type A = [u32; { | ________________- LL | | impl Uto for *mut Test {} - | | ^^^^^---^^^^^--------- - | | | | - | | | `*mut Test` is not local + | | ^^^^^---^^^^^^^^^^---- + | | | | + | | | `Test` is not local | | `Uto` is not local LL | | ... | @@ -62,9 +62,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam LL | type A = [u32; { | ____________________- LL | | impl Uto for &Test {} - | | ^^^^^---^^^^^----- - | | | | - | | | `&'_ Test` is not local + | | ^^^^^---^^^^^^---- + | | | | + | | | `Test` is not local | | `Uto` is not local LL | | ... | @@ -81,9 +81,9 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam LL | fn a(_: [u32; { | ___________________- LL | | impl Uto for &(Test,) {} - | | ^^^^^---^^^^^-------- - | | | | - | | | `&'_ (Test,)` is not local + | | ^^^^^---^^^^^^^----^^ + | | | | + | | | `Test` is not local | | `Uto` is not local LL | | ... | @@ -100,9 +100,10 @@ warning: non-local `impl` definition, `impl` blocks should be written at the sam LL | fn b() -> [u32; { | _____________________- LL | | impl Uto for &(Test,Test) {} - | | ^^^^^---^^^^^------------ - | | | | - | | | `&'_ (Test, Test)` is not local + | | ^^^^^---^^^^^^^----^----^ + | | | | | + | | | | `Test` is not local + | | | `Test` is not local | | `Uto` is not local LL | | ... | |