From 77e729ea92fde42e407c30325a379fcbd47f51eb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 31 May 2022 21:47:19 +0200 Subject: [PATCH 1/2] Add Symbol into rustdoc JSON ID to prevent conflicts between reexports --- src/librustdoc/json/conversions.rs | 77 +++++++++++++++++++----------- src/librustdoc/json/mod.rs | 24 ++++++---- 2 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 6a8e4787676e1..51a2abc50bc2b 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -10,7 +10,7 @@ use std::fmt; use rustc_ast::ast; use rustc_hir::{def::CtorKind, def_id::DefId}; use rustc_middle::ty::{self, TyCtxt}; -use rustc_span::Pos; +use rustc_span::{Pos, Symbol}; use rustc_target::spec::abi::Abi as RustcAbi; use rustdoc_json_types::*; @@ -29,7 +29,9 @@ impl JsonRenderer<'_> { .get(&item.item_id) .into_iter() .flatten() - .map(|clean::ItemLink { link, did, .. }| (link.clone(), from_item_id((*did).into()))) + .map(|clean::ItemLink { link, did, .. }| { + (link.clone(), from_item_id((*did).into(), self.tcx)) + }) .collect(); let docs = item.attrs.collapsed_doc_value(); let attrs = item @@ -45,7 +47,7 @@ impl JsonRenderer<'_> { _ => from_clean_item(item, self.tcx), }; Some(Item { - id: from_item_id(item_id), + id: from_item_id_with_name(item_id, self.tcx, name), crate_id: item_id.krate().as_u32(), name: name.map(|sym| sym.to_string()), span: self.convert_span(span), @@ -84,7 +86,7 @@ impl JsonRenderer<'_> { Inherited => Visibility::Default, Restricted(did) if did.is_crate_root() => Visibility::Crate, Restricted(did) => Visibility::Restricted { - parent: from_item_id(did.into()), + parent: from_item_id(did.into(), self.tcx), path: self.tcx.def_path(did).to_string_no_crate_verbose(), }, } @@ -173,22 +175,39 @@ impl FromWithTcx for TypeBindingKind { } } -pub(crate) fn from_item_id(item_id: ItemId) -> Id { - struct DisplayDefId(DefId); +/// It generates an ID as follows: +/// +/// `CRATE_ID:ITEM_ID[:NAME_ID]` (if there is no name, NAME_ID is not generated). +pub(crate) fn from_item_id(item_id: ItemId, tcx: TyCtxt<'_>) -> Id { + from_item_id_with_name(item_id, tcx, None) +} + +// FIXME: this function (and appending the name at the end of the ID) should be removed when +// reexports are not inlined anymore for json format. It should be done in #93518. +pub(crate) fn from_item_id_with_name(item_id: ItemId, tcx: TyCtxt<'_>, name: Option) -> Id { + struct DisplayDefId<'a>(DefId, TyCtxt<'a>, Option); - impl fmt::Display for DisplayDefId { + impl<'a> fmt::Display for DisplayDefId<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}:{}", self.0.krate.as_u32(), u32::from(self.0.index)) + let name = match self.2 { + Some(name) => format!(":{}", name.as_u32()), + None => self + .1 + .opt_item_name(self.0) + .map(|n| format!(":{}", n.as_u32())) + .unwrap_or_default(), + }; + write!(f, "{}:{}{}", self.0.krate.as_u32(), u32::from(self.0.index), name) } } match item_id { - ItemId::DefId(did) => Id(format!("{}", DisplayDefId(did))), + ItemId::DefId(did) => Id(format!("{}", DisplayDefId(did, tcx, name))), ItemId::Blanket { for_, impl_id } => { - Id(format!("b:{}-{}", DisplayDefId(impl_id), DisplayDefId(for_))) + Id(format!("b:{}-{}", DisplayDefId(impl_id, tcx, None), DisplayDefId(for_, tcx, name))) } ItemId::Auto { for_, trait_ } => { - Id(format!("a:{}-{}", DisplayDefId(trait_), DisplayDefId(for_))) + Id(format!("a:{}-{}", DisplayDefId(trait_, tcx, None), DisplayDefId(for_, tcx, name))) } ItemId::Primitive(ty, krate) => Id(format!("p:{}:{}", krate.as_u32(), ty.as_sym())), } @@ -201,7 +220,7 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum { let header = item.fn_header(tcx); match *item.kind { - ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items) }), + ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items, tcx) }), ImportItem(i) => ItemEnum::Import(i.into_tcx(tcx)), StructItem(s) => ItemEnum::Struct(s.into_tcx(tcx)), UnionItem(u) => ItemEnum::Union(u.into_tcx(tcx)), @@ -255,7 +274,7 @@ impl FromWithTcx for Struct { struct_type: from_ctor_kind(struct_type), generics: generics.into_tcx(tcx), fields_stripped, - fields: ids(fields), + fields: ids(fields, tcx), impls: Vec::new(), // Added in JsonRenderer::item } } @@ -268,7 +287,7 @@ impl FromWithTcx for Union { Union { generics: generics.into_tcx(tcx), fields_stripped, - fields: ids(fields), + fields: ids(fields, tcx), impls: Vec::new(), // Added in JsonRenderer::item } } @@ -413,7 +432,7 @@ impl FromWithTcx for Type { match ty { clean::Type::Path { path } => Type::ResolvedPath { name: path.whole_name(), - id: from_item_id(path.def_id().into()), + id: from_item_id(path.def_id().into(), tcx), args: path.segments.last().map(|args| Box::new(args.clone().args.into_tcx(tcx))), param_names: Vec::new(), }, @@ -422,7 +441,7 @@ impl FromWithTcx for Type { Type::ResolvedPath { name: first_trait.whole_name(), - id: from_item_id(first_trait.def_id().into()), + id: from_item_id(first_trait.def_id().into(), tcx), args: first_trait .segments .last() @@ -517,7 +536,7 @@ impl FromWithTcx for Trait { Trait { is_auto, is_unsafe: unsafety == rustc_hir::Unsafety::Unsafe, - items: ids(items), + items: ids(items, tcx), generics: generics.into_tcx(tcx), bounds: bounds.into_iter().map(|x| x.into_tcx(tcx)).collect(), implementations: Vec::new(), // Added in JsonRenderer::item @@ -550,7 +569,7 @@ impl FromWithTcx for Impl { .collect(), trait_, for_: for_.into_tcx(tcx), - items: ids(items), + items: ids(items, tcx), negative: negative_polarity, synthetic, blanket_impl: blanket_impl.map(|x| x.into_tcx(tcx)), @@ -593,21 +612,21 @@ impl FromWithTcx for Enum { Enum { generics: generics.into_tcx(tcx), variants_stripped, - variants: ids(variants), + variants: ids(variants, tcx), impls: Vec::new(), // Added in JsonRenderer::item } } } impl FromWithTcx for Struct { - fn from_tcx(struct_: clean::VariantStruct, _tcx: TyCtxt<'_>) -> Self { + fn from_tcx(struct_: clean::VariantStruct, tcx: TyCtxt<'_>) -> Self { let fields_stripped = struct_.has_stripped_entries(); let clean::VariantStruct { struct_type, fields } = struct_; Struct { struct_type: from_ctor_kind(struct_type), generics: Default::default(), fields_stripped, - fields: ids(fields), + fields: ids(fields, tcx), impls: Vec::new(), } } @@ -630,25 +649,25 @@ impl FromWithTcx for Variant { }) .collect(), ), - Struct(s) => Variant::Struct(ids(s.fields)), + Struct(s) => Variant::Struct(ids(s.fields, tcx)), } } } impl FromWithTcx for Import { - fn from_tcx(import: clean::Import, _tcx: TyCtxt<'_>) -> Self { + fn from_tcx(import: clean::Import, tcx: TyCtxt<'_>) -> Self { use clean::ImportKind::*; match import.kind { Simple(s) => Import { source: import.source.path.whole_name(), name: s.to_string(), - id: import.source.did.map(ItemId::from).map(from_item_id), + id: import.source.did.map(ItemId::from).map(|i| from_item_id(i, tcx)), glob: false, }, Glob => Import { source: import.source.path.whole_name(), name: import.source.path.last().to_string(), - id: import.source.did.map(ItemId::from).map(from_item_id), + id: import.source.did.map(ItemId::from).map(|i| from_item_id(i, tcx)), glob: true, }, } @@ -742,6 +761,10 @@ impl FromWithTcx for ItemKind { } } -fn ids(items: impl IntoIterator) -> Vec { - items.into_iter().filter(|x| !x.is_stripped()).map(|i| from_item_id(i.item_id)).collect() +fn ids(items: impl IntoIterator, tcx: TyCtxt<'_>) -> Vec { + items + .into_iter() + .filter(|x| !x.is_stripped()) + .map(|i| from_item_id_with_name(i.item_id, tcx, i.name)) + .collect() } diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 08f61056d853f..f338050bee0f9 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -16,6 +16,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def_id::DefId; use rustc_middle::ty::TyCtxt; use rustc_session::Session; +use rustc_span::def_id::LOCAL_CRATE; use rustdoc_json_types as types; @@ -25,7 +26,7 @@ use crate::docfs::PathError; use crate::error::Error; use crate::formats::cache::Cache; use crate::formats::FormatRenderer; -use crate::json::conversions::{from_item_id, IntoWithTcx}; +use crate::json::conversions::{from_item_id, from_item_id_with_name, IntoWithTcx}; use crate::{clean, try_err}; #[derive(Clone)] @@ -54,7 +55,7 @@ impl<'tcx> JsonRenderer<'tcx> { .map(|i| { let item = &i.impl_item; self.item(item.clone()).unwrap(); - from_item_id(item.item_id) + from_item_id_with_name(item.item_id, self.tcx, item.name) }) .collect() }) @@ -86,7 +87,7 @@ impl<'tcx> JsonRenderer<'tcx> { if item.item_id.is_local() || is_primitive_impl { self.item(item.clone()).unwrap(); - Some(from_item_id(item.item_id)) + Some(from_item_id_with_name(item.item_id, self.tcx, item.name)) } else { None } @@ -105,10 +106,11 @@ impl<'tcx> JsonRenderer<'tcx> { if !id.is_local() { let trait_item = &trait_item.trait_; trait_item.items.clone().into_iter().for_each(|i| self.item(i).unwrap()); + let item_id = from_item_id(id.into(), self.tcx); Some(( - from_item_id(id.into()), + item_id.clone(), types::Item { - id: from_item_id(id.into()), + id: item_id, crate_id: id.krate.as_u32(), name: self .cache @@ -176,6 +178,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { // Flatten items that recursively store other items item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap()); + let name = item.name; let item_id = item.item_id; if let Some(mut new_item) = self.convert_item(item) { if let types::ItemEnum::Trait(ref mut t) = new_item.inner { @@ -187,7 +190,10 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { } else if let types::ItemEnum::Union(ref mut u) = new_item.inner { u.impls = self.get_impls(item_id.expect_def_id()) } - let removed = self.index.borrow_mut().insert(from_item_id(item_id), new_item.clone()); + let removed = self + .index + .borrow_mut() + .insert(from_item_id_with_name(item_id, self.tcx, name), new_item.clone()); // FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check // to make sure the items are unique. The main place this happens is when an item, is @@ -211,13 +217,15 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { self.get_impls(*primitive); } + let e = ExternalCrate { crate_num: LOCAL_CRATE }; + let mut index = (*self.index).clone().into_inner(); index.extend(self.get_trait_items()); // This needs to be the default HashMap for compatibility with the public interface for // rustdoc-json-types #[allow(rustc::default_hash_types)] let output = types::Crate { - root: types::Id(String::from("0:0")), + root: types::Id(format!("0:0:{}", e.name(self.tcx).as_u32())), crate_version: self.cache.crate_version.clone(), includes_private: self.cache.document_private, index: index.into_iter().collect(), @@ -229,7 +237,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { .chain(self.cache.external_paths.clone().into_iter()) .map(|(k, (path, kind))| { ( - from_item_id(k.into()), + from_item_id(k.into(), self.tcx), types::ItemSummary { crate_id: k.krate.as_u32(), path: path.iter().map(|s| s.to_string()).collect(), From 5adca7305e327de66ac389e7fe32964f5ba8c4bf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 31 May 2022 22:45:31 +0200 Subject: [PATCH 2/2] Add regression test for json reexport bug --- .../same_type_reexported_more_than_once.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs diff --git a/src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs b/src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs new file mode 100644 index 0000000000000..fd6ac8372d976 --- /dev/null +++ b/src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs @@ -0,0 +1,17 @@ +// Regression test for https://github.com/rust-lang/rust/issues/97432. + +#![feature(no_core)] +#![no_std] +#![no_core] + +// @has same_type_reexported_more_than_once.json +// @set trait_id = - "$.index[*][?(@.name=='Trait')].id" +// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $trait_id +pub use inner::Trait; +// @set reexport_id = - "$.index[*][?(@.name=='Reexport')].id" +// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $reexport_id +pub use inner::Trait as Reexport; + +mod inner { + pub trait Trait {} +}