Skip to content

Commit

Permalink
Auto merge of #128252 - EtomicBomb:pre-rfc, r=notriddle
Browse files Browse the repository at this point in the history
modularize rustdoc's write_shared

Refactor src/librustdoc/html/render/write_shared.rs to reduce code duplication, adding unit tests

* Extract + unit test code for sorting and rendering JSON, which is duplicated 9 times in the current impl
* Extract + unit test code for encoding JSON as single quoted strings, which is duplicated twice in the current impl
* Unit tests for cross-crate information file formats
* Generic interface to add new kinds of cross-crate information files in the future
* Intended to match current behavior exactly, except for a merge info comment it adds to the bottom of cci files
* This PR is intended to reduce the review burden from my [mergeable rustdoc rfc](rust-lang/rfcs#3662) implementation PR, which is a [small commit based on this branch](https://github.com/EtomicBomb/rust/tree/rfc). This code is agnostic to the RFC and does not include any of the flags discussed there, but cleanly enables the addition of these flags in the future because it is more modular
  • Loading branch information
bors committed Aug 20, 2024
2 parents 4d5b3b1 + b4f057f commit 5aea140
Show file tree
Hide file tree
Showing 9 changed files with 1,551 additions and 683 deletions.
9 changes: 1 addition & 8 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rustc_span::edition::Edition;
use rustc_span::{sym, FileName, Symbol};

use super::print_item::{full_path, item_path, print_item};
use super::search_index::build_index;
use super::sidebar::{print_sidebar, sidebar_module_like, Sidebar};
use super::write_shared::write_shared;
use super::{collect_spans_and_sources, scrape_examples_help, AllTypes, LinkFromSrc, StylePath};
Expand Down Expand Up @@ -573,13 +572,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
}

if !no_emit_shared {
// Build our search index
let index = build_index(&krate, &mut Rc::get_mut(&mut cx.shared).unwrap().cache, tcx);

// Write shared runs within a flock; disable thread dispatching of IO temporarily.
Rc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(true);
write_shared(&mut cx, &krate, index, &md_opts)?;
Rc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(false);
write_shared(&mut cx, &krate, &md_opts, tcx)?;
}

Ok((cx, krate))
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ pub(crate) mod search_index;
mod tests;

mod context;
mod ordered_json;
mod print_item;
pub(crate) mod sidebar;
mod sorted_template;
mod span_map;
mod type_layout;
mod write_shared;
Expand Down
83 changes: 83 additions & 0 deletions src/librustdoc/html/render/ordered_json.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use std::borrow::Borrow;
use std::fmt;

use itertools::Itertools as _;
use serde::{Deserialize, Serialize};
use serde_json::Value;

/// Prerendered json.
///
/// 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 OrderedJson(String);

impl OrderedJson {
/// If you pass in an array, it will not be sorted.
pub(crate) fn serialize<T: Serialize>(item: T) -> Result<Self, serde_json::Error> {
Ok(Self(serde_json::to_string(&item)?))
}

/// Serializes and sorts
pub(crate) fn array_sorted<T: Borrow<Self>, 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()));
Self(format!("[{}]", items))
}

pub(crate) fn array_unsorted<T: Borrow<Self>, I: IntoIterator<Item = T>>(items: I) -> Self {
let items = items.into_iter().format_with(",", |item, f| f(item.borrow()));
Self(format!("[{items}]"))
}
}

impl fmt::Display for OrderedJson {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.fmt(f)
}
}

impl From<Value> for OrderedJson {
fn from(value: Value) -> Self {
let serialized =
serde_json::to_string(&value).expect("Serializing a Value to String should never fail");
Self(serialized)
}
}

impl From<OrderedJson> for Value {
fn from(json: OrderedJson) -> Self {
serde_json::from_str(&json.0).expect("OrderedJson should always store valid JSON")
}
}

/// For use in JSON.parse('{...}').
///
/// Assumes we are going to be wrapped in single quoted strings.
///
/// JSON.parse loads faster than raw JS source,
/// so this is used for large objects.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub(crate) struct EscapedJson(OrderedJson);

