Skip to content

Commit fa55e90

Browse files
Auto merge of #148866 - GuillaumeGomez:rustdoc-failed_merged_doctest_compilation, r=<try>
Add new `failed_merged_doctest_compilation` rustdoc lint
2 parents 95aeb46 + 3d4bd83 commit fa55e90

20 files changed

+255
-147
lines changed

‎src/librustdoc/doctest.rs‎

Lines changed: 92 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use rustc_hir as hir;
2323
use rustc_hir::CRATE_HIR_ID;
2424
use rustc_hir::def_id::LOCAL_CRATE;
2525
use rustc_interface::interface;
26+
use rustc_middle::ty::TyCtxt;
2627
use rustc_session::config::{self, CrateType, ErrorOutputType, Input};
2728
use rustc_session::lint;
2829
use rustc_span::edition::Edition;
@@ -204,13 +205,14 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
204205
Err(error) => return crate::wrap_return(dcx, Err(error)),
205206
};
206207
let args_path = temp_dir.path().join("rustdoc-cfgs");
208+
let temp_dir_path = temp_dir.path().to_path_buf();
207209
crate::wrap_return(dcx, generate_args_file(&args_path, &options));
208210

209211
let extract_doctests = options.output_format == OutputFormat::Doctest;
210212
let result = interface::run_compiler(config, |compiler| {
211213
let krate = rustc_interface::passes::parse(&compiler.sess);
212214

213-
let collector = rustc_interface::create_and_enter_global_ctxt(compiler, krate, |tcx| {
215+
rustc_interface::create_and_enter_global_ctxt(compiler, krate, |tcx| {
214216
let crate_name = tcx.crate_name(LOCAL_CRATE).to_string();
215217
let crate_attrs = tcx.hir_attrs(CRATE_HIR_ID);
216218
let opts = scrape_test_config(crate_name, crate_attrs, args_path);
@@ -220,6 +222,8 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
220222
tcx,
221223
);
222224
let tests = hir_collector.collect_crate();
225+
tcx.dcx().abort_if_errors();
226+
223227
if extract_doctests {
224228
let mut collector = extracted::ExtractedDocTests::new();
225229
tests.into_iter().for_each(|t| collector.add_test(t, &opts, &options));
@@ -228,90 +232,87 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
228232
let mut stdout = stdout.lock();
229233
if let Err(error) = serde_json::ser::to_writer(&mut stdout, &collector) {
230234
eprintln!();
231-
Err(format!("Failed to generate JSON output for doctests: {error:?}"))
235+
return Err(format!("Failed to generate JSON output for doctests: {error:?}"));
232236
} else {
233-
Ok(None)
237+
return Ok(());
234238
}
235-
} else {
236-
let mut collector = CreateRunnableDocTests::new(options, opts);
237-
tests.into_iter().for_each(|t| collector.add_test(t, Some(compiler.sess.dcx())));
238-
239-
Ok(Some(collector))
240239
}
241-
});
242-
compiler.sess.dcx().abort_if_errors();
240+
let mut collector = CreateRunnableDocTests::new(options, opts);
241+
tests.into_iter().for_each(|t| collector.add_test(t, Some(compiler.sess.dcx())));
243242

244-
collector
245-
});
243+
let CreateRunnableDocTests {
244+
standalone_tests,
245+
mergeable_tests,
246+
rustdoc_options,
247+
opts,
248+
unused_extern_reports,
249+
compiling_test_count,
250+
..
251+
} = collector;
246252

247-
let CreateRunnableDocTests {
248-
standalone_tests,
249-
mergeable_tests,
250-
rustdoc_options,
251-
opts,
252-
unused_extern_reports,
253-
compiling_test_count,
254-
..
255-
} = match result {
256-
Ok(Some(collector)) => collector,
257-
Ok(None) => return,
258-
Err(error) => {
259-
eprintln!("{error}");
260-
// Since some files in the temporary folder are still owned and alive, we need
261-
// to manually remove the folder.
262-
let _ = std::fs::remove_dir_all(temp_dir.path());
263-
std::process::exit(1);
264-
}
265-
};
253+
run_tests(
254+
opts,
255+
&rustdoc_options,
256+
&unused_extern_reports,
257+
standalone_tests,
258+
mergeable_tests,
259+
Some(temp_dir),
260+
Some(tcx),
261+
);
266262

267-
run_tests(
268-
opts,
269-
&rustdoc_options,
270-
&unused_extern_reports,
271-
standalone_tests,
272-
mergeable_tests,
273-
Some(temp_dir),
274-
);
263+
let compiling_test_count = compiling_test_count.load(Ordering::SeqCst);
264+
265+
// Collect and warn about unused externs, but only if we've gotten
266+
// reports for each doctest
267+
if json_unused_externs.is_enabled() {
268+
let unused_extern_reports: Vec<_> =
269+
std::mem::take(&mut unused_extern_reports.lock().unwrap());
270+
if unused_extern_reports.len() == compiling_test_count {
271+
let extern_names =
272+
externs.iter().map(|(name, _)| name).collect::<FxIndexSet<&String>>();
273+
let mut unused_extern_names = unused_extern_reports
274+
.iter()
275+
.map(|uexts| {
276+
uexts.unused_extern_names.iter().collect::<FxIndexSet<&String>>()
277+
})
278+
.fold(extern_names, |uextsa, uextsb| {
279+
uextsa.intersection(&uextsb).copied().collect::<FxIndexSet<&String>>()
280+
})
281+
.iter()
282+
.map(|v| (*v).clone())
283+
.collect::<Vec<String>>();
284+
unused_extern_names.sort();
285+
// Take the most severe lint level
286+
let lint_level = unused_extern_reports
287+
.iter()
288+
.map(|uexts| uexts.lint_level.as_str())
289+
.max_by_key(|v| match *v {
290+
"warn" => 1,
291+
"deny" => 2,
292+
"forbid" => 3,
293+
// The allow lint level is not expected,
294+
// as if allow is specified, no message
295+
// is to be emitted.
296+
v => unreachable!("Invalid lint level '{v}'"),
297+
})
298+
.unwrap_or("warn")
299+
.to_string();
300+
let uext = UnusedExterns { lint_level, unused_extern_names };
301+
let unused_extern_json = serde_json::to_string(&uext).unwrap();
302+
eprintln!("{unused_extern_json}");
303+
}
304+
}
275305

276-
let compiling_test_count = compiling_test_count.load(Ordering::SeqCst);
277-
278-
// Collect and warn about unused externs, but only if we've gotten
279-
// reports for each doctest
280-
if json_unused_externs.is_enabled() {
281-
let unused_extern_reports: Vec<_> =
282-
std::mem::take(&mut unused_extern_reports.lock().unwrap());
283-
if unused_extern_reports.len() == compiling_test_count {
284-
let extern_names =
285-
externs.iter().map(|(name, _)| name).collect::<FxIndexSet<&String>>();
286-
let mut unused_extern_names = unused_extern_reports
287-
.iter()
288-
.map(|uexts| uexts.unused_extern_names.iter().collect::<FxIndexSet<&String>>())
289-
.fold(extern_names, |uextsa, uextsb| {
290-
uextsa.intersection(&uextsb).copied().collect::<FxIndexSet<&String>>()
291-
})
292-
.iter()
293-
.map(|v| (*v).clone())
294-
.collect::<Vec<String>>();
295-
unused_extern_names.sort();
296-
// Take the most severe lint level
297-
let lint_level = unused_extern_reports
298-
.iter()
299-
.map(|uexts| uexts.lint_level.as_str())
300-
.max_by_key(|v| match *v {
301-
"warn" => 1,
302-
"deny" => 2,
303-
"forbid" => 3,
304-
// The allow lint level is not expected,
305-
// as if allow is specified, no message
306-
// is to be emitted.
307-
v => unreachable!("Invalid lint level '{v}'"),
308-
})
309-
.unwrap_or("warn")
310-
.to_string();
311-
let uext = UnusedExterns { lint_level, unused_extern_names };
312-
let unused_extern_json = serde_json::to_string(&uext).unwrap();
313-
eprintln!("{unused_extern_json}");
314-
}
306+
Ok(())
307+
})
308+
});
309+
310+
if let Err(error) = result {
311+
eprintln!("{error}");
312+
// Since some files in the temporary folder are still owned and alive, we need
313+
// to manually remove the folder.
314+
let _ = std::fs::remove_dir_all(temp_dir_path);
315+
std::process::exit(1);
315316
}
316317
}
317318

