Merge pull request #41 from cqse/cr/33868_non_colliding_fids

TS-33868 Non Colliding File Identifiers

packages/teamscale-javascript-instrumenter/CHANGELOG.md

@@ -2,6 +2,8 @@ We use [Semantic Versioning](https://semver.org/).
 
 # New Release
 
+- [fix] The instrumenter created colliding identifiers in case no Ecmascript modules were used.
+
 # 0.0.1-beta.50
 
 - [feature] Unquote inputs in nested quotes.

packages/teamscale-javascript-instrumenter/src/instrumenter/Postprocessor.ts

@@ -14,9 +14,7 @@ import {
 	StringLiteral,
 	isVariableDeclaration,
 	VariableDeclaration,
-	VariableDeclarator,
 	isFunctionDeclaration,
-	ExpressionStatement,
 	isSequenceExpression,
 	SequenceExpression,
 	isUpdateExpression
@@ -46,6 +44,34 @@ type FunctionCoverageIncrement = CoverageIncrement & {
 	functionId: number;
 };
 
+/**
+ * Generator for identifiers that are unique across files to instrument.
+ * Relevant in case no Ecmascript modules are used.
+ *
+ * We assume that the files to be executed in a browser can
+ * stem from different runs of the instrumenter. We have to decrease
+ * the probability of colliding identifiers.
+ */
+const fileIdSeqGenerator: { next: () => string } = (() => {
+	const instrumenterRunId = process.pid;
+	let fileIdSeq = 0;
+
+	return {
+		next: () => {
+			fileIdSeq++;
+			let num: number;
+			if (fileIdSeq < 10000) {
+				num = instrumenterRunId * 10000 + fileIdSeq;
+			} else if (fileIdSeq < 100000) {
+				num = instrumenterRunId * 100000 + fileIdSeq;
+			} else {
+				throw new Error(`Not more that 100k files supported to be instrumented in one run.`)
+			}
+			return num.toString(36);
+		}
+	}
+})();
+
 function getIstanbulCoverageFunctionDeclarationName(node: Node | undefined): string | undefined {
 	if (!isFunctionDeclaration(node)) {
 		return undefined;
@@ -63,7 +89,7 @@ function getIstanbulCoverageFunctionDeclarationName(node: Node | undefined): str
  * Adds constants with the file id to the header of the file to process
  * which are then used in the coverage broadcast functions as arguments.
  *
- * For example, `_$fid0` is introduced as `const _$fid0 = "6822844a804c1e9986ac4bd4a45b85893bde8b33";`
+ * For example, `_$f0` is introduced as `const _$f0 = "6822844a804c1e9986ac4bd4a45b85893bde8b33";`
  * based on
  * ```
  * function cov_oqh6rsgrd() {
@@ -81,8 +107,7 @@ type FileIdMappingHandler = {
 	getFileHashForCoverageObjectId: (coverageObjectId: string) => string | undefined;
 };
 function createFileIdMappingHandler(): FileIdMappingHandler {
-	const fileIdMap: Map<string, string> = new Map<string, string>();
-	let fileIdSeq = 0;
+	const fileIdMap: Map<string, string> = new Map<string, string>();	
 
 	return {
 		enterPath(path: NodePath): void {
@@ -98,7 +123,7 @@ function createFileIdMappingHandler(): FileIdMappingHandler {
 					const declarator = declaration.declarations[0];
 					if (isIdentifier(declarator.id) && declarator.id.name === 'hash') {
 						// We take note of the hash that is stored within the `cov_*' function.
-						const fileIdVarName = `_$fid${fileIdSeq++}`;
+						const fileIdVarName = `_$f${fileIdSeqGenerator.next()}`;
 						const fileId = (declarator.init as StringLiteral).value;
 						fileIdMap.set(coverageFunctionName, fileIdVarName);
 						grandParentPath.insertBefore(newStringConstDeclarationNode(fileIdVarName, fileId) as any);

packages/teamscale-javascript-instrumenter/test/unit/Postprocessor.test.ts

@@ -71,7 +71,7 @@ test('Remove Function Coverage Increments', () => {
 		loc => loc.start.line === 9
 	);
 	expect(cleaned).toContain('f++');
-	expect(cleaned).toContain('_$stmtCov(_$fid0, 0)');
+	expect(cleaned).toContain('_$stmtCov(_$f');
 	expect(cleaned).not.toContain('f[0]++');
 	expect(cleaned).not.toContain('f[1]++');
 });
@@ -134,8 +134,7 @@ test('Remove unsupported coverage only', () => {
 		loc => true
 	);
 	expect(cleaned).toContain('f++');
-	expect(cleaned).toContain('_$brCov(_$fid1, 2, 0)');
-	expect(cleaned).toContain('_$brCov(_$fid1, 2, 1)');
+	expect(cleaned).toContain('_$brCov(_$f');
 });
 
 test('Also handle coverage increments in sequence expressions.', () => {
@@ -157,7 +156,7 @@ test('Also handle coverage increments in sequence expressions.', () => {
 		loc => true
 	);
 	expect(cleaned).not.toContain('b[3][0]++');
-	expect(cleaned).toContain('_$brCov(_$fid0, 3, 0)');
+	expect(cleaned).toContain('_$brCov(_$f');
 });
 
 test('Remove the coverage from an instrument Angular bundle.', () => {