impl From<OrderedJson> for EscapedJson {
fn from(json: OrderedJson) -> Self {
Self(json)
}
}

impl fmt::Display for EscapedJson {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// All these `replace` calls are because we have to go through JS string
// for JSON content.
// We need to escape double quotes for the JSON
let json = self.0.0.replace('\\', r"\\").replace('\'', r"\'").replace("\\\"", "\\\\\"");
json.fmt(f)
}
}

#[cfg(test)]
mod tests;
121 changes: 121 additions & 0 deletions src/librustdoc/html/render/ordered_json/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
use super::super::ordered_json::*;

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: 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: OrderedJson = serde_json::from_str(&json).unwrap();

assert_eq!(json.to_string(), serialized);
assert_eq!(serde_json::to_string(&json).unwrap(), serialized);
}

// Make sure there is no extra level of string, plus number of escapes.
#[test]
fn escape_json_number() {
let json = OrderedJson::serialize(3).unwrap();
let json = EscapedJson::from(json);
assert_eq!(format!("{json}"), "3");
}

#[test]
fn escape_json_single_quote() {
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 = 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 = 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 = 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 = 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 = OrderedJson::serialize(3).unwrap();
let serialized = "3";
check(json, serialized);
}

#[test]
fn boolean() {
let json = OrderedJson::serialize(true).unwrap();
let serialized = "true";
check(json, serialized);
}

#[test]
fn string() {
let json = OrderedJson::serialize("he\"llo").unwrap();
let serialized = r#""he\"llo""#;
check(json, serialized);
}

#[test]
fn serialize_array() {
let json = OrderedJson::serialize([3, 1, 2]).unwrap();
let serialized = "[3,1,2]";
check(json, serialized);
}

#[test]
fn sorted_array() {
let items = ["c", "a", "b"];
let serialized = r#"["a","b","c"]"#;
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 = 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);
}

#[test]
fn array_unsorted() {
let items = ["c", "a", "b"];
let serialized = r#"["c","a","b"]"#;
let items: Vec<OrderedJson> =
items.into_iter().map(OrderedJson::serialize).collect::<Result<Vec<_>, _>>().unwrap();
let json = OrderedJson::array_unsorted(items);
check(json, serialized);
}
34 changes: 15 additions & 19 deletions src/librustdoc/html/render/search_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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::ordered_json::OrderedJson;
use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, RenderTypeId};

/// The serialized search description sharded version
Expand Down Expand Up @@ -46,7 +47,7 @@ use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, Re
/// [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: String,
pub(crate) index: OrderedJson,
pub(crate) desc: Vec<(usize, String)>,
}

Expand Down Expand Up @@ -683,24 +684,19 @@ pub(crate) fn build_index<'tcx>(
// The index, which is actually used to search, is JSON
// It uses `JSON.parse(..)` to actually load, since JSON
// parses faster than the full JavaScript syntax.
let index = format!(
r#"["{}",{}]"#,
krate.name(tcx),
serde_json::to_string(&CrateData {
items: crate_items,
paths: crate_paths,
aliases: &aliases,
associated_item_disambiguators: &associated_item_disambiguators,
desc_index,
empty_desc,
})
.expect("failed serde conversion")
// All these `replace` calls are because we have to go through JS string for JSON content.
.replace('\\', r"\\")
.replace('\'', r"\'")
// We need to escape double quotes for the JSON.
.replace("\\\"", "\\\\\"")
);
let crate_name = krate.name(tcx);
let data = CrateData {
items: crate_items,
paths: crate_paths,
aliases: &aliases,
associated_item_disambiguators: &associated_item_disambiguators,
desc_index,
empty_desc,
};
let index = OrderedJson::array_unsorted([
OrderedJson::serialize(crate_name.as_str()).unwrap(),
OrderedJson::serialize(data).unwrap(),
]);
SerializedSearchIndex { index, desc }
}

Expand Down
Loading

0 comments on commit 5aea140

Please sign in to comment.