From 2855bf039a574865c13c67a43cefbf8cead49c1b Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 1 Jun 2021 14:02:09 -0700 Subject: [PATCH] Factor scraping and rendering into separate calls to rustdoc Simplify toggle UI logic, add workspace root for URLs --- src/librustdoc/clean/types.rs | 6 +- src/librustdoc/config.rs | 38 +++++- src/librustdoc/doctest.rs | 10 +- src/librustdoc/html/render/mod.rs | 11 +- src/librustdoc/html/static/css/rustdoc.css | 13 +- src/librustdoc/html/static/js/main.js | 55 ++------- src/librustdoc/lib.rs | 26 ++-- src/librustdoc/scrape_examples.rs | 134 +++++++++------------ 8 files changed, 139 insertions(+), 154 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 2e0be44d9322..ab5b6000a185 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -42,7 +42,7 @@ use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; use crate::html::render::cache::ExternalLocation; use crate::html::render::Context; -use crate::scrape_examples::FnCallLocations; +use crate::scrape_examples::{self, FnCallLocations}; use self::FnRetTy::*; use self::ItemKind::*; @@ -1261,9 +1261,9 @@ crate struct Function { impl Function { crate fn load_call_locations(&mut self, def_id: hir::def_id::DefId, cx: &DocContext<'_>) { if let Some(call_locations) = cx.render_options.call_locations.as_ref() { - let key = cx.tcx.def_path(def_id).to_string_no_crate_verbose(); + let key = scrape_examples::def_id_call_key(cx.tcx, def_id); self.call_locations = call_locations.get(&key).cloned(); - debug!("call_locations: {} -- {:?}", key, self.call_locations); + debug!("call_locations: {:?} -- {:?}", key, self.call_locations); } } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index f34f773ea56d..65c38566a058 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::convert::TryFrom; use std::ffi::OsStr; use std::fmt; +use std::fs; use std::path::PathBuf; use std::str::FromStr; @@ -160,7 +161,12 @@ crate struct Options { /// Whether to skip capturing stdout and stderr of tests. crate nocapture: bool, - crate scrape_examples: Vec, + // Options for scraping call sites from examples/ directory + /// Path to output file to write JSON of call sites. If this option is Some(..) then + /// the compiler will scrape examples and not generate documentation. + crate scrape_examples: Option, + /// Path to the root of the workspace, used to generate workspace-relative file paths. + crate workspace_root: Option, } impl fmt::Debug for Options { @@ -677,7 +683,32 @@ impl Options { } let repository_url = matches.opt_str("repository-url"); - let scrape_examples = matches.opt_strs("scrape-examples"); + let scrape_examples = matches.opt_str("scrape-examples").map(PathBuf::from); + let workspace_root = matches.opt_str("workspace-root").map(PathBuf::from); + let with_examples = matches.opt_strs("with-examples"); + let each_call_locations = with_examples + .into_iter() + .map(|path| { + let bytes = fs::read(&path).map_err(|e| format!("{} (for path {})", e, path))?; + let calls: AllCallLocations = + serde_json::from_slice(&bytes).map_err(|e| format!("{}", e))?; + Ok(calls) + }) + .collect::, _>>() + .map_err(|e: String| { + diag.err(&format!("failed to load examples with error: {}", e)); + 1 + })?; + let call_locations = (each_call_locations.len() > 0).then(move || { + each_call_locations.into_iter().fold(FxHashMap::default(), |mut acc, map| { + for (function, calls) in map.into_iter() { + acc.entry(function) + .or_insert_with(FxHashMap::default) + .extend(calls.into_iter()); + } + acc + }) + }); let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format); @@ -745,13 +776,14 @@ impl Options { ), emit, generate_link_to_definition, - call_locations: None, + call_locations, repository_url, }, crate_name, output_format, json_unused_externs, scrape_examples, + workspace_root, }) } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 43abcf095d85..f8f5e6be9d51 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -45,7 +45,7 @@ crate struct TestOptions { crate attrs: Vec, } -crate fn run(options: Options) -> Result<(), ErrorReported> { +crate fn make_rustc_config(options: &Options) -> interface::Config { let input = config::Input::File(options.input.clone()); let invalid_codeblock_attributes_name = crate::lint::INVALID_CODEBLOCK_ATTRIBUTES.name; @@ -87,7 +87,7 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { let mut cfgs = options.cfgs.clone(); cfgs.push("doc".to_owned()); cfgs.push("doctest".to_owned()); - let config = interface::Config { + interface::Config { opts: sessopts, crate_cfg: interface::parse_cfgspecs(cfgs), input, @@ -103,7 +103,11 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { override_queries: None, make_codegen_backend: None, registry: rustc_driver::diagnostics_registry(), - }; + } +} + +crate fn run(options: Options) -> Result<(), ErrorReported> { + let config = make_rustc_config(&options); let test_args = options.test_args.clone(); let display_doctest_warnings = options.display_doctest_warnings; diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index e83e085dc116..aa65ca474dee 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2528,9 +2528,16 @@ fn render_call_locations( write_example(w, it.next().unwrap()); if n_examples > 1 { - write!(w, r#""); } write!(w, ""); diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 212362b94e00..557a1d119480 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -2066,15 +2066,12 @@ details.undocumented[open] > summary::before { background: #f6fdb0; } +.more-examples-toggle summary { + color: #999; +} + .more-scraped-examples { padding-left: 10px; border-left: 1px solid #ccc; -} - -.toggle-examples .collapse-toggle { - position: relative; -} - -.toggle-examples a { - color: #999 !important; // FIXME(wcrichto): why is important needed + margin-left: 24px; } diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index c6671623c234..fea1b4ecbf1e 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -1136,54 +1136,21 @@ function hideThemeButtonState() { } function updateScrapedExamples() { - onEach(document.getElementsByClassName('scraped-example-list'), function (exampleSet) { - updateScrapedExample( - exampleSet.querySelector(".small-section-header + .scraped-example") - ); - }); - - onEach(document.getElementsByClassName("more-scraped-examples"), function (more) { - var toggle = createSimpleToggle(true); - var label = "More examples"; - var wrapper = createToggle(toggle, label, 14, "toggle-examples", false); - more.parentNode.insertBefore(wrapper, more); - var examples_init = false; - - // Show additional examples on click - wrapper.onclick = function () { - if (hasClass(this, "collapsed")) { - removeClass(this, "collapsed"); - onEachLazy(this.parentNode.getElementsByClassName("hidden"), function (x) { - if (hasClass(x, "content") === false) { - removeClass(x, "hidden"); - addClass(x, "x") - } - }, true); - this.querySelector('.toggle-label').innerHTML = "Hide examples"; - this.querySelector('.inner').innerHTML = labelForToggleButton(false); - if (!examples_init) { - examples_init = true; - onEach(more.getElementsByClassName('scraped-example'), - updateScrapedExample); - } - } else { - addClass(this, "collapsed"); - onEachLazy(this.parentNode.getElementsByClassName("x"), function (x) { - if (hasClass(x, "content") === false) { - addClass(x, "hidden"); - removeClass(x, "x") - } - }, true); - this.querySelector('.toggle-label').innerHTML = label; - this.querySelector('.inner').innerHTML = labelForToggleButton(true); - } - }; + var firstExamples = document.querySelectorAll('.scraped-example-list > .scraped-example'); + onEach(firstExamples, updateScrapedExample); + onEach(document.querySelectorAll('.more-examples-toggle'), function(toggle) { + var moreExamples = toggle.querySelectorAll('.scraped-example'); + toggle.querySelector('summary').addEventListener('click', function() { + // Wrapping in setTimeout ensures the update happens after the elements are actually + // visible. This is necessary since updateScrapedExample calls scrollToLoc which + // depends on offsetHeight, a property that requires an element to be visible to + // compute correctly. + setTimeout(function() { onEach(moreExamples, updateScrapedExample); }); + }, {once: true}); }); } - var start = Date.now(); updateScrapedExamples(); - console.log("updated examples took", Date.now() - start, "ms"); }()); (function () { diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index ed7f656f4068..9bcdbc406a6f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -619,8 +619,10 @@ fn opts() -> Vec { "Make the identifiers in the HTML source code pages navigable", ) }), - unstable("scrape-examples", |o| o.optmulti("", "scrape-examples", "", "")), + unstable("scrape-examples", |o| o.optopt("", "scrape-examples", "", "")), + unstable("workspace-root", |o| o.optopt("", "workspace-root", "", "")), unstable("repository-url", |o| o.optopt("", "repository-url", "", "")), + unstable("with-examples", |o| o.optmulti("", "with-examples", "", "")), ] } @@ -700,28 +702,20 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>( } } -fn main_options(mut options: config::Options) -> MainResult { +fn main_options(options: config::Options) -> MainResult { let diag = core::new_handler(options.error_format, None, &options.debugging_opts); - match (options.should_test, options.markdown_input()) { - (true, true) => return wrap_return(&diag, markdown::test(options)), - (true, false) => return doctest::run(options), - (false, true) => { + match (options.should_test, options.markdown_input(), options.scrape_examples.is_some()) { + (_, _, true) => return scrape_examples::run(options), + (true, true, false) => return wrap_return(&diag, markdown::test(options)), + (true, false, false) => return doctest::run(options), + (false, true, false) => { return wrap_return( &diag, markdown::render(&options.input, options.render_options, options.edition), ); } - (false, false) => {} - } - - if options.scrape_examples.len() > 0 { - if let Some(crate_name) = &options.crate_name { - options.render_options.call_locations = - Some(scrape_examples::scrape(&options.scrape_examples, crate_name)?); - } else { - // raise an error? - } + (false, false, false) => {} } // need to move these items separately because we lose them by the time the closure is called, diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index bc1626977b3e..67d80d01be75 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -1,7 +1,8 @@ //! This module analyzes provided crates to find examples of uses for items in the //! current crate being documented. -use rayon::prelude::*; +use crate::config::Options; +use crate::doctest::make_rustc_config; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{ self as hir, @@ -10,10 +11,12 @@ use rustc_hir::{ use rustc_interface::interface; use rustc_middle::hir::map::Map; use rustc_middle::ty::{self, TyCtxt}; -use rustc_span::symbol::Symbol; +use rustc_span::def_id::DefId; +use std::fs; +crate type DefIdCallKey = String; crate type FnCallLocations = FxHashMap>; -crate type AllCallLocations = FxHashMap; +crate type AllCallLocations = FxHashMap; /// Visitor for traversing a crate and finding instances of function calls. struct FindCalls<'a, 'tcx> { @@ -22,16 +25,20 @@ struct FindCalls<'a, 'tcx> { /// Workspace-relative path to the root of the crate. Used to remember /// which example a particular call came from. - file_name: String, - - /// Name of the crate being documented, to filter out calls to irrelevant - /// functions. - krate: Symbol, + file_path: String, /// Data structure to accumulate call sites across all examples. calls: &'a mut AllCallLocations, } +crate fn def_id_call_key(tcx: TyCtxt<'_>, def_id: DefId) -> DefIdCallKey { + format!( + "{}{}", + tcx.crate_name(def_id.krate).to_ident_string(), + tcx.def_path(def_id).to_string_no_crate_verbose() + ) +} + impl<'a, 'tcx> Visitor<'tcx> for FindCalls<'a, 'tcx> where 'tcx: 'a, @@ -46,9 +53,11 @@ where intravisit::walk_expr(self, ex); // Get type of function if expression is a function call - let types = self.tcx.typeck(ex.hir_id.owner); let (ty, span) = match ex.kind { - hir::ExprKind::Call(f, _) => (types.node_type(f.hir_id), ex.span), + hir::ExprKind::Call(f, _) => { + let types = self.tcx.typeck(ex.hir_id.owner); + (types.node_type(f.hir_id), ex.span) + } hir::ExprKind::MethodCall(_, _, _, span) => { let types = self.tcx.typeck(ex.hir_id.owner); let def_id = types.type_dependent_def_id(ex.hir_id).unwrap(); @@ -59,80 +68,55 @@ where } }; - // Save call site if the function resovles to a concrete definition + // Save call site if the function resolves to a concrete definition if let ty::FnDef(def_id, _) = ty.kind() { - if self.tcx.crate_name(def_id.krate) == self.krate { - let key = self.tcx.def_path(*def_id).to_string_no_crate_verbose(); - let entries = self.calls.entry(key).or_insert_with(FxHashMap::default); - entries - .entry(self.file_name.clone()) - .or_insert_with(Vec::new) - .push((span.lo().0 as usize, span.hi().0 as usize)); - } + let key = def_id_call_key(self.tcx, *def_id); + let entries = self.calls.entry(key).or_insert_with(FxHashMap::default); + entries + .entry(self.file_path.clone()) + .or_insert_with(Vec::new) + .push((span.lo().0 as usize, span.hi().0 as usize)); } } } -struct Callbacks { - calls: AllCallLocations, - krate: String, - file_name: String, -} +crate fn run(options: Options) -> interface::Result<()> { + let inner = move || { + let config = make_rustc_config(&options); -impl rustc_driver::Callbacks for Callbacks { - fn after_analysis<'tcx>( - &mut self, - _compiler: &rustc_interface::interface::Compiler, - queries: &'tcx rustc_interface::Queries<'tcx>, - ) -> rustc_driver::Compilation { - queries.global_ctxt().unwrap().take().enter(|tcx| { - let mut finder = FindCalls { - calls: &mut self.calls, - tcx, - map: tcx.hir(), - file_name: self.file_name.clone(), - krate: Symbol::intern(&self.krate), - }; - tcx.hir().krate().visit_all_item_likes(&mut finder.as_deep_visitor()); - }); + // Get input file path as relative to workspace root + let file_path = options + .input + .strip_prefix(options.workspace_root.as_ref().unwrap()) + .map_err(|e| format!("{}", e))?; - rustc_driver::Compilation::Stop - } -} + interface::run_compiler(config, |compiler| { + compiler.enter(|queries| { + let mut global_ctxt = queries.global_ctxt().unwrap().take(); + global_ctxt.enter(|tcx| { + // Run call-finder on all items + let mut calls = FxHashMap::default(); + let mut finder = FindCalls { + calls: &mut calls, + tcx, + map: tcx.hir(), + file_path: file_path.display().to_string(), + }; + tcx.hir().krate().visit_all_item_likes(&mut finder.as_deep_visitor()); -/// Executes rustc on each example and collects call locations into a single structure. -/// -/// # Arguments: -/// * `examples` is an array of invocations to rustc, generated by Cargo. -/// * `krate` is the name of the crate being documented. -pub fn scrape(examples: &[String], krate: &str) -> interface::Result { - // Scrape each crate in parallel - // FIXME(wcrichto): do we need optional support for no rayon? - let maps = examples - .par_iter() - .map(|example| { - // FIXME(wcrichto): is there a more robust way to get arguments than split(" ")? - let mut args = example.split(" ").map(|s| s.to_owned()).collect::>(); - let file_name = args[0].clone(); - args.insert(0, "_".to_string()); + // Save output JSON to provided path + let calls_json = serde_json::to_string(&calls).map_err(|e| format!("{}", e))?; + fs::write(options.scrape_examples.as_ref().unwrap(), &calls_json) + .map_err(|e| format!("{}", e))?; - // FIXME(wcrichto): is there any setup / cleanup that needs to be performed - // here upon the invocation of rustc_driver? - debug!("Scraping examples from krate {} with args:\n{:?}", krate, args); - let mut callbacks = - Callbacks { calls: FxHashMap::default(), file_name, krate: krate.to_string() }; - rustc_driver::RunCompiler::new(&args, &mut callbacks).run()?; - Ok(callbacks.calls) + Ok(()) + }) + }) }) - .collect::>>()?; + }; - // Merge the call locations into a single result - let mut all_map = FxHashMap::default(); - for map in maps { - for (function, calls) in map.into_iter() { - all_map.entry(function).or_insert_with(FxHashMap::default).extend(calls.into_iter()); - } - } - - Ok(all_map) + inner().map_err(|e: String| { + eprintln!("{}", e); + rustc_errors::ErrorReported + }) }