From b2cbbe40bd0a20d90809b0c249bfd13642c2136f Mon Sep 17 00:00:00 2001 From: Brent Wilton Date: Fri, 21 Jul 2017 12:48:48 +1200 Subject: [PATCH 1/6] Single pass ReplaceSource node Update ReplaceSource .node to process all replacements in a single pass --- lib/ReplaceSource.js | 260 +++++++++++++++++++++++++++++++++---------- 1 file changed, 200 insertions(+), 60 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index c7999b8..46ac648 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -66,30 +66,18 @@ class ReplaceSource extends Source { } node(options) { - this._sortReplacements(); - var result = [this._source.node(options)]; - this.replacements.forEach(function(repl) { - var remSource = result.pop(); - var splitted1 = this._splitSourceNode(remSource, Math.floor(repl[1] + 1)); - var splitted2; - if(Array.isArray(splitted1)) { - splitted2 = this._splitSourceNode(splitted1[0], Math.floor(repl[0])); - if(Array.isArray(splitted2)) { - result.push(splitted1[1], this._replacementToSourceNode(splitted2[1], repl[2]), splitted2[0]); - } else { - result.push(splitted1[1], this._replacementToSourceNode(splitted1[1], repl[2]), splitted1[0]); - } - } else { - splitted2 = this._splitSourceNode(remSource, Math.floor(repl[0])); - if(Array.isArray(splitted2)) { - result.push(this._replacementToSourceNode(splitted2[1], repl[2]), splitted2[0]); - } else { - result.push(repl[2], remSource); - } - } - }, this); - result = result.reverse(); - return new SourceNode(null, null, null, result); + var node = this._source.node(options); + if(this.replacements.length === 0) { + return node; + } + this._sortReplacementsAscending(); + var replace = new ReplacementEnumerator(this.replacements); + var output = []; + this._prependStartNodes(output, replace); + this._replaceInNode(output, replace, node, 0, null); + this._appendRemainingNodes(output, replace); + var result = new SourceNode(null, null, null, output); + return result; } listMap(options) { @@ -144,52 +132,204 @@ class ReplaceSource extends Source { return map; } - _replacementToSourceNode(oldNode, newString) { - var map = oldNode.toStringWithSourceMap({ - file: "?" - }).map; - var original = new SourceMapConsumer(map.toJSON()).originalPositionFor({ - line: 1, - column: 0 + _splitString(str, position) { + return position <= 0 ? ["", str] : [str.substr(0, position), str.substr(position)]; + } + + _sortReplacementsAscending() { + this.replacements.sort(function(a, b) { + var diff = a[1] - b[1]; // end + if(diff !== 0) + return diff; + diff = a[0] - b[0]; // start + if(diff !== 0) + return diff; + return a[3] - b[3]; // insert order }); - if(original) { - return new SourceNode(original.line, original.column, original.source, newString); - } else { - return newString; + } + + _replaceInNode(output, replace, node, position) { + var outputChildren = []; + var allChildrenAreStrings = true; + + for(var i = 0, len = node.children.length; i < len; i++) { + var child = node.children[i]; + if(typeof child !== 'string') { + position = this._replaceInNode(outputChildren, replace, child, position); + allChildrenAreStrings = false; + } else { + position = this._replaceInStringNode(outputChildren, replace, child, position, node); + } } + if(outputChildren.length > 0) { + if(allChildrenAreStrings) { + for(i = 0; i < outputChildren.length; i++) + output.push(outputChildren[i]); + } else { + var outputNode = new SourceNode( + node.line, + node.column, + node.source, + outputChildren, + node.name + ); + if(node.sourceContents) + outputNode.sourceContents = node.sourceContents; + output.push(outputNode); + } + } + return position; } - _splitSourceNode(node, position) { - if(typeof node === "string") { - if(node.length <= position) return position - node.length; - return position <= 0 ? ["", node] : [node.substr(0, position), node.substr(position)]; + _replaceInStringNode(output, replace, node, position, parent) { + var splitPosition = replace.position - position; + // If multiple replaces occur in the same location then the splitPosition may be + // before the current position for the subsequent splits. Ensure it is >= 0 + if(splitPosition < 0) + splitPosition = 0; + if(splitPosition >= node.length || replace.done) { + if(replace.emit) { + var nodeEnd = new SourceNode( + parent.line, + parent.column, + parent.source, + node, + parent.name + ); + if(parent.sourceContents) + nodeEnd.sourceContents = parent.sourceContents; + output.push(nodeEnd); + } + return(position + node.length); + } + var emit = replace.next(); + if(!emit) { + // Stop emitting when we have found the beginning of the string to replace. + // Emit the part of the string before splitPosition + var nodeStart = new SourceNode( + parent.line, + parent.column, + parent.source, + node.substr(0, splitPosition), + parent.name + ); + if(parent.sourceContents) + nodeStart.sourceContents = parent.sourceContents; + output.push(nodeStart); + + // We should advance the current column position by splitPosition characters at this point. + // However if multiple ReplaceSource's are chained (as occurs when using ModuleConcatenationPlugin) + // then the column position shows the split position of the intermediate source map and not the original + + // Emit the replacement value + if(replace.value && replace.value.length > 0) { + output.push(new SourceNode( + parent.line, + parent.column, + parent.source, + replace.value, + parent.name + )); + } + } + + // Recurse with remainder of the string as there may be multiple replaces within a single node + var remainder = node.substr(splitPosition, node.length); + return this._replaceInStringNode(output, replace, remainder, position + splitPosition, parent); + } + + _prependStartNodes(output, replace) { + // If any replacements occur before the start of the original file, then we prepend them + // directly to the start of the output + var startValues = replace.header(); + for(var i = 0; i < startValues.length; i++) { + output.push(startValues[i]); + } + } + + _appendRemainingNodes(output, replace) { + // If any replacements occur after the end of the original file, then we append them + // directly to the end of the output + var remainingValues = replace.footer(); + for(var i = 0; i < remainingValues.length; i++) { + output.push(remainingValues[i]); + } + } +} + +class ReplacementEnumerator { + constructor(replacements) { + this.emit = true; + this.done = !replacements || replacements.length === 0; + this.index = 0; + this.replacements = replacements; + if(!this.done) { + // Set initial start position in case .header is not called + var repl = replacements[0]; + this.position = Math.floor(repl[0]); + if(this.position < 0) + this.position = 0; + } + } + + next() { + if(this.done) + return true; + if(this.emit) { + // Start point found. stop emitting. set position to find end + var repl = this.replacements[this.index]; + var end = Math.floor(repl[1] + 1); + this.position = end; + this.value = repl[2]; } else { - for(var i = 0; i < node.children.length; i++) { - position = this._splitSourceNode(node.children[i], position); - if(Array.isArray(position)) { - var leftNode = new SourceNode( - node.line, - node.column, - node.source, - node.children.slice(0, i).concat([position[0]]), - node.name - ); - var rightNode = new SourceNode( - node.line, - node.column, - node.source, [position[1]].concat(node.children.slice(i + 1)), - node.name - ); - leftNode.sourceContents = node.sourceContents; - return [leftNode, rightNode]; + // End point found. start emitting. set position to find next start + this.index++; + if(this.index >= this.replacements.length) { + this.done = true; + } else { + var nextRepl = this.replacements[this.index]; + var start = Math.floor(nextRepl[0]); + this.position = start; + } + } + if(this.position < 0) + this.position = 0; + this.emit = !this.emit; + return this.emit; + } + + header() { + var output = []; + var inHeader = true; + while(inHeader && !this.done) { + var repl = this.replacements[this.index]; + var start = Math.floor(repl[0]); + this.position = start; + + // Replicate previous ReplaceSource behavior: + // In order to generate identical source maps to the previous version of webpack we should not output the last header + // node as a string if the next replace starts above position 0 + var nextReplAtStart = this.index == (this.replacements.length - 1) ? + false : + Math.floor(this.replacements[this.index + 1][0]) <= 0; + + if(start < 0 && nextReplAtStart) { + output.push(repl[2]); + this.index++; + if(this.index >= this.replacements.length) { + this.done = true; } + } else { + inHeader = false; } - return position; } + if(this.position < 0) + this.position = 0; + return output; } - _splitString(str, position) { - return position <= 0 ? ["", str] : [str.substr(0, position), str.substr(position)]; + footer() { + return this.done ? [] : this.replacements.slice(this.index).map(repl => repl[2]); } } From 6ac5075c9b5334f9dbaead411d69ee6708ad8f43 Mon Sep 17 00:00:00 2001 From: Brent Wilton Date: Mon, 31 Jul 2017 11:36:08 +1200 Subject: [PATCH 2/6] Add unit test coverage for ReplaceSource --- lib/ReplaceSource.js | 3 -- test/ReplaceSource.js | 71 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index 46ac648..24e510c 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -316,9 +316,6 @@ class ReplacementEnumerator { if(start < 0 && nextReplAtStart) { output.push(repl[2]); this.index++; - if(this.index >= this.replacements.length) { - this.done = true; - } } else { inHeader = false; } diff --git a/test/ReplaceSource.js b/test/ReplaceSource.js index 1729f19..dbaf820 100644 --- a/test/ReplaceSource.js +++ b/test/ReplaceSource.js @@ -73,4 +73,75 @@ describe("ReplaceSource", function() { resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); resultListMap.map.mappings.should.be.eql("AAAA,cACA"); }); + + it("should prepend items correctly", function() { + var source = new ReplaceSource( + new OriginalSource("Line 1", "file.txt") + ); + source.insert(-1, "Line 0\n"); + var resultText = source.source(); + var resultMap = source.sourceAndMap({ + columns: true + }); + var resultListMap = source.sourceAndMap({ + columns: false + }); + resultText.should.be.eql("Line 0\nLine 1"); + resultMap.source.should.be.eql(resultText); + resultListMap.source.should.be.eql(resultText); + resultListMap.map.file.should.be.eql(resultMap.map.file); + resultListMap.map.version.should.be.eql(resultMap.map.version); + resultListMap.map.sources.should.be.eql(resultMap.map.sources); + resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); + resultMap.map.mappings.should.be.eql("AAAA;AAAA"); + }); + + it("should prepend items with replace at start correctly", function() { + var source = new ReplaceSource( + new OriginalSource([ + "Line 1", + "Line 2" + ].join("\n"), "file.txt") + ); + source.insert(-1, "Line 0\n"); + source.replace(0, 5, "Hello"); + var resultText = source.source(); + var resultMap = source.sourceAndMap({ + columns: true + }); + var resultListMap = source.sourceAndMap({ + columns: false + }); + resultText.should.be.eql("Line 0\nHello\nLine 2"); + resultMap.source.should.be.eql(resultText); + resultListMap.source.should.be.eql(resultText); + resultListMap.map.file.should.be.eql(resultMap.map.file); + resultListMap.map.version.should.be.eql(resultMap.map.version); + resultListMap.map.sources.should.be.eql(resultMap.map.sources); + resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); + resultMap.map.mappings.should.be.eql(";AAAA;AACA"); + }); + + it("should append items correctly", function() { + var line1; + var source = new ReplaceSource( + new OriginalSource(line1 = "Line 1", "file.txt") + ); + source.insert(line1.length + 1, "\nLine 2"); + var resultText = source.source(); + var resultMap = source.sourceAndMap({ + columns: true + }); + var resultListMap = source.sourceAndMap({ + columns: false + }); + resultText.should.be.eql("Line 1\nLine 2"); + resultMap.source.should.be.eql(resultText); + resultListMap.source.should.be.eql(resultText); + resultListMap.map.file.should.be.eql(resultMap.map.file); + resultListMap.map.version.should.be.eql(resultMap.map.version); + resultListMap.map.sources.should.be.eql(resultMap.map.sources); + resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); + resultMap.map.mappings.should.be.eql("AAAA,M"); + }); }); From d178f187b58497a7e8c4f989c3c1a172a17b1b4c Mon Sep 17 00:00:00 2001 From: Brent Wilton Date: Mon, 11 Dec 2017 16:24:44 +1300 Subject: [PATCH 3/6] Fix double emit on in progress replace and adjust node column for overlapping replaces --- lib/ReplaceSource.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index 9fa3369..9516474 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -190,8 +190,11 @@ class ReplaceSource extends Source { var splitPosition = replace.position - position; // If multiple replaces occur in the same location then the splitPosition may be // before the current position for the subsequent splits. Ensure it is >= 0 - if(splitPosition < 0) + var originalSplitPosition = 0; + if(splitPosition < 0) { + originalSplitPosition = splitPosition; splitPosition = 0; + } if(splitPosition >= node.length || replace.done) { if(replace.emit) { var nodeEnd = new SourceNode( @@ -227,10 +230,12 @@ class ReplaceSource extends Source { // then the column position shows the split position of the intermediate source map and not the original // Emit the replacement value - if(replace.value && replace.value.length > 0) { + if(replace.value) { + // If the split position was < 0 due to overlapping replaces, adjust the column to match + var column = parent.column + originalSplitPosition; output.push(new SourceNode( parent.line, - parent.column, + column < 0 ? 0 : column, parent.source, replace.value, parent.name @@ -331,6 +336,8 @@ class ReplacementEnumerator { } footer() { + if(!this.done && !this.emit) + this.next(); // If we finished _replaceInNode mid emit we advance to next entry return this.done ? [] : this.replacements.slice(this.index).map(repl => repl[2]); } } From 441ef8b217a9037328eaf7f237ecaf0a773e3427 Mon Sep 17 00:00:00 2001 From: Brent Wilton Date: Fri, 2 Mar 2018 14:27:24 +1300 Subject: [PATCH 4/6] Move ReplaceSource replacement sort method outside of class --- lib/ReplaceSource.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index 9516474..0ce462a 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -75,7 +75,7 @@ class ReplaceSource extends Source { if(this.replacements.length === 0) { return node; } - this._sortReplacementsAscending(); + this.replacements.sort(sortReplacementsAscending); var replace = new ReplacementEnumerator(this.replacements); var output = []; this._prependStartNodes(output, replace); @@ -141,18 +141,6 @@ class ReplaceSource extends Source { return position <= 0 ? ["", str] : [str.substr(0, position), str.substr(position)]; } - _sortReplacementsAscending() { - this.replacements.sort(function(a, b) { - var diff = a[1] - b[1]; // end - if(diff !== 0) - return diff; - diff = a[0] - b[0]; // start - if(diff !== 0) - return diff; - return a[3] - b[3]; // insert order - }); - } - _replaceInNode(output, replace, node, position) { var outputChildren = []; var allChildrenAreStrings = true; @@ -267,6 +255,16 @@ class ReplaceSource extends Source { } } +function sortReplacementsAscending(a, b) { + var diff = a[1] - b[1]; // end + if(diff !== 0) + return diff; + diff = a[0] - b[0]; // start + if(diff !== 0) + return diff; + return a[3] - b[3]; // insert order +} + class ReplacementEnumerator { constructor(replacements) { this.emit = true; From 89e8ebf06b755b99e7ff894cadbd6f83494cb4e3 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 29 Aug 2018 12:15:42 +0200 Subject: [PATCH 5/6] Simplify and inline methods improve tests --- lib/ReplaceSource.js | 209 +++++++++++++++--------------------------- test/ReplaceSource.js | 24 +++-- 2 files changed, 87 insertions(+), 146 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index 0ce462a..a8235c6 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -78,10 +78,30 @@ class ReplaceSource extends Source { this.replacements.sort(sortReplacementsAscending); var replace = new ReplacementEnumerator(this.replacements); var output = []; - this._prependStartNodes(output, replace); - this._replaceInNode(output, replace, node, 0, null); - this._appendRemainingNodes(output, replace); + var position = 0; + + // We build a new list of SourceNodes in "output" + // from the original mapping data + + var replaceInStringNode = this._replaceInStringNode.bind(this, output, replace); + node.walk(function(chunk, mapping) { + position = replaceInStringNode(chunk, position, mapping); + }); + + // If any replacements occur after the end of the original file, then we append them + // directly to the end of the output + var remaining = replace.footer(); + if(remaining) { + output.push(remaining); + } + var result = new SourceNode(null, null, null, output); + + // We need to add source contents afterwards + node.walkSourceContents(function(sourceFile, sourceContent) { + result.setSourceContent(sourceFile, sourceContent); + }); + return result; } @@ -141,117 +161,57 @@ class ReplaceSource extends Source { return position <= 0 ? ["", str] : [str.substr(0, position), str.substr(position)]; } - _replaceInNode(output, replace, node, position) { - var outputChildren = []; - var allChildrenAreStrings = true; - - for(var i = 0, len = node.children.length; i < len; i++) { - var child = node.children[i]; - if(typeof child !== 'string') { - position = this._replaceInNode(outputChildren, replace, child, position); - allChildrenAreStrings = false; - } else { - position = this._replaceInStringNode(outputChildren, replace, child, position, node); - } - } - if(outputChildren.length > 0) { - if(allChildrenAreStrings) { - for(i = 0; i < outputChildren.length; i++) - output.push(outputChildren[i]); - } else { - var outputNode = new SourceNode( - node.line, - node.column, - node.source, - outputChildren, - node.name - ); - if(node.sourceContents) - outputNode.sourceContents = node.sourceContents; - output.push(outputNode); - } - } - return position; - } - _replaceInStringNode(output, replace, node, position, parent) { - var splitPosition = replace.position - position; - // If multiple replaces occur in the same location then the splitPosition may be - // before the current position for the subsequent splits. Ensure it is >= 0 - var originalSplitPosition = 0; - if(splitPosition < 0) { - originalSplitPosition = splitPosition; - splitPosition = 0; - } - if(splitPosition >= node.length || replace.done) { - if(replace.emit) { - var nodeEnd = new SourceNode( - parent.line, - parent.column, - parent.source, - node, - parent.name - ); - if(parent.sourceContents) - nodeEnd.sourceContents = parent.sourceContents; - output.push(nodeEnd); + do { + var splitPosition = replace.position - position; + // If multiple replaces occur in the same location then the splitPosition may be + // before the current position for the subsequent splits. Ensure it is >= 0 + if(splitPosition < 0) { + splitPosition = 0; } - return(position + node.length); - } - var emit = replace.next(); - if(!emit) { - // Stop emitting when we have found the beginning of the string to replace. - // Emit the part of the string before splitPosition - var nodeStart = new SourceNode( - parent.line, - parent.column, - parent.source, - node.substr(0, splitPosition), - parent.name - ); - if(parent.sourceContents) - nodeStart.sourceContents = parent.sourceContents; - output.push(nodeStart); - - // We should advance the current column position by splitPosition characters at this point. - // However if multiple ReplaceSource's are chained (as occurs when using ModuleConcatenationPlugin) - // then the column position shows the split position of the intermediate source map and not the original - - // Emit the replacement value - if(replace.value) { - // If the split position was < 0 due to overlapping replaces, adjust the column to match - var column = parent.column + originalSplitPosition; - output.push(new SourceNode( - parent.line, - column < 0 ? 0 : column, - parent.source, - replace.value, - parent.name - )); + if(splitPosition >= node.length || replace.done) { + if(replace.emit) { + var nodeEnd = new SourceNode( + parent.line, + parent.column, + parent.source, + node, + parent.name + ); + output.push(nodeEnd); + } + return position + node.length; } - } - - // Recurse with remainder of the string as there may be multiple replaces within a single node - var remainder = node.substr(splitPosition, node.length); - return this._replaceInStringNode(output, replace, remainder, position + splitPosition, parent); - } + var emit = replace.next(); + if(!emit) { + // Stop emitting when we have found the beginning of the string to replace. + // Emit the part of the string before splitPosition + if(splitPosition > 0) { + var nodeStart = new SourceNode( + parent.line, + parent.column, + parent.source, + node.substr(0, splitPosition), + parent.name + ); + output.push(nodeStart); + } - _prependStartNodes(output, replace) { - // If any replacements occur before the start of the original file, then we prepend them - // directly to the start of the output - var startValues = replace.header(); - for(var i = 0; i < startValues.length; i++) { - output.push(startValues[i]); - } - } + // Emit the replacement value + if(replace.value) { + output.push(new SourceNode( + parent.line, + parent.column, + parent.source, + replace.value + )); + } + } - _appendRemainingNodes(output, replace) { - // If any replacements occur after the end of the original file, then we append them - // directly to the end of the output - var remainingValues = replace.footer(); - for(var i = 0; i < remainingValues.length; i++) { - output.push(remainingValues[i]); - } + // Recurse with remainder of the string as there may be multiple replaces within a single node + node = node.substr(splitPosition); + position += splitPosition; + } while (true); } } @@ -306,37 +266,12 @@ class ReplacementEnumerator { return this.emit; } - header() { - var output = []; - var inHeader = true; - while(inHeader && !this.done) { - var repl = this.replacements[this.index]; - var start = Math.floor(repl[0]); - this.position = start; - - // Replicate previous ReplaceSource behavior: - // In order to generate identical source maps to the previous version of webpack we should not output the last header - // node as a string if the next replace starts above position 0 - var nextReplAtStart = this.index == (this.replacements.length - 1) ? - false : - Math.floor(this.replacements[this.index + 1][0]) <= 0; - - if(start < 0 && nextReplAtStart) { - output.push(repl[2]); - this.index++; - } else { - inHeader = false; - } - } - if(this.position < 0) - this.position = 0; - return output; - } - footer() { if(!this.done && !this.emit) this.next(); // If we finished _replaceInNode mid emit we advance to next entry - return this.done ? [] : this.replacements.slice(this.index).map(repl => repl[2]); + return this.done ? [] : this.replacements.slice(this.index).map(function(repl) { + return repl[2]; + }).join(""); } } diff --git a/test/ReplaceSource.js b/test/ReplaceSource.js index dbaf820..228641b 100644 --- a/test/ReplaceSource.js +++ b/test/ReplaceSource.js @@ -44,7 +44,8 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultListMap.map.mappings.should.be.eql("AAAA;AACA;AAAA;AAAA;AAIA,KACA"); + resultMap.map.mappings.should.be.eql("AAAA;AACA;AAAA;AAAA;AAIA,KACA"); + resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); }); it("should replace multiple items correctly", function() { @@ -71,13 +72,15 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultListMap.map.mappings.should.be.eql("AAAA,cACA"); + resultMap.map.mappings.should.be.eql("AAAA,cACA"); + resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); }); it("should prepend items correctly", function() { var source = new ReplaceSource( new OriginalSource("Line 1", "file.txt") ); + source.insert(-1, "Line -1\n"); source.insert(-1, "Line 0\n"); var resultText = source.source(); var resultMap = source.sourceAndMap({ @@ -86,14 +89,15 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); - resultText.should.be.eql("Line 0\nLine 1"); + resultText.should.be.eql("Line -1\nLine 0\nLine 1"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText); resultListMap.map.file.should.be.eql(resultMap.map.file); resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql("AAAA;AAAA"); + resultMap.map.mappings.should.be.eql("AAAA;AAAA;AAAA"); + resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); }); it("should prepend items with replace at start correctly", function() { @@ -119,15 +123,16 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql(";AAAA;AACA"); + resultMap.map.mappings.should.be.eql("AAAA;AAAA;AACA"); + resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); }); it("should append items correctly", function() { var line1; var source = new ReplaceSource( - new OriginalSource(line1 = "Line 1", "file.txt") + new OriginalSource(line1 = "Line 1\n", "file.txt") ); - source.insert(line1.length + 1, "\nLine 2"); + source.insert(line1.length + 1, "Line 2\n"); var resultText = source.source(); var resultMap = source.sourceAndMap({ columns: true @@ -135,13 +140,14 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); - resultText.should.be.eql("Line 1\nLine 2"); + resultText.should.be.eql("Line 1\nLine 2\n"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText); resultListMap.map.file.should.be.eql(resultMap.map.file); resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql("AAAA,M"); + resultMap.map.mappings.should.be.eql("AAAA"); + resultListMap.map.mappings.should.be.eql("AAAA;;"); }); }); From cf11460a68846c4637361b472df8756d1a78ea5f Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 29 Aug 2018 13:45:53 +0200 Subject: [PATCH 6/6] Improve the quality of the source maps in some cases --- lib/ReplaceSource.js | 83 ++++++++++++++++++++++++++++++++----------- test/ReplaceSource.js | 18 ++++++---- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/lib/ReplaceSource.js b/lib/ReplaceSource.js index a8235c6..f28a0fe 100644 --- a/lib/ReplaceSource.js +++ b/lib/ReplaceSource.js @@ -79,11 +79,38 @@ class ReplaceSource extends Source { var replace = new ReplacementEnumerator(this.replacements); var output = []; var position = 0; + var sources = Object.create(null); + var sourcesInLines = Object.create(null); // We build a new list of SourceNodes in "output" // from the original mapping data - var replaceInStringNode = this._replaceInStringNode.bind(this, output, replace); + var result = new SourceNode(); + + // We need to add source contents manually + // because "walk" will not handle it + node.walkSourceContents(function(sourceFile, sourceContent) { + result.setSourceContent(sourceFile, sourceContent); + sources["$" + sourceFile] = sourceContent; + }); + + var replaceInStringNode = this._replaceInStringNode.bind(this, output, replace, function getOriginalSource(mapping) { + var key = "$" + mapping.source; + var lines = sourcesInLines[key]; + if(!lines) { + var source = sources[key]; + if(!source) return null; + lines = source.split("\n").map(function(line) { + return line + "\n"; + }); + sourcesInLines[key] = lines; + } + // line is 1-based + if(mapping.line > lines.length) return null; + var line = lines[mapping.line - 1]; + return line.substr(mapping.column); + }); + node.walk(function(chunk, mapping) { position = replaceInStringNode(chunk, position, mapping); }); @@ -95,12 +122,7 @@ class ReplaceSource extends Source { output.push(remaining); } - var result = new SourceNode(null, null, null, output); - - // We need to add source contents afterwards - node.walkSourceContents(function(sourceFile, sourceContent) { - result.setSourceContent(sourceFile, sourceContent); - }); + result.add(output); return result; } @@ -161,7 +183,9 @@ class ReplaceSource extends Source { return position <= 0 ? ["", str] : [str.substr(0, position), str.substr(position)]; } - _replaceInStringNode(output, replace, node, position, parent) { + _replaceInStringNode(output, replace, getOriginalSource, node, position, mapping) { + var original = undefined; + do { var splitPosition = replace.position - position; // If multiple replaces occur in the same location then the splitPosition may be @@ -172,27 +196,46 @@ class ReplaceSource extends Source { if(splitPosition >= node.length || replace.done) { if(replace.emit) { var nodeEnd = new SourceNode( - parent.line, - parent.column, - parent.source, + mapping.line, + mapping.column, + mapping.source, node, - parent.name + mapping.name ); output.push(nodeEnd); } return position + node.length; } + + var originalColumn = mapping.column; + + // Try to figure out if generated code matches original code of this segement + // If this is the case we assume that it's allowed to move mapping.column + // Because getOriginalSource can be expensive we only do it when neccessary + + var nodePart; + if(splitPosition > 0) { + nodePart = node.slice(0, splitPosition); + if(original === undefined) { + original = getOriginalSource(mapping); + } + if(original && original.length >= splitPosition && original.startsWith(nodePart)) { + mapping.column += splitPosition; + original = original.substr(splitPosition); + } + } + var emit = replace.next(); if(!emit) { // Stop emitting when we have found the beginning of the string to replace. // Emit the part of the string before splitPosition if(splitPosition > 0) { var nodeStart = new SourceNode( - parent.line, - parent.column, - parent.source, - node.substr(0, splitPosition), - parent.name + mapping.line, + originalColumn, + mapping.source, + nodePart, + mapping.name ); output.push(nodeStart); } @@ -200,9 +243,9 @@ class ReplaceSource extends Source { // Emit the replacement value if(replace.value) { output.push(new SourceNode( - parent.line, - parent.column, - parent.source, + mapping.line, + mapping.column, + mapping.source, replace.value )); } diff --git a/test/ReplaceSource.js b/test/ReplaceSource.js index 228641b..d11f14f 100644 --- a/test/ReplaceSource.js +++ b/test/ReplaceSource.js @@ -44,8 +44,8 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql("AAAA;AACA;AAAA;AAAA;AAIA,KACA"); - resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); + resultMap.map.mappings.should.be.eql("AAAA,CAAC,EAAI,KAAE,IAAC;AACR,CAAC;AAAA;AAAA;AAID,IAAI,CACJ"); + resultListMap.map.mappings.should.be.eql("AAAA;AACA;AAAA;AAAA;AAIA,KACA"); }); it("should replace multiple items correctly", function() { @@ -65,6 +65,7 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); + resultText.should.be.eql("Message: Hey Ad!"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText); @@ -72,8 +73,8 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql("AAAA,cACA"); - resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); + resultMap.map.mappings.should.be.eql("AAAA,WAAE,GACE"); + resultListMap.map.mappings.should.be.eql("AAAA,cACA"); }); it("should prepend items correctly", function() { @@ -89,6 +90,7 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); + resultText.should.be.eql("Line -1\nLine 0\nLine 1"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText); @@ -97,7 +99,7 @@ describe("ReplaceSource", function() { resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); resultMap.map.mappings.should.be.eql("AAAA;AAAA;AAAA"); - resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); + resultListMap.map.mappings.should.be.eql("AAAA;AAAA;AAAA"); }); it("should prepend items with replace at start correctly", function() { @@ -116,6 +118,7 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); + resultText.should.be.eql("Line 0\nHello\nLine 2"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText); @@ -123,8 +126,8 @@ describe("ReplaceSource", function() { resultListMap.map.version.should.be.eql(resultMap.map.version); resultListMap.map.sources.should.be.eql(resultMap.map.sources); resultListMap.map.sourcesContent.should.be.eql(resultMap.map.sourcesContent); - resultMap.map.mappings.should.be.eql("AAAA;AAAA;AACA"); - resultListMap.map.mappings.should.be.eql(resultMap.map.mappings); + resultMap.map.mappings.should.be.eql("AAAA;AAAA,KAAM;AACN"); + resultListMap.map.mappings.should.be.eql("AAAA;AAAA;AACA"); }); it("should append items correctly", function() { @@ -140,6 +143,7 @@ describe("ReplaceSource", function() { var resultListMap = source.sourceAndMap({ columns: false }); + resultText.should.be.eql("Line 1\nLine 2\n"); resultMap.source.should.be.eql(resultText); resultListMap.source.should.be.eql(resultText);