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

ref: Persist SourceMap name in the cache. #698

Merged
merged 7 commits into from
Oct 17, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Features**:

- Added an Object type for Portable PDB files. ([#696](https://github.com/getsentry/symbolic/pull/696))
- Version 2 of the sourcemapcache format additionally saves the names of source locations. ([#698](https://github.com/getsentry/symbolic/pull/698))

## 9.2.1

Expand Down
2 changes: 1 addition & 1 deletion symbolic-sourcemapcache/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ A fast lookup cache for JavaScript Source Maps.
edition = "2021"

[dependencies]
js-source-scopes = "0.1.0"
js-source-scopes = "0.2.0"
thiserror = "1.0.31"
sourcemap = "6.1.0"
tracing = "0.1.36"
Expand Down
15 changes: 15 additions & 0 deletions symbolic-sourcemapcache/src/lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct SourceLocation<'data> {
line: u32,
/// The source column.
column: u32,
/// The `name` of the source location.
name: Option<&'data str>,
/// The scope containing this source location.
scope: ScopeLookupResult<'data>,
}
Expand All @@ -33,6 +35,13 @@ impl<'data> SourceLocation<'data> {
self.column
}

/// The `name` of this source location as it is defined in the SourceMap.
///
/// This can be useful when inferring the name of a scope from the callers call expression.
pub fn name(&self) -> Option<&'data str> {
self.name
}

/// The contents of the source line.
pub fn line_contents(&self) -> Option<&'data str> {
self.file().and_then(|file| file.line(self.line as usize))
Expand Down Expand Up @@ -169,6 +178,11 @@ impl<'data> SourceMapCache<'data> {
.get(sl.file_idx as usize)
.and_then(|raw_file| self.resolve_file(raw_file));

let name = match sl.name_idx {
raw::NO_NAME_SENTINEL => None,
idx => self.get_string(idx),
};

let scope = match sl.scope_idx {
raw::GLOBAL_SCOPE_SENTINEL => ScopeLookupResult::Unknown,
raw::ANONYMOUS_SCOPE_SENTINEL => ScopeLookupResult::AnonymousScope,
Expand All @@ -181,6 +195,7 @@ impl<'data> SourceMapCache<'data> {
file,
line,
column,
name,
scope,
})
}
Expand Down
13 changes: 11 additions & 2 deletions symbolic-sourcemapcache/src/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ pub const SOURCEMAPCACHE_MAGIC: u32 = u32::from_le_bytes(SOURCEMAPCACHE_MAGIC_BY
pub const SOURCEMAPCACHE_MAGIC_FLIPPED: u32 = SOURCEMAPCACHE_MAGIC.swap_bytes();

/// The current Format version
pub const SOURCEMAPCACHE_VERSION: u32 = 1;
///
/// # Version History
///
/// - 2: Added `name` reference
/// - 1: Initial version
pub const SOURCEMAPCACHE_VERSION: u32 = 2;

/// The SourceMapCache binary Header.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -59,6 +64,8 @@ impl From<SourcePosition> for MinifiedSourcePosition {

/// Sentinel value used to denote unknown file.
pub const NO_FILE_SENTINEL: u32 = u32::MAX;
/// Sentinel value used to denote no `name`.
pub const NO_NAME_SENTINEL: u32 = u32::MAX;
/// Sentinel value used to denote unknown/global scope.
pub const GLOBAL_SCOPE_SENTINEL: u32 = u32::MAX;
/// Sentinel value used to denote anonymous function scope.
Expand All @@ -74,6 +81,8 @@ pub struct OriginalSourceLocation {
pub line: u32,
/// The original column number.
pub column: u32,
/// The optional `name` of this token (offset into string table).
pub name_idx: u32,
/// The optional scope name (offset into string table).
pub scope_idx: u32,
}
Expand Down Expand Up @@ -117,7 +126,7 @@ mod tests {
assert_eq!(mem::size_of::<MinifiedSourcePosition>(), 8);
assert_eq!(mem::align_of::<MinifiedSourcePosition>(), 4);

assert_eq!(mem::size_of::<OriginalSourceLocation>(), 16);
assert_eq!(mem::size_of::<OriginalSourceLocation>(), 20);
assert_eq!(mem::align_of::<OriginalSourceLocation>(), 4);
}
}
28 changes: 22 additions & 6 deletions symbolic-sourcemapcache/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use js_source_scopes::{
use sourcemap::DecodedMap;
use watto::{Pod, StringTable, Writer};

use super::raw::{self, ANONYMOUS_SCOPE_SENTINEL, GLOBAL_SCOPE_SENTINEL, NO_FILE_SENTINEL};
use super::raw;
use super::{ScopeLookupResult, SourcePosition};

/// A structure that allows quick resolution of minified source position
Expand Down Expand Up @@ -57,7 +57,16 @@ impl SourceMapCacheWriter {
};

// parse scopes out of the minified source
let scopes = extract_scope_names(source);
let scopes = match extract_scope_names(source) {
Ok(scopes) => scopes,
Err(err) => {
let err: &dyn std::error::Error = &err;
tracing::error!(error = err, "failed parsing minified source");
// even if the minified source failed parsing, we can still use the information
// from the sourcemap itself.
vec![]
}
};

// resolve scopes to original names
let ctx = SourceContext::new(source).map_err(SourceMapCacheErrorInner::SourceContext)?;
Expand Down Expand Up @@ -153,21 +162,28 @@ impl SourceMapCacheWriter {
let mut file_idx = token.get_src_id();

if file_idx >= files.len() as u32 {
file_idx = NO_FILE_SENTINEL;
file_idx = raw::NO_FILE_SENTINEL;
}

let scope_idx = match scope {
ScopeLookupResult::NamedScope(name) => {
std::cmp::min(string_table.insert(name) as u32, GLOBAL_SCOPE_SENTINEL)
std::cmp::min(string_table.insert(name) as u32, raw::GLOBAL_SCOPE_SENTINEL)
}
ScopeLookupResult::AnonymousScope => ANONYMOUS_SCOPE_SENTINEL,
ScopeLookupResult::Unknown => GLOBAL_SCOPE_SENTINEL,
ScopeLookupResult::AnonymousScope => raw::ANONYMOUS_SCOPE_SENTINEL,
ScopeLookupResult::Unknown => raw::GLOBAL_SCOPE_SENTINEL,
};

let name = token.get_name();
let name_idx = match name {
Some(name) => string_table.insert(name) as u32,
None => raw::NO_NAME_SENTINEL,
};

let sl = raw::OriginalSourceLocation {
file_idx,
line,
column,
name_idx,
scope_idx,
};

Expand Down
6 changes: 5 additions & 1 deletion symbolic-sourcemapcache/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ fn resolves_inlined_function() {
assert_eq!(sl.column(), 2);
assert_eq!(sl.scope(), ScopeLookupResult::NamedScope("bar"));

// NOTE: The last source position itself does not have a named scope, it truely is an
// anonymous function. However, the *call* itself has a `name` which we use in its place.
assert_eq!(sl.name(), Some("foo"));

let sl = cache.lookup(SourcePosition::new(0, 33)).unwrap();
assert_eq!(sl.file_name(), Some("../src/foo.js"));
assert_eq!(sl.line(), 1);
assert_eq!(sl.column(), 8);
assert_eq!(sl.scope(), ScopeLookupResult::NamedScope("foo"));
assert_eq!(sl.scope(), ScopeLookupResult::AnonymousScope);
}

#[test]
Expand Down