@@ -323,6 +324,7 @@ pub(crate) fn run_tests(
323324
mergeable_tests: FxIndexMap<MergeableTestKey, Vec<(DocTestBuilder, ScrapedDocTest)>>,
324325
// We pass this argument so we can drop it manually before using `exit`.
325326
mut temp_dir: Option<TempDir>,
327+
tcx: Option<TyCtxt<'_>>,
326328
) {
327329
let mut test_args = Vec::with_capacity(rustdoc_options.test_args.len() + 1);
328330
test_args.insert(0, "rustdoctest".to_string());
@@ -368,6 +370,18 @@ pub(crate) fn run_tests(
368370
}
369371
continue;
370372
}
373+
if let Some(tcx) = tcx {
374+
tcx.node_span_lint(
375+
crate::lint::FAILED_MERGED_DOCTEST_COMPILATION,
376+
CRATE_HIR_ID,
377+
tcx.hir_span(CRATE_HIR_ID),
378+
|lint| {
379+
lint.primary_message(
380+
format!("failed to compile merged doctests for edition {edition}. Reverting to standalone doctests."),
381+
);
382+
},
383+
);
384+
}
371385
// We failed to compile all compatible tests as one so we push them into the
372386
// `standalone_tests` doctests.
373387
debug!("Failed to compile compatible doctests for edition {} all at once", edition);

