per djkoloski's review

rust-lang · Jul 26, 2024 · b6bd191 · b6bd191
1 parent d3876e1
commit b6bd191
Show file tree

Hide file tree

Showing 9 changed files with 214 additions and 179 deletions.
diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml
@@ -16,7 +16,7 @@ minifier = "0.3.0"
 pulldown-cmark-old = { version = "0.9.6", package = "pulldown-cmark", default-features = false }
 regex = "1"
 rustdoc-json-types = { path = "../rustdoc-json-types" }
-serde_json = { version = "1.0", features = ["preserve_order"] }
+serde_json = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 smallvec = "1.8.1"
 tempfile = "3"

diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
@@ -29,9 +29,9 @@ pub(crate) mod search_index;
 mod tests;
 
 mod context;
+mod ordered_json;
 mod print_item;
 mod sidebar;
-mod sorted_json;
 mod sorted_template;
 mod span_map;
 mod type_layout;

diff --git a/src/librustdoc/html/render/sorted_json.rs → src/librustdoc/html/render/ordered_json.rs b/src/librustdoc/html/render/sorted_json.rs → src/librustdoc/html/render/ordered_json.rs
@@ -6,55 +6,54 @@ use std::fmt;
 
 /// Prerenedered json.
 ///
-/// Arrays are sorted by their stringified entries, and objects are sorted by their stringified
-/// keys.
-///
-/// Must use serde_json with the preserve_order feature.
-///
 /// Both the Display and serde_json::to_string implementations write the serialized json
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 #[serde(from = "Value")]
 #[serde(into = "Value")]
-pub(crate) struct SortedJson(String);
+pub(crate) struct OrderedJson(String);
 
