From ce21447c01c8ea6ce4f4c9dd2c18266439200f1d Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Wed, 24 Mar 2021 15:52:47 +0000 Subject: [PATCH 1/3] Better errors in jsondocck --- Cargo.lock | 7 +++++++ src/tools/jsondocck/Cargo.toml | 1 + src/tools/jsondocck/src/cache.rs | 10 ++++++++-- src/tools/jsondocck/src/main.rs | 20 ++++++++++++++++++-- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5a7b7d9b6056..9b0f310e3ae44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1219,6 +1219,12 @@ dependencies = [ "rustc-std-workspace-core", ] +[[package]] +name = "fs-err" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bcd1163ae48bda72a20ae26d66a04d3094135cadab911cff418ae5e33f253431" + [[package]] name = "fs_extra" version = "1.1.0" @@ -1748,6 +1754,7 @@ checksum = "92c245af8786f6ac35f95ca14feca9119e71339aaab41e878e7cdd655c97e9e5" name = "jsondocck" version = "0.1.0" dependencies = [ + "fs-err", "getopts", "jsonpath_lib", "lazy_static", diff --git a/src/tools/jsondocck/Cargo.toml b/src/tools/jsondocck/Cargo.toml index 97052ef58d6f2..a6efc4c9a6b5b 100644 --- a/src/tools/jsondocck/Cargo.toml +++ b/src/tools/jsondocck/Cargo.toml @@ -12,3 +12,4 @@ lazy_static = "1.4" shlex = "0.1" serde = "1.0" serde_json = "1.0" +fs-err = "2.5.0" diff --git a/src/tools/jsondocck/src/cache.rs b/src/tools/jsondocck/src/cache.rs index 8a6a911321c34..a188750c56ae3 100644 --- a/src/tools/jsondocck/src/cache.rs +++ b/src/tools/jsondocck/src/cache.rs @@ -1,8 +1,10 @@ use crate::error::CkError; use serde_json::Value; use std::collections::HashMap; +use std::io; use std::path::{Path, PathBuf}; -use std::{fs, io}; + +use fs_err as fs; #[derive(Debug)] pub struct Cache { @@ -31,7 +33,11 @@ impl Cache { self.last_path = Some(resolve.clone()); resolve } else { - self.last_path.as_ref().unwrap().clone() + self.last_path + .as_ref() + // FIXME: Point to a line number + .expect("No last path set. Make sure to specify a full path before using `-`") + .clone() } } diff --git a/src/tools/jsondocck/src/main.rs b/src/tools/jsondocck/src/main.rs index bcb3f6922efaa..216890d59ad6c 100644 --- a/src/tools/jsondocck/src/main.rs +++ b/src/tools/jsondocck/src/main.rs @@ -239,7 +239,20 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> { let val = cache.get_value(&command.args[0])?; let results = select(&val, &command.args[1]).unwrap(); let pat = string_to_value(&command.args[2], cache); - results.len() == 1 && results[0] == pat.as_ref() + let is = results.len() == 1 && results[0] == pat.as_ref(); + if !command.negated && !is { + return Err(CkError::FailedCheck( + format!( + "{} matched to {:?}, but expected {:?}", + &command.args[1], + results, + pat.as_ref() + ), + command, + )); + } else { + is + } } CommandKind::Set => { // @set = @@ -299,7 +312,10 @@ fn check_command(command: Command, cache: &mut Cache) -> Result<(), CkError> { fn string_to_value<'a>(s: &str, cache: &'a Cache) -> Cow<'a, Value> { if s.starts_with("$") { - Cow::Borrowed(&cache.variables[&s[1..]]) + Cow::Borrowed(&cache.variables.get(&s[1..]).unwrap_or_else(|| { + // FIXME(adotinthevoid): Show line number + panic!("No variable: `{}`. Current state: `{:?}`", &s[1..], cache.variables) + })) } else { Cow::Owned(serde_json::from_str(s).unwrap()) } From 9ba92972ed6a1a39afa5993da97e016eea907be2 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Wed, 24 Mar 2021 15:54:20 +0000 Subject: [PATCH 2/3] Don't call `item` on modules for json renderer Closes #80664 --- src/librustdoc/formats/renderer.rs | 7 ++++++- src/librustdoc/html/render/context.rs | 2 ++ src/librustdoc/json/conversions.rs | 3 ++- src/librustdoc/json/mod.rs | 6 +++++- .../rustdoc-json/reexport/in_root_and_mod.rs | 15 ++++++++++++++ .../reexport/in_root_and_mod_pub.rs | 20 +++++++++++++++++++ .../rustdoc-json/reexport/rename_private.rs | 14 +++++++++++++ 7 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 src/test/rustdoc-json/reexport/in_root_and_mod.rs create mode 100644 src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs create mode 100644 src/test/rustdoc-json/reexport/rename_private.rs diff --git a/src/librustdoc/formats/renderer.rs b/src/librustdoc/formats/renderer.rs index 9dcef3a20d6c0..4e0f3a4e3c317 100644 --- a/src/librustdoc/formats/renderer.rs +++ b/src/librustdoc/formats/renderer.rs @@ -13,6 +13,11 @@ crate trait FormatRenderer<'tcx>: Sized { /// Gives a description of the renderer. Used for performance profiling. fn descr() -> &'static str; + /// Whether to call `item` recursivly for modules + /// + /// This is true for html, and false for json. See #80664 + const RUN_ON_MODULE: bool; + /// Sets up any state required for the renderer. When this is called the cache has already been /// populated. fn init( @@ -68,7 +73,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>( let unknown = Symbol::intern(""); while let Some((mut cx, item)) = work.pop() { - if item.is_mod() { + if item.is_mod() && T::RUN_ON_MODULE { // modules are special because they add a namespace. We also need to // recurse into the items of the module as well. let name = item.name.as_ref().unwrap().to_string(); diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 64d413a5f3119..0ffb4d616da1a 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -290,6 +290,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { "html" } + const RUN_ON_MODULE: bool = true; + fn init( mut krate: clean::Crate, options: RenderOptions, diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 073209c2468a0..6da0d8816408c 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -179,7 +179,8 @@ fn from_clean_item_kind(item: clean::ItemKind, tcx: TyCtxt<'_>, name: &Option from_clean_item_kind(*inner, tcx, name), + // `convert_item` early returns `None` for striped items + StrippedItem(_) => unreachable!(), PrimitiveItem(_) | KeywordItem(_) => { panic!("{:?} is not supported for JSON output", item) } diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index a4cdad69865ff..5d9c598549e0e 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -129,6 +129,8 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { "json" } + const RUN_ON_MODULE: bool = false; + fn init( krate: clean::Crate, options: RenderOptions, @@ -169,8 +171,10 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { e.impls = self.get_impls(id) } let removed = self.index.borrow_mut().insert(from_def_id(id), new_item.clone()); + // FIXME(adotinthevoid): Currently, the index is duplicated. This is a sanity check - // to make sure the items are unique. + // to make sure the items are unique. The main place this happens is when an item, is + // reexported in more than one place. See `rustdoc-json/reexport/in_root_and_mod` if let Some(old_item) = removed { assert_eq!(old_item, new_item); } diff --git a/src/test/rustdoc-json/reexport/in_root_and_mod.rs b/src/test/rustdoc-json/reexport/in_root_and_mod.rs new file mode 100644 index 0000000000000..e3cecbdd7ff2f --- /dev/null +++ b/src/test/rustdoc-json/reexport/in_root_and_mod.rs @@ -0,0 +1,15 @@ +#![feature(no_core)] +#![no_core] + +mod foo { + // @set foo_id = in_root_and_mod.json "$.index[*][?(@.name=='Foo')].id" + pub struct Foo; +} + +// @has - "$.index[*][?(@.name=='in_root_and_mod')].inner.items[*]" $foo_id +pub use foo::Foo; + +pub mod bar { + // @has - "$.index[*][?(@.name=='bar')].inner.items[*]" $foo_id + pub use crate::foo::Foo; +} diff --git a/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs b/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs new file mode 100644 index 0000000000000..8adb05f7be88b --- /dev/null +++ b/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs @@ -0,0 +1,20 @@ +#![feature(no_core)] +#![no_core] + +pub mod foo { + // @set bar_id = in_root_and_mod_pub.json "$.index[*][?(@.name=='Bar')].id" + // @has - "$.index[*][?(@.name=='foo')].inner.items[*]" $bar_id + pub struct Bar; +} + +// @set root_import_id = - "$.index[*][?(@.inner.span=='foo::Bar')].id" +// @is - "$.index[*][?(@.inner.span=='foo::Bar')].inner.id" $bar_id +// @has - "$.index[*][?(@.name=='in_root_and_mod_pub')].inner.items[*]" $root_import_id +pub use foo::Bar; + +pub mod baz { + // @set baz_import_id = - "$.index[*][?(@.inner.span=='crate::foo::Bar')].id" + // @is - "$.index[*][?(@.inner.span=='crate::foo::Bar')].inner.id" $bar_id + // @has - "$.index[*][?(@.name=='baz')].inner.items[*]" $baz_import_id + pub use crate::foo::Bar; +} diff --git a/src/test/rustdoc-json/reexport/rename_private.rs b/src/test/rustdoc-json/reexport/rename_private.rs new file mode 100644 index 0000000000000..fb8296f23374a --- /dev/null +++ b/src/test/rustdoc-json/reexport/rename_private.rs @@ -0,0 +1,14 @@ +// edition:2018 + +#![no_core] +#![feature(no_core)] +// @!has rename_private.json "$.index[*][?(@.name=='inner')]" +mod inner { + // @!has - "$.index[*][?(@.name=='Public')]" + pub struct Public; +} + +// @set newname_id = - "$.index[*][?(@.name=='NewName')].id" +// @is - "$.index[*][?(@.name=='NewName')].kind" \"struct\" +// @has - "$.index[*][?(@.name=='rename_private')].inner.items[*]" $newname_id +pub use inner::Public as NewName; From d9e2d8d6652fef745a5658178a6f85fc809f6453 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Wed, 24 Mar 2021 19:44:23 +0000 Subject: [PATCH 3/3] Rename `span` to `source` Caused by https://github.com/rust-lang/rust/commit/b0659f9b1bfb92626c40dabceb3268f88bb26224#diff-ede26372490522288745c5b3df2b6b2a1cc913dcd09b29af3a49935afe00c7e6L464-R464 --- src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs b/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs index 8adb05f7be88b..2daadf7620ca0 100644 --- a/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs +++ b/src/test/rustdoc-json/reexport/in_root_and_mod_pub.rs @@ -7,14 +7,14 @@ pub mod foo { pub struct Bar; } -// @set root_import_id = - "$.index[*][?(@.inner.span=='foo::Bar')].id" -// @is - "$.index[*][?(@.inner.span=='foo::Bar')].inner.id" $bar_id +// @set root_import_id = - "$.index[*][?(@.inner.source=='foo::Bar')].id" +// @is - "$.index[*][?(@.inner.source=='foo::Bar')].inner.id" $bar_id // @has - "$.index[*][?(@.name=='in_root_and_mod_pub')].inner.items[*]" $root_import_id pub use foo::Bar; pub mod baz { - // @set baz_import_id = - "$.index[*][?(@.inner.span=='crate::foo::Bar')].id" - // @is - "$.index[*][?(@.inner.span=='crate::foo::Bar')].inner.id" $bar_id + // @set baz_import_id = - "$.index[*][?(@.inner.source=='crate::foo::Bar')].id" + // @is - "$.index[*][?(@.inner.source=='crate::foo::Bar')].inner.id" $bar_id // @has - "$.index[*][?(@.name=='baz')].inner.items[*]" $baz_import_id pub use crate::foo::Bar; }