Change removal of sourcemap comment (#3987)

* Change removal of sourcemap comment

Do the removal without relying on the module parsing the code. This way even if
a plugin uses the `parse` method in the PluginContext the comments are removed.

* Add test: plugin-parse-ast-remove-sourcemapping

* Remove dead code

* Remove unused proprty `comments` from Module

* Remove sourceMappingURL Comment only on top-level

* Fix linting error

* Add test plugin-parse-ast-receives-comments

* Rewrite findSourceMappingURLComments

Better readability and hopefully more performant

Co-authored-by: Yannay Livneh <you@example.com>
Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>

src/Module.ts

@@ -15,7 +15,7 @@ import Literal from './ast/nodes/Literal';
 import MetaProperty from './ast/nodes/MetaProperty';
 import * as NodeType from './ast/nodes/NodeType';
 import Program from './ast/nodes/Program';
-import { ExpressionNode, NodeBase } from './ast/nodes/shared/Node';
+import { ExpressionNode, GenericEsTreeNode, NodeBase } from './ast/nodes/shared/Node';
 import TemplateLiteral from './ast/nodes/TemplateLiteral';
 import VariableDeclaration from './ast/nodes/VariableDeclaration';
 import ModuleScope from './ast/scopes/ModuleScope';
@@ -60,7 +60,7 @@ import { makeLegal } from './utils/identifierHelpers';
 import { basename, extname } from './utils/path';
 import relativeId from './utils/relativeId';
 import { RenderOptions } from './utils/renderHelpers';
-import { SOURCEMAPPING_URL_RE } from './utils/sourceMappingURL';
+import { SOURCEMAPPING_URL_COMMENT_RE } from './utils/sourceMappingURL';
 import { timeEnd, timeStart } from './utils/timers';
 import { markModuleAndImpureDependenciesAsExecuted } from './utils/traverseStaticDependencies';
 import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';
@@ -119,6 +119,31 @@ const MISSING_EXPORT_SHIM_DESCRIPTION: ExportDescription = {
 	localName: MISSING_EXPORT_SHIM_VARIABLE
 };
 
+function findSourceMappingURLComments(ast: acorn.Node, code: string): [number, number][] {
+	const ret: [number, number][] = [];
+
+	const addCommentsPos = (start: number, end: number): void => {
+		if (start == end) {
+			return;
+		}
+
+		let sourcemappingUrlMatch;
+		const interStatmentCode = code.slice(start, end);
+		while (sourcemappingUrlMatch = SOURCEMAPPING_URL_COMMENT_RE.exec(interStatmentCode)) {
+			ret.push([start + sourcemappingUrlMatch.index, start + SOURCEMAPPING_URL_COMMENT_RE.lastIndex]);
+		}
+	};
+
+	let prevStmtEnd = 0;
+	for (const stmt of (ast as GenericEsTreeNode).body) {
+		addCommentsPos(prevStmtEnd, stmt.start);
+		prevStmtEnd = stmt.end;
+	}
+	addCommentsPos(prevStmtEnd, code.length);
+
+	return ret;
+}
+
 function getVariableForExportNameRecursive(
 	target: Module | ExternalModule,
 	name: string,
@@ -181,7 +206,6 @@ export default class Module {
 	ast: Program | null = null;
 	chunkFileNames = new Set<string>();
 	chunkName: string | null = null;
-	comments: acorn.Comment[] = [];
 	cycles = new Set<Symbol>();
 	dependencies = new Set<Module | ExternalModule>();
 	dynamicDependencies = new Set<Module | ExternalModule>();
@@ -683,12 +707,8 @@ export default class Module {
 		this.alwaysRemovedCode = alwaysRemovedCode || [];
 		if (!ast) {
 			ast = this.tryParse();
-			for (const comment of this.comments) {
-				if (comment.type != "Block" && SOURCEMAPPING_URL_RE.test(comment.value)) {
-					this.alwaysRemovedCode.push([comment.start, comment.end]);
-				}
-			}
 		}
+		this.alwaysRemovedCode.push(...findSourceMappingURLComments(ast, this.info.code));
 
 		timeEnd('generate ast', 3);
 
@@ -798,10 +818,7 @@ export default class Module {
 
 	tryParse(): acorn.Node {
 		try {
-			return this.graph.contextParse(this.info.code!, {
-				onComment: (block: boolean, text: string, start: number, end: number) =>
-					this.comments.push({ type: block ? "Block" : "Line", value: text, start, end })
-			});
+			return this.graph.contextParse(this.info.code!);
 		} catch (err) {
 			let message = err.message.replace(/ \(\d+:\d+\)$/, '');
 			if (this.id.endsWith('.json')) {

src/utils/sourceMappingURL.ts

@@ -2,7 +2,11 @@
 // this for an actual sourceMappingURL
 let SOURCEMAPPING_URL = 'sourceMa';
 SOURCEMAPPING_URL += 'ppingURL';
+const whiteSpaceNoNewline = '[ \\f\\r\\t\\v\\u00a0\\u1680\\u2000-\\u200a\\u2028\\u2029\\u202f\\u205f\\u3000\\ufeff]';
 
-const SOURCEMAPPING_URL_RE = new RegExp(`^#\\s+${SOURCEMAPPING_URL}=.+\\n?`);
+const SOURCEMAPPING_URL_LINE_COMMENT_RE = `//#${whiteSpaceNoNewline}+${SOURCEMAPPING_URL}=.+`;
+const SOURCEMAPPING_URL_BLOCK_COMMENT_RE = `/\\*#${whiteSpaceNoNewline}+${SOURCEMAPPING_URL}=.+\\*/`;
+const SOURCEMAPPING_URL_COMMENT_RE = new RegExp(
+  `(${SOURCEMAPPING_URL_LINE_COMMENT_RE})|(${SOURCEMAPPING_URL_BLOCK_COMMENT_RE})`, 'g');
 
-export { SOURCEMAPPING_URL, SOURCEMAPPING_URL_RE };
+export { SOURCEMAPPING_URL, SOURCEMAPPING_URL_COMMENT_RE};

test/function/samples/plugin-parse-ast-receives-comments/_config.js

@@ -0,0 +1,22 @@
+const assert = require('assert');
+
+const comments = [];
+
+module.exports = {
+	description: 'plugin parse ast receives comments',
+	options: {
+		plugins:[{
+			transform(code, _id) {
+				const ast = this.parse(code, {
+					onComment(...args) {
+						comments.push(args);
+					},
+				});
+				return {ast, code, map: null};
+			},
+		}],
+	},
+	after() {
+		assert.ok(comments.length > 0);
+	},
+};

test/function/samples/plugin-parse-ast-receives-comments/main.js

@@ -0,0 +1,4 @@
+/* this is a comment */
+export function main() {
+	// this is also a comment
+}

test/function/samples/plugin-parse-ast-remove-sourcemapping/_config.js

@@ -0,0 +1,16 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'remove source mapping comment even if code is parsed by PluginContext.parse method',
+	options: {
+		plugins:[{
+			transform(code, _id) {
+				const ast = this.parse(code);
+				return {ast, code, map: null};
+			},
+		}],
+	},
+	code(code) {
+		assert.ok(code.search(/sourceMappingURL/) === -1);
+	}
+};