‎src/librustdoc/doctest/markdown.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
124124
standalone_tests,
125125
mergeable_tests,
126126
None,
127+
None,
127128
);
128129
Ok(())
129130
}

‎src/librustdoc/lint.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ declare_rustdoc_lint! {
196196
"detects redundant explicit links in doc comments"
197197
}
198198

199+
declare_rustdoc_lint! {
200+
/// This lint is **warn-by-default**. It warns when merged doctests fail to compile
201+
/// when running doctests. This is a `rustdoc` only lint, see the documentation in
202+
/// the [rustdoc book].
203+
///
204+
/// [rustdoc book]: ../../../rustdoc/lints.html#failed_merged_doctest_compilation
205+
FAILED_MERGED_DOCTEST_COMPILATION,
206+
Warn,
207+
"warns when merged doctest fail to compile when running doctests"
208+
}
209+
199210
pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
200211
vec![
201212
BROKEN_INTRA_DOC_LINKS,
@@ -209,6 +220,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
209220
MISSING_CRATE_LEVEL_DOCS,
210221
UNESCAPED_BACKTICKS,
211222
REDUNDANT_EXPLICIT_LINKS,
223+
FAILED_MERGED_DOCTEST_COMPILATION,
212224
]
213225
});
214226

‎tests/rustdoc-ui/doctest/dead-code-2024.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//@ normalize-stdout: "compilation took \d+\.\d+s" -> "compilation took $$TIME"
99
//@ failure-status: 101
1010

11+
#![allow(rustdoc::failed_merged_doctest_compilation)]
1112
#![doc(test(attr(allow(unused_variables), deny(warnings))))]
1213

1314
/// Example

‎tests/rustdoc-ui/doctest/dead-code-2024.stdout‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11

22
running 1 test
3-
test $DIR/dead-code-2024.rs - f (line 15) - compile ... FAILED
3+
test $DIR/dead-code-2024.rs - f (line 16) - compile ... FAILED
44

55
failures:
66

7-
---- $DIR/dead-code-2024.rs - f (line 15) stdout ----
7+
---- $DIR/dead-code-2024.rs - f (line 16) stdout ----
88
error: trait `T` is never used
9-
--> $DIR/dead-code-2024.rs:16:7
9+
--> $DIR/dead-code-2024.rs:17:7
1010
|
1111
LL | trait T { fn f(); }
1212
| ^
1313
|
1414
note: the lint level is defined here
15-
--> $DIR/dead-code-2024.rs:14:9
15+
--> $DIR/dead-code-2024.rs:15:9
1616
|
1717
LL | #![deny(warnings)]
1818
| ^^^^^^^^
@@ -23,7 +23,7 @@ error: aborting due to 1 previous error
2323
Couldn't compile the test.
2424

2525
failures:
26-
$DIR/dead-code-2024.rs - f (line 15)
26+
$DIR/dead-code-2024.rs - f (line 16)
2727

2828
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
2929

‎tests/rustdoc-ui/doctest/dead-code-items.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//@ failure-status: 101
1010

1111
#![doc(test(attr(deny(warnings))))]
12+
#![allow(rustdoc::failed_merged_doctest_compilation)]
1213

1314
#[doc(test(attr(allow(dead_code))))]
1415
/// Example

0 commit comments

Comments
 (0)