Skip to content

Commit 6a21450

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#126422
Rollup merge of rust-lang#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 rust-lang#124568. Fixes rust-lang#126339 r? ```@fmease```
2 parents 4e63822 + 94c2821 commit 6a21450

File tree

6 files changed

+125
-46
lines changed

6 files changed

+125
-46
lines changed

‎compiler/rustc_lint/messages.ftl

+1
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks sho
549549
.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`
550550
.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`
551551
.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
552+
.doctest = make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}`
552553
.exception = items in an anonymous const item (`const _: () = {"{"} ... {"}"}`) are treated as in the same scope as the anonymous const's declaration
553554
.const_anon = use a const-anon item to suppress this lint
554555
.macro_to_change = the {$macro_kind} `{$macro_to_change}` defines the non-local `impl`, and may need to be changed

‎compiler/rustc_lint/src/lints.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -1358,6 +1358,7 @@ pub enum NonLocalDefinitionsDiag {
13581358
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
13591359
const_anon: Option<Option<Span>>,
13601360
move_to: Option<(Span, Vec<Span>)>,
1361+
doctest: bool,
13611362
may_remove: Option<(Span, String)>,
13621363
has_trait: bool,
13631364
self_ty_str: String,
@@ -1368,8 +1369,7 @@ pub enum NonLocalDefinitionsDiag {
13681369
depth: u32,
13691370
body_kind_descr: &'static str,
13701371
body_name: String,
1371-
help: Option<()>,
1372-
doctest_help: Option<()>,
1372+
doctest: bool,
13731373
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
13741374
},
13751375
}
@@ -1384,6 +1384,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13841384
cargo_update,
13851385
const_anon,
13861386
move_to,
1387+
doctest,
13871388
may_remove,
13881389
has_trait,
13891390
self_ty_str,
@@ -1422,6 +1423,9 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
14221423
}
14231424
diag.span_help(ms, fluent::lint_non_local_definitions_impl_move_help);
14241425
}
1426+
if doctest {
1427+
diag.help(fluent::lint_doctest);
1428+
}
14251429

14261430
if let Some((span, part)) = may_remove {
14271431
diag.arg("may_remove_part", part);
@@ -1451,20 +1455,18 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
14511455
depth,
14521456
body_kind_descr,
14531457
body_name,
1454-
help,
1455-
doctest_help,
1458+
doctest,
14561459
cargo_update,
14571460
} => {
14581461
diag.primary_message(fluent::lint_non_local_definitions_macro_rules);
14591462
diag.arg("depth", depth);
14601463
diag.arg("body_kind_descr", body_kind_descr);
14611464
diag.arg("body_name", body_name);
14621465

1463-
if let Some(()) = help {
1464-
diag.help(fluent::lint_help);
1465-
}
1466-
if let Some(()) = doctest_help {
1466+
if doctest {
14671467
diag.help(fluent::lint_help_doctest);
1468+
} else {
1469+
diag.help(fluent::lint_help);
14681470
}
14691471

14701472
diag.note(fluent::lint_non_local);

‎compiler/rustc_lint/src/non_local_def.rs

+45-38
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
111111
}
112112
};
113113

114+
// determining if we are in a doctest context can't currently be determined
115+
// by the code itself (there are no specific attributes), but fortunately rustdoc
116+
// sets a perma-unstable env var for libtest so we just reuse that for now
117+
let is_at_toplevel_doctest =
118+
|| self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok();
119+
114120
match item.kind {
115121
ItemKind::Impl(impl_) => {
116122
// The RFC states:
@@ -191,29 +197,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
191197
None
192198
};
193199

194-
let mut collector = PathCollector { paths: Vec::new() };
195-
collector.visit_ty(&impl_.self_ty);
196-
if let Some(of_trait) = &impl_.of_trait {
197-
collector.visit_trait_ref(of_trait);
198-
}
199-
collector.visit_generics(&impl_.generics);
200-
201-
let mut may_move: Vec<Span> = collector
202-
.paths
203-
.into_iter()
204-
.filter_map(|path| {
205-
if let Some(did) = path.res.opt_def_id()
206-
&& did_has_local_parent(did, cx.tcx, parent, parent_parent)
207-
{
208-
Some(cx.tcx.def_span(did))
209-
} else {
210-
None
211-
}
212-
})
213-
.collect();
214-
may_move.sort();
215-
may_move.dedup();
216-
217200
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
218201
.then_some(span_for_const_anon_suggestion);
219202

@@ -248,14 +231,44 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
248231
} else {
249232
None
250233
};
251-
let move_to = if may_move.is_empty() {
252-
ms.push_span_label(
253-
cx.tcx.def_span(parent),
254-
fluent::lint_non_local_definitions_impl_move_help,
255-
);
256-
None
234+
235+
let (doctest, move_to) = if is_at_toplevel_doctest() {
236+
(true, None)
257237
} else {
258-
Some((cx.tcx.def_span(parent), may_move))
238+
let mut collector = PathCollector { paths: Vec::new() };
239+
collector.visit_ty(&impl_.self_ty);
240+
if let Some(of_trait) = &impl_.of_trait {
241+
collector.visit_trait_ref(of_trait);
242+
}
243+
collector.visit_generics(&impl_.generics);
244+
245+
let mut may_move: Vec<Span> = collector
246+
.paths
247+
.into_iter()
248+
.filter_map(|path| {
249+
if let Some(did) = path.res.opt_def_id()
250+
&& did_has_local_parent(did, cx.tcx, parent, parent_parent)
251+
{
252+
Some(cx.tcx.def_span(did))
253+
} else {
254+
None
255+
}
256+
})
257+
.collect();
258+
may_move.sort();
259+
may_move.dedup();
260+
261+
let move_to = if may_move.is_empty() {
262+
ms.push_span_label(
263+
cx.tcx.def_span(parent),
264+
fluent::lint_non_local_definitions_impl_move_help,
265+
);
266+
None
267+
} else {
268+
Some((cx.tcx.def_span(parent), may_move))
269+
};
270+
271+
(false, move_to)
259272
};
260273

261274
let macro_to_change =
@@ -279,6 +292,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
279292
self_ty_str,
280293
of_trait_str,
281294
move_to,
295+
doctest,
282296
may_remove,
283297
has_trait: impl_.of_trait.is_some(),
284298
macro_to_change,
@@ -288,12 +302,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
288302
ItemKind::Macro(_macro, MacroKind::Bang)
289303
if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) =>
290304
{
291-
// determining we if are in a doctest context can't currently be determined
292-
// by the code it-self (no specific attrs), but fortunatly rustdoc sets a
293-
// perma-unstable env for libtest so we just re-use that env for now
294-
let is_at_toplevel_doctest =
295-
self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok();
296-
297305
cx.emit_span_lint(
298306
NON_LOCAL_DEFINITIONS,
299307
item.span,
@@ -304,8 +312,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
304312
.map(|s| s.to_ident_string())
305313
.unwrap_or_else(|| "<unnameable>".to_string()),
306314
cargo_update: cargo_update(),
307-
help: (!is_at_toplevel_doctest).then_some(()),
308-
doctest_help: is_at_toplevel_doctest.then_some(()),
315+
doctest: is_at_toplevel_doctest(),
309316
},
310317
)
311318
}

‎tests/rustdoc-ui/doctest/auxiliary/pub_trait.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub trait Trait {}

‎tests/rustdoc-ui/doctest/non-local-defs-impl.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//@ check-fail
2+
//@ edition:2018
3+
//@ failure-status: 101
4+
//@ aux-build:pub_trait.rs
5+
//@ compile-flags: --test --test-args --test-threads=1
6+
//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR"
7+
//@ normalize-stdout-test "finished in \d+\.\d+s" -> "finished in $$TIME"
8+
9+
#![doc(test(attr(deny(non_local_definitions))))]
10+
#![doc(test(attr(allow(dead_code))))]
11+
12+
/// This will produce a warning:
13+
/// ```rust,no_run
14+
/// # extern crate pub_trait;
15+
/// # use pub_trait::Trait;
16+
///
17+
/// struct Local;
18+
/// impl Trait for &Local {}
19+
/// ```
20+
///
21+
/// But this shoudln't produce a warning:
22+
/// ```rust,no_run
23+
/// # extern crate pub_trait;
24+
/// # use pub_trait::Trait;
25+
///
26+
/// struct Local;
27+
/// impl Trait for &Local {}
28+
///
29+
/// # fn main() {}
30+
/// ```
31+
pub fn doctest() {}

