From 7680b164b00af55e7af64bdc8027b6deb4fb8233 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 2 Jan 2023 19:30:39 +0000 Subject: [PATCH 1/4] jsondoclint: Parse args with clap. --- Cargo.lock | 18 +++++++++++++++++- src/tools/jsondoclint/Cargo.toml | 1 + src/tools/jsondoclint/src/main.rs | 14 ++++++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f99e58e59b8e5..0483d50fc2a69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -597,7 +597,7 @@ checksum = "23b71c3ce99b7611011217b366d923f1d0a7e07a92bb2dbf1e84508c673ca3bd" dependencies = [ "atty", "bitflags", - "clap_derive", + "clap_derive 3.2.18", "clap_lex 0.2.2", "indexmap", "once_cell", @@ -614,7 +614,9 @@ checksum = "6bf8832993da70a4c6d13c581f4463c2bdda27b9bf1c5498dc4365543abe6d6f" dependencies = [ "atty", "bitflags", + "clap_derive 4.0.13", "clap_lex 0.3.0", + "once_cell", "strsim", "termcolor", ] @@ -641,6 +643,19 @@ dependencies = [ "syn", ] +[[package]] +name = "clap_derive" +version = "4.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c42f169caba89a7d512b5418b09864543eeb4d497416c917d7137863bd2076ad" +dependencies = [ + "heck", + "proc-macro-error", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "clap_lex" version = "0.2.2" @@ -2097,6 +2112,7 @@ name = "jsondoclint" version = "0.1.0" dependencies = [ "anyhow", + "clap 4.0.15", "fs-err", "rustdoc-json-types", "serde_json", diff --git a/src/tools/jsondoclint/Cargo.toml b/src/tools/jsondoclint/Cargo.toml index 84a6c7f96c464..0dda3935ed873 100644 --- a/src/tools/jsondoclint/Cargo.toml +++ b/src/tools/jsondoclint/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] anyhow = "1.0.62" +clap = { version = "4.0.15", features = ["derive"] } fs-err = "2.8.1" rustdoc-json-types = { version = "0.1.0", path = "../../rustdoc-json-types" } serde_json = "1.0.85" diff --git a/src/tools/jsondoclint/src/main.rs b/src/tools/jsondoclint/src/main.rs index fc54c421b4b22..91a7aeb063f09 100644 --- a/src/tools/jsondoclint/src/main.rs +++ b/src/tools/jsondoclint/src/main.rs @@ -1,6 +1,5 @@ -use std::env; - -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; +use clap::Parser; use fs_err as fs; use rustdoc_json_types::{Crate, Id, FORMAT_VERSION}; use serde_json::Value; @@ -21,8 +20,15 @@ enum ErrorKind { Custom(String), } +#[derive(Parser)] +struct Cli { + /// The path to the json file to be linted + path: String, +} + fn main() -> Result<()> { - let path = env::args().nth(1).ok_or_else(|| anyhow!("no path given"))?; + let Cli { path } = Cli::parse(); + let contents = fs::read_to_string(&path)?; let krate: Crate = serde_json::from_str(&contents)?; assert_eq!(krate.format_version, FORMAT_VERSION); From 855b7e8cf3963b9f187249f1f38b6bb2baa9c353 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 2 Jan 2023 19:40:00 +0000 Subject: [PATCH 2/4] jsondoclint: Add `--verbose` flag. Without verbose: 0:61941:36627 not in index or paths, but refered to at '$.index["0:62007"].inner.for.inner.id' and 12 more With verbose: 0:10808:27206 not in index or paths, but refered to at '$.index["0:10813"].inner.for.inner.id', '$.index["0:52495"].inner.for.inner.id', '$.index["a:0:2666:215-0:10808:27206"].inner.for.inner.id', '$.index["a:0:2680:223-0:10808:27206"].inner.for.inner.id', '$.index["a:0:2730:7845-0:10808:27206"].inner.for.inner.id', '$.index["a:0:7731:21706-0:10808:27206"].inner.for.inner.id', '$.index["a:0:7732:21705-0:10808:27206"].inner.for.inner.id' --- src/tools/jsondoclint/src/main.rs | 32 +++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/tools/jsondoclint/src/main.rs b/src/tools/jsondoclint/src/main.rs index 91a7aeb063f09..89965dc24039c 100644 --- a/src/tools/jsondoclint/src/main.rs +++ b/src/tools/jsondoclint/src/main.rs @@ -24,10 +24,14 @@ enum ErrorKind { struct Cli { /// The path to the json file to be linted path: String, + + /// Show verbose output + #[arg(long)] + verbose: bool, } fn main() -> Result<()> { - let Cli { path } = Cli::parse(); + let Cli { path, verbose } = Cli::parse(); let contents = fs::read_to_string(&path)?; let krate: Crate = serde_json::from_str(&contents)?; @@ -53,11 +57,27 @@ fn main() -> Result<()> { err.id.0, json_find::to_jsonpath(&sel) ), - [sel, ..] => eprintln!( - "{} not in index or paths, but refered to at '{}' and more", - err.id.0, - json_find::to_jsonpath(&sel) - ), + [sel, ..] => { + if verbose { + let sels = sels + .iter() + .map(json_find::to_jsonpath) + .map(|i| format!("'{i}'")) + .collect::>() + .join(", "); + eprintln!( + "{} not in index or paths, but refered to at {sels}", + err.id.0 + ); + } else { + eprintln!( + "{} not in index or paths, but refered to at '{}' and {} more", + err.id.0, + json_find::to_jsonpath(&sel), + sels.len() - 1, + ) + } + } } } ErrorKind::Custom(msg) => eprintln!("{}: {}", err.id.0, msg), From 95329080d317f01f22ddce1dec8be693ef4b29f4 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 2 Jan 2023 19:54:08 +0000 Subject: [PATCH 3/4] jsondoclint: Find selector for missing ID when error is created, not reported. This is needed for json output, but even without that, it increases performance massivly. On my machine, in reduces the time to check core.json from 40.190s to 11.333s. --- src/tools/jsondoclint/src/main.rs | 73 ++++++++++---------- src/tools/jsondoclint/src/validator.rs | 13 +++- src/tools/jsondoclint/src/validator/tests.rs | 20 +++++- 3 files changed, 64 insertions(+), 42 deletions(-) diff --git a/src/tools/jsondoclint/src/main.rs b/src/tools/jsondoclint/src/main.rs index 89965dc24039c..266900ea3a22d 100644 --- a/src/tools/jsondoclint/src/main.rs +++ b/src/tools/jsondoclint/src/main.rs @@ -16,7 +16,7 @@ struct Error { #[derive(Debug, PartialEq, Eq)] enum ErrorKind { - NotFound, + NotFound(Vec), Custom(String), } @@ -37,49 +37,48 @@ fn main() -> Result<()> { let krate: Crate = serde_json::from_str(&contents)?; assert_eq!(krate.format_version, FORMAT_VERSION); - let mut validator = validator::Validator::new(&krate); + let krate_json: Value = serde_json::from_str(&contents)?; + + let mut validator = validator::Validator::new(&krate, krate_json); validator.check_crate(); if !validator.errs.is_empty() { 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[..] { - [] => unreachable!( - "id must be in crate, or it wouldn't be reported as not found" - ), - [sel] => eprintln!( - "{} not in index or paths, but refered to at '{}'", - err.id.0, - json_find::to_jsonpath(&sel) - ), - [sel, ..] => { - if verbose { - let sels = sels - .iter() - .map(json_find::to_jsonpath) - .map(|i| format!("'{i}'")) - .collect::>() - .join(", "); - eprintln!( - "{} not in index or paths, but refered to at {sels}", - err.id.0 - ); - } else { - eprintln!( - "{} not in index or paths, but refered to at '{}' and {} more", - err.id.0, - json_find::to_jsonpath(&sel), - sels.len() - 1, - ) - } + ErrorKind::NotFound(sels) => match &sels[..] { + [] => { + unreachable!( + "id {:?} must be in crate, or it wouldn't be reported as not found", + err.id + ) + } + [sel] => eprintln!( + "{} not in index or paths, but refered to at '{}'", + err.id.0, + json_find::to_jsonpath(&sel) + ), + [sel, ..] => { + if verbose { + let sels = sels + .iter() + .map(json_find::to_jsonpath) + .map(|i| format!("'{i}'")) + .collect::>() + .join(", "); + eprintln!( + "{} not in index or paths, but refered to at {sels}", + err.id.0 + ); + } else { + eprintln!( + "{} not in index or paths, but refered to at '{}' and {} more", + err.id.0, + json_find::to_jsonpath(&sel), + sels.len() - 1, + ) } } - } + }, ErrorKind::Custom(msg) => eprintln!("{}: {}", err.id.0, msg), } } diff --git a/src/tools/jsondoclint/src/validator.rs b/src/tools/jsondoclint/src/validator.rs index 291d02d67bd62..f1b9c1acbaec0 100644 --- a/src/tools/jsondoclint/src/validator.rs +++ b/src/tools/jsondoclint/src/validator.rs @@ -7,8 +7,9 @@ use rustdoc_json_types::{ 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, Error, ErrorKind}; +use crate::{item_kind::Kind, json_find, Error, ErrorKind}; /// The Validator walks over the JSON tree, and ensures it is well formed. /// It is made of several parts. @@ -22,6 +23,7 @@ use crate::{item_kind::Kind, Error, ErrorKind}; pub struct Validator<'a> { pub(crate) errs: Vec, krate: &'a Crate, + krate_json: Value, /// Worklist of Ids to check. todo: HashSet<&'a Id>, /// Ids that have already been visited, so don't need to be checked again. @@ -39,9 +41,10 @@ enum PathKind { } impl<'a> Validator<'a> { - pub fn new(krate: &'a Crate) -> Self { + pub fn new(krate: &'a Crate, krate_json: Value) -> Self { Self { krate, + krate_json, errs: Vec::new(), seen_ids: HashSet::new(), todo: HashSet::new(), @@ -373,7 +376,11 @@ impl<'a> Validator<'a> { } else { if !self.missing_ids.contains(id) { self.missing_ids.insert(id); - self.fail(id, ErrorKind::NotFound) + + let sels = json_find::find_selector(&self.krate_json, &Value::String(id.0.clone())); + assert_ne!(sels.len(), 0); + + self.fail(id, ErrorKind::NotFound(sels)) } } } diff --git a/src/tools/jsondoclint/src/validator/tests.rs b/src/tools/jsondoclint/src/validator/tests.rs index c4aeee9c53b76..37b826153efb1 100644 --- a/src/tools/jsondoclint/src/validator/tests.rs +++ b/src/tools/jsondoclint/src/validator/tests.rs @@ -2,11 +2,16 @@ use std::collections::HashMap; use rustdoc_json_types::{Crate, Item, Visibility}; +use crate::json_find::SelectorPart; + use super::*; #[track_caller] fn check(krate: &Crate, errs: &[Error]) { - let mut validator = Validator::new(krate); + let krate_string = serde_json::to_string(krate).unwrap(); + let krate_json = serde_json::from_str(&krate_string).unwrap(); + + let mut validator = Validator::new(krate, krate_json); validator.check_crate(); assert_eq!(errs, &validator.errs[..]); @@ -46,5 +51,16 @@ fn errors_on_missing_links() { format_version: rustdoc_json_types::FORMAT_VERSION, }; - check(&k, &[Error { kind: ErrorKind::NotFound, id: id("1") }]); + check( + &k, + &[Error { + kind: ErrorKind::NotFound(vec![vec![ + SelectorPart::Field("index".to_owned()), + SelectorPart::Field("0".to_owned()), + SelectorPart::Field("links".to_owned()), + SelectorPart::Field("Not Found".to_owned()), + ]]), + id: id("1"), + }], + ); } From 226ab7fd759d94e81d6831f869e41502a7f183c7 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 2 Jan 2023 20:15:45 +0000 Subject: [PATCH 4/4] jsondoclint: Add option to dump errors as json. The output looks like: { "errors": [ { "id": "2:2017:1833", "kind": { "NotFound": [ [ {"Field": "index"}, {"Field": "0:0:1571"}, {"Field": "links"}, {"Field": "pointer::read"} ] ] } } ], "path": "/home/nixon/dev/rust/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/intra-doc-links/pointer_method/pointer_method.json" } --- Cargo.lock | 1 + src/tools/jsondoclint/Cargo.toml | 1 + src/tools/jsondoclint/src/json_find.rs | 3 ++- src/tools/jsondoclint/src/main.rs | 25 ++++++++++++++++++++++--- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0483d50fc2a69..bb438f288f2e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2115,6 +2115,7 @@ dependencies = [ "clap 4.0.15", "fs-err", "rustdoc-json-types", + "serde", "serde_json", ] diff --git a/src/tools/jsondoclint/Cargo.toml b/src/tools/jsondoclint/Cargo.toml index 0dda3935ed873..8990310a4f474 100644 --- a/src/tools/jsondoclint/Cargo.toml +++ b/src/tools/jsondoclint/Cargo.toml @@ -10,4 +10,5 @@ anyhow = "1.0.62" clap = { version = "4.0.15", features = ["derive"] } fs-err = "2.8.1" rustdoc-json-types = { version = "0.1.0", path = "../../rustdoc-json-types" } +serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.85" diff --git a/src/tools/jsondoclint/src/json_find.rs b/src/tools/jsondoclint/src/json_find.rs index 70e7440f73085..a183c4068ce85 100644 --- a/src/tools/jsondoclint/src/json_find.rs +++ b/src/tools/jsondoclint/src/json_find.rs @@ -1,8 +1,9 @@ use std::fmt::Write; +use serde::Serialize; use serde_json::Value; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub enum SelectorPart { Field(String), Index(usize), diff --git a/src/tools/jsondoclint/src/main.rs b/src/tools/jsondoclint/src/main.rs index 266900ea3a22d..05e938f4f7df4 100644 --- a/src/tools/jsondoclint/src/main.rs +++ b/src/tools/jsondoclint/src/main.rs @@ -1,25 +1,34 @@ +use std::io::{BufWriter, Write}; + use anyhow::{bail, Result}; use clap::Parser; use fs_err as fs; use rustdoc_json_types::{Crate, Id, FORMAT_VERSION}; +use serde::Serialize; use serde_json::Value; pub(crate) mod item_kind; mod json_find; mod validator; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Serialize, Clone)] struct Error { kind: ErrorKind, id: Id, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Serialize, Clone)] enum ErrorKind { NotFound(Vec), Custom(String), } +#[derive(Debug, Serialize)] +struct JsonOutput { + path: String, + errors: Vec, +} + #[derive(Parser)] struct Cli { /// The path to the json file to be linted @@ -28,10 +37,13 @@ struct Cli { /// Show verbose output #[arg(long)] verbose: bool, + + #[arg(long)] + json_output: Option, } fn main() -> Result<()> { - let Cli { path, verbose } = Cli::parse(); + let Cli { path, verbose, json_output } = Cli::parse(); let contents = fs::read_to_string(&path)?; let krate: Crate = serde_json::from_str(&contents)?; @@ -42,6 +54,13 @@ fn main() -> Result<()> { let mut validator = validator::Validator::new(&krate, krate_json); validator.check_crate(); + if let Some(json_output) = json_output { + let output = JsonOutput { path: path.clone(), errors: validator.errs.clone() }; + let mut f = BufWriter::new(fs::File::create(json_output)?); + serde_json::to_writer(&mut f, &output)?; + f.flush()?; + } + if !validator.errs.is_empty() { for err in validator.errs { match err.kind {