Make doctests collect and emit the unused externs

This commit is contained in:
est31 2020-08-01 12:57:35 +02:00
parent 2d5200605f
commit 13371b59ee
3 changed files with 88 additions and 7 deletions

View file

@ -485,6 +485,10 @@ impl Externs {
pub fn iter(&self) -> BTreeMapIter<'_, String, ExternEntry> { pub fn iter(&self) -> BTreeMapIter<'_, String, ExternEntry> {
self.0.iter() self.0.iter()
} }
pub fn len(&self) -> usize {
self.0.len()
}
} }
impl ExternEntry { impl ExternEntry {

View file

@ -153,6 +153,8 @@ crate struct Options {
/// If this option is set to `true`, rustdoc will only run checks and not generate /// If this option is set to `true`, rustdoc will only run checks and not generate
/// documentation. /// documentation.
crate run_check: bool, crate run_check: bool,
/// Whether doctests should emit unused externs
crate json_unused_externs: bool,
} }
impl fmt::Debug for Options { impl fmt::Debug for Options {
@ -323,7 +325,8 @@ impl Options {
} }
let color = config::parse_color(&matches); let color = config::parse_color(&matches);
let config::JsonConfig { json_rendered, .. } = config::parse_json(&matches); let config::JsonConfig { json_rendered, json_unused_externs, .. } =
config::parse_json(&matches);
let error_format = config::parse_error_format(&matches, color, json_rendered); let error_format = config::parse_error_format(&matches, color, json_rendered);
let codegen_options = build_codegen_options(matches, error_format); let codegen_options = build_codegen_options(matches, error_format);
@ -644,6 +647,7 @@ impl Options {
}, },
crate_name, crate_name,
output_format, output_format,
json_unused_externs,
}) })
} }

View file