‎tests/rustdoc-ui/doctest/non-local-defs-impl.stdout

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
2+
running 2 tests
3+
test $DIR/non-local-defs-impl.rs - doctest (line 13) - compile ... FAILED
4+
test $DIR/non-local-defs-impl.rs - doctest (line 22) - compile ... ok
5+
6+
failures:
7+
8+
---- $DIR/non-local-defs-impl.rs - doctest (line 13) stdout ----
9+
error: non-local `impl` definition, `impl` blocks should be written at the same level as their item
10+
--> $DIR/non-local-defs-impl.rs:18:1
11+
|
12+
LL | impl Trait for &Local {}
13+
| ^^^^^-----^^^^^------
14+
| | |
15+
| | `&'_ Local` is not local
16+
| | help: remove `&` to make the `impl` local
17+
| `Trait` is not local
18+
|
19+
= 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
20+
= 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`
21+
= help: make this doc-test a standalone test with its own `fn main() { ... }`
22+
= note: 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>
23+
note: the lint level is defined here
24+
--> $DIR/non-local-defs-impl.rs:11:9
25+
|
26+
LL | #![deny(non_local_definitions)]
27+
| ^^^^^^^^^^^^^^^^^^^^^
28+
29+
error: aborting due to 1 previous error
30+
31+
Couldn't compile the test.
32+
33+
failures:
34+
$DIR/non-local-defs-impl.rs - doctest (line 13)
35+
36+
test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
37+

0 commit comments

Comments
 (0)