From 867ac8da355cb39bc30c40c416809b0c7c07d1c9 Mon Sep 17 00:00:00 2001 From: Joseph Ryan Date: Wed, 12 Aug 2020 12:22:45 -0500 Subject: [PATCH] Respond to comments and start adding tests --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/clean/types.rs | 36 ++++ src/librustdoc/json/conversions.rs | 9 +- src/librustdoc/json/mod.rs | 124 +++++++------ src/librustdoc/json/types.rs | 41 +++-- .../run-make-fulldeps/rustdoc-json/Makefile | 5 + .../rustdoc-json/check_missing_items.py | 170 ++++++++++++++++++ .../run-make-fulldeps/rustdoc-json/test.rs | 17 ++ 8 files changed, 329 insertions(+), 75 deletions(-) create mode 100644 src/test/run-make-fulldeps/rustdoc-json/Makefile create mode 100644 src/test/run-make-fulldeps/rustdoc-json/check_missing_items.py create mode 100644 src/test/run-make-fulldeps/rustdoc-json/test.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 801d06e610169..4cf53476c1ab2 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2262,7 +2262,7 @@ impl Clean> for doctree::Import<'_> { name: None, attrs: self.attrs.clean(cx), source: self.whence.clean(cx), - def_id: DefId::local(CRATE_DEF_INDEX), + def_id: cx.tcx.hir().local_def_id(self.id).to_def_id(), visibility: self.vis.clean(cx), stability: None, deprecation: None, diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index ab5255f6b5a4b..592e1ce3d41b2 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -279,6 +279,42 @@ pub enum ItemEnum { } impl ItemEnum { + /// Some items contain others such as structs (for their fields) and Enums + /// (for their variants). This method returns those contained items. + pub fn inner_items(&self) -> impl Iterator { + match self { + StructItem(s) => s.fields.iter(), + UnionItem(u) => u.fields.iter(), + VariantItem(Variant { kind: VariantKind::Struct(v) }) => v.fields.iter(), + EnumItem(e) => e.variants.iter(), + TraitItem(t) => t.items.iter(), + ImplItem(i) => i.items.iter(), + ModuleItem(m) => m.items.iter(), + ExternCrateItem(_, _) + | ImportItem(_) + | FunctionItem(_) + | TypedefItem(_, _) + | OpaqueTyItem(_, _) + | StaticItem(_) + | ConstantItem(_) + | TraitAliasItem(_) + | TyMethodItem(_) + | MethodItem(_) + | StructFieldItem(_) + | VariantItem(_) + | ForeignFunctionItem(_) + | ForeignStaticItem(_) + | ForeignTypeItem + | MacroItem(_) + | ProcMacroItem(_) + | PrimitiveItem(_) + | AssocConstItem(_,_) + | AssocTypeItem(_,_) + | StrippedItem(_) + | KeywordItem(_) => [].iter(), + } + } + pub fn is_associated(&self) -> bool { match *self { ItemEnum::TypedefItem(_, _) | ItemEnum::AssocTypeItem(_, _) => true, diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 6855931def1e6..d0e38ceb8a023 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -1,3 +1,7 @@ +//! These from impls are used to create the JSON types which get serialized. They're very close to +//! the `clean` types but with some fields removed or stringified to simplify the output and not +//! expose unstable compiler internals. + use std::convert::From; use rustc_ast::ast; @@ -22,7 +26,7 @@ impl From for Item { deprecation, } = item; Item { - crate_num: def_id.krate.as_u32(), + crate_id: def_id.krate.as_u32(), name, source: source.into(), visibility: visibility.into(), @@ -329,14 +333,13 @@ impl From for Type { ResolvedPath { path, param_names, did, is_generic: _ } => Type::ResolvedPath { name: path.whole_name(), id: did.into(), - args: Box::new(path.segments.last().map(|args| args.clone().args.into())), + args: path.segments.last().map(|args| Box::new(args.clone().args.into())), param_names: param_names .map(|v| v.into_iter().map(Into::into).collect()) .unwrap_or_default(), }, Generic(s) => Type::Generic(s), Primitive(p) => Type::Primitive(p.as_str().to_string()), - // TODO: check if there's a more idiomatic way of calling `into` on Box BareFunction(f) => Type::FunctionPointer(Box::new((*f).into())), Tuple(t) => Type::Tuple(t.into_iter().map(Into::into).collect()), Slice(t) => Type::Slice(Box::new((*t).into())), diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 5f4034d8753de..de858ff79aa2a 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -1,8 +1,15 @@ +//! Rustdoc's JSON backend +//! +//! This module contains the logic for rendering a crate as JSON rather than the normal static HTML +//! output. See [the RFC](https://github.com/rust-lang/rfcs/pull/2963) and the [`types`] module +//! docs for usage and details. + mod conversions; mod types; use std::cell::RefCell; use std::fs::File; +use std::path::PathBuf; use std::rc::Rc; use rustc_data_structures::fx::FxHashMap; @@ -17,10 +24,17 @@ use crate::html::render::cache::ExternalLocation; #[derive(Clone)] pub struct JsonRenderer { + /// A mapping of IDs that contains all local items for this crate which gets output as a top + /// level field of the JSON blob. index: Rc>>, + /// The directory where the blob will be written to. + out_path: PathBuf, } impl JsonRenderer { + /// Inserts an item into the index. This should be used rather than directly calling insert on + /// the hashmap because certain items (traits and types) need to have their mappings for trait + /// implementations filled out before they're inserted. fn insert(&self, item: clean::Item, cache: &Cache) { let id = item.def_id; let mut new_item: types::Item = item.into(); @@ -75,33 +89,74 @@ impl JsonRenderer { }) .unwrap_or_default() } + + fn get_trait_items(&self, cache: &Cache) -> Vec<(types::Id, types::Item)> { + cache + .traits + .iter() + .filter_map(|(id, trait_item)| { + // only need to synthesize items for external traits + if !id.is_local() { + trait_item.items.clone().into_iter().for_each(|i| self.insert(i, cache)); + Some(( + (*id).into(), + types::Item { + crate_id: id.krate.as_u32(), + name: cache + .paths + .get(&id) + .unwrap_or_else(|| { + cache + .external_paths + .get(&id) + .expect("Trait should either be in local or external paths") + }) + .0 + .last() + .map(Clone::clone), + visibility: types::Visibility::Public, + kind: types::ItemKind::Trait, + inner: types::ItemEnum::TraitItem(trait_item.clone().into()), + source: None, + docs: Default::default(), + links: Default::default(), + attrs: Default::default(), + deprecation: Default::default(), + }, + )) + } else { + None + } + }) + .collect() + } } impl FormatRenderer for JsonRenderer { fn init( krate: clean::Crate, - _options: RenderOptions, + options: RenderOptions, _render_info: RenderInfo, _edition: Edition, _cache: &mut Cache, ) -> Result<(Self, clean::Crate), Error> { debug!("Initializing json renderer"); - Ok((JsonRenderer { index: Rc::new(RefCell::new(FxHashMap::default())) }, krate)) + Ok(( + JsonRenderer { + index: Rc::new(RefCell::new(FxHashMap::default())), + out_path: options.output, + }, + krate, + )) } fn item(&mut self, item: clean::Item, cache: &Cache) -> Result<(), Error> { use clean::ItemEnum::*; - // Flatten items that recursively store other items by putting their children in the index - match item.inner.clone() { - StructItem(s) => s.fields.into_iter().for_each(|i| self.insert(i, cache)), - UnionItem(u) => u.fields.into_iter().for_each(|i| self.insert(i, cache)), - VariantItem(clean::Variant { kind: clean::VariantKind::Struct(v) }) => { - v.fields.into_iter().for_each(|i| self.insert(i, cache)); - } - EnumItem(e) => e.variants.into_iter().for_each(|i| self.item(i, cache).unwrap()), - TraitItem(t) => t.items.into_iter().for_each(|i| self.insert(i, cache)), - ImplItem(i) => i.items.into_iter().for_each(|i| self.insert(i, cache)), - _ => {} + // Flatten items that recursively store other items by inserting them into the index + if let ModuleItem(_) = &item.inner { + // but ignore modules because we handle recursing into them separately + } else { + item.inner.inner_items().for_each(|i| self.item(i.clone(), cache).unwrap()) } self.insert(item.clone(), cache); Ok(()) @@ -124,41 +179,7 @@ impl FormatRenderer for JsonRenderer { fn after_krate(&mut self, krate: &clean::Crate, cache: &Cache) -> Result<(), Error> { debug!("Done with crate"); let mut index = (*self.index).clone().into_inner(); - let trait_items = cache.traits.iter().filter_map(|(id, trait_item)| { - // only need to synthesize items for external traits - if !id.is_local() { - trait_item.items.clone().into_iter().for_each(|i| self.insert(i, cache)); - Some(( - (*id).into(), - types::Item { - crate_num: id.krate.as_u32(), - name: cache - .paths - .get(&id) - .unwrap_or_else(|| { - cache - .external_paths - .get(&id) - .expect("Trait should either be in local or external paths") - }) - .0 - .last() - .map(Clone::clone), - visibility: types::Visibility::Public, - kind: types::ItemKind::Trait, - inner: types::ItemEnum::TraitItem(trait_item.clone().into()), - source: None, - docs: Default::default(), - links: Default::default(), - attrs: Default::default(), - deprecation: Default::default(), - }, - )) - } else { - None - } - }); - index.extend(trait_items); + index.extend(self.get_trait_items(cache)); let output = types::Crate { root: types::Id(String::from("0:0")), version: krate.version.clone(), @@ -172,7 +193,7 @@ impl FormatRenderer for JsonRenderer { .map(|(k, (path, kind))| { ( k.into(), - types::ItemSummary { crate_num: k.krate.as_u32(), path, kind: kind.into() }, + types::ItemSummary { crate_id: k.krate.as_u32(), path, kind: kind.into() }, ) }) .collect(), @@ -194,7 +215,10 @@ impl FormatRenderer for JsonRenderer { .collect(), format_version: 1, }; - serde_json::ser::to_writer_pretty(&File::create("test.json").unwrap(), &output).unwrap(); + let mut p = self.out_path.clone(); + p.push(output.index.get(&output.root).unwrap().name.clone().unwrap()); + p.set_extension("json"); + serde_json::ser::to_writer_pretty(&File::create(p).unwrap(), &output).unwrap(); Ok(()) } diff --git a/src/librustdoc/json/types.rs b/src/librustdoc/json/types.rs index f888d00b95158..b3db6ff33f24f 100644 --- a/src/librustdoc/json/types.rs +++ b/src/librustdoc/json/types.rs @@ -1,6 +1,6 @@ //! Rustdoc's JSON output interface //! -//! These types are the public API exposed through the `--output-format json` flag. The [`Crate`][] +//! These types are the public API exposed through the `--output-format json` flag. The [`Crate`] //! struct is the root of the JSON blob and all other items are contained within. use std::path::PathBuf; @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; /// tools to find or link to them. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Crate { - /// The id of the root [`Module`][] item of the local crate. + /// The id of the root [`Module`] item of the local crate. pub root: Id, /// The version string given to `--crate-version`, if any. pub version: Option, @@ -22,10 +22,9 @@ pub struct Crate { /// A collection of all items in the local crate as well as some external traits and their /// items that are referenced locally. pub index: FxHashMap, - /// Maps ids to fully qualified paths (e.g. `["std", "io", "lazy", "Lazy"]` for - /// `std::io::lazy::Lazy`) as well as their `ItemKind` + /// Maps IDs to fully qualified paths and other info helpful for generating links. pub paths: FxHashMap, - /// Maps `crate_num` of items to a crate name and html_root_url if it exists + /// Maps `crate_id` of items to a crate name and html_root_url if it exists. pub external_crates: FxHashMap, /// A single version number to be used in the future when making backwards incompatible changes /// to the JSON output. @@ -38,31 +37,36 @@ pub struct ExternalCrate { pub html_root_url: Option, } -/// For external items (stuff not defined in the local crate), you don't get the same level of +/// For external (not defined in the local crate) items, you don't get the same level of /// information. This struct should contain enough to generate a link/reference to the item in /// question, or can be used by a tool that takes the json output of multiple crates to find /// the actual item definition with all the relevant info. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct ItemSummary { - pub crate_num: u32, + /// Can be used to look up the name and html_root_url of the crate this item came from in the + /// `external_crates` map. + pub crate_id: u32, + /// The list of path components for the fully qualified path of this item (e.g. + /// `["std", "io", "lazy", "Lazy"]` for `std::io::lazy::Lazy`). pub path: Vec, + /// Whether this item is a struct, trait, macro, etc. pub kind: ItemKind, } #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Item { - /// This can be used as a key to the `external_crates` map of [`Crate`][] to see which crate + /// This can be used as a key to the `external_crates` map of [`Crate`] to see which crate /// this item came from. - pub crate_num: u32, + pub crate_id: u32, /// Some items such as impls don't have names. pub name: Option, - /// The source location of this item. May not be present if it came from a macro expansion, - /// inline assembly, other "virtual" files. + /// The source location of this item (absent if it came from a macro expansion or inline + /// assembly). pub source: Option, - /// Usually documented items are all public, but you can tell rustdoc to output private items + /// By default all documented items are public, but you can tell rustdoc to output private items /// so this field is needed to differentiate. pub visibility: Visibility, - /// The full docstring of this item. + /// The full markdown docstring of this item. pub docs: String, /// This mapping resolves [intradoc links](https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md) from the docstring to their IDs pub links: FxHashMap, @@ -71,10 +75,6 @@ pub struct Item { pub deprecation: Option, pub kind: ItemKind, pub inner: ItemEnum, - // TODO: should we stringify the cfg attrs as well, or should we preserve their structure so - // the consumer doesn't have to parse an arbitrarily nested tree to figure out what platforms - // the item is available on? - // TODO: should we have a "stability" field if it's only used by the standard library? } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -97,6 +97,8 @@ pub struct Deprecation { #[derive(Clone, Debug, Serialize, Deserialize)] pub enum Visibility { Public, + /// For the most part items are private by default. The exceptions are associated items of + /// public traits and variants of public enums. Default, Crate, // TODO: Restricted(Id, String), @@ -336,7 +338,7 @@ pub enum Type { ResolvedPath { name: String, id: Id, - args: Box>, + args: Option>, param_names: Vec, }, /// Parameterized types @@ -429,9 +431,6 @@ pub struct Impl { pub blanket_impl: Option, } -// TODO: this is currently broken because imports have the same ID as the module that contains -// them. The only obvious fix is to modify the clean types to renumber imports so that IDs are -// actually unique. #[serde(rename_all = "snake_case")] #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Import { diff --git a/src/test/run-make-fulldeps/rustdoc-json/Makefile b/src/test/run-make-fulldeps/rustdoc-json/Makefile new file mode 100644 index 0000000000000..b66bcb8035daa --- /dev/null +++ b/src/test/run-make-fulldeps/rustdoc-json/Makefile @@ -0,0 +1,5 @@ +-include ../tools.mk + +tests: *.rs + $(RUSTDOC) $< -o $(TMPDIR) --output-format json + $(PYTHON) check_missing_items.py $(TMPDIR)/$(basename $<).json diff --git a/src/test/run-make-fulldeps/rustdoc-json/check_missing_items.py b/src/test/run-make-fulldeps/rustdoc-json/check_missing_items.py new file mode 100644 index 0000000000000..224b8f82ca9cc --- /dev/null +++ b/src/test/run-make-fulldeps/rustdoc-json/check_missing_items.py @@ -0,0 +1,170 @@ +#!/usr/bin/env python + +import sys +import json + +crate = json.load(open(sys.argv[1])) + + +def get_local_item(item_id): + if item_id in crate["index"]: + return crate["index"][item_id] + print("Missing local ID:", item_id) + sys.exit(1) + + +# local IDs have to be in `index`, external ones can sometimes be in `index` but otherwise have +# to be in `paths` +def valid_id(item_id): + return item_id in crate["index"] or item_id[0] != "0" and item_id in crate["paths"] + + +def check_generics(generics): + for param in generics["params"]: + check_generic_param(param) + for where_predicate in generics["where_predicates"]: + if "bound_predicate" in where_predicate: + pred = where_predicate["bound_predicate"] + check_type(pred["ty"]) + for bound in pred["bounds"]: + check_generic_bound(bound) + elif "region_predicate" in where_predicate: + pred = where_predicate["region_predicate"] + for bound in pred["bounds"]: + check_generic_bound(bound) + elif "eq_predicate" in where_predicate: + pred = where_predicate["eq_predicate"] + check_type(pred["rhs"]) + check_type(pred["lhs"]) + + +def check_generic_param(param): + if "type" in param["kind"]: + ty = param["kind"]["type"] + if ty["default"]: + check_type(ty["default"]) + for bound in ty["bounds"]: + check_generic_bound(bound) + elif "const" in param["kind"]: + check_type(param["kind"]["const"]) + + +def check_generic_bound(bound): + if "trait_bound" in bound: + for param in bound["trait_bound"]["generic_params"]: + check_generic_param(param) + check_type(bound["trait_bound"]["trait"]) + + +def check_decl(decl): + for (_name, ty) in decl["inputs"]: + check_type(ty) + if decl["output"]: + check_type(decl["output"]) + + +def check_type(ty): + if ty["kind"] == "resolved_path": + for bound in ty["inner"]["param_names"]: + check_generic_bound(bound) + if args := ty["inner"]["args"]: + if "angle_bracketed" in args: + for arg in args["angle_bracketed"]["args"]: + if "type" in arg: + check_type(arg["type"]) + elif "const" in arg: + check_type(arg["const"]["type"]) + for binding in args["angle_bracketed"]["bindings"]: + if "equality" in binding["binding"]: + check_type(binding["binding"]["equality"]) + elif "constraint" in binding["binding"]: + for bound in binding["binding"]["constraint"]: + check_generic_bound(bound) + elif "parenthesized" in args: + for ty in args["parenthesized"]["inputs"]: + check_type(ty) + if args["parenthesized"]["output"]: + ckeck_ty(args["parenthesized"]["output"]) + if not valid_id(ty["inner"]["id"]): + print("Type contained an invalid ID:", ty["inner"]["id"]) + sys.exit(1) + elif ty["kind"] == "tuple": + for ty in ty["inner"]: + check_type(ty) + elif ty["kind"] == "slice": + check_type(ty["inner"]) + elif ty["kind"] == "impl_trait": + for bound in ty["inner"]: + check_generic_bound(bound) + elif ty["kind"] in ("raw_pointer", "borrowed_ref", "array"): + check_type(ty["inner"]["type"]) + elif ty["kind"] == "function_pointer": + for param in ty["inner"]["generic_params"]: + check_generic_param(param) + check_decl(ty["inner"]["inner"]) + elif ty["kind"] == "qualified_path": + check_type(ty["inner"]["self_type"]) + check_type(ty["inner"]["trait"]) + + +work_list = set([crate["root"]]) +visited = work_list.copy() + +while work_list: + current = work_list.pop() + visited.add(current) + item = get_local_item(current) + # check intradoc links + work_list |= set(item["links"].values()) - visited + + # check all fields that reference types such as generics as well as nested items + # (modules, structs, traits, and enums) + if item["kind"] == "module": + work_list |= set(item["inner"]["items"]) - visited + elif item["kind"] == "struct": + check_generics(item["inner"]["generics"]) + work_list |= (set(item["inner"]["fields"]) | set(item["inner"]["impls"])) - visited + elif item["kind"] == "struct_field": + check_type(item["inner"]) + elif item["kind"] == "enum": + check_generics(item["inner"]["generics"]) + work_list |= (set(item["inner"]["variants"]) | set(item["inner"]["impls"])) - visited + elif item["kind"] == "variant": + if item["inner"]["variant_kind"] == "tuple": + for ty in item["inner"]["variant_inner"]: + check_type(ty) + elif item["inner"]["variant_kind"] == "struct": + work_list |= set(item["inner"]["variant_inner"]) - visited + elif item["kind"] in ("function", "method"): + check_generics(item["inner"]["generics"]) + check_decl(item["inner"]["decl"]) + elif item["kind"] in ("static", "constant", "assoc_const"): + check_type(item["inner"]["type"]) + elif item["kind"] == "typedef": + check_type(item["inner"]["type"]) + check_generics(item["inner"]["generics"]) + elif item["kind"] in ("opaque_ty", "trait_alias"): + check_generics(item["inner"]["generics"]) + for bound in item["inner"]["bounds"]: + check_generic_bound(bound) + elif item["kind"] == "trait": + check_generics(item["inner"]["generics"]) + for bound in item["inner"]["bounds"]: + check_generic_bound(bound) + work_list |= (set(item["inner"]["items"]) | set(item["inner"]["implementors"])) - visited + elif item["kind"] == "impl": + check_generics(item["inner"]["generics"]) + if item["inner"]["trait"]: + check_type(item["inner"]["trait"]) + if item["inner"]["blanket_impl"]: + check_type(item["inner"]["blanket_impl"]) + check_type(item["inner"]["for"]) + for assoc_item in item["inner"]["items"]: + if not valid_id(assoc_item): + print("Impl block referenced a missing ID:", assoc_item) + sys.exit(1) + elif item["kind"] == "assoc_type": + for bound in item["inner"]["bounds"]: + check_generic_bound(bound) + if item["inner"]["default"]: + check_type(item["inner"]["default"]) diff --git a/src/test/run-make-fulldeps/rustdoc-json/test.rs b/src/test/run-make-fulldeps/rustdoc-json/test.rs new file mode 100644 index 0000000000000..e467f0d4cccc2 --- /dev/null +++ b/src/test/run-make-fulldeps/rustdoc-json/test.rs @@ -0,0 +1,17 @@ +use std::collections::HashMap; + +pub struct Normal {} + +pub struct Tuple(); + +pub struct Unit; + +pub struct WithPrimitives<'a> { + num: u32, + s: &'a str, +} + +pub struct WithGenerics { + stuff: Vec, + things: HashMap, +}