Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(sourcemap)!: avoid passing Results #4541

Merged
merged 1 commit into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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