Skip to content

Commit

Permalink
Rollup merge of rust-lang#106546 - aDotInTheVoid:jsondoclint-path-loc…
Browse files Browse the repository at this point in the history
…al-item, r=notriddle

jsondoclint: Check local items in `paths` are also in `index`.

Would have caught rust-lang#104064 (if core.json was linted in CI).

Closes rust-lang#106433.

r? rustdoc
  • Loading branch information
Yuki Okushi authored Jan 8, 2023
2 parents 3edc7e0 + d4139b3 commit 7997ff6
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 4 deletions.
29 changes: 26 additions & 3 deletions src/tools/jsondoclint/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@ use std::hash::Hash;

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

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

// This is a rustc implementation detail that we rely on here
const LOCAL_CRATE_ID: u32 = 0;

/// The Validator walks over the JSON tree, and ensures it is well formed.
/// It is made of several parts.
///
Expand Down Expand Up @@ -53,12 +56,19 @@ impl<'a> Validator<'a> {
}

pub fn check_crate(&mut self) {
// Graph traverse the index
let root = &self.krate.root;
self.add_mod_id(root);
while let Some(id) = set_remove(&mut self.todo) {
self.seen_ids.insert(id);
self.check_item(id);
}

let root_crate_id = self.krate.index[root].crate_id;
assert_eq!(root_crate_id, LOCAL_CRATE_ID, "LOCAL_CRATE_ID is wrong");
for (id, item_info) in &self.krate.paths {
self.check_item_info(id, item_info);
}
}

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

fn check_item_info(&mut self, id: &Id, item_info: &ItemSummary) {
// FIXME: Their should be a better way to determine if an item is local, rather than relying on `LOCAL_CRATE_ID`,
// which encodes rustc implementation details.
if item_info.crate_id == LOCAL_CRATE_ID && !self.krate.index.contains_key(id) {
self.errs.push(Error {
id: id.clone(),
kind: ErrorKind::Custom(
"Id for local item in `paths` but not in `index`".to_owned(),
),
})
}
}

fn add_id_checked(&mut self, id: &'a Id, valid: fn(Kind) -> bool, expected: &str) {
if let Some(kind) = self.kind_of(id) {
if valid(kind) {
Expand Down
100 changes: 99 additions & 1 deletion src/tools/jsondoclint/src/validator/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::HashMap;

use rustdoc_json_types::{Crate, Item, Visibility};
use rustdoc_json_types::{Crate, Item, ItemKind, ItemSummary, Visibility, FORMAT_VERSION};

use crate::json_find::SelectorPart;

Expand Down Expand Up @@ -64,3 +64,101 @@ fn errors_on_missing_links() {
}],
);
}

// Test we would catch
// https://github.com/rust-lang/rust/issues/104064#issuecomment-1368589718
#[test]
fn errors_on_local_in_paths_and_not_index() {
let krate = Crate {
root: id("0:0:1572"),
crate_version: None,
includes_private: false,
index: HashMap::from_iter([
(
id("0:0:1572"),
Item {
id: id("0:0:1572"),
crate_id: 0,
name: Some("microcore".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: HashMap::from_iter([(("prim@i32".to_owned(), id("0:1:1571")))]),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Module(Module {
is_crate: true,
items: vec![id("0:1:717")],
is_stripped: false,
}),
},
),
(
id("0:1:717"),
Item {
id: id("0:1:717"),
crate_id: 0,
name: Some("i32".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: HashMap::default(),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Primitive(Primitive { name: "i32".to_owned(), impls: vec![] }),
},
),
]),
paths: HashMap::from_iter([(
id("0:1:1571"),
ItemSummary {
crate_id: 0,
path: vec!["microcore".to_owned(), "i32".to_owned()],
kind: ItemKind::Primitive,
},
)]),
external_crates: HashMap::default(),
format_version: rustdoc_json_types::FORMAT_VERSION,
};

check(
&krate,
&[Error {
id: id("0:1:1571"),
kind: ErrorKind::Custom("Id for local item in `paths` but not in `index`".to_owned()),
}],
);
}

#[test]
#[should_panic = "LOCAL_CRATE_ID is wrong"]
fn checks_local_crate_id_is_correct() {
let krate = Crate {
root: id("root"),
crate_version: None,
includes_private: false,
index: HashMap::from_iter([(
id("root"),
Item {
id: id("root"),
crate_id: LOCAL_CRATE_ID.wrapping_add(1),
name: Some("irrelavent".to_owned()),
span: None,
visibility: Visibility::Public,
docs: None,
links: HashMap::default(),
attrs: Vec::new(),
deprecation: None,
inner: ItemEnum::Module(Module {
is_crate: true,
items: vec![],
is_stripped: false,
}),
},
)]),
paths: HashMap::default(),
external_crates: HashMap::default(),
format_version: FORMAT_VERSION,
};
check(&krate, &[]);
}

0 comments on commit 7997ff6

Please sign in to comment.