Rollup merge of #126422 - Urgau:doctest-impl-non-local-def, r=fmease
Suggest using a standalone doctest for non-local impl defs This PR tweaks the lint output of the `non_local_definitions` lint to suggest using a standalone doctest instead of a moving the `impl` def to an impossible place as was already done with `macro_rules!` case in https://github.com/rust-lang/rust/pull/124568. Fixes #126339 r? ```@fmease```
This commit is contained in:
许杰友 Jieyou Xu (Joe)
GitHub
commit
081cc5cc2d
6 changed files with 125 additions and 46 deletions
|
|
@ -549,6 +549,7 @@ lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks sho
|
|||
.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
|
||||
.doctest = make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}`
|
||||
.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
|
||||
.macro_to_change = the {$macro_kind} `{$macro_to_change}` defines the non-local `impl`, and may need to be changed
|
||||
|
|
|
|||
|
|
@ -1358,6 +1358,7 @@ pub enum NonLocalDefinitionsDiag {
|
|||
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
|
||||
const_anon: Option<Option<Span>>,
|
||||
move_to: Option<(Span, Vec<Span>)>,
|
||||
doctest: bool,
|
||||
may_remove: Option<(Span, String)>,
|
||||
has_trait: bool,
|
||||
self_ty_str: String,
|
||||
|
|
@ -1368,8 +1369,7 @@ pub enum NonLocalDefinitionsDiag {
|
|||
depth: u32,
|
||||
body_kind_descr: &'static str,
|
||||
body_name: String,
|
||||
help: Option<()>,
|
||||
doctest_help: Option<()>,
|
||||
doctest: bool,
|
||||
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
|
||||
},
|
||||
}
|
||||
|
|
@ -1384,6 +1384,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
|
|||
cargo_update,
|
||||
const_anon,
|
||||
move_to,
|
||||
doctest,
|
||||
may_remove,
|
||||
has_trait,
|
||||
self_ty_str,
|
||||
|
|
@ -1422,6 +1423,9 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
|
|||
}
|
||||
diag.span_help(ms, fluent::lint_non_local_definitions_impl_move_help);
|
||||
}
|
||||
if doctest {
|
||||
diag.help(fluent::lint_doctest);
|
||||
}
|
||||
|
||||
if let Some((span, part)) = may_remove {
|
||||
diag.arg("may_remove_part", part);
|
||||
|
|
@ -1451,8 +1455,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
|
|||
depth,
|
||||
body_kind_descr,
|
||||
body_name,
|
||||
help,
|
||||
doctest_help,
|
||||
doctest,
|
||||
cargo_update,
|
||||
} => {
|
||||
diag.primary_message(fluent::lint_non_local_definitions_macro_rules);
|
||||
|
|
@ -1460,11 +1463,10 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
|
|||
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 {
|
||||
if doctest {
|
||||
diag.help(fluent::lint_help_doctest);
|
||||
} else {
|
||||
diag.help(fluent::lint_help);
|
||||
}
|
||||
|
||||
diag.note(fluent::lint_non_local);
|
||||
|
|
|
|||
|
|
@ -111,6 +111,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
}
|
||||
};
|
||||
|
||||
// determining if we are in a doctest context can't currently be determined
|
||||
// by the code itself (there are no specific attributes), but fortunately rustdoc
|
||||
// sets a perma-unstable env var for libtest so we just reuse that for now
|
||||
let is_at_toplevel_doctest =
|
||||
|| self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok();
|
||||
|
||||
match item.kind {
|
||||
ItemKind::Impl(impl_) => {
|
||||
// The RFC states:
|
||||
|
|
@ -191,29 +197,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
None
|
||||
};
|
||||
|
||||
let mut collector = PathCollector { paths: Vec::new() };
|
||||
collector.visit_ty(&impl_.self_ty);
|
||||
if let Some(of_trait) = &impl_.of_trait {
|
||||
collector.visit_trait_ref(of_trait);
|
||||
}
|
||||
collector.visit_generics(&impl_.generics);
|
||||
|
||||
let mut may_move: Vec<Span> = collector
|
||||
.paths
|
||||
.into_iter()
|
||||
.filter_map(|path| {
|
||||
if let Some(did) = path.res.opt_def_id()
|
||||
&& did_has_local_parent(did, cx.tcx, parent, parent_parent)
|
||||
{
|
||||
Some(cx.tcx.def_span(did))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
may_move.sort();
|
||||
may_move.dedup();
|
||||
|
||||
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
|
||||
.then_some(span_for_const_anon_suggestion);
|
||||
|
||||
|
|
@ -248,14 +231,44 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
} else {
|
||||
None
|
||||
};
|
||||
let move_to = if may_move.is_empty() {
|
||||
ms.push_span_label(
|
||||
cx.tcx.def_span(parent),
|
||||
fluent::lint_non_local_definitions_impl_move_help,
|
||||
);
|
||||
None
|
||||
|
||||
let (doctest, move_to) = if is_at_toplevel_doctest() {
|
||||
(true, None)
|
||||
} else {
|
||||
Some((cx.tcx.def_span(parent), may_move))
|
||||
let mut collector = PathCollector { paths: Vec::new() };
|
||||
collector.visit_ty(&impl_.self_ty);
|
||||
if let Some(of_trait) = &impl_.of_trait {
|
||||
collector.visit_trait_ref(of_trait);
|
||||
}
|
||||
collector.visit_generics(&impl_.generics);
|
||||
|
||||
let mut may_move: Vec<Span> = collector
|
||||
.paths
|
||||
.into_iter()
|
||||
.filter_map(|path| {
|
||||
if let Some(did) = path.res.opt_def_id()
|
||||
&& did_has_local_parent(did, cx.tcx, parent, parent_parent)
|
||||
{
|
||||
Some(cx.tcx.def_span(did))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
may_move.sort();
|
||||
may_move.dedup();
|
||||
|
||||
let move_to = if may_move.is_empty() {
|
||||
ms.push_span_label(
|
||||
cx.tcx.def_span(parent),
|
||||
fluent::lint_non_local_definitions_impl_move_help,
|
||||
);
|
||||
None
|
||||
} else {
|
||||
Some((cx.tcx.def_span(parent), may_move))
|
||||
};
|
||||
|
||||
(false, move_to)
|
||||
};
|
||||
|
||||
let macro_to_change =
|
||||
|
|
@ -279,6 +292,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
self_ty_str,
|
||||
of_trait_str,
|
||||
move_to,
|
||||
doctest,
|
||||
may_remove,
|
||||
has_trait: impl_.of_trait.is_some(),
|
||||
macro_to_change,
|
||||
|
|
@ -288,12 +302,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
ItemKind::Macro(_macro, MacroKind::Bang)
|
||||
if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) =>
|
||||
{
|
||||
// determining we if are in a doctest context can't currently be determined
|
||||
// by the code it-self (no specific attrs), but fortunatly rustdoc sets a
|
||||
// perma-unstable env for libtest so we just re-use that env for now
|
||||
let is_at_toplevel_doctest =
|
||||
self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok();
|
||||
|
||||
cx.emit_span_lint(
|
||||
NON_LOCAL_DEFINITIONS,
|
||||
item.span,
|
||||
|
|
@ -304,8 +312,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
|
|||
.map(|s| s.to_ident_string())
|
||||
.unwrap_or_else(|| "<unnameable>".to_string()),
|
||||
cargo_update: cargo_update(),
|
||||
help: (!is_at_toplevel_doctest).then_some(()),
|
||||
doctest_help: is_at_toplevel_doctest.then_some(()),
|
||||
doctest: is_at_toplevel_doctest(),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue