From ac2e52574ba6edc460b328921aa7d6478eaadfcc Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 30 Dec 2021 14:38:27 -0500 Subject: [PATCH 1/5] fix(sourcemap): fall back to low-resolution line mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …inside `Link.traceSegment` instead of returning null, so that a low-resolution sourcemap preceding a high-resolution sourcemap in the sourcemap chain doesn't fail. --- src/utils/collapseSourcemaps.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/utils/collapseSourcemaps.ts b/src/utils/collapseSourcemaps.ts index 784eadf6696..1d8c302006a 100644 --- a/src/utils/collapseSourcemaps.ts +++ b/src/utils/collapseSourcemaps.ts @@ -117,6 +117,23 @@ class Link { const segments = this.mappings[line]; if (!segments) return null; + // Sometimes a high-resolution sourcemap will be preceded in the sourcemap chain + // by a low-resolution sourcemap. We can detect this by checking if the mappings + // array for this line only contains a segment for column zero. In that case, we + // want to fall back to a low-resolution mapping instead of returning null. + if (segments.length == 1 && segments[0][0] == 0) { + const segment = segments[0]; + if (segment.length == 1) { + return null; + } + const source = this.sources[segment[1]] || null; + return source?.traceSegment( + segment[2], + segment[3], + segment.length === 5 ? this.names[segment[4]] : name + ); + } + // binary search through segments for the given column let i = 0; let j = segments.length - 1; From eeca9a7f53f76eb4d4153ebb2beca4ef357ea4b6 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Sat, 12 Feb 2022 19:43:21 -0500 Subject: [PATCH 2/5] fix: fall back to low resolution mapping in `getOriginalLocation` --- src/utils/getOriginalLocation.ts | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/utils/getOriginalLocation.ts b/src/utils/getOriginalLocation.ts index 692888dd472..94fa68eb6b0 100644 --- a/src/utils/getOriginalLocation.ts +++ b/src/utils/getOriginalLocation.ts @@ -14,18 +14,23 @@ export function getOriginalLocation( let locationFound = false; if (line !== undefined) { - for (const segment of line) { - if (segment[0] >= location.column) { - if (segment.length === 1) break; - location = { - column: segment[3], - line: segment[2] + 1, - name: segment.length === 5 ? sourcemap.names[segment[4]] : undefined, - source: sourcemap.sources[segment[1]] - }; - locationFound = true; - break; - } + // Sometimes a high-resolution sourcemap will be preceded in the sourcemap chain + // by a low-resolution sourcemap. We can detect this by checking if the mappings + // array for this line only contains a segment for column zero. In that case, we + // want to fall back to a low-resolution mapping instead of throwing an error. + const segment = + line.length == 1 && line[0][0] == 0 + ? line[0] + : line.find(segment => segment[0] >= location.column); + + if (segment && segment.length !== 1) { + locationFound = true; + location = { + column: segment[3], + line: segment[2] + 1, + name: segment.length === 5 ? sourcemap.names[segment[4]] : undefined, + source: sourcemap.sources[segment[1]] + }; } } if (!locationFound) { From a25b540ef3ab4d9997c9b58e8ebb3d894ebe39fc Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Mar 2022 14:04:30 +0100 Subject: [PATCH 3/5] Generalize low-resolution sourcemap handling and add tests --- src/utils/collapseSourcemaps.ts | 39 ++++++--------- src/utils/getOriginalLocation.ts | 38 ++++++--------- .../_config.js | 37 +++++++++++++++ .../warning-low-resolution-location/main.js | 1 + .../transform-low-resolution/_config.js | 47 +++++++++++++++++++ .../samples/transform-low-resolution/main.js | 1 + 6 files changed, 114 insertions(+), 49 deletions(-) create mode 100644 test/function/samples/warning-low-resolution-location/_config.js create mode 100644 test/function/samples/warning-low-resolution-location/main.js create mode 100644 test/sourcemaps/samples/transform-low-resolution/_config.js create mode 100644 test/sourcemaps/samples/transform-low-resolution/main.js diff --git a/src/utils/collapseSourcemaps.ts b/src/utils/collapseSourcemaps.ts index 1d8c302006a..f8a6cefcd4c 100644 --- a/src/utils/collapseSourcemaps.ts +++ b/src/utils/collapseSourcemaps.ts @@ -117,34 +117,23 @@ class Link { const segments = this.mappings[line]; if (!segments) return null; - // Sometimes a high-resolution sourcemap will be preceded in the sourcemap chain - // by a low-resolution sourcemap. We can detect this by checking if the mappings - // array for this line only contains a segment for column zero. In that case, we - // want to fall back to a low-resolution mapping instead of returning null. - if (segments.length == 1 && segments[0][0] == 0) { - const segment = segments[0]; - if (segment.length == 1) { - return null; - } - const source = this.sources[segment[1]] || null; - return source?.traceSegment( - segment[2], - segment[3], - segment.length === 5 ? this.names[segment[4]] : name - ); - } - // binary search through segments for the given column - let i = 0; - let j = segments.length - 1; + let searchStart = 0; + let searchEnd = segments.length - 1; - while (i <= j) { - const m = (i + j) >> 1; + while (searchStart <= searchEnd) { + const m = (searchStart + searchEnd) >> 1; const segment = segments[m]; - if (segment[0] === column) { + + // If a sourcemap does not have sufficient resolution to contain a + // necessary mapping, e.g. because it only contains line information, we + // use the best approximation we could find + if (segment[0] === column || searchStart === searchEnd) { if (segment.length == 1) return null; const source = this.sources[segment[1]]; - if (!source) return null; + if (!source) { + return null; + } return source.traceSegment( segment[2], @@ -153,9 +142,9 @@ class Link { ); } if (segment[0] > column) { - j = m - 1; + searchEnd = m - 1; } else { - i = m + 1; + searchStart = m + 1; } } diff --git a/src/utils/getOriginalLocation.ts b/src/utils/getOriginalLocation.ts index 94fa68eb6b0..216acfef814 100644 --- a/src/utils/getOriginalLocation.ts +++ b/src/utils/getOriginalLocation.ts @@ -7,35 +7,25 @@ export function getOriginalLocation( const filteredSourcemapChain = sourcemapChain.filter( (sourcemap): sourcemap is ExistingDecodedSourceMap => !!sourcemap.mappings ); - - while (filteredSourcemapChain.length > 0) { + traceSourcemap: while (filteredSourcemapChain.length > 0) { const sourcemap = filteredSourcemapChain.pop()!; const line = sourcemap.mappings[location.line - 1]; - let locationFound = false; - - if (line !== undefined) { - // Sometimes a high-resolution sourcemap will be preceded in the sourcemap chain - // by a low-resolution sourcemap. We can detect this by checking if the mappings - // array for this line only contains a segment for column zero. In that case, we - // want to fall back to a low-resolution mapping instead of throwing an error. - const segment = - line.length == 1 && line[0][0] == 0 - ? line[0] - : line.find(segment => segment[0] >= location.column); - if (segment && segment.length !== 1) { - locationFound = true; - location = { - column: segment[3], - line: segment[2] + 1, - name: segment.length === 5 ? sourcemap.names[segment[4]] : undefined, - source: sourcemap.sources[segment[1]] - }; + if (line?.length) { + for (const segment of line) { + if (segment[0] >= location.column || line.length === 1) { + if (segment.length === 1) break; + location = { + column: segment[3], + line: segment[2] + 1, + name: segment.length === 5 ? sourcemap.names[segment[4]] : undefined, + source: sourcemap.sources[segment[1]] + }; + continue traceSourcemap; + } } } - if (!locationFound) { - throw new Error("Can't resolve original location of error."); - } + throw new Error("Can't resolve original location of error."); } return location; } diff --git a/test/function/samples/warning-low-resolution-location/_config.js b/test/function/samples/warning-low-resolution-location/_config.js new file mode 100644 index 00000000000..01d2d4001c7 --- /dev/null +++ b/test/function/samples/warning-low-resolution-location/_config.js @@ -0,0 +1,37 @@ +const path = require('path'); +const { encode } = require('sourcemap-codec'); +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = { + description: 'handles when a low resolution sourcemap is used to report an error', + options: { + plugins: { + name: 'test-plugin', + transform() { + // each entry of each line consist of + // [generatedColumn, sourceIndex, sourceLine, sourceColumn]; + // this mapping only maps the first line to itself + const decodedMap = [[[0, 0, 0, 0]]]; + return { code: 'export default this', map: { mappings: encode(decodedMap), sources: [] } }; + } + } + }, + warnings: [ + { + code: 'THIS_IS_UNDEFINED', + frame: ` +1: console.log('original source'); + ^`, + id: ID_MAIN, + loc: { + column: 0, + file: ID_MAIN, + line: 1 + }, + message: + "The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten", + pos: 15, + url: 'https://rollupjs.org/guide/en/#error-this-is-undefined' + } + ] +}; diff --git a/test/function/samples/warning-low-resolution-location/main.js b/test/function/samples/warning-low-resolution-location/main.js new file mode 100644 index 00000000000..2f40c1f1894 --- /dev/null +++ b/test/function/samples/warning-low-resolution-location/main.js @@ -0,0 +1 @@ +console.log('original source'); diff --git a/test/sourcemaps/samples/transform-low-resolution/_config.js b/test/sourcemaps/samples/transform-low-resolution/_config.js new file mode 100644 index 00000000000..ad66c2b15d7 --- /dev/null +++ b/test/sourcemaps/samples/transform-low-resolution/_config.js @@ -0,0 +1,47 @@ +const assert = require('assert'); +const MagicString = require('magic-string'); +const { SourceMapConsumer } = require('source-map'); +const { encode } = require('sourcemap-codec'); +const getLocation = require('../../getLocation'); + +module.exports = { + description: 'handles combining low-resolution and high-resolution source-maps when transforming', + options: { + output: { name: 'bundle' }, + plugins: [ + { + transform(code) { + // each entry of each line consist of + // [generatedColumn, sourceIndex, sourceLine, sourceColumn]; + // this mapping only maps the second line to the first with no column + // details + const decodedMap = [[], [[0, 0, 0, 0]]]; + return { + code: `console.log('added');\n${code}`, + map: { mappings: encode(decodedMap) } + }; + } + }, + { + transform(code) { + const s = new MagicString(code); + s.prepend("console.log('second');\n"); + + return { + code: s.toString(), + map: s.generateMap({ hires: true }) + }; + } + } + ] + }, + async test(code, map) { + const smc = await new SourceMapConsumer(map); + + const generatedLoc = getLocation(code, code.indexOf("'baz'")); + const originalLoc = smc.originalPositionFor(generatedLoc); + + assert.strictEqual(originalLoc.line, 1); + assert.strictEqual(originalLoc.column, 0); + } +}; diff --git a/test/sourcemaps/samples/transform-low-resolution/main.js b/test/sourcemaps/samples/transform-low-resolution/main.js new file mode 100644 index 00000000000..e4161efb281 --- /dev/null +++ b/test/sourcemaps/samples/transform-low-resolution/main.js @@ -0,0 +1 @@ +export let foo = 'bar'; foo += 'baz'; From c87ffbead794e888914716c5a2047dce35f3782f Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Mar 2022 14:37:09 +0100 Subject: [PATCH 4/5] Slightly speed up mapping generation --- src/utils/collapseSourcemaps.ts | 41 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/utils/collapseSourcemaps.ts b/src/utils/collapseSourcemaps.ts index 9431de1c526..66c6284f0f1 100644 --- a/src/utils/collapseSourcemaps.ts +++ b/src/utils/collapseSourcemaps.ts @@ -47,6 +47,7 @@ class Link { traceMappings() { const sources: string[] = []; + const sourceIndexMap = new Map(); const sourcesContent: string[] = []; const names: string[] = []; const nameIndexMap = new Map(); @@ -68,36 +69,34 @@ class Link { ); if (traced) { - // newer sources are more likely to be used, so search backwards. - let sourceIndex = sources.lastIndexOf(traced.source.filename); - if (sourceIndex === -1) { + const { + column, + line, + name, + source: { content, filename } + } = traced; + let sourceIndex = sourceIndexMap.get(filename); + if (sourceIndex === undefined) { sourceIndex = sources.length; - sources.push(traced.source.filename); - sourcesContent[sourceIndex] = traced.source.content; + sources.push(filename); + sourceIndexMap.set(filename, sourceIndex); + sourcesContent[sourceIndex] = content; } else if (sourcesContent[sourceIndex] == null) { - sourcesContent[sourceIndex] = traced.source.content; - } else if ( - traced.source.content != null && - sourcesContent[sourceIndex] !== traced.source.content - ) { + sourcesContent[sourceIndex] = content; + } else if (content != null && sourcesContent[sourceIndex] !== content) { return error({ - message: `Multiple conflicting contents for sourcemap source ${traced.source.filename}` + message: `Multiple conflicting contents for sourcemap source ${filename}` }); } - const tracedSegment: SourceMapSegment = [ - segment[0], - sourceIndex, - traced.line, - traced.column - ]; + const tracedSegment: SourceMapSegment = [segment[0], sourceIndex, line, column]; - if (traced.name) { - let nameIndex = nameIndexMap.get(traced.name); + if (name) { + let nameIndex = nameIndexMap.get(name); if (nameIndex === undefined) { nameIndex = names.length; - names.push(traced.name); - nameIndexMap.set(traced.name, nameIndex); + names.push(name); + nameIndexMap.set(name, nameIndex); } (tracedSegment as SourceMapSegment)[4] = nameIndex; From 4041d4c37397f120bdad4b90a41be5d169e1c6e9 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 3 Mar 2022 15:09:56 +0100 Subject: [PATCH 5/5] Improve coverage --- src/utils/collapseSourcemaps.ts | 6 ++---- src/utils/getOriginalLocation.ts | 18 +++++++++--------- .../_config.js | 2 +- .../warning-low-resolution-location/_config.js | 2 +- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/utils/collapseSourcemaps.ts b/src/utils/collapseSourcemaps.ts index 66c6284f0f1..e4c87734cd1 100644 --- a/src/utils/collapseSourcemaps.ts +++ b/src/utils/collapseSourcemaps.ts @@ -58,7 +58,7 @@ class Link { const tracedLine: SourceMapSegment[] = []; for (const segment of line) { - if (segment.length == 1) continue; + if (segment.length === 1) continue; const source = this.sources[segment[1]]; if (!source) continue; @@ -130,9 +130,7 @@ class Link { if (segment[0] === column || searchStart === searchEnd) { if (segment.length == 1) return null; const source = this.sources[segment[1]]; - if (!source) { - return null; - } + if (!source) return null; return source.traceSegment( segment[2], diff --git a/src/utils/getOriginalLocation.ts b/src/utils/getOriginalLocation.ts index e0c53e6d26d..4303a1c3003 100644 --- a/src/utils/getOriginalLocation.ts +++ b/src/utils/getOriginalLocation.ts @@ -2,7 +2,7 @@ import type { DecodedSourceMapOrMissing, ExistingDecodedSourceMap } from '../rol export function getOriginalLocation( sourcemapChain: readonly DecodedSourceMapOrMissing[], - location: { column: number; line: number; name?: string; source?: string } + location: { column: number; line: number } ): { column: number; line: number } { const filteredSourcemapChain = sourcemapChain.filter( (sourcemap): sourcemap is ExistingDecodedSourceMap => !!sourcemap.mappings @@ -10,16 +10,16 @@ export function getOriginalLocation( traceSourcemap: while (filteredSourcemapChain.length > 0) { const sourcemap = filteredSourcemapChain.pop()!; const line = sourcemap.mappings[location.line - 1]; - - if (line?.length) { - for (const segment of line) { - if (segment[0] >= location.column || line.length === 1) { - if (segment.length === 1) break; + if (line) { + const filteredLine = line.filter( + (segment): segment is [number, number, number, number] => segment.length > 1 + ); + const lastSegment = filteredLine[filteredLine.length - 1]; + for (const segment of filteredLine) { + if (segment[0] >= location.column || segment === lastSegment) { location = { column: segment[3], - line: segment[2] + 1, - name: segment.length === 5 ? sourcemap.names[segment[4]] : undefined, - source: sourcemap.sources[segment[1]] + line: segment[2] + 1 }; continue traceSourcemap; } diff --git a/test/function/samples/cannot-resolve-sourcemap-warning/_config.js b/test/function/samples/cannot-resolve-sourcemap-warning/_config.js index a803b5b72e8..3ec5a853772 100644 --- a/test/function/samples/cannot-resolve-sourcemap-warning/_config.js +++ b/test/function/samples/cannot-resolve-sourcemap-warning/_config.js @@ -7,7 +7,7 @@ module.exports = { plugins: { name: 'test-plugin', transform() { - return { code: 'export default this', map: { mappings: 'X' } }; + return { code: 'export default this', map: { mappings: '' } }; } } }, diff --git a/test/function/samples/warning-low-resolution-location/_config.js b/test/function/samples/warning-low-resolution-location/_config.js index 01d2d4001c7..a8e18a9d87c 100644 --- a/test/function/samples/warning-low-resolution-location/_config.js +++ b/test/function/samples/warning-low-resolution-location/_config.js @@ -11,7 +11,7 @@ module.exports = { // each entry of each line consist of // [generatedColumn, sourceIndex, sourceLine, sourceColumn]; // this mapping only maps the first line to itself - const decodedMap = [[[0, 0, 0, 0]]]; + const decodedMap = [[[0], [0, 0, 0, 0], [1]]]; return { code: 'export default this', map: { mappings: encode(decodedMap), sources: [] } }; } }