Skip to content

Commit

Permalink
refactor(sourcemap)!: avoid passing Results (#4541)
Browse files Browse the repository at this point in the history
Refactor building sourcemap JSON to avoid passing `Result`s. `Serialize::serialize` is infallible here as writing to a `Vec<u8>` is infallible.
  • Loading branch information
overlookmotel committed Jul 30, 2024
1 parent 9fcd9ae commit 27fd062
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 37 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_codegen/examples/sourcemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn main() -> std::io::Result<()> {
.build(&ret.program);

if let Some(source_map) = source_map {
let result = source_map.to_json_string().unwrap();
let result = source_map.to_json_string();
let hash = BASE64_STANDARD.encode(format!(
"{}\0{}{}\0{}",
source_text.len(),
Expand Down
40 changes: 19 additions & 21 deletions crates/oxc_sourcemap/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::borrow::Cow;
#[cfg(feature = "concurrent")]
use rayon::prelude::*;

use crate::Error;
use crate::JSONSourceMap;
/// Port from https://github.com/getsentry/rust-sourcemap/blob/master/src/encoder.rs
/// It is a helper for encode `SourceMap` to vlq sourcemap string, but here some different.
Expand All @@ -27,7 +26,7 @@ pub fn encode(sourcemap: &SourceMap) -> JSONSourceMap {

// Here using `serde_json` to serialize `names` / `source_contents` / `sources`.
// It will escape the string to avoid invalid JSON string.
pub fn encode_to_string(sourcemap: &SourceMap) -> Result<String, Error> {
pub fn encode_to_string(sourcemap: &SourceMap) -> String {
let max_segments = 12
+ sourcemap.names.len() * 2
+ sourcemap.sources.len() * 2
Expand All @@ -49,11 +48,10 @@ pub fn encode_to_string(sourcemap: &SourceMap) -> Result<String, Error> {
}

contents.push("\"names\":[".into());
contents.push_list(sourcemap.names.iter().map(escape_json_string))?;
contents.push_list(sourcemap.names.iter().map(escape_json_string));

contents.push("],\"sources\":[".into());

contents.push_list(sourcemap.sources.iter().map(escape_json_string))?;
contents.push_list(sourcemap.sources.iter().map(escape_json_string));

// Quote `source_content` in parallel
if let Some(source_contents) = &sourcemap.source_contents {
Expand All @@ -64,7 +62,7 @@ pub fn encode_to_string(sourcemap: &SourceMap) -> Result<String, Error> {
.par_iter()
.map(escape_json_string)
.collect();
contents.push_list(quoted_source_contents.into_iter())?;
contents.push_list(quoted_source_contents.into_iter());
} else {
contents.push_list(source_contents.iter().map(escape_json_string));
}
Expand All @@ -73,8 +71,7 @@ pub fn encode_to_string(sourcemap: &SourceMap) -> Result<String, Error> {

if let Some(x_google_ignore_list) = &sourcemap.x_google_ignore_list {
contents.push("],\"x_google_ignoreList\":[".into());
contents
.push_list(x_google_ignore_list.iter().map(std::string::ToString::to_string).map(Ok))?;
contents.push_list(x_google_ignore_list.iter().map(ToString::to_string));
}

contents.push("],\"mappings\":\"".into());
Expand All @@ -84,7 +81,7 @@ pub fn encode_to_string(sourcemap: &SourceMap) -> Result<String, Error> {
// Check we calculated number of segments required correctly
debug_assert!(contents.num_segments() <= max_segments);

Ok(contents.consume())
contents.consume()
}

#[allow(clippy::cast_possible_truncation)]
Expand Down Expand Up @@ -211,20 +208,19 @@ impl<'a> PreAllocatedString<'a> {
}

#[inline]
fn push_list<I>(&mut self, mut iter: I) -> Result<(), Error>
fn push_list<I>(&mut self, mut iter: I)
where
I: Iterator<Item = Result<String, Error>>,
I: Iterator<Item = String>,
{
let Some(first) = iter.next() else {
return Ok(());
return;
};
self.push(Cow::Owned(first?));
self.push(Cow::Owned(first));

for other in iter {
self.push(Cow::Borrowed(","));
self.push(Cow::Owned(other?));
self.push(Cow::Owned(other));
}
Ok(())
}

#[inline]
Expand All @@ -239,12 +235,14 @@ impl<'a> PreAllocatedString<'a> {
}
}

fn escape_json_string<S: AsRef<str>>(s: S) -> Result<String, Error> {
fn escape_json_string<S: AsRef<str>>(s: S) -> String {
let s = s.as_ref();
let mut escaped_buf = Vec::with_capacity(s.len() * 2 + 2);
serde::Serialize::serialize(s, &mut serde_json::Serializer::new(&mut escaped_buf))?;
// This call is infallible as only error it can return is if the writer errors.
// Writing to a `Vec<u8>` is infallible, so that's not possible here.
serde::Serialize::serialize(s, &mut serde_json::Serializer::new(&mut escaped_buf)).unwrap();
// Safety: `escaped_buf` is valid utf8.
Ok(unsafe { String::from_utf8_unchecked(escaped_buf) })
unsafe { String::from_utf8_unchecked(escaped_buf) }
}

#[test]
Expand All @@ -267,7 +265,7 @@ fn test_escape_json_string() {
for (c, expected) in FIXTURES {
let mut input = String::new();
input.push(*c);
assert_eq!(escape_json_string(input).unwrap(), *expected);
assert_eq!(escape_json_string(input), *expected);
}
}

Expand All @@ -281,7 +279,7 @@ fn test_encode() {
"mappings": "AAAA,GAAIA,GAAI,EACR,IAAIA,GAAK,EAAG,CACVC,MAAM"
}"#;
let sm = SourceMap::from_json_string(input).unwrap();
let sm2 = SourceMap::from_json_string(&sm.to_json_string().unwrap()).unwrap();
let sm2 = SourceMap::from_json_string(&sm.to_json_string()).unwrap();

for (tok1, tok2) in sm.get_tokens().zip(sm2.get_tokens()) {
assert_eq!(tok1, tok2);
Expand All @@ -302,7 +300,7 @@ fn test_encode_escape_string() {
);
sm.set_x_google_ignore_list(vec![0]);
assert_eq!(
sm.to_json_string().unwrap(),
sm.to_json_string(),
r#"{"version":3,"names":["name_length_greater_than_16_\u0000"],"sources":["\u0000"],"sourcesContent":["emoji-👀-\u0000"],"x_google_ignoreList":[0],"mappings":""}"#
);
}
17 changes: 4 additions & 13 deletions crates/oxc_sourcemap/src/sourcemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,28 +63,19 @@ impl SourceMap {
}

/// Convert `SourceMap` to vlq sourcemap.
/// # Errors
///
/// The `serde_json` serialization Error.
pub fn to_json(&self) -> JSONSourceMap {
encode(self)
}

/// Convert `SourceMap` to vlq sourcemap string.
/// # Errors
///
/// The `serde_json` serialization Error.
pub fn to_json_string(&self) -> Result<String> {
pub fn to_json_string(&self) -> String {
encode_to_string(self)
}

/// Convert `SourceMap` to vlq sourcemap data url.
/// # Errors
///
/// The `serde_json` serialization Error.
pub fn to_data_url(&self) -> Result<String> {
let base_64_str = base64_simd::STANDARD.encode_to_string(self.to_json_string()?.as_bytes());
Ok(format!("data:application/json;charset=utf-8;base64,{base_64_str}"))
pub fn to_data_url(&self) -> String {
let base_64_str = base64_simd::STANDARD.encode_to_string(self.to_json_string().as_bytes());
format!("data:application/json;charset=utf-8;base64,{base_64_str}")
}

pub fn get_file(&self) -> Option<&str> {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_sourcemap/src/sourcemap_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,5 @@ fn test_sourcemap_builder() {
assert_eq!(sm.get_file(), Some("file"));

let expected = r#"{"version":3,"file":"file","names":["x"],"sources":["baz.js"],"sourcesContent":[""],"mappings":""}"#;
assert_eq!(expected, sm.to_json_string().unwrap());
assert_eq!(expected, sm.to_json_string());
}
2 changes: 1 addition & 1 deletion tasks/benchmark/benches/sourcemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn bench_sourcemap(criterion: &mut Criterion) {
for i in 0..2 {
concat_sourcemap_builder.add_sourcemap(&sourcemap, lines * i);
}
concat_sourcemap_builder.into_sourcemap().to_json_string().unwrap();
concat_sourcemap_builder.into_sourcemap().to_json_string();
}
});
});
Expand Down

0 comments on commit 27fd062

Please sign in to comment.