-impl SortedJson {
+impl OrderedJson {
     /// If you pass in an array, it will not be sorted.
-    pub(crate) fn serialize<T: Serialize>(item: T) -> Self {
-        SortedJson(serde_json::to_string(&item).unwrap())
+    pub(crate) fn serialize<T: Serialize>(item: T) -> Result<Self, serde_json::Error> {
+        Ok(OrderedJson(serde_json::to_string(&item)?))
     }
 
     /// Serializes and sorts
-    pub(crate) fn array<T: Borrow<SortedJson>, I: IntoIterator<Item = T>>(items: I) -> Self {
+    pub(crate) fn array_sorted<T: Borrow<OrderedJson>, I: IntoIterator<Item = T>>(
+        items: I,
+    ) -> Self {
         let items = items
             .into_iter()
             .sorted_unstable_by(|a, b| a.borrow().cmp(&b.borrow()))
             .format_with(",", |item, f| f(item.borrow()));
-        SortedJson(format!("[{}]", items))
+        OrderedJson(format!("[{}]", items))
     }
 
-    pub(crate) fn array_unsorted<T: Borrow<SortedJson>, I: IntoIterator<Item = T>>(
+    pub(crate) fn array_unsorted<T: Borrow<OrderedJson>, I: IntoIterator<Item = T>>(
         items: I,
     ) -> Self {
         let items = items.into_iter().format_with(",", |item, f| f(item.borrow()));
-        SortedJson(format!("[{items}]"))
+        OrderedJson(format!("[{items}]"))
     }
 }
 
-impl fmt::Display for SortedJson {
+impl fmt::Display for OrderedJson {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.0)
+        self.0.fmt(f)
     }
 }
 
-impl From<Value> for SortedJson {
+impl From<Value> for OrderedJson {
     fn from(value: Value) -> Self {
-        SortedJson(serde_json::to_string(&value).unwrap())
+        let serialized =
+            serde_json::to_string(&value).expect("Serializing a Value to String should never fail");
+        OrderedJson(serialized)
     }
 }
 
-impl From<SortedJson> for Value {
-    fn from(json: SortedJson) -> Self {
-        serde_json::from_str(&json.0).unwrap()
+impl From<OrderedJson> for Value {
+    fn from(json: OrderedJson) -> Self {
+        serde_json::from_str(&json.0).expect("OrderedJson should always store valid JSON")
     }
 }
 
@@ -65,10 +64,10 @@ impl From<SortedJson> for Value {
 /// JSON.parse loads faster than raw JS source,
 /// so this is used for large objects.
 #[derive(Debug, Clone, Serialize, Deserialize)]
-pub(crate) struct EscapedJson(SortedJson);
+pub(crate) struct EscapedJson(OrderedJson);
 
-impl From<SortedJson> for EscapedJson {
-    fn from(json: SortedJson) -> Self {
+impl From<OrderedJson> for EscapedJson {
+    fn from(json: OrderedJson) -> Self {
         EscapedJson(json)
     }
 }
@@ -79,7 +78,7 @@ impl fmt::Display for EscapedJson {
         // for JSON content.
         // We need to escape double quotes for the JSON
         let json = self.0.0.replace('\\', r"\\").replace('\'', r"\'").replace("\\\"", "\\\\\"");
-        write!(f, "{}", json)
+        json.fmt(f)
     }
 }
 

diff --git a/...brustdoc/html/render/sorted_json/tests.rs → ...rustdoc/html/render/ordered_json/tests.rs b/...brustdoc/html/render/sorted_json/tests.rs → ...rustdoc/html/render/ordered_json/tests.rs
@@ -1,17 +1,17 @@
-use super::super::sorted_json::*;
+use super::super::ordered_json::*;
 
-fn check(json: SortedJson, serialized: &str) {
+fn check(json: OrderedJson, serialized: &str) {
     assert_eq!(json.to_string(), serialized);
     assert_eq!(serde_json::to_string(&json).unwrap(), serialized);
 
     let json = json.to_string();
-    let json: SortedJson = serde_json::from_str(&json).unwrap();
+    let json: OrderedJson = serde_json::from_str(&json).unwrap();
 
     assert_eq!(json.to_string(), serialized);
     assert_eq!(serde_json::to_string(&json).unwrap(), serialized);
 
     let json = serde_json::to_string(&json).unwrap();
-    let json: SortedJson = serde_json::from_str(&json).unwrap();
+    let json: OrderedJson = serde_json::from_str(&json).unwrap();
 
     assert_eq!(json.to_string(), serialized);
     assert_eq!(serde_json::to_string(&json).unwrap(), serialized);
@@ -20,71 +20,71 @@ fn check(json: SortedJson, serialized: &str) {
 // Make sure there is no extra level of string, plus number of escapes.
 #[test]
 fn escape_json_number() {
-    let json = SortedJson::serialize(3);
+    let json = OrderedJson::serialize(3).unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), "3");
 }
 
 #[test]
 fn escape_json_single_quote() {
-    let json = SortedJson::serialize("he's");
+    let json = OrderedJson::serialize("he's").unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), r#""he\'s""#);
 }
 
 #[test]
 fn escape_json_array() {
-    let json = SortedJson::serialize([1, 2, 3]);
+    let json = OrderedJson::serialize([1, 2, 3]).unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), r#"[1,2,3]"#);
 }
 
 #[test]
 fn escape_json_string() {
-    let json = SortedJson::serialize(r#"he"llo"#);
+    let json = OrderedJson::serialize(r#"he"llo"#).unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), r#""he\\\"llo""#);
 }
 
 #[test]
 fn escape_json_string_escaped() {
-    let json = SortedJson::serialize(r#"he\"llo"#);
+    let json = OrderedJson::serialize(r#"he\"llo"#).unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), r#""he\\\\\\\"llo""#);
 }
 
 #[test]
 fn escape_json_string_escaped_escaped() {
-    let json = SortedJson::serialize(r#"he\\"llo"#);
+    let json = OrderedJson::serialize(r#"he\\"llo"#).unwrap();
     let json = EscapedJson::from(json);
     assert_eq!(format!("{json}"), r#""he\\\\\\\\\\\"llo""#);
 }
 
 // Testing round trip + making sure there is no extra level of string
 #[test]
 fn number() {
-    let json = SortedJson::serialize(3);
+    let json = OrderedJson::serialize(3).unwrap();
     let serialized = "3";
     check(json, serialized);
 }
 
 #[test]
 fn boolean() {
-    let json = SortedJson::serialize(true);
+    let json = OrderedJson::serialize(true).unwrap();
     let serialized = "true";
     check(json, serialized);
 }
 
 #[test]
 fn string() {
-    let json = SortedJson::serialize("he\"llo");
+    let json = OrderedJson::serialize("he\"llo").unwrap();
     let serialized = r#""he\"llo""#;
     check(json, serialized);
 }
 
 #[test]
 fn serialize_array() {
-    let json = SortedJson::serialize([3, 1, 2]);
+    let json = OrderedJson::serialize([3, 1, 2]).unwrap();
     let serialized = "[3,1,2]";
     check(json, serialized);
 }
@@ -93,18 +93,19 @@ fn serialize_array() {
 fn sorted_array() {
     let items = ["c", "a", "b"];
     let serialized = r#"["a","b","c"]"#;
-    let items: Vec<SortedJson> = items.into_iter().map(SortedJson::serialize).collect();
-    let json = SortedJson::array(items);
+    let items: Vec<OrderedJson> =
+        items.into_iter().map(OrderedJson::serialize).collect::<Result<Vec<_>, _>>().unwrap();
+    let json = OrderedJson::array_sorted(items);
     check(json, serialized);
 }
 
 #[test]
 fn nested_array() {
-    let a = SortedJson::serialize(3);
-    let b = SortedJson::serialize(2);
-    let c = SortedJson::serialize(1);
-    let d = SortedJson::serialize([1, 3, 2]);
-    let json = SortedJson::array([a, b, c, d]);
+    let a = OrderedJson::serialize(3).unwrap();
+    let b = OrderedJson::serialize(2).unwrap();
+    let c = OrderedJson::serialize(1).unwrap();
+    let d = OrderedJson::serialize([1, 3, 2]).unwrap();
+    let json = OrderedJson::array_sorted([a, b, c, d]);
     let serialized = r#"[1,2,3,[1,3,2]]"#;
     check(json, serialized);
 }
@@ -113,7 +114,8 @@ fn nested_array() {
 fn array_unsorted() {
     let items = ["c", "a", "b"];
     let serialized = r#"["c","a","b"]"#;
-    let items: Vec<SortedJson> = items.into_iter().map(SortedJson::serialize).collect();
-    let json = SortedJson::array_unsorted(items);
+    let items: Vec<OrderedJson> =
+        items.into_iter().map(OrderedJson::serialize).collect::<Result<Vec<_>, _>>().unwrap();
+    let json = OrderedJson::array_unsorted(items);
     check(json, serialized);
 }
diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs
@@ -17,7 +17,7 @@ use crate::formats::cache::{Cache, OrphanImplItem};
 use crate::formats::item_type::ItemType;
 use crate::html::format::join_with_double_colon;
 use crate::html::markdown::short_markdown_summary;
-use crate::html::render::sorted_json::SortedJson;
+use crate::html::render::ordered_json::OrderedJson;
 use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, RenderTypeId};
 
 use encode::{bitmap_to_string, write_vlqhex_to_string};
@@ -48,7 +48,7 @@ use encode::{bitmap_to_string, write_vlqhex_to_string};
 /// [2]: https://en.wikipedia.org/wiki/Sliding_window_protocol#Basic_concept
 /// [3]: https://learn.microsoft.com/en-us/troubleshoot/windows-server/networking/description-tcp-features
 pub(crate) struct SerializedSearchIndex {
-    pub(crate) index: SortedJson,
+    pub(crate) index: OrderedJson,
     pub(crate) desc: Vec<(usize, String)>,
 }
 
@@ -694,9 +694,9 @@ pub(crate) fn build_index<'tcx>(
         desc_index,
         empty_desc,
     };
-    let index = SortedJson::array_unsorted([
-        SortedJson::serialize(crate_name.as_str()),
-        SortedJson::serialize(data),
+    let index = OrderedJson::array_unsorted([
+        OrderedJson::serialize(crate_name.as_str()).unwrap(),
+        OrderedJson::serialize(data).unwrap(),
     ]);
     SerializedSearchIndex { index, desc }
 }