Skip to content

Commit

Permalink
Rollup merge of #135348 - aDotInTheVoid:pathspathspaths, r=GuillaumeG…
Browse files Browse the repository at this point in the history
…omez

rustdoc-json: Include items in stripped modules in `Crate::paths`.

Closes #135309

When we're running rustdoc-json, we should err on the side of adding more items to `Cache::paths`, as that directly becomes `Crate::paths` in the output.

r? ``@GuillaumeGomez.`` Best reviewed commit-by-commit.
  • Loading branch information
GuillaumeGomez authored Jan 12, 2025
2 parents 627513a + 2a2b090 commit 9d3ae11
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 13 deletions.
23 changes: 15 additions & 8 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ struct CacheBuilder<'a, 'tcx> {
/// This field is used to prevent duplicated impl blocks.
impl_ids: DefIdMap<DefIdSet>,
tcx: TyCtxt<'tcx>,
is_json_output: bool,
}

impl Cache {
Expand Down Expand Up @@ -184,8 +185,13 @@ impl Cache {
}

let (krate, mut impl_ids) = {
let mut cache_builder =
CacheBuilder { tcx, cache: &mut cx.cache, impl_ids: Default::default() };
let is_json_output = cx.is_json_output();
let mut cache_builder = CacheBuilder {
tcx,
cache: &mut cx.cache,
impl_ids: Default::default(),
is_json_output,
};
krate = cache_builder.fold_crate(krate);
(krate, cache_builder.impl_ids)
};
Expand Down Expand Up @@ -307,12 +313,13 @@ impl DocFolder for CacheBuilder<'_, '_> {
| clean::ProcMacroItem(..)
| clean::VariantItem(..) => {
use rustc_data_structures::fx::IndexEntry as Entry;
if !self.cache.stripped_mod
&& !matches!(
item.stability.map(|stab| stab.level),
Some(StabilityLevel::Stable { allowed_through_unstable_modules: true, .. })
)
{

let skip_because_unstable = matches!(
item.stability.map(|stab| stab.level),
Some(StabilityLevel::Stable { allowed_through_unstable_modules: true, .. })
);

if (!self.cache.stripped_mod && !skip_because_unstable) || self.is_json_output {
// Re-exported items mean that the same id can show up twice
// in the rustdoc ast that we're looking at. We know,
// however, that a re-exported item doesn't show up in the
Expand Down
27 changes: 23 additions & 4 deletions src/tools/jsondocck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ enum CommandKind {
/// Checks the path doesn't exist.
HasNotPath,

/// `//@ !has <path> <value>`
///
/// Checks the path exists, but doesn't have the given value.
HasNotValue { value: String },

/// `//@ is <path> <value>`
///
/// Check the path is the given value.
Expand Down Expand Up @@ -128,10 +133,11 @@ impl CommandKind {
[_path, value] => Self::HasValue { value: value.clone() },
_ => panic!("`//@ has` must have 2 or 3 arguments, but got {args:?}"),
},
("has", true) => {
assert_eq!(args.len(), 1, "args={args:?}");
Self::HasNotPath
}
("has", true) => match args {
[_path] => Self::HasNotPath,
[_path, value] => Self::HasNotValue { value: value.clone() },
_ => panic!("`//@ !has` must have 2 or 3 arguments, but got {args:?}"),
},

(_, false) if KNOWN_DIRECTIVE_NAMES.contains(&command_name) => {
return None;
Expand Down Expand Up @@ -223,6 +229,19 @@ fn check_command(command: &Command, cache: &mut Cache) -> Result<(), String> {
return Err(format!("matched to {matches:?}, which didn't contain {want_value:?}"));
}
}
CommandKind::HasNotValue { value } => {
let wantnt_value = string_to_value(value, cache);
if matches.contains(&wantnt_value.as_ref()) {
return Err(format!(
"matched to {matches:?}, which contains unwanted {wantnt_value:?}"
));
} else if matches.is_empty() {
return Err(format!(
"got no matches, but expected some matched (not containing {wantnt_value:?}"
));
}
}

CommandKind::Is { value } => {
let want_value = string_to_value(value, cache);
let matched = get_one(&matches)?;
Expand Down
6 changes: 6 additions & 0 deletions src/tools/jsondoclint/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ impl<'a> Validator<'a> {
PathKind::Trait => self.add_trait_or_alias_id(&x.id),
PathKind::Type => self.add_type_id(&x.id),
}

// FIXME: More robust support for checking things in $.index also exist in $.paths
if !self.krate.paths.contains_key(&x.id) {
self.fail(&x.id, ErrorKind::Custom(format!("No entry in '$.paths' for {x:?}")));
}

if let Some(args) = &x.args {
self.check_generic_args(&**args);
}
Expand Down
97 changes: 96 additions & 1 deletion src/tools/jsondoclint/src/validator/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_hash::FxHashMap;
use rustdoc_json_types::{FORMAT_VERSION, Item, ItemKind, Visibility};
use rustdoc_json_types::{Abi, FORMAT_VERSION, FunctionHeader, Item, ItemKind, Visibility};

use super::*;
use crate::json_find::SelectorPart;
Expand Down Expand Up @@ -102,6 +102,101 @@ fn errors_on_local_in_paths_and_not_index() {
}]);
}

#[test]
fn errors_on_missing_path() {
// crate-name=foo
// ```
// pub struct Bar;
// pub fn mk_bar() -> Bar { ... }
// ```

let generics = Generics { params: vec![], where_predicates: vec![] };

let krate = Crate {
root: Id(0),
crate_version: None,
includes_private: false,
index: FxHashMap::from_iter([
(Id(0), Item {
id: Id(0),
crate_id: 0,
name: Some("foo".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: FxHashMap::default(),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Module(Module {
is_crate: true,
items: vec![Id(1), Id(2)],
is_stripped: false,
}),
}),
(Id(1), Item {
id: Id(0),
crate_id: 0,
name: Some("Bar".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: FxHashMap::default(),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Struct(Struct {
kind: StructKind::Unit,
generics: generics.clone(),
impls: vec![],
}),
}),
(Id(2), Item {
id: Id(0),
crate_id: 0,
name: Some("mk_bar".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: FxHashMap::default(),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Function(Function {
sig: FunctionSignature {
inputs: vec![],
output: Some(Type::ResolvedPath(Path {
name: "Bar".to_owned(),
id: Id(1),
args: None,
})),
is_c_variadic: false,
},
generics,
header: FunctionHeader {
is_const: false,
is_unsafe: false,
is_async: false,
abi: Abi::Rust,
},
has_body: true,
}),
}),
]),
paths: FxHashMap::from_iter([(Id(0), ItemSummary {
crate_id: 0,
path: vec!["foo".to_owned()],
kind: ItemKind::Module,
})]),
external_crates: FxHashMap::default(),
format_version: rustdoc_json_types::FORMAT_VERSION,
};

check(&krate, &[Error {
kind: ErrorKind::Custom(
r#"No entry in '$.paths' for Path { name: "Bar", id: Id(1), args: None }"#.to_owned(),
),
id: Id(1),
}]);
}

#[test]
#[should_panic = "LOCAL_CRATE_ID is wrong"]
fn checks_local_crate_id_is_correct() {
Expand Down
6 changes: 6 additions & 0 deletions tests/rustdoc-json/reexport/simple_private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ mod inner {
pub use inner::Public;

//@ ismany "$.index[*][?(@.name=='simple_private')].inner.module.items[*]" $use_id

// Test for https://github.com/rust-lang/rust/issues/135309
//@ has "$.paths[*][?(@.kind=='module')].path" '["simple_private"]'
//@ !has "$.paths[*].path" '["simple_private", "inner"]'
//@ has "$.paths[*][?(@.kind=='struct')].path" '["simple_private", "inner", "Public"]'
//@ !has "$.paths[*].path" '["simple_private", "Public"]'
5 changes: 5 additions & 0 deletions tests/rustdoc-json/reexport/simple_public.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,8 @@ pub mod inner {
pub use inner::Public;

//@ ismany "$.index[*][?(@.name=='simple_public')].inner.module.items[*]" $import_id $inner_id

//@ has "$.paths[*][?(@.kind=='module')].path" '["simple_public"]'
//@ has "$.paths[*][?(@.kind=='module')].path" '["simple_public", "inner"]'
//@ has "$.paths[*][?(@.kind=='struct')].path" '["simple_public", "inner", "Public"]'
//@ !has "$.paths[*].path" '["simple_public", "Public"]'

0 comments on commit 9d3ae11

Please sign in to comment.