From ab4b8b577f4621aee3ab1233584d254f572ffbd0 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 20 Sep 2013 18:29:42 -0700 Subject: [PATCH 1/4] Extend the test runner so that we can choose to run only a specific test file. --- test/run-tests.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/test/run-tests.js b/test/run-tests.js index 8bf2256f..626c53f7 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -22,27 +22,20 @@ function run(tests) { try { tests[i].testCase[k](assert, util); passed++; - process.stdout.write('.'); } catch (e) { - failures.push({ - name: tests[i].name + ': ' + k, - error: e - }); - process.stdout.write('E'); + console.log('FAILED ' + tests[i].name + ': ' + k + '!'); + console.log(e.stack); } } } } - process.stdout.write('\n'); + console.log(""); console.log(passed + ' / ' + total + ' tests passed.'); + console.log(""); failures.forEach(function (f) { - console.log('================================================================================'); - console.log(f.name); - console.log('--------------------------------------------------------------------------------'); - console.log(f.error.stack); }); return failures.length; @@ -55,14 +48,19 @@ process.stdout.on('close', function () { }); function isTestFile(f) { - return /^test\-.*?\.js/.test(f); + var testToRun = process.argv[2]; + return testToRun + ? path.basename(testToRun) === f + : /^test\-.*?\.js/.test(f); } function toModule(f) { return './source-map/' + f.replace(/\.js$/, ''); } -var requires = fs.readdirSync(path.join(__dirname, 'source-map')).filter(isTestFile).map(toModule); +var requires = fs.readdirSync(path.join(__dirname, 'source-map')) + .filter(isTestFile) + .map(toModule); code = run(requires.map(require).map(function (mod, i) { return { From 6dcb1bd95c767ce46eb7fd74fe0a22f368346fd8 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 20 Sep 2013 18:35:06 -0700 Subject: [PATCH 2/4] Refactor SourceMapConsumer and SourceMapGenerator to share the same internal representation of a single mapping, and to share comparators of those mappings. --- lib/source-map/binary-search.js | 2 +- lib/source-map/source-map-consumer.js | 36 +---------- lib/source-map/source-map-generator.js | 61 +++++++----------- lib/source-map/util.js | 87 ++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 72 deletions(-) diff --git a/lib/source-map/binary-search.js b/lib/source-map/binary-search.js index 7dfd0ea3..ff347c68 100644 --- a/lib/source-map/binary-search.js +++ b/lib/source-map/binary-search.js @@ -30,7 +30,7 @@ define(function (require, exports, module) { // element which is less than the one we are searching for, so we // return null. var mid = Math.floor((aHigh - aLow) / 2) + aLow; - var cmp = aCompare(aNeedle, aHaystack[mid]); + var cmp = aCompare(aNeedle, aHaystack[mid], true); if (cmp === 0) { // Found the element we are looking for. return aHaystack[mid]; diff --git a/lib/source-map/source-map-consumer.js b/lib/source-map/source-map-consumer.js index 6eaa0706..4f6c02fd 100644 --- a/lib/source-map/source-map-consumer.js +++ b/lib/source-map/source-map-consumer.js @@ -195,37 +195,7 @@ define(function (require, exports, module) { } } - this._originalMappings.sort(this._compareOriginalPositions); - }; - - /** - * Comparator between two mappings where the original positions are compared. - */ - SourceMapConsumer.prototype._compareOriginalPositions = - function SourceMapConsumer_compareOriginalPositions(mappingA, mappingB) { - if (mappingA.source > mappingB.source) { - return 1; - } - else if (mappingA.source < mappingB.source) { - return -1; - } - else { - var cmp = mappingA.originalLine - mappingB.originalLine; - return cmp === 0 - ? mappingA.originalColumn - mappingB.originalColumn - : cmp; - } - }; - - /** - * Comparator between two mappings where the generated positions are compared. - */ - SourceMapConsumer.prototype._compareGeneratedPositions = - function SourceMapConsumer_compareGeneratedPositions(mappingA, mappingB) { - var cmp = mappingA.generatedLine - mappingB.generatedLine; - return cmp === 0 - ? mappingA.generatedColumn - mappingB.generatedColumn - : cmp; + this._originalMappings.sort(util.compareByOriginalPositions); }; /** @@ -278,7 +248,7 @@ define(function (require, exports, module) { this._generatedMappings, "generatedLine", "generatedColumn", - this._compareGeneratedPositions); + util.compareByGeneratedPositions); if (mapping) { var source = util.getArg(mapping, 'source', null); @@ -372,7 +342,7 @@ define(function (require, exports, module) { this._originalMappings, "originalLine", "originalColumn", - this._compareOriginalPositions); + util.compareByOriginalPositions); if (mapping) { return { diff --git a/lib/source-map/source-map-generator.js b/lib/source-map/source-map-generator.js index f4970907..da6cf3b5 100644 --- a/lib/source-map/source-map-generator.js +++ b/lib/source-map/source-map-generator.js @@ -107,8 +107,10 @@ define(function (require, exports, module) { } this._mappings.push({ - generated: generated, - original: original, + generatedLine: generated.line, + generatedColumn: generated.column, + originalLine: original != null && original.line, + originalColumn: original != null && original.column, source: source, name: name }); @@ -169,11 +171,11 @@ define(function (require, exports, module) { // Find mappings for the "aSourceFile" this._mappings.forEach(function (mapping) { - if (mapping.source === aSourceFile && mapping.original) { + if (mapping.source === aSourceFile && mapping.originalLine) { // Check if it can be mapped by the source map, then update the mapping. var original = aSourceMapConsumer.originalPositionFor({ - line: mapping.original.line, - column: mapping.original.column + line: mapping.originalLine, + column: mapping.originalColumn }); if (original.source !== null) { // Copy mapping @@ -182,8 +184,8 @@ define(function (require, exports, module) { } else { mapping.source = original.source; } - mapping.original.line = original.line; - mapping.original.column = original.column; + mapping.originalLine = original.line; + mapping.originalColumn = original.column; if (original.name !== null && mapping.name !== null) { // Only use the identifier name if it's an identifier // in both SourceMaps @@ -251,24 +253,6 @@ define(function (require, exports, module) { } }; - function cmpLocation(loc1, loc2) { - var cmp = (loc1 && loc1.line) - (loc2 && loc2.line); - return cmp ? cmp : (loc1 && loc1.column) - (loc2 && loc2.column); - } - - function strcmp(str1, str2) { - str1 = str1 || ''; - str2 = str2 || ''; - return (str1 > str2) - (str1 < str2); - } - - function cmpMapping(mappingA, mappingB) { - return cmpLocation(mappingA.generated, mappingB.generated) || - cmpLocation(mappingA.original, mappingB.original) || - strcmp(mappingA.source, mappingB.source) || - strcmp(mappingA.name, mappingB.name); - } - /** * Serialize the accumulated mappings in to the stream of base 64 VLQs * specified by the source map format. @@ -289,44 +273,44 @@ define(function (require, exports, module) { // via the ';' separators) will be all messed up. Note: it might be more // performant to maintain the sorting as we insert them, rather than as we // serialize them, but the big O is the same either way. - this._mappings.sort(cmpMapping); + this._mappings.sort(util.compareByGeneratedPositions); for (var i = 0, len = this._mappings.length; i < len; i++) { mapping = this._mappings[i]; - if (mapping.generated.line !== previousGeneratedLine) { + if (mapping.generatedLine !== previousGeneratedLine) { previousGeneratedColumn = 0; - while (mapping.generated.line !== previousGeneratedLine) { + while (mapping.generatedLine !== previousGeneratedLine) { result += ';'; previousGeneratedLine++; } } else { if (i > 0) { - if (!cmpMapping(mapping, this._mappings[i - 1])) { + if (!util.compareByGeneratedPositions(mapping, this._mappings[i - 1])) { continue; } result += ','; } } - result += base64VLQ.encode(mapping.generated.column + result += base64VLQ.encode(mapping.generatedColumn - previousGeneratedColumn); - previousGeneratedColumn = mapping.generated.column; + previousGeneratedColumn = mapping.generatedColumn; - if (mapping.source && mapping.original) { + if (mapping.source) { result += base64VLQ.encode(this._sources.indexOf(mapping.source) - previousSource); previousSource = this._sources.indexOf(mapping.source); // lines are stored 0-based in SourceMap spec version 3 - result += base64VLQ.encode(mapping.original.line - 1 + result += base64VLQ.encode(mapping.originalLine - 1 - previousOriginalLine); - previousOriginalLine = mapping.original.line - 1; + previousOriginalLine = mapping.originalLine - 1; - result += base64VLQ.encode(mapping.original.column + result += base64VLQ.encode(mapping.originalColumn - previousOriginalColumn); - previousOriginalColumn = mapping.original.column; + previousOriginalColumn = mapping.originalColumn; if (mapping.name) { result += base64VLQ.encode(this._names.indexOf(mapping.name) @@ -359,12 +343,13 @@ define(function (require, exports, module) { if (map.sourceRoot) { source = util.relative(map.sourceRoot, source); } - return Object.prototype.hasOwnProperty.call( - this._sourcesContents, util.toSetString(source)) + return Object.prototype.hasOwnProperty.call(this._sourcesContents, + util.toSetString(source)) ? this._sourcesContents[util.toSetString(source)] : null; }, this); } + return map; }; diff --git a/lib/source-map/util.js b/lib/source-map/util.js index 10cd38fb..87946d3f 100644 --- a/lib/source-map/util.js +++ b/lib/source-map/util.js @@ -115,4 +115,91 @@ define(function (require, exports, module) { } exports.relative = relative; + function strcmp(aStr1, aStr2) { + var s1 = aStr1 || ""; + var s2 = aStr2 || ""; + return (s1 > s2) - (s1 < s2); + } + + /** + * Comparator between two mappings where the original positions are compared. + * + * Optionally pass in `true` as `onlyCompareGenerated` to consider two + * mappings with the same original source/line/column, but different generated + * line and column the same. Useful when searching for a mapping with a + * stubbed out mapping. + */ + function compareByOriginalPositions(mappingA, mappingB, onlyCompareOriginal) { + var cmp; + + cmp = strcmp(mappingA.source, mappingB.source); + if (cmp) { + return cmp; + } + + cmp = mappingA.originalLine - mappingB.originalLine; + if (cmp) { + return cmp; + } + + cmp = mappingA.originalColumn - mappingB.originalColumn; + if (cmp || onlyCompareOriginal) { + return cmp; + } + + cmp = strcmp(mappingA.name, mappingB.name); + if (cmp) { + return cmp; + } + + cmp = mappingA.generatedLine - mappingB.generatedLine; + if (cmp) { + return cmp; + } + + return mappingA.generatedColumn - mappingB.generatedColumn; + }; + exports.compareByOriginalPositions = compareByOriginalPositions; + + /** + * Comparator between two mappings where the generated positions are + * compared. + * + * Optionally pass in `true` as `onlyCompareGenerated` to consider two + * mappings with the same generated line and column, but different + * source/name/original line and column the same. Useful when searching for a + * mapping with a stubbed out mapping. + */ + function compareByGeneratedPositions(mappingA, mappingB, onlyCompareGenerated) { + var cmp; + + cmp = mappingA.generatedLine - mappingB.generatedLine; + if (cmp) { + return cmp; + } + + cmp = mappingA.generatedColumn - mappingB.generatedColumn; + if (cmp || onlyCompareGenerated) { + return cmp; + } + + cmp = strcmp(mappingA.source, mappingB.source); + if (cmp) { + return cmp; + } + + cmp = mappingA.originalLine - mappingB.originalLine; + if (cmp) { + return cmp; + } + + cmp = mappingA.originalColumn - mappingB.originalColumn; + if (cmp) { + return cmp; + } + + return strcmp(mappingA.name, mappingB.name); + }; + exports.compareByGeneratedPositions = compareByGeneratedPositions; + }); From f2094c490189ff845e4bc7f56c0a7f98a6ae6302 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 23 Sep 2013 09:48:18 -0700 Subject: [PATCH 3/4] Add SourceMapConsumer.fromSourceMap to create SourceMapConsumer instances from SourceMapGenerator instances. --- lib/source-map/source-map-consumer.js | 26 +++++++++ lib/source-map/source-map-generator.js | 24 +++++---- test/source-map/test-source-map-consumer.js | 60 +++++++++++++++++++++ 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/lib/source-map/source-map-consumer.js b/lib/source-map/source-map-consumer.js index 4f6c02fd..8f0d9fed 100644 --- a/lib/source-map/source-map-consumer.js +++ b/lib/source-map/source-map-consumer.js @@ -100,6 +100,32 @@ define(function (require, exports, module) { this._parseMappings(mappings, sourceRoot); } + /** + * Create a SourceMapConsumer from a SourceMapGenerator. + * + * @param SourceMapGenerator aSourceMap + * The source map that will be consumed. + * @returns SourceMapConsumer + */ + SourceMapConsumer.fromSourceMap = + function SourceMapConsumer_fromSourceMap(aSourceMap) { + var smc = Object.create(SourceMapConsumer.prototype); + + smc._names = ArraySet.fromArray(aSourceMap._names.toArray(), true); + smc._sources = ArraySet.fromArray(aSourceMap._sources.toArray(), true); + smc.sourceRoot = aSourceMap._sourceRoot; + smc.sourcesContent = aSourceMap._generateSourcesContent(smc._sources.toArray(), + smc.sourceRoot); + smc.file = aSourceMap._file; + + smc._generatedMappings = aSourceMap._mappings.slice() + .sort(util.compareByGeneratedPositions); + smc._originalMappings = aSourceMap._mappings.slice() + .sort(util.compareByOriginalPositions); + + return smc; + }; + /** * The version of the source mapping spec that we are consuming. */ diff --git a/lib/source-map/source-map-generator.js b/lib/source-map/source-map-generator.js index da6cf3b5..f113d4e1 100644 --- a/lib/source-map/source-map-generator.js +++ b/lib/source-map/source-map-generator.js @@ -323,6 +323,20 @@ define(function (require, exports, module) { return result; }; + SourceMapGenerator.prototype._generateSourcesContent = + function SourceMapGenerator_generateSourcesContent(aSources, aSourceRoot) { + return aSources.map(function (source) { + if (aSourceRoot) { + source = util.relative(aSourceRoot, source); + } + var key = util.toSetString(source); + return Object.prototype.hasOwnProperty.call(this._sourcesContents, + key) + ? this._sourcesContents[key] + : null; + }, this); + }; + /** * Externalize the source map. */ @@ -339,15 +353,7 @@ define(function (require, exports, module) { map.sourceRoot = this._sourceRoot; } if (this._sourcesContents) { - map.sourcesContent = map.sources.map(function (source) { - if (map.sourceRoot) { - source = util.relative(map.sourceRoot, source); - } - return Object.prototype.hasOwnProperty.call(this._sourcesContents, - util.toSetString(source)) - ? this._sourcesContents[util.toSetString(source)] - : null; - }, this); + map.sourcesContent = this._generateSourcesContent(map.sources, map.sourceRoot); } return map; diff --git a/test/source-map/test-source-map-consumer.js b/test/source-map/test-source-map-consumer.js index ffab45a3..f2c65a7f 100644 --- a/test/source-map/test-source-map-consumer.js +++ b/test/source-map/test-source-map-consumer.js @@ -388,4 +388,64 @@ define(function (require, exports, module) { assert.equal(pos.column, 5); }; + exports['test SourceMapConsumer.fromSourceMap'] = function (assert, util) { + var smg = new SourceMapGenerator({ + sourceRoot: 'http://example.com/', + file: 'foo.js' + }); + smg.addMapping({ + original: { line: 1, column: 1 }, + generated: { line: 2, column: 2 }, + source: 'bar.js' + }); + smg.addMapping({ + original: { line: 2, column: 2 }, + generated: { line: 4, column: 4 }, + source: 'baz.js', + name: 'dirtMcGirt' + }); + smg.setSourceContent('baz.js', 'baz.js content'); + + var smc = SourceMapConsumer.fromSourceMap(smg); + assert.equal(smc.file, 'foo.js'); + assert.equal(smc.sourceRoot, 'http://example.com/'); + assert.equal(smc.sources.length, 2); + assert.equal(smc.sources[0], 'http://example.com/bar.js'); + assert.equal(smc.sources[1], 'http://example.com/baz.js'); + assert.equal(smc.sourceContentFor('baz.js'), 'baz.js content'); + + var pos = smc.originalPositionFor({ + line: 2, + column: 2 + }); + assert.equal(pos.line, 1); + assert.equal(pos.column, 1); + assert.equal(pos.source, 'http://example.com/bar.js'); + assert.equal(pos.name, null); + + pos = smc.generatedPositionFor({ + line: 1, + column: 1, + source: 'http://example.com/bar.js' + }); + assert.equal(pos.line, 2); + assert.equal(pos.column, 2); + + pos = smc.originalPositionFor({ + line: 4, + column: 4 + }); + assert.equal(pos.line, 2); + assert.equal(pos.column, 2); + assert.equal(pos.source, 'http://example.com/baz.js'); + assert.equal(pos.name, 'dirtMcGirt'); + + pos = smc.generatedPositionFor({ + line: 2, + column: 2, + source: 'http://example.com/baz.js' + }); + assert.equal(pos.line, 4); + assert.equal(pos.column, 4); + }; }); From 753f95b7b716eb669cea14543a34b733323e156b Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 24 Sep 2013 12:29:07 -0700 Subject: [PATCH 4/4] Use for loops instead of .forEach for some perf gains --- lib/source-map/source-node.js | 22 +++++++++++++--------- test/source-map/test-source-node.js | 16 +++++++++++----- test/source-map/util.js | 3 ++- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/lib/source-map/source-node.js b/lib/source-map/source-node.js index 9c9cf1c3..626cb657 100644 --- a/lib/source-map/source-node.js +++ b/lib/source-map/source-node.js @@ -193,7 +193,9 @@ define(function (require, exports, module) { * @param aFn The traversal function. */ SourceNode.prototype.walk = function SourceNode_walk(aFn) { - this.children.forEach(function (chunk) { + var chunk; + for (var i = 0, len = this.children.length; i < len; i++) { + chunk = this.children[i]; if (chunk instanceof SourceNode) { chunk.walk(aFn); } @@ -205,7 +207,7 @@ define(function (require, exports, module) { name: this.name }); } } - }, this); + } }; /** @@ -271,14 +273,16 @@ define(function (require, exports, module) { */ SourceNode.prototype.walkSourceContents = function SourceNode_walkSourceContents(aFn) { - this.children.forEach(function (chunk) { - if (chunk instanceof SourceNode) { - chunk.walkSourceContents(aFn); + for (var i = 0, len = this.children.length; i < len; i++) { + if (this.children[i] instanceof SourceNode) { + this.children[i].walkSourceContents(aFn); } - }, this); - Object.keys(this.sourceContents).forEach(function (sourceFileKey) { - aFn(util.fromSetString(sourceFileKey), this.sourceContents[sourceFileKey]); - }, this); + } + + var sources = Object.keys(this.sourceContents); + for (var i = 0, len = sources.length; i < len; i++) { + aFn(util.fromSetString(sources[i]), this.sourceContents[sources[i]]); + } }; /** diff --git a/test/source-map/test-source-node.js b/test/source-map/test-source-node.js index 93e8971c..6e0eca82 100644 --- a/test/source-map/test-source-node.js +++ b/test/source-map/test-source-node.js @@ -257,11 +257,17 @@ define(function (require, exports, module) { exports['test .fromStringWithSourceMap() merging duplicate mappings'] = function (assert, util) { var input = new SourceNode(null, null, null, [ - new SourceNode(1, 0, "a.js", "(function"), new SourceNode(1, 0, "a.js", "() {\n"), - " ", new SourceNode(1, 0, "a.js", "var Test = "), new SourceNode(1, 0, "b.js", "{};\n"), - new SourceNode(2, 0, "b.js", "Test"), new SourceNode(2, 0, "b.js", ".A", "A"), new SourceNode(2, 20, "b.js", " = { value: 1234 };\n", "A"), - "}());\n", - "/* Generated Source */"]); + new SourceNode(1, 0, "a.js", "(function"), + new SourceNode(1, 0, "a.js", "() {\n"), + " ", + new SourceNode(1, 0, "a.js", "var Test = "), + new SourceNode(1, 0, "b.js", "{};\n"), + new SourceNode(2, 0, "b.js", "Test"), + new SourceNode(2, 0, "b.js", ".A", "A"), + new SourceNode(2, 20, "b.js", " = { value: 1234 };\n", "A"), + "}());\n", + "/* Generated Source */" + ]); input = input.toStringWithSourceMap({ file: 'foo.js' }); diff --git a/test/source-map/util.js b/test/source-map/util.js index 8a9c53e2..288046bf 100644 --- a/test/source-map/util.js +++ b/test/source-map/util.js @@ -143,7 +143,8 @@ define(function (require, exports, module) { expectedMap.sourceRoot, "sourceRoot mismatch: " + actualMap.sourceRoot + " != " + expectedMap.sourceRoot); - assert.equal(actualMap.mappings, expectedMap.mappings, "mappings mismatch"); + assert.equal(actualMap.mappings, expectedMap.mappings, + "mappings mismatch:\nActual: " + actualMap.mappings + "\nExpected: " + expectedMap.mappings); if (actualMap.sourcesContent) { assert.equal(actualMap.sourcesContent.length, expectedMap.sourcesContent.length,