Use the correct source when skipping trivia

petebacondarwin · petebacondarwin · commit 331b9bcfde86 · 2019-01-24T10:28:10.000Z
A custom `SourceMapSource` can optionally provide its own `skipTrivia` function. If this is not provided then the compiler will use the default function designed for TypeScript source files. Previously, when calling this default function we were passing the current `sourceMapSource` rather than the specified `source` whose trivia needs to be skipped. This resulted in the `pos` being incorrectly calculated for external source files that need mapping. **Side note:** There are actually two possible constructors available for creating `SourceMapSource` objects. One of them defaults to an identity function for the `skipTrivia` function if it is not provided (see https://github.com/Microsoft/TypeScript/blob/49689894d747714d6b2b8461b9033020efec9625/src/compiler/utilities.ts#L6972-L6976) and the other one leaves the `skipTrivia` field `undefined` (see https://github.com/Microsoft/TypeScript/blob/5fc8f1dd801dbacfe7e2d624f80b7a6a3868d180/src/services/services.ts#L776-L797) Unfortunately, it appears that the second of these two constructors is the one available when importing the "typescript" module in node.js code.
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -4372,10 +4372,10 @@ namespace ts {
         }
 
         /**
-         * Skips trivia such as comments and white-space that can optionally overriden by the source map source
+         * Skips trivia such as comments and white-space that can be optionally overridden by the source-map source
          */
         function skipSourceTrivia(source: SourceMapSource, pos: number): number {
-            return source.skipTrivia ? source.skipTrivia(pos) : skipTrivia(sourceMapSource.text, pos);
+            return source.skipTrivia ? source.skipTrivia(pos) : skipTrivia(source.text, pos);
         }
 
         /**
diff --git a/src/testRunner/unittests/customTransforms.ts b/src/testRunner/unittests/customTransforms.ts
@@ -129,5 +129,34 @@ namespace ts {
             },
             { sourceMap: true }
         );
+
+        emitsCorrectly("skipTriviaExternalSourceFiles",
+            [
+                {
+                    file: "source.ts",
+                    // The source file contains preceding trivia (e.g. whitespace) to try to confuse the `skipSourceTrivia` function.
+                    text: "         original;"
+                },
+            ],
+            {
+                before: [
+                    context => node => visitNode(node, function visitor(node: Node): Node {
+                        if (isIdentifier(node) && node.text === "original") {
+                            const newNode = createIdentifier("changed");
+                            setSourceMapRange(newNode, {
+                              pos: 0,
+                              end: 7,
+                              // Do not provide a custom skipTrivia function for `source`.
+                              source: createSourceMapSource("another.html", "changed;")
+                            });
+                            return newNode;
+                        }
+                        return visitEachChild(node, visitor, context);
+                    })
+                ]
+            },
+            { sourceMap: true }
+        );
+
     });
 }
diff --git a/tests/baselines/reference/customTransforms/skipTriviaExternalSourceFiles.js b/tests/baselines/reference/customTransforms/skipTriviaExternalSourceFiles.js

Original file line number	Diff line number	Diff line change
`@@ -4372,10 +4372,10 @@ namespace ts {`
`4372`	`4372`	`}`
`4373`	`4373`
`4374`	`4374`	`/**`
`4375`		`- * Skips trivia such as comments and white-space that can optionally overriden by the source map source`
	`4375`	`+ * Skips trivia such as comments and white-space that can be optionally overridden by the source-map source`
`4376`	`4376`	`*/`
`4377`	`4377`	`function skipSourceTrivia(source: SourceMapSource, pos: number): number {`
`4378`		`- return source.skipTrivia ? source.skipTrivia(pos) : skipTrivia(sourceMapSource.text, pos);`
	`4378`	`+ return source.skipTrivia ? source.skipTrivia(pos) : skipTrivia(source.text, pos);`
`4379`	`4379`	`}`
`4380`	`4380`
`4381`	`4381`	`/**`