jsondoclint: Document validator

3 files changed, 29 insertions, 22 deletions

diff --git a/src/tools/jsondoclint/src/item_kind.rs b/src/tools/jsondoclint/src/item_kind.rs
index f46da20fd58..b3e88a90813 100644
--- a/src/tools/jsondoclint/src/item_kind.rs
+++ b/src/tools/jsondoclint/src/item_kind.rs
@@ -1,7 +1,6 @@
 use rustdoc_json_types::{Item, ItemEnum, ItemKind, ItemSummary};
 
-// We want a univeral way to represent an `ItemEnum` or `ItemKind`
-
+/// A univeral way to represent an [`ItemEnum`] or [`ItemKind`]
 #[derive(Debug)]
 pub(crate) enum Kind {
     Module,
diff --git a/src/tools/jsondoclint/src/main.rs b/src/tools/jsondoclint/src/main.rs
index 1d02482421b..70d7a82a576 100644
--- a/src/tools/jsondoclint/src/main.rs
+++ b/src/tools/jsondoclint/src/main.rs
@@ -25,8 +25,6 @@ fn main() -> Result<()> {
     let path = env::args().nth(1).ok_or_else(|| anyhow!("no path given"))?;
     let contents = fs::read_to_string(&path)?;
     let krate: Crate = serde_json::from_str(&contents)?;
-    // TODO: Only load if nessessary.
-    let krate_json: Value = serde_json::from_str(&contents)?;
     assert_eq!(krate.format_version, FORMAT_VERSION);
 
     let mut validator = validator::Validator::new(&krate);
@@ -36,6 +34,8 @@ fn main() -> Result<()> {
         for err in validator.errs {
             match err.kind {
                 ErrorKind::NotFound => {
+                    let krate_json: Value = serde_json::from_str(&contents)?;
+
                     let sels =
                         json_find::find_selector(&krate_json, &Value::String(err.id.0.clone()));
                     match &sels[..] {
diff --git a/src/tools/jsondoclint/src/validator.rs b/src/tools/jsondoclint/src/validator.rs
index 06b114f6c2f..efe2c165b6c 100644
--- a/src/tools/jsondoclint/src/validator.rs
+++ b/src/tools/jsondoclint/src/validator.rs
@@ -10,22 +10,24 @@ use rustdoc_json_types::{
 
 use crate::{item_kind::Kind, Error, ErrorKind};
 
+/// The Validator walks over the JSON tree, and ensures it is well formed.
+/// It is made of several parts.
+///
+/// - `check_*`: These take a type from [`rustdoc_json_types`], and check that
+///              it is well formed. This involves calling `check_*` functions on
+///              fields of that item, and `add_*` functions on [`Id`]s.
+/// - `add_*`: These add an [`Id`] to the worklist, after validating it to check if
+///            the `Id` is a kind expected in this suituation.
 #[derive(Debug)]
 pub struct Validator<'a> {
     pub(crate) errs: Vec<Error>,
     krate: &'a Crate,
+    /// Worklist of Ids to check.
+    todo: HashSet<&'a Id>,
+    /// Ids that have already been visited, so don't need to be checked again.
     seen_ids: HashSet<&'a Id>,
+    /// Ids that have already been reported missing.
     missing_ids: HashSet<&'a Id>,
-    todo: HashSet<&'a Id>,
-}
-
-fn set_remove<T: Hash + Eq + Clone>(set: &mut HashSet<T>) -> Option<T> {
-    if let Some(id) = set.iter().next() {
-        let id = id.clone();
-        set.take(&id)
-    } else {
-        None
-    }
 }
 
 impl<'a> Validator<'a> {
@@ -82,6 +84,8 @@ impl<'a> Validator<'a> {
                     }
                 }
             }
+        } else {
+            assert!(self.krate.paths.contains_key(id));
         }
     }
 
@@ -336,17 +340,12 @@ impl<'a> Validator<'a> {
         fp.generic_params.iter().for_each(|gpd| self.check_generic_param_def(gpd));
     }
 
-    // Aux functions
-    fn add_id(&mut self, id: &'a Id) {
-        if !self.seen_ids.contains(id) {
-            self.todo.insert(id);
-        }
-    }
-
     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) {
-                self.add_id(id);
+                if !self.seen_ids.contains(id) {
+                    self.todo.insert(id);
+                }
             } else {
                 self.fail_expecting(id, expected);
             }
@@ -402,3 +401,12 @@ impl<'a> Validator<'a> {
         }
     }
 }
+
+fn set_remove<T: Hash + Eq + Clone>(set: &mut HashSet<T>) -> Option<T> {
+    if let Some(id) = set.iter().next() {
+        let id = id.clone();
+        set.take(&id)
+    } else {
+        None
+    }
+}