Fix Index source map lookup

src/detector.rs

@@ -51,7 +51,7 @@ impl SourceMapRef {
         if url.starts_with("data:") {
             return None;
         }
-        resolve_url(url, &Url::from_file_path(&minified_path).ok()?)
+        resolve_url(url, &Url::from_file_path(minified_path).ok()?)
             .and_then(|x| x.to_file_path().ok())
     }
 

src/hermes.rs

@@ -3,9 +3,9 @@ use crate::encoder::{encode, Encodable};
 use crate::errors::{Error, Result};
 use crate::jsontypes::{FacebookScopeMapping, FacebookSources, RawSourceMap};
 use crate::types::{DecodedMap, RewriteOptions, SourceMap};
+use crate::utils::greatest_lower_bound;
 use crate::vlq::parse_vlq_segment_into;
 use crate::Token;
-use std::cmp::Ordering;
 use std::io::{Read, Write};
 use std::ops::{Deref, DerefMut};
 
@@ -104,24 +104,12 @@ impl SourceMapHermes {
 
         // Find the closest mapping, just like here:
         // https://github.com/facebook/metro/blob/63b523eb20e7bdf62018aeaf195bb5a3a1a67f36/packages/metro-symbolicate/src/SourceMetadataMapConsumer.js#L204-L231
-        let mapping =
-            function_map
-                .mappings
-                .binary_search_by(|o| match o.line.cmp(&token.get_src_line()) {
-                    Ordering::Equal => o.column.cmp(&token.get_src_col()),
-                    x => x,
-                });
-        let name_index = function_map
-            .mappings
-            .get(match mapping {
-                Ok(a) => a,
-                Err(a) => a.saturating_sub(1),
-            })?
-            .name_index;
-
+        let mapping = greatest_lower_bound(&function_map.mappings, &token.get_src(), |o| {
+            (o.line, o.column)
+        })?;
         function_map
             .names
-            .get(name_index as usize)
+            .get(mapping.name_index as usize)
             .map(|n| n.as_str())
     }
 

src/types.rs

@@ -10,7 +10,7 @@ use crate::encoder::encode;
 use crate::errors::{Error, Result};
 use crate::hermes::SourceMapHermes;
 use crate::sourceview::SourceView;
-use crate::utils::find_common_prefix;
+use crate::utils::{find_common_prefix, greatest_lower_bound};
 
 /// Controls the `SourceMap::rewrite` behavior
 ///
@@ -614,24 +614,8 @@ impl SourceMap {
 
     /// Looks up the closest token to a given 0-indexed line and column.
     pub fn lookup_token(&self, line: u32, col: u32) -> Option<Token<'_>> {
-        let mut low = 0;
-        let mut high = self.index.len();
-
-        while low < high {
-            let mid = (low + high) / 2;
-            let ii = &self.index[mid as usize];
-            if (line, col) < (ii.0, ii.1) {
-                high = mid;
-            } else {
-                low = mid + 1;
-            }
-        }
-
-        if low > 0 && low <= self.index.len() {
-            self.get_token(self.index[low as usize - 1].2)
-        } else {
-            None
-        }
+        let ii = greatest_lower_bound(&self.index, &(line, col), |ii| (ii.0, ii.1))?;
+        self.get_token(ii.2)
     }
 
     /// Given a location, name and minified source file resolve a minified
@@ -935,18 +919,14 @@ impl SourceMapIndex {
     /// If a sourcemap is encountered that is not embedded but just
     /// externally referenced it is silently skipped.
     pub fn lookup_token(&self, line: u32, col: u32) -> Option<Token<'_>> {
-        for section in self.sections() {
-            let (off_line, off_col) = section.get_offset();
-            if off_line < line || off_col < col {
-                continue;
-            }
-            if let Some(map) = section.get_sourcemap() {
-                if let Some(tok) = map.lookup_token(line - off_line, col - off_col) {
-                    return Some(tok);
-                }
-            }
-        }
-        None
+        let section =
+            greatest_lower_bound(&self.sections, &(line, col), SourceMapSection::get_offset)?;
+        let map = section.get_sourcemap()?;
+        let (off_line, off_col) = section.get_offset();
+        map.lookup_token(
+            line - off_line,
+            if line == off_line { col - off_col } else { col },
+        )
     }
 
     /// Flattens an indexed sourcemap into a regular one.  This requires

src/utils.rs

@@ -144,6 +144,23 @@ pub fn make_relative_path(base: &str, target: &str) -> String {
     }
 }
 
+pub fn greatest_lower_bound<'a, T, K: Ord, F: Fn(&'a T) -> K>(
+    slice: &'a [T],
+    key: &K,
+    map: F,
+) -> Option<&'a T> {
+    match slice.binary_search_by_key(key, map) {
+        Ok(index) => Some(&slice[index]),
+        Err(index) => {
+            if index > 0 {
+                Some(&slice[index - 1])
+            } else {
+                None
+            }
+        }
+    }
+}
+
 #[test]
 fn test_is_abs_path() {
     assert!(is_abs_path("C:\\foo.txt"));

tests/test_index.rs

@@ -23,7 +23,7 @@ fn test_basic_indexed_sourcemap() {
             {
                 "offset": {
                     "line": 1,
-                    "column": 0
+                    "column": 1
                 },
                 "map": {
                     "version":3,
@@ -95,6 +95,27 @@ fn test_basic_indexed_sourcemap() {
             .unwrap_or_else(|| panic!("no source for {}", filename));
         assert_eq!(&view.lines().collect::<Vec<_>>(), ref_contents);
     }
+
+    assert_eq!(
+        ism.lookup_token(0, 0).unwrap().to_tuple(),
+        ("file1.js", 0, 0, None)
+    );
+    assert_eq!(
+        ism.lookup_token(1, 0).unwrap().to_tuple(),
+        ("file1.js", 2, 12, Some("b"))
+    );
+    assert_eq!(
+        ism.lookup_token(1, 1).unwrap().to_tuple(),
+        ("file2.js", 0, 0, None)
+    );
+    assert_eq!(
+        ism.lookup_token(1, 8).unwrap().to_tuple(),
+        ("file2.js", 0, 0, None)
+    );
+    assert_eq!(
+        ism.lookup_token(1, 9).unwrap().to_tuple(),
+        ("file2.js", 0, 9, Some("multiply"))
+    );
 }
 
 #[test]

tests/test_regular.rs

@@ -0,0 +1,37 @@
+use sourcemap::SourceMap;
+
+#[test]
+fn test_basic_sourcemap() {
+    let input: &[_] = b"{
+        \"version\":3,
+        \"sources\":[\"coolstuff.js\"],
+        \"names\":[\"x\",\"alert\"],
+        \"mappings\":\"AAAA,GAAIA,GAAI,EACR,IAAIA,GAAK,EAAG,CACVC,MAAM\"
+    }";
+    let sm = SourceMap::from_reader(input).unwrap();
+
+    assert_eq!(
+        sm.lookup_token(0, 0).unwrap().to_tuple(),
+        ("coolstuff.js", 0, 0, None)
+    );
+    assert_eq!(
+        sm.lookup_token(0, 3).unwrap().to_tuple(),
+        ("coolstuff.js", 0, 4, Some("x"))
+    );
+    assert_eq!(
+        sm.lookup_token(0, 24).unwrap().to_tuple(),
+        ("coolstuff.js", 2, 8, None)
+    );
+
+    // Lines continue out to infinity
+    assert_eq!(
+        sm.lookup_token(0, 1000).unwrap().to_tuple(),
+        ("coolstuff.js", 2, 8, None)
+    );
+
+    // Token can return prior lines.
+    assert_eq!(
+        sm.lookup_token(1000, 0).unwrap().to_tuple(),
+        ("coolstuff.js", 2, 8, None)
+    );
+}