Skip to content

Commit

Permalink
Rollup merge of #105182 - aDotInTheVoid:rdj-no-foreign-traits, r=Ense…
Browse files Browse the repository at this point in the history
…lic,GuillaumeGomez

Rustdoc-Json: Don't inline foreign traits

It wasn't done correctly, and [we want to move towards only having local items in the index, and making foreign items easier to resolved](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Rustdoc.20JSON.3A.20Include.20All.20Foreign.20Items.3F)

Fixes #105025. This means #105015 is included to test this

Fixes #105022

r? `@GuillaumeGomez`
  • Loading branch information
Yuki Okushi authored Dec 3, 2022
2 parents 8f36866 + 79d897b commit 9afa850
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 61 deletions.
53 changes: 1 addition & 52 deletions src/librustdoc/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,53 +99,6 @@ impl<'tcx> JsonRenderer<'tcx> {
})
.unwrap_or_default()
}

fn get_trait_items(&mut self) -> Vec<(types::Id, types::Item)> {
debug!("Adding foreign trait items");
Rc::clone(&self.cache)
.traits
.iter()
.filter_map(|(&id, trait_item)| {
// only need to synthesize items for external traits
if !id.is_local() {
for item in &trait_item.items {
trace!("Adding subitem to {id:?}: {:?}", item.item_id);
self.item(item.clone()).unwrap();
}
let item_id = from_item_id(id.into(), self.tcx);
Some((
item_id.clone(),
types::Item {
id: item_id,
crate_id: id.krate.as_u32(),
name: self
.cache
.paths
.get(&id)
.unwrap_or_else(|| {
self.cache
.external_paths
.get(&id)
.expect("Trait should either be in local or external paths")
})
.0
.last()
.map(|s| s.to_string()),
visibility: types::Visibility::Public,
inner: types::ItemEnum::Trait(trait_item.clone().into_tcx(self.tcx)),
span: None,
docs: Default::default(),
links: Default::default(),
attrs: Default::default(),
deprecation: Default::default(),
},
))
} else {
None
}
})
.collect()
}
}

impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
Expand Down Expand Up @@ -276,11 +229,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {

let e = ExternalCrate { crate_num: LOCAL_CRATE };

// FIXME(adotinthevoid): Remove this, as it's not consistent with not
// inlining foreign items.
let foreign_trait_items = self.get_trait_items();
let mut index = (*self.index).clone().into_inner();
index.extend(foreign_trait_items);
let index = (*self.index).clone().into_inner();

debug!("Constructing Output");
// This needs to be the default HashMap for compatibility with the public interface for
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
pub trait Trait {
/// [`Enum::Variant`]
fn method() {}
}

pub enum Enum {
Variant,
}
13 changes: 13 additions & 0 deletions src/test/rustdoc-json/intra-doc-links/foreign_variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Regression test for <https://github.com/rust-lang/rust/issues/105025>
// aux-build: enum_variant_in_trait_method.rs

extern crate enum_variant_in_trait_method;

pub struct Local;

/// local impl
impl enum_variant_in_trait_method::Trait for Local {}

// @!has "$.index[*][?(@.name == 'Trait')]"
// @!has "$.index[*][?(@.name == 'method')]"
// @count "$.index[*][?(@.docs == 'local impl')].inner.items[*]" 0
2 changes: 2 additions & 0 deletions src/test/rustdoc-json/reexport/auxiliary/trait_with_docs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/// The Docs
pub trait HasDocs {}
10 changes: 10 additions & 0 deletions src/test/rustdoc-json/reexport/synthesize_trait_with_docs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Regression test for <https://github.com/rust-lang/rust/issues/105022>
// aux-build: trait_with_docs.rs

extern crate trait_with_docs;

pub struct Local;

impl trait_with_docs::HasDocs for Local {}

// @!has "$.index[*][?(@.name == 'HasDocs')]"
11 changes: 2 additions & 9 deletions src/test/rustdoc-json/traits/uses_extern_trait.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
#![no_std]
pub fn drop_default<T: core::default::Default>(_x: T) {}

// FIXME(adotinthevoid): Theses shouldn't be here
// @has "$.index[*][?(@.name=='Debug')]"

// Debug may have several items. All we depend on here the that `fmt` is first. See
// https://github.com/rust-lang/rust/pull/104525#issuecomment-1331087852 for why we
// can't use [*].

// @set Debug_fmt = "$.index[*][?(@.name=='Debug')].inner.items[0]"
// @has "$.index[*][?(@.name=='fmt')].id" $Debug_fmt
// @!has "$.index[*][?(@.name=='Debug')]"
// @!has "$.index[*][?(@.name=='Default')]"
9 changes: 9 additions & 0 deletions src/tools/jsondoclint/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ impl<'a> Validator<'a> {

fn check_item(&mut self, id: &'a Id) {
if let Some(item) = &self.krate.index.get(id) {
item.links.values().for_each(|id| self.add_any_id(id));

match &item.inner {
ItemEnum::Import(x) => self.check_import(x),
ItemEnum::Union(x) => self.check_union(x),
Expand Down Expand Up @@ -376,6 +378,10 @@ impl<'a> Validator<'a> {
}
}

fn add_any_id(&mut self, id: &'a Id) {
self.add_id_checked(id, |_| true, "any kind of item");
}

fn add_field_id(&mut self, id: &'a Id) {
self.add_id_checked(id, Kind::is_struct_field, "StructField");
}
Expand Down Expand Up @@ -446,3 +452,6 @@ fn set_remove<T: Hash + Eq + Clone>(set: &mut HashSet<T>) -> Option<T> {
None
}
}

#[cfg(test)]
mod tests;
50 changes: 50 additions & 0 deletions src/tools/jsondoclint/src/validator/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use std::collections::HashMap;

use rustdoc_json_types::{Crate, Item, Visibility};

use super::*;

#[track_caller]
fn check(krate: &Crate, errs: &[Error]) {
let mut validator = Validator::new(krate);
validator.check_crate();

assert_eq!(errs, &validator.errs[..]);
}

fn id(s: &str) -> Id {
Id(s.to_owned())
}

#[test]
fn errors_on_missing_links() {
let k = Crate {
root: id("0"),
crate_version: None,
includes_private: false,
index: HashMap::from_iter([(
id("0"),
Item {
name: Some("root".to_owned()),
id: id(""),
crate_id: 0,
span: None,
visibility: Visibility::Public,
docs: None,
links: HashMap::from_iter([("Not Found".to_owned(), id("1"))]),
attrs: vec![],
deprecation: None,
inner: ItemEnum::Module(Module {
is_crate: true,
items: vec![],
is_stripped: false,
}),
},
)]),
paths: HashMap::new(),
external_crates: HashMap::new(),
format_version: rustdoc_json_types::FORMAT_VERSION,
};

check(&k, &[Error { kind: ErrorKind::NotFound, id: id("1") }]);
}

0 comments on commit 9afa850

Please sign in to comment.