Off by one(1) error in column addressing: fixing originalPositionFor() API for indexed maps + added test adapted from the 0.6.x dev tree

lib/source-map-consumer.js

@@ -527,7 +527,11 @@ class BasicSourceMapConsumer extends SourceMapConsumer {
    */
   sourceContentFor(aSource, nullOnMissing) {
     if (!this.sourcesContent) {
-      return null;
+      if (nullOnMissing) {
+        return null;
+      }
+
+      throw new Error('"' + aSource + '" is not in the SourceMap.');
     }
 
     const index = this._findSourceIndex(aSource);
@@ -785,8 +789,7 @@ class IndexedSourceMapConsumer extends SourceMapConsumer {
           return cmp;
         }
 
-        return (aNeedle.generatedColumn -
-                section.generatedOffset.generatedColumn);
+        return aNeedle.generatedColumn - (section.generatedOffset.generatedColumn - 1);
       });
     const section = this._sections[sectionIndex];
 

lib/source-map-generator.js

@@ -71,15 +71,15 @@ class SourceMapGenerator {
     });
     aSourceMapConsumer.sources.forEach(function(sourceFile) {
       let sourceRelative = sourceFile;
-      if (sourceRoot !== null) {
+      if (sourceRoot != null) {
         sourceRelative = util.relative(sourceRoot, sourceFile);
       }
 
       if (!generator._sources.has(sourceRelative)) {
         generator._sources.add(sourceRelative);
       }
 
-      const content = aSourceMapConsumer.sourceContentFor(sourceFile);
+      const content = aSourceMapConsumer.sourceContentFor(sourceFile, true);
       if (content != null) {
         generator.setSourceContent(sourceFile, content);
       }
@@ -238,7 +238,7 @@ class SourceMapGenerator {
 
     // Copy sourcesContents of applied map.
     aSourceMapConsumer.sources.forEach(function(srcFile) {
-      const content = aSourceMapConsumer.sourceContentFor(srcFile);
+      const content = aSourceMapConsumer.sourceContentFor(srcFile, true);
       if (content != null) {
         if (aSourceMapPath != null) {
           srcFile = util.join(aSourceMapPath, srcFile);

lib/source-node.js

@@ -137,7 +137,7 @@ class SourceNode {
 
     // Copy sourcesContent into SourceNode
     aSourceMapConsumer.sources.forEach(function(sourceFile) {
-      const content = aSourceMapConsumer.sourceContentFor(sourceFile);
+      const content = aSourceMapConsumer.sourceContentFor(sourceFile, true);
       if (content != null) {
         if (aRelativePath != null) {
           sourceFile = util.join(aRelativePath, sourceFile);

test/test-source-map-consumer.js

@@ -95,7 +95,6 @@ exports["test that the source root is reflected in a mapping's source field"] =
   assert.equal(mapping.source, "/the/root/one.js");
   map.destroy();
 
-
   map = await new SourceMapConsumer(util.testMapNoSourceRoot);
 
   mapping = map.originalPositionFor({
@@ -111,7 +110,6 @@ exports["test that the source root is reflected in a mapping's source field"] =
   assert.equal(mapping.source, "one.js");
   map.destroy();
 
-
   map = await new SourceMapConsumer(util.testMapEmptySourceRoot);
 
   mapping = map.originalPositionFor({
@@ -583,7 +581,6 @@ exports["test sourceRoot + generatedPositionFor"] = async function(assert) {
     source: "bang.coffee"
   });
 
-
   map = await new SourceMapConsumer(map.toString(), "http://example.com/");
 
   // Should handle without sourceRoot.
@@ -692,6 +689,55 @@ exports["test index map + generatedPositionFor"] = async function(assert) {
   map.destroy();
 };
 
+exports["test index map + originalPositionFor"] = async function(assert) {
+  var map = await new SourceMapConsumer(
+    util.indexedTestMapWithMappingsAtSectionStart
+  );
+  assert.ok(map instanceof IndexedSourceMapConsumer);
+
+  var pos = map.originalPositionFor({
+    line: 1,
+    column: 0
+  });
+
+  assert.equal(pos.line, 1);
+  assert.equal(pos.column, 0);
+  assert.equal(pos.source, "foo.js");
+  assert.equal(pos.name, "first");
+
+  pos = map.originalPositionFor({
+    line: 1,
+    column: 1
+  });
+
+  assert.equal(pos.line, 2);
+  assert.equal(pos.column, 1);
+  assert.equal(pos.source, "bar.js");
+  assert.equal(pos.name, "second");
+
+  pos = map.originalPositionFor({
+    line: 1,
+    column: 2
+  });
+
+  assert.equal(pos.line, 1);
+  assert.equal(pos.column, 0);
+  assert.equal(pos.source, "baz.js");
+  assert.equal(pos.name, "third");
+
+  pos = map.originalPositionFor({
+    line: 1,
+    column: 3
+  });
+
+  assert.equal(pos.line, 2);
+  assert.equal(pos.column, 1);
+  assert.equal(pos.source, "quux.js");
+  assert.equal(pos.name, "fourth");
+
+  map.destroy();
+};
+
 exports["test allGeneratedPositionsFor for line"] = async function(assert) {
   let map = new SourceMapGenerator({
     file: "generated.js"

test/util.js

@@ -265,6 +265,29 @@ exports.indexedTestMapColumnOffset = {
     }
   ]
 };
+exports.indexedTestMapWithMappingsAtSectionStart = {
+  version: 3,
+  sections: [
+    {
+      offset: { line: 0, column: 0 },
+      map: {
+        version: 3,
+        names: ["first", "second"],
+        sources: ["foo.js", "bar.js"],
+        mappings: "AAAAA,CCCCC"
+      }
+    },
+    {
+      offset: { line: 0, column: 2 },
+      map: {
+        version: 3,
+        names: ["third", "fourth"],
+        sources: ["baz.js", "quux.js"],
+        mappings: "AAAAA,CCCCC"
+      }
+    }
+  ]
+};
 exports.testMapWithSourcesContent = {
   version: 3,
   file: "min.js",