Skip to content

Commit 7997ff6

Browse files
author
Yuki Okushi
authored
Rollup merge of #106546 - aDotInTheVoid:jsondoclint-path-local-item, r=notriddle
jsondoclint: Check local items in `paths` are also in `index`. Would have caught #104064 (if core.json was linted in CI). Closes #106433. r? rustdoc
2 parents 3edc7e0 + d4139b3 commit 7997ff6

File tree

2 files changed

+125
-4
lines changed

2 files changed

+125
-4
lines changed

src/tools/jsondoclint/src/validator.rs

+26-3
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@ use std::hash::Hash;
33

44
use rustdoc_json_types::{
55
Constant, Crate, DynTrait, Enum, FnDecl, Function, FunctionPointer, GenericArg, GenericArgs,
6-
GenericBound, GenericParamDef, Generics, Id, Impl, Import, ItemEnum, Module, OpaqueTy, Path,
7-
Primitive, ProcMacro, Static, Struct, StructKind, Term, Trait, TraitAlias, Type, TypeBinding,
8-
TypeBindingKind, Typedef, Union, Variant, VariantKind, WherePredicate,
6+
GenericBound, GenericParamDef, Generics, Id, Impl, Import, ItemEnum, ItemSummary, Module,
7+
OpaqueTy, Path, Primitive, ProcMacro, Static, Struct, StructKind, Term, Trait, TraitAlias,
8+
Type, TypeBinding, TypeBindingKind, Typedef, Union, Variant, VariantKind, WherePredicate,
99
};
1010
use serde_json::Value;
1111

1212
use crate::{item_kind::Kind, json_find, Error, ErrorKind};
1313

14+
// This is a rustc implementation detail that we rely on here
15+
const LOCAL_CRATE_ID: u32 = 0;
16+
1417
/// The Validator walks over the JSON tree, and ensures it is well formed.
1518
/// It is made of several parts.
1619
///
@@ -53,12 +56,19 @@ impl<'a> Validator<'a> {
5356
}
5457

5558
pub fn check_crate(&mut self) {
59+
// Graph traverse the index
5660
let root = &self.krate.root;
5761
self.add_mod_id(root);
5862
while let Some(id) = set_remove(&mut self.todo) {
5963
self.seen_ids.insert(id);
6064
self.check_item(id);
6165
}
66+
67+
let root_crate_id = self.krate.index[root].crate_id;
68+
assert_eq!(root_crate_id, LOCAL_CRATE_ID, "LOCAL_CRATE_ID is wrong");
69+
for (id, item_info) in &self.krate.paths {
70+
self.check_item_info(id, item_info);
71+
}
6272
}
6373

6474
fn check_item(&mut self, id: &'a Id) {
@@ -364,6 +374,19 @@ impl<'a> Validator<'a> {
364374
fp.generic_params.iter().for_each(|gpd| self.check_generic_param_def(gpd));
365375
}
366376

377+
fn check_item_info(&mut self, id: &Id, item_info: &ItemSummary) {
378+
// FIXME: Their should be a better way to determine if an item is local, rather than relying on `LOCAL_CRATE_ID`,
379+
// which encodes rustc implementation details.
380+
if item_info.crate_id == LOCAL_CRATE_ID && !self.krate.index.contains_key(id) {
381+
self.errs.push(Error {
382+
id: id.clone(),
383+
kind: ErrorKind::Custom(
384+
"Id for local item in `paths` but not in `index`".to_owned(),
385+
),
386+
})
387+
}
388+
}
389+
367390
fn add_id_checked(&mut self, id: &'a Id, valid: fn(Kind) -> bool, expected: &str) {
368391
if let Some(kind) = self.kind_of(id) {
369392
if valid(kind) {

src/tools/jsondoclint/src/validator/tests.rs

+99-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashMap;
22

3-
use rustdoc_json_types::{Crate, Item, Visibility};
3+
use rustdoc_json_types::{Crate, Item, ItemKind, ItemSummary, Visibility, FORMAT_VERSION};
44

55
use crate::json_find::SelectorPart;
66

@@ -64,3 +64,101 @@ fn errors_on_missing_links() {
6464
}],
6565
);
6666
}
67+
68+
// Test we would catch
69+
// https://github.com/rust-lang/rust/issues/104064#issuecomment-1368589718
70+
#[test]
71+
fn errors_on_local_in_paths_and_not_index() {
72+
let krate = Crate {
73+
root: id("0:0:1572"),
74+
crate_version: None,
75+
includes_private: false,
76+
index: HashMap::from_iter([
77+
(
78+
id("0:0:1572"),
79+
Item {
80+
id: id("0:0:1572"),
81+
crate_id: 0,
82+
name: Some("microcore".to_owned()),
83+
span: None,
84+
visibility: Visibility::Public,
85+
docs: None,
86+
links: HashMap::from_iter([(("prim@i32".to_owned(), id("0:1:1571")))]),
87+
attrs: Vec::new(),
88+
deprecation: None,
89+
inner: ItemEnum::Module(Module {
90+
is_crate: true,
91+
items: vec![id("0:1:717")],
92+
is_stripped: false,
93+
}),
94+
},
95+
),
96+
(
97+
id("0:1:717"),
98+
Item {
99+
id: id("0:1:717"),
100+
crate_id: 0,
101+
name: Some("i32".to_owned()),
102+
span: None,
103+
visibility: Visibility::Public,
104+
docs: None,
105+
links: HashMap::default(),
106+
attrs: Vec::new(),
107+
deprecation: None,
108+
inner: ItemEnum::Primitive(Primitive { name: "i32".to_owned(), impls: vec![] }),
109+
},
110+
),
111+
]),
112+
paths: HashMap::from_iter([(
113+
id("0:1:1571"),
114+
ItemSummary {
115+
crate_id: 0,
116+
path: vec!["microcore".to_owned(), "i32".to_owned()],
117+
kind: ItemKind::Primitive,
118+
},
119+
)]),
120+
external_crates: HashMap::default(),
121+
format_version: rustdoc_json_types::FORMAT_VERSION,
122+
};
123+
124+
check(
125+
&krate,
126+
&[Error {
127+
id: id("0:1:1571"),
128+
kind: ErrorKind::Custom("Id for local item in `paths` but not in `index`".to_owned()),
129+
}],
130+
);
131+
}
132+
133+
#[test]
134+
#[should_panic = "LOCAL_CRATE_ID is wrong"]
135+
fn checks_local_crate_id_is_correct() {
136+
let krate = Crate {
137+
root: id("root"),
138+
crate_version: None,
139+
includes_private: false,
140+
index: HashMap::from_iter([(
141+
id("root"),
142+
Item {
143+
id: id("root"),
144+
crate_id: LOCAL_CRATE_ID.wrapping_add(1),
145+
name: Some("irrelavent".to_owned()),
146+
span: None,
147+
visibility: Visibility::Public,
148+
docs: None,
149+
links: HashMap::default(),
150+
attrs: Vec::new(),
151+
deprecation: None,
152+
inner: ItemEnum::Module(Module {
153+
is_crate: true,
154+
items: vec![],
155+
is_stripped: false,
156+
}),
157+
},
158+
)]),
159+
paths: HashMap::default(),
160+
external_crates: HashMap::default(),
161+
format_version: FORMAT_VERSION,
162+
};
163+
check(&krate, &[]);
164+
}

0 commit comments

Comments
 (0)