Skip to content

Rustdoc-Json: Don't inline foreign traits #105182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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") }]);
}