fix: source mapping original path instead of user-provided input (#978)

* fix: Add debugging information for sourcemapper

* Fix lint issue

* fix: source mapping original path instead of user-provided input

* Fix lint issue

* Add test case for more coverage in sourcemapper

* Move comments in test-sourcemmapper to the correct place

* Update comments about the entry of getMapInfoOutput

* Fix typo

Co-authored-by: James McTavish <[email protected]>

Author: Louis-Ye

src/agent/io/sourcemapper.ts

@@ -35,6 +35,11 @@ export interface MapInfoInput {
   // process's working directory).
   outputFile: string;
 
+  // The path of the original source file (relative to the process's working
+  // directory). In the example above, this will be either "src/index1.ts" or
+  // "src/index2.ts".
+  inputFile: string;
+
   // The source map's path (relative to the process's working directory). For
   // the same example above, this field is "dist/index.js.map".
   mapFile: string;
@@ -158,8 +163,10 @@ async function processSourcemap(
   }
 
   for (const src of normalizedSourcesRelToProc) {
-    infoMap.set(path.normalize(src), {
+    const inputFile = path.normalize(src);
+    infoMap.set(inputFile, {
       outputFile: outputPath,
+      inputFile,
       mapFile: mapPath,
       mapConsumer: consumer,
       sources: sourcesRelToSrcmap,
@@ -218,13 +225,13 @@ export class SourceMapper {
   }
 
   /**
-   * @param {string} inputPath The path to an input file that could possibly
-   *  be the input to a transpilation process.  The path should be relative to
-   *  the process's current working directory
    * @param {number} The line number in the input file where the line number is
    *   zero-based.
    * @param {number} (Optional) The column number in the line of the file
    *   specified where the column number is zero-based.
+   * @param {string} The entry of the source map info in the sourceMapper. Such
+   *     an entry is supposed to be got by the getMapInfoInput method.
+   *
    * @return {Object} The object returned has a "file" attribute for the
    *   path of the output file associated with the given input file (where the
    *   path is relative to the process's current working directory),
@@ -237,17 +244,14 @@ export class SourceMapper {
    *   with it then null is returned.
    */
   getMapInfoOutput(
-    inputPath: string,
     lineNumber: number,
     colNumber: number,
     entry: MapInfoInput
   ): MapInfoOutput | null {
-    this.logger.debug(`sourcemapper inputPath: ${inputPath}`);
-
-    inputPath = path.normalize(inputPath);
+    this.logger.debug(`sourcemapper entry.inputFile: ${entry.inputFile}`);
 
     const relPath = path
-      .relative(path.dirname(entry.mapFile), inputPath)
+      .relative(path.dirname(entry.mapFile), entry.inputFile)
       .replace(/\\/g, '/');
 
     /**

src/agent/v8/inspector-debugapi.ts

@@ -198,7 +198,6 @@ export class InspectorDebugApi implements debugapi.DebugApi {
       const line = breakpoint.location.line;
       const column = 0;
       const mapInfo = this.sourcemapper.getMapInfoOutput(
-        baseScriptPath,
         line,
         column,
         mapInfoInput

src/agent/v8/legacy-debugapi.ts

@@ -145,7 +145,6 @@ export class V8DebugApi implements debugapi.DebugApi {
       const line = breakpoint.location.line;
       const column = 0;
       const mapInfo = this.sourcemapper.getMapInfoOutput(
-        baseScriptPath,
         line,
         column,
         mapInfoInput

test/test-sourcemapper.ts

@@ -25,24 +25,6 @@ import {MockLogger} from './mock-logger';
 const BASE_PATH = path.join(__dirname, 'fixtures', 'sourcemapper');
 const QUICK_MILLISECONDS = 300;
 
-/**
- * @param {string} tool The name of the tool that was used to generate the
- *  given sourcemap data
- * @param {string} relativeMapFilePath The path to the sourcemap file of a
- *  transpilation to test
- * @param {string} relativeInputFilePath The path to the input file that was
- *  transpiled to generate the specified sourcemap file
- * @param {string} relativeOutputFilePath The path to the output file that was
- *  generated during the transpilation process that constructed the
- *  specified sourcemap file
- * @param {Array.<Array.<number, number>>} inToOutLineNums An array of arrays
- *  where each element in the array is a pair of numbers.  The first number
- *  in the pair is the line number from the input file and the second number
- *  in the pair is the expected line number in the corresponding output file
- *
- *  Note: The line numbers are zero-based
- */
-
 describe('sourcemapper debug info', () => {
   const logger = new MockLogger();
   const mapFilePath = path.join(
@@ -84,45 +66,56 @@ describe('sourcemapper debug info', () => {
 
     const mapInfoInput = sourcemapper.getMapInfoInput(inputFilePath);
     assert.notEqual(mapInfoInput, null);
-    sourcemapper.getMapInfoOutput(inputFilePath, 1, 0, mapInfoInput!);
+    sourcemapper.getMapInfoOutput(1, 0, mapInfoInput!);
 
     // Verify if the debugging information is correctly printed.
     // We use 'indexOf' here instead of 'match' to avoid parsing regular
     // expression, which will confuse the result when having '\' in the path
     // name on platform like Windows.
+    const expectedDebugMessages = [
+      `sourcemapper entry.inputFile: ${inputFilePath}`,
+      'sourcePos: {"source":"in.ts","line":2,"column":0}',
+      'mappedPos: {"line":6,"column":0,"lastColumn":null}',
+    ].reverse();
     const debugsLength = logger.debugs.length;
-    assert.notStrictEqual(
-      logger.debugs[debugsLength - 3].args[0].indexOf(
-        `sourcemapper inputPath: ${inputFilePath}`
-      ),
-      -1
-    );
-    assert.notStrictEqual(
-      logger.debugs[debugsLength - 2].args[0].indexOf(
-        'sourcePos: {"source":"in.ts","line":2,"column":0}'
-      ),
-      -1
-    );
-    assert.notStrictEqual(
-      logger.debugs[debugsLength - 1].args[0].indexOf(
-        'mappedPos: {"line":6,"column":0,"lastColumn":null}'
-      ),
-      -1
-    );
+    for (let i = 0; i < expectedDebugMessages.length; i++) {
+      assert.notStrictEqual(
+        logger.debugs[debugsLength - i - 1].args[0].indexOf(
+          expectedDebugMessages[i]
+        ),
+        -1,
+        `'${logger.debugs[debugsLength - i - 1].args[0]}' does not match '${
+          expectedDebugMessages[i]
+        }'`
+      );
+    }
   });
 });
 
+/**
+ * @param {string} tool The name of the tool that was used to generate the
+ *  given sourcemap data
+ * @param {string} mapFilePath The path to the sourcemap file of a
+ *  transpilation to test
+ * @param {string} inputFilePath The path to the input file that was
+ *  transpiled to generate the specified sourcemap file
+ * @param {string} outputFilePath The path to the output file that was
+ *  generated during the transpilation process that constructed the
+ *  specified sourcemap file
+ * @param {Array.<Array.<number, number>>} inToOutLineNums An array of arrays
+ *  where each element in the array is a pair of numbers.  The first number
+ *  in the pair is the line number from the input file and the second number
+ *  in the pair is the expected line number in the corresponding output file
+ *
+ *  Note: The line numbers are zero-based
+ */
 function testTool(
   tool: string,
-  relativeMapFilePath: string,
-  relativeInputFilePath: string,
-  relativeOutputFilePath: string,
+  mapFilePath: string,
+  inputFilePath: string,
+  outputFilePath: string,
   inToOutLineNums: number[][]
 ) {
-  const mapFilePath = path.join(BASE_PATH, relativeMapFilePath);
-  const inputFilePath = path.join(BASE_PATH, relativeInputFilePath);
-  const outputFilePath = path.join(BASE_PATH, relativeOutputFilePath);
-
   describe('sourcemapper for tool ' + tool, () => {
     const logger = new MockLogger();
     let sourcemapper: sm.SourceMapper;
@@ -160,10 +153,7 @@ function testTool(
           sourcemapper.getMapInfoInput(inputFilePath),
           null
         );
-        const movedPath = path.join(
-          '/some/other/base/dir/',
-          relativeInputFilePath
-        );
+        const movedPath = path.join('/some/other/base/dir/', inputFilePath);
         assert.notStrictEqual(
           sourcemapper.getMapInfoInput(inputFilePath),
           null,
@@ -189,20 +179,34 @@ function testTool(
       }
     );
 
+    it(
+      'for tool ' +
+        tool +
+        ' it can get mapping info output when input file does not exist in the source map',
+      done => {
+        const mapInfoInput = sourcemapper.getMapInfoInput(inputFilePath);
+        assert.notEqual(mapInfoInput, null);
+
+        const mapInfoOutput = sourcemapper.getMapInfoOutput(10, 0, {
+          outputFile: mapInfoInput!.outputFile,
+          inputFile: 'some/file/not/in/sourcemap',
+          mapFile: mapInfoInput!.mapFile,
+          mapConsumer: mapInfoInput!.mapConsumer,
+          sources: mapInfoInput!.sources,
+        });
+
+        assert.notEqual(mapInfoOutput, null);
+        assert.strictEqual(mapInfoOutput!.file, mapInfoInput!.outputFile);
+        assert.strictEqual(mapInfoOutput!.line, -1);
+        done();
+      }
+    );
+
     const testLineMapping = (inputLine: number, expectedOutputLine: number) => {
       const mapInfoInput = sourcemapper.getMapInfoInput(inputFilePath);
       assert.notEqual(mapInfoInput, null);
-      const info = sourcemapper.getMapInfoOutput(
-        inputFilePath,
-        inputLine,
-        0,
-        mapInfoInput!
-      );
-      assert.notStrictEqual(
-        info,
-        null,
-        'The mapping info for file ' + inputFilePath + ' must be non-null'
-      );
+      const info = sourcemapper.getMapInfoOutput(inputLine, 0, mapInfoInput!);
+
       assert.strictEqual(info!.file, outputFilePath);
       assert.strictEqual(
         info!.line,
@@ -228,9 +232,9 @@ function testTool(
 
 testTool(
   'Babel',
-  path.join('babel', 'out.js.map'),
-  path.join('babel', 'in.js'),
-  path.join('babel', 'out.js'),
+  path.join(BASE_PATH, path.join('babel', 'out.js.map')),
+  path.join(BASE_PATH, path.join('babel', 'in.js')),
+  path.join(BASE_PATH, path.join('babel', 'out.js')),
   [
     [1, 14],
     [2, 15],
@@ -287,9 +291,9 @@ testTool(
 
 testTool(
   'Typescript',
-  path.join('typescript', 'out.js.map'),
-  path.join('typescript', 'in.ts'),
-  path.join('typescript', 'out.js'),
+  path.join(BASE_PATH, path.join('typescript', 'out.js.map')),
+  path.join(BASE_PATH, path.join('typescript', 'in.ts')),
+  path.join(BASE_PATH, path.join('typescript', 'out.js')),
   [
     [1, 5],
     [2, 6],
@@ -307,11 +311,21 @@ testTool(
   ]
 );
 
+// This test is for testing that by providing a partial partial match of the
+// source file, the breakpoint can still be set correctly.
+testTool(
+  'Typescript with sub path',
+  path.join(BASE_PATH, path.join('typescript', 'out.js.map')),
+  'in.ts',
+  path.join(BASE_PATH, path.join('typescript', 'out.js')),
+  [[1, 5]]
+);
+
 testTool(
   'Coffeescript',
-  path.join('coffeescript', 'in.js.map'),
-  path.join('coffeescript', 'in.coffee'),
-  path.join('coffeescript', 'in.js'),
+  path.join(BASE_PATH, path.join('coffeescript', 'in.js.map')),
+  path.join(BASE_PATH, path.join('coffeescript', 'in.coffee')),
+  path.join(BASE_PATH, path.join('coffeescript', 'in.js')),
   [
     [1, 1],
     [2, 7],
@@ -333,9 +347,9 @@ testTool(
 
 testTool(
   'Webpack with Typescript',
-  path.join('webpack-ts', 'out.js.map'),
-  path.join('webpack-ts', 'in.ts_'),
-  path.join('webpack-ts', 'out.js'),
+  path.join(BASE_PATH, path.join('webpack-ts', 'out.js.map')),
+  path.join(BASE_PATH, path.join('webpack-ts', 'in.ts_')),
+  path.join(BASE_PATH, path.join('webpack-ts', 'out.js')),
   [
     [3, 93],
     [4, 94],