@ -1,5 +1,5 @@
use rustc_ast as ast; use rustc_ast as ast;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc; use rustc_data_structures::sync::Lrc;
use rustc_errors::{ColorConfig, ErrorReported}; use rustc_errors::{ColorConfig, ErrorReported};
use rustc_hir as hir; use rustc_hir as hir;
@ -23,6 +23,8 @@ use std::panic;
use std::path::PathBuf; use std::path::PathBuf;
use std::process::{self, Command, Stdio}; use std::process::{self, Command, Stdio};
use std::str; use std::str;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex};
use crate::clean::Attributes; use crate::clean::Attributes;
use crate::config::Options; use crate::config::Options;
@ -103,8 +105,10 @@ crate fn run(options: Options) -> Result<(), ErrorReported> {
let mut test_args = options.test_args.clone(); let mut test_args = options.test_args.clone();
let display_warnings = options.display_warnings; let display_warnings = options.display_warnings;
let externs = options.externs.clone();
let json_unused_externs = options.json_unused_externs;
let tests = interface::run_compiler(config, |compiler| { let res = interface::run_compiler(config, |compiler| {
compiler.enter(|queries| { compiler.enter(|queries| {
let lower_to_hir = queries.lower_to_hir()?; let lower_to_hir = queries.lower_to_hir()?;
@ -147,12 +151,15 @@ crate fn run(options: Options) -> Result<(), ErrorReported> {
}); });
compiler.session().abort_if_errors(); compiler.session().abort_if_errors();
let ret: Result<_, ErrorReported> = Ok(collector.tests); let unused_extern_reports = collector.unused_extern_reports.clone();
let compiling_test_count = collector.compiling_test_count.load(Ordering::SeqCst);
let ret: Result<_, ErrorReported> =
Ok((collector.tests, unused_extern_reports, compiling_test_count));
ret ret
}) })
}); });
let tests = match tests { let (tests, unused_extern_reports, compiling_test_count) = match res {
Ok(tests) => tests, Ok(res) => res,
Err(ErrorReported) => return Err(ErrorReported), Err(ErrorReported) => return Err(ErrorReported),
}; };
@ -164,6 +171,29 @@ crate fn run(options: Options) -> Result<(), ErrorReported> {
Some(testing::Options::new().display_output(display_warnings)), Some(testing::Options::new().display_output(display_warnings)),
); );
// Collect and warn about unused externs, but only if we've gotten
// reports for each doctest
if json_unused_externs {
let unused_extern_reports: Vec<_> =
std::mem::take(&mut unused_extern_reports.lock().unwrap());
if unused_extern_reports.len() == compiling_test_count {
let extern_names = externs.iter().map(|(name, _)| name).collect::<FxHashSet<&String>>();
let mut unused_extern_names = unused_extern_reports
.iter()
.map(|uexts| uexts.unused_extern_names.iter().collect::<FxHashSet<&String>>())
.fold(extern_names, |uextsa, uextsb| {
uextsa.intersection(&uextsb).map(|v| *v).collect::<FxHashSet<&String>>()
})
.iter()
.map(|v| (*v).clone())
.collect::<Vec<String>>();
unused_extern_names.sort();
let unused_extern_json =
serde_json::to_string(&UnusedExterns { unused_extern_names }).unwrap();
eprintln!("{}", unused_extern_json);
}
}
Ok(()) Ok(())
} }
@ -233,6 +263,12 @@ impl DirState {
} }
} }
#[derive(serde::Serialize, serde::Deserialize)]
struct UnusedExterns {
/// List of unused externs by their names.
unused_extern_names: Vec<String>,
}
fn run_test( fn run_test(
test: &str, test: &str,
cratename: &str, cratename: &str,
@ -251,6 +287,7 @@ fn run_test(
outdir: DirState, outdir: DirState,
path: PathBuf, path: PathBuf,
test_id: &str, test_id: &str,
report_unused_externs: impl Fn(UnusedExterns),
) -> Result<(), TestFailure> { ) -> Result<(), TestFailure> {
let (test, line_offset, supports_color) = let (test, line_offset, supports_color) =
make_test(test, Some(cratename), as_test_harness, opts, edition, Some(test_id)); make_test(test, Some(cratename), as_test_harness, opts, edition, Some(test_id));
@ -276,6 +313,11 @@ fn run_test(
if as_test_harness { if as_test_harness {
compiler.arg("--test"); compiler.arg("--test");
} }
if options.json_unused_externs && !compile_fail {
compiler.arg("--error-format=json");
compiler.arg("--json").arg("unused-externs");
compiler.arg("-Z").arg("unstable-options");
}
for lib_str in &options.lib_strs { for lib_str in &options.lib_strs {
compiler.arg("-L").arg(&lib_str); compiler.arg("-L").arg(&lib_str);
} }
@ -335,7 +377,26 @@ fn run_test(
eprint!("{}", self.0); eprint!("{}", self.0);
} }
} }
let out = str::from_utf8(&output.stderr).unwrap(); let mut out_lines = str::from_utf8(&output.stderr)
.unwrap()
.lines()
.filter(|l| {
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(l) {
report_unused_externs(uext);
false
} else {
true
}
})
.collect::<Vec<_>>();
// Add a \n to the end to properly terminate the last line,
// but only if there was output to be printed
if out_lines.len() > 0 {
out_lines.push("");
}
let out = out_lines.join("\n");
let _bomb = Bomb(&out); let _bomb = Bomb(&out);
match (output.status.success(), compile_fail) { match (output.status.success(), compile_fail) {
(true, true) => { (true, true) => {
@ -719,6 +780,8 @@ crate struct Collector {
source_map: Option<Lrc<SourceMap>>, source_map: Option<Lrc<SourceMap>>,
filename: Option<PathBuf>, filename: Option<PathBuf>,
visited_tests: FxHashMap<(String, usize), usize>, visited_tests: FxHashMap<(String, usize), usize>,
unused_extern_reports: Arc<Mutex<Vec<UnusedExterns>>>,
compiling_test_count: AtomicUsize,
} }
impl Collector { impl Collector {
@ -743,6 +806,8 @@ impl Collector {
source_map, source_map,
filename, filename,
visited_tests: FxHashMap::default(), visited_tests: FxHashMap::default(),
unused_extern_reports: Default::default(),
compiling_test_count: AtomicUsize::new(0),
} }
} }
@ -789,6 +854,10 @@ impl Tester for Collector {
let runtool_args = self.options.runtool_args.clone(); let runtool_args = self.options.runtool_args.clone();
let target = self.options.target.clone(); let target = self.options.target.clone();
let target_str = target.to_string(); let target_str = target.to_string();
let unused_externs = self.unused_extern_reports.clone();
if !config.compile_fail {
self.compiling_test_count.fetch_add(1, Ordering::SeqCst);
}
// FIXME(#44940): if doctests ever support path remapping, then this filename // FIXME(#44940): if doctests ever support path remapping, then this filename
// needs to be the result of `SourceMap::span_to_unmapped_path`. // needs to be the result of `SourceMap::span_to_unmapped_path`.
@ -844,6 +913,9 @@ impl Tester for Collector {
test_type: testing::TestType::DocTest, test_type: testing::TestType::DocTest,
}, },
testfn: testing::DynTestFn(box move || { testfn: testing::DynTestFn(box move || {
let report_unused_externs = |uext| {
unused_externs.lock().unwrap().push(uext);
};
let res = run_test( let res = run_test(
&test, &test,
&cratename, &cratename,
@ -862,6 +934,7 @@ impl Tester for Collector {
outdir, outdir,
path, path,
&test_id, &test_id,
report_unused_externs,
); );
if let Err(err) = res { if let Err(err) = res {