Improve findClosestMapping (#50)

* find closest mapping should return null if no mapping is found

* add test and fix another edge case

* use .at as it checks boundaries

* use lower_bound

* code cleanup

* remove segfault-handler for now...

* more test

* bump version

Author: DeMoorJasper

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@parcel/source-map",
-  "version": "2.0.0-alpha.4.20",
+  "version": "2.0.0-alpha.4.21",
   "main": "./dist/node.js",
   "types": "index.d.ts",
   "browser": "./dist/wasm-browser.js",

src/MappingContainer.cpp

@@ -188,32 +188,12 @@ std::string MappingContainer::toVLQMappings() {
 }
 
 Mapping MappingContainer::findClosestMapping(int lineIndex, int columnIndex) {
-    if (lineIndex <= _generated_lines) {
+    if (lineIndex <= _generated_lines && lineIndex >= 0) {
         auto &line = _mapping_lines.at(lineIndex);
-        auto &segments = line._segments;
-        unsigned int segmentsCount = segments.size();
-
-        int startIndex = 0;
-        int stopIndex = segmentsCount - 1;
-        int middleIndex = ((stopIndex + startIndex) / 2);
-        while (startIndex < stopIndex) {
-            Mapping &mapping = segments[middleIndex];
-            int diff = mapping.generated.column - columnIndex;
-            if (diff > 0) {
-                --stopIndex;
-            } else if (diff < 0) {
-                ++startIndex;
-            } else {
-                break;
-            }
-
-            middleIndex = ((stopIndex + startIndex) / 2);
-        }
-
-        return segments[middleIndex];
+        return line.findClosestMapping(columnIndex);
+    } else {
+        return Mapping{Position{-1, -1}, Position{-1, -1}, -1, -1};
     }
-
-    return Mapping{Position{-1, -1}, Position{-1, -1}, -1, -1};
 }
 
 int MappingContainer::getTotalSegments() {

src/MappingLine.cpp

@@ -34,6 +34,42 @@ int MappingLine::lineNumber() {
     return _line_number;
 }
 
+int MappingLine::lastColumn() {
+    return _last_column;
+}
+
+Mapping MappingLine::findClosestMapping(int columnIndex) {
+    // Ensure mappings are sorted
+    sort();
+
+    if (_segments.size() > 0) {
+        Mapping searchValue = Mapping{Position{_line_number, columnIndex}, Position{-1, -1}, -1, -1};
+        std::vector<Mapping>::iterator low = std::lower_bound(_segments.begin(), _segments.end(), searchValue, [](const Mapping& mappingA, const Mapping& mappingB){
+            return mappingA.generated.column < mappingB.generated.column;
+        });
+
+        if (low != _segments.end()) {
+            Mapping currMapping = *low;
+            if (++low != _segments.end()) {
+                Mapping nextMapping = *low;
+
+                int nextMappingDiff = nextMapping.generated.column - columnIndex;
+                int currMappingDiff = columnIndex - currMapping.generated.column;
+
+                if (nextMappingDiff < currMappingDiff) {
+                    return nextMapping;
+                }
+            }
+
+            return currMapping;
+        } else {
+            return _segments.at(0);
+        }
+    }
+
+    return Mapping{Position{-1, -1}, Position{-1, -1}, -1, -1};
+}
+
 void MappingLine::clearMappings() {
     this->_segments.clear();
     this->_last_column = 0;

src/MappingLine.h

@@ -15,6 +15,10 @@ class MappingLine {
 
     int lineNumber();
 
+    int lastColumn();
+
+    Mapping findClosestMapping(int columnIndex);
+
     std::vector<Mapping> _segments;
 
 private:

src/node.js

@@ -46,8 +46,12 @@ export default class NodeSourceMap extends SourceMap {
 
   findClosestMapping(line: number, column: number): ?IndexedMapping<string> {
     let mapping = this.sourceMapInstance.findClosestMapping(line, column);
-    let v = this.indexedMappingToStringMapping(mapping);
-    return v;
+    if (mapping.generated.line === -1 || mapping.generated.column === -1) {
+      return null;
+    } else {
+      let v = this.indexedMappingToStringMapping(mapping);
+      return v;
+    }
   }
 
   delete() {}

src/wasm.js

@@ -96,7 +96,7 @@ export default class WasmSourceMap extends SourceMap {
 
   findClosestMapping(line: number, column: number): ?IndexedMapping<string> {
     let mapping = this.sourceMapInstance.findClosestMapping(line, column);
-    if (mapping.generated.line === -1) {
+    if (mapping.generated.line === -1 || mapping.generated.column === -1) {
       return null;
     } else {
       let m = { ...mapping };

test/find.test.js

@@ -10,7 +10,14 @@ describe('SourceMap - Find', () => {
       names: [],
     });
 
-    let mapping = map.findClosestMapping(2, 14);
+    let mapping = map.findClosestMapping(2, 8);
+    assert.deepEqual(mapping, {
+      generated: { line: 2, column: 9 },
+      original: { line: 1, column: 7 },
+      source: './helloworld.coffee',
+    });
+
+    mapping = map.findClosestMapping(2, 14);
     assert.deepEqual(mapping, {
       generated: { line: 2, column: 14 },
       original: { line: 1, column: 12 },
@@ -23,5 +30,22 @@ describe('SourceMap - Find', () => {
       original: { line: 1, column: 0 },
       source: './helloworld.coffee',
     });
+
+    mapping = map.findClosestMapping(3, 15);
+    assert.deepEqual(mapping, {
+      generated: { line: 3, column: 0 },
+      original: { line: 1, column: 0 },
+      source: './helloworld.coffee',
+    });
+
+    // Edge cases
+    mapping = map.findClosestMapping(50, 15);
+    assert.equal(mapping, null);
+
+    mapping = map.findClosestMapping(0, 0);
+    assert.deepEqual(mapping, null);
+
+    mapping = map.findClosestMapping(-2, -6);
+    assert.deepEqual(mapping, null);
   });
 });