Merge pull request #11395 from webpack/refactor/asi

refactor how asi handled

lib/DefinePlugin.js

@@ -15,6 +15,7 @@ const {
 } = require("./javascript/JavascriptParserHelpers");
 
 /** @typedef {import("./Compiler")} Compiler */
+/** @typedef {import("estree").Expression} Expression */
 /** @typedef {import("./javascript/JavascriptParser")} JavascriptParser */
 /** @typedef {null|undefined|RegExp|Function|string|number|boolean|bigint|undefined} CodeValuePrimitive */
 /** @typedef {RecursiveArrayOrRecord<CodeValuePrimitive|RuntimeValue>} CodeValue */
@@ -39,25 +40,41 @@ class RuntimeValue {
 	}
 }
 
-const stringifyObj = (obj, parser, ecmaVersion) => {
-	if (Array.isArray(obj)) {
-		return (
-			"Object([" +
-			obj.map(code => toCode(code, parser, ecmaVersion)).join(",") +
-			"])"
-		);
-	}
-
-	return (
-		"Object({" +
-		Object.keys(obj)
+/**
+ * @param {any[]|{[k: string]: any}} obj obj
+ * @param {JavascriptParser} parser Parser
+ * @param {number} ecmaVersion EcmaScript version
+ * @param {boolean|undefined|null=} asiSafe asi safe (undefined: unknown, null: unneeded)
+ * @returns {string} code converted to string that evaluates
+ */
+const stringifyObj = (obj, parser, ecmaVersion, asiSafe) => {
+	let code;
+	let arr = Array.isArray(obj);
+	if (arr) {
+		code = `[${obj
+			.map(code => toCode(code, parser, ecmaVersion, null))
+			.join(",")}]`;
+	} else {
+		code = `{${Object.keys(obj)
 			.map(key => {
 				const code = obj[key];
-				return JSON.stringify(key) + ":" + toCode(code, parser, ecmaVersion);
+				return (
+					JSON.stringify(key) + ":" + toCode(code, parser, ecmaVersion, null)
+				);
 			})
-			.join(",") +
-		"})"
-	);
+			.join(",")}}`;
+	}
+
+	switch (asiSafe) {
+		case null:
+			return code;
+		case true:
+			return arr ? code : `(${code})`;
+		case false:
+			return arr ? `;${code}` : `;(${code})`;
+		default:
+			return `Object(${code})`;
+	}
 };
 
 /**
@@ -65,9 +82,10 @@ const stringifyObj = (obj, parser, ecmaVersion) => {
  * @param {CodeValue} code Code to evaluate
  * @param {JavascriptParser} parser Parser
  * @param {number} ecmaVersion EcmaScript version
+ * @param {boolean|undefined|null=} asiSafe asi safe (undefined: unknown, null: unneeded)
  * @returns {string} code converted to string that evaluates
  */
-const toCode = (code, parser, ecmaVersion) => {
+const toCode = (code, parser, ecmaVersion, asiSafe) => {
 	if (code === null) {
 		return "null";
 	}
@@ -78,7 +96,7 @@ const toCode = (code, parser, ecmaVersion) => {
 		return "-0";
 	}
 	if (code instanceof RuntimeValue) {
-		return toCode(code.exec(parser), parser, ecmaVersion);
+		return toCode(code.exec(parser), parser, ecmaVersion, asiSafe);
 	}
 	if (code instanceof RegExp && code.toString) {
 		return code.toString();
@@ -87,7 +105,7 @@ const toCode = (code, parser, ecmaVersion) => {
 		return "(" + code.toString() + ")";
 	}
 	if (typeof code === "object") {
-		return stringifyObj(code, parser, ecmaVersion);
+		return stringifyObj(code, parser, ecmaVersion, asiSafe);
 	}
 	if (typeof code === "bigint") {
 		return ecmaVersion >= 11 ? `${code}n` : `BigInt("${code}")`;
@@ -195,14 +213,19 @@ class DefinePlugin {
 									if (recurse) return;
 									recurse = true;
 									const res = parser.evaluate(
-										toCode(code, parser, ecmaVersion)
+										toCode(code, parser, ecmaVersion, null)
 									);
 									recurse = false;
 									res.setRange(expr.range);
 									return res;
 								});
 							parser.hooks.expression.for(key).tap("DefinePlugin", expr => {
-								const strCode = toCode(code, parser, ecmaVersion);
+								const strCode = toCode(
+									code,
+									parser,
+									ecmaVersion,
+									!parser.isAsiPosition(expr.range[0])
+								);
 								if (/__webpack_require__\s*(!?\.)/.test(strCode)) {
 									return toConstantDependency(parser, strCode, [
 										RuntimeGlobals.require
@@ -228,8 +251,8 @@ class DefinePlugin {
 							if (recurseTypeof) return;
 							recurseTypeof = true;
 							const typeofCode = isTypeof
-								? toCode(code, parser, ecmaVersion)
-								: "typeof (" + toCode(code, parser, ecmaVersion) + ")";
+								? toCode(code, parser, ecmaVersion, null)
+								: "typeof (" + toCode(code, parser, ecmaVersion, null) + ")";
 							const res = parser.evaluate(typeofCode);
 							recurseTypeof = false;
 							res.setRange(expr.range);
@@ -237,8 +260,8 @@ class DefinePlugin {
 						});
 						parser.hooks.typeof.for(key).tap("DefinePlugin", expr => {
 							const typeofCode = isTypeof
-								? toCode(code, parser, ecmaVersion)
-								: "typeof (" + toCode(code, parser, ecmaVersion) + ")";
+								? toCode(code, parser, ecmaVersion, null)
+								: "typeof (" + toCode(code, parser, ecmaVersion, null) + ")";
 							const res = parser.evaluate(typeofCode);
 							if (!res.isString()) return;
 							return toConstantDependency(
@@ -268,7 +291,12 @@ class DefinePlugin {
 							.for(key)
 							.tap("DefinePlugin", evaluateToString("object"));
 						parser.hooks.expression.for(key).tap("DefinePlugin", expr => {
-							const strCode = stringifyObj(obj, parser, ecmaVersion);
+							const strCode = stringifyObj(
+								obj,
+								parser,
+								ecmaVersion,
+								!parser.isAsiPosition(expr.range[0])
+							);
 
 							if (/__webpack_require__\s*(!?\.)/.test(strCode)) {
 								return toConstantDependency(parser, strCode, [

lib/RuntimeTemplate.js

@@ -625,7 +625,7 @@ class RuntimeTemplate {
 	 * @param {string} options.request the request
 	 * @param {string | string[]} options.exportName the export name
 	 * @param {Module} options.originModule the origin module
-	 * @param {boolean} options.asiSafe true, if location is safe for ASI, a bracket can be emitted
+	 * @param {boolean|undefined} options.asiSafe true, if location is safe for ASI, a bracket can be emitted
 	 * @param {boolean} options.isCall true, if expression will be called
 	 * @param {boolean} options.callContext when false, call context will not be preserved
 	 * @param {boolean} options.defaultInterop when true and accessing the default exports, interop code will be generated
@@ -669,10 +669,12 @@ class RuntimeTemplate {
 					case "dynamic":
 						if (isCall) {
 							return `${importVar}_default()${propertyAccess(exportName, 1)}`;
-						} else if (asiSafe) {
-							return `(${importVar}_default()${propertyAccess(exportName, 1)})`;
 						} else {
-							return `${importVar}_default.a${propertyAccess(exportName, 1)}`;
+							return asiSafe
+								? `(${importVar}_default()${propertyAccess(exportName, 1)})`
+								: asiSafe === false
+								? `;(${importVar}_default()${propertyAccess(exportName, 1)})`
+								: `${importVar}_default.a${propertyAccess(exportName, 1)}`;
 						}
 					case "default-only":
 					case "default-with-named":
@@ -700,7 +702,7 @@ class RuntimeTemplate {
 					)
 				);
 				return `/*#__PURE__*/ ${
-					asiSafe ? "" : "Object"
+					asiSafe ? "" : asiSafe === false ? ";" : "Object"
 				}(${importVar}_namespace_cache || (${importVar}_namespace_cache = ${
 					RuntimeGlobals.createFakeNamespaceObject
 				}(${importVar}${exportsType === "default-only" ? "" : ", 2"})))`;
@@ -721,11 +723,11 @@ class RuntimeTemplate {
 				: Template.toNormalComment(propertyAccess(exportName)) + " ";
 			const access = `${importVar}${comment}${propertyAccess(used)}`;
 			if (isCall && callContext === false) {
-				if (asiSafe) {
-					return `(0,${access})`;
-				} else {
-					return `Object(${access})`;
-				}
+				return asiSafe
+					? `(0,${access})`
+					: asiSafe === false
+					? `;(0,${access})`
+					: `Object(${access})`;
 			}
 			return access;
 		} else {

lib/dependencies/CommonJsExportRequireDependency.js

@@ -37,7 +37,7 @@ class CommonJsExportRequireDependency extends ModuleDependency {
 		this.names = names;
 		this.ids = ids;
 		this.resultUsed = resultUsed;
-		this.asiSafe = false;
+		this.asiSafe = undefined;
 	}
 
 	get type() {
@@ -260,6 +260,7 @@ class CommonJsExportRequireDependency extends ModuleDependency {
 
 	serialize(context) {
 		const { write } = context;
+		write(this.asiSafe);
 		write(this.range);
 		write(this.valueRange);
 		write(this.base);
@@ -271,6 +272,7 @@ class CommonJsExportRequireDependency extends ModuleDependency {
 
 	deserialize(context) {
 		const { read } = context;
+		this.asiSafe = read();
 		this.range = read();
 		this.valueRange = read();
 		this.base = read();

lib/dependencies/CommonJsFullRequireDependency.js

@@ -29,7 +29,7 @@ class CommonJsFullRequireDependency extends ModuleDependency {
 		this.range = range;
 		this.names = names;
 		this.call = false;
-		this.asiSafe = false;
+		this.asiSafe = undefined;
 	}
 
 	/**

lib/dependencies/HarmonyImportSpecifierDependency.js

@@ -36,7 +36,7 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
 		this.call = undefined;
 		this.directImport = undefined;
 		this.shorthand = undefined;
-		this.asiSafe = false;
+		this.asiSafe = undefined;
 		/** @type {Set<string> | boolean} */
 		this.usedByExports = undefined;
 	}
@@ -272,7 +272,7 @@ HarmonyImportSpecifierDependency.Template = class HarmonyImportSpecifierDependen
 			request: dep.request,
 			exportName: ids,
 			originModule: module,
-			asiSafe: dep.asiSafe || dep.shorthand,
+			asiSafe: dep.shorthand ? true : dep.asiSafe,
 			isCall: dep.call,
 			callContext: !dep.directImport,
 			defaultInterop: true,

lib/dependencies/ImportMetaPlugin.js

@@ -69,7 +69,10 @@ class ImportMetaPlugin {
 									metaProperty.loc
 								)
 							);
-							const dep = new ConstDependency("Object()", metaProperty.range);
+							const dep = new ConstDependency(
+								`${parser.isAsiPosition(metaProperty.range[0]) ? ";" : ""}({})`,
+								metaProperty.range
+							);
 							dep.loc = metaProperty.loc;
 							parser.state.module.addPresentationalDependency(dep);
 							return true;

lib/javascript/JavascriptParser.js

@@ -3256,6 +3256,10 @@ class JavascriptParser extends Parser {
 		);
 	}
 
+	/**
+	 * @param {number} pos source code position
+	 * @returns {boolean} true when a semicolon has been inserted before this position, false if not
+	 */
 	isAsiPosition(pos) {
 		if (this.prevStatement === undefined) return false;
 		const currentStatement = this.statementPath[this.statementPath.length - 1];

lib/javascript/JavascriptParserHelpers.js

@@ -10,6 +10,7 @@ const ConstDependency = require("../dependencies/ConstDependency");
 const BasicEvaluatedExpression = require("./BasicEvaluatedExpression");
 
 /** @typedef {import("estree").Expression} ExpressionNode */
+/** @typedef {import("estree").Node} Node */
 /** @typedef {import("./JavascriptParser")} JavascriptParser */
 
 /**

lib/optimize/ConcatenatedModule.js

@@ -241,7 +241,7 @@ const ensureNsObjSource = (
  * @param {boolean} asCall asCall
  * @param {boolean} callContext callContext
  * @param {boolean} strictHarmonyModule strictHarmonyModule
- * @param {boolean} asiSafe asiSafe
+ * @param {boolean | undefined} asiSafe asiSafe
  * @returns {string} expression to get value of external module
  */
 const getExternalImport = (
@@ -299,6 +299,8 @@ const getExternalImport = (
 						? `${info.interopDefaultAccessName}()`
 						: asiSafe
 						? `(${info.interopDefaultAccessName}())`
+						: asiSafe === false
+						? `;(${info.interopDefaultAccessName}())`
 						: `${info.interopDefaultAccessName}.a`;
 					exportName = exportName.slice(1);
 				}
@@ -316,7 +318,11 @@ const getExternalImport = (
 		: Template.toNormalComment(`${exportName.join(".")}`);
 	const reference = `${exprStart}${comment}${propertyAccess(used)}`;
 	if (asCall && callContext === false) {
-		return asiSafe ? `(0,${reference})` : `Object(${reference})`;
+		return asiSafe
+			? `(0,${reference})`
+			: asiSafe === false
+			? `;(0,${reference})`
+			: `Object(${reference})`;
 	}
 	return reference;
 };
@@ -407,7 +413,7 @@ const getFinalBinding = (
  * @param {boolean} asCall asCall
  * @param {boolean} callContext callContext
  * @param {boolean} strictHarmonyModule strictHarmonyModule
- * @param {boolean} asiSafe asiSafe
+ * @param {boolean | undefined} asiSafe asiSafe
  * @returns {string} the final name
  */
 const getFinalName = (
@@ -581,14 +587,18 @@ const createModuleReference = ({
 	const callFlag = call ? "_call" : "";
 	const directImportFlag = directImport ? "_directImport" : "";
 	const strictFlag = strict ? "_strict" : "";
-	const asiSafeFlag = asiSafe ? "_asiSafe" : "";
+	const asiSafeFlag = asiSafe
+		? "_asiSafe1"
+		: asiSafe === false
+		? "_asiSafe0"
+		: "";
 	const exportData = ids
 		? Buffer.from(JSON.stringify(ids), "utf-8").toString("hex")
 		: "ns";
 	return `__WEBPACK_MODULE_REFERENCE__${info.index}_${exportData}${callFlag}${directImportFlag}${strictFlag}${asiSafeFlag}__`;
 };
 
-const MODULE_REFERENCE_REGEXP = /^__WEBPACK_MODULE_REFERENCE__(\d+)_([\da-f]+|ns)(_call)?(_directImport)?(_strict)?(_asiSafe)?__$/;
+const MODULE_REFERENCE_REGEXP = /^__WEBPACK_MODULE_REFERENCE__(\d+)_([\da-f]+|ns)(_call)?(_directImport)?(_strict)?(?:_asiSafe(\d))?__$/;
 
 const isModuleReference = name => {
 	return MODULE_REFERENCE_REGEXP.test(name);
@@ -598,6 +608,7 @@ const matchModuleReference = (name, modulesWithInfo) => {
 	const match = MODULE_REFERENCE_REGEXP.exec(name);
 	if (!match) return null;
 	const index = +match[1];
+	const asiSafe = match[6];
 	return {
 		index,
 		info: modulesWithInfo[index],
@@ -608,7 +619,7 @@ const matchModuleReference = (name, modulesWithInfo) => {
 		call: !!match[3],
 		directImport: !!match[4],
 		strict: !!match[5],
-		asiSafe: !!match[6]
+		asiSafe: asiSafe ? asiSafe === "1" : undefined
 	};
 };
 

test/cases/parsing/asi/a.js

@@ -0,0 +1 @@
+export function a() {}

test/cases/parsing/asi/b.json

@@ -0,0 +1 @@
+{}

test/cases/parsing/asi/index.js

@@ -0,0 +1,15 @@
+import {a as b} from "./a";
+import * as c from "./b";
+
+function donotcallme() {
+	expect("asi unsafe call happened").toBe(false);
+}
+
+it("should respect asi flag", () => {
+	(donotcallme)
+	import.meta;
+	(donotcallme)
+	b();
+	(donotcallme)
+	c;
+});

test/cases/parsing/asi/warnings.js

@@ -0,0 +1,3 @@
+module.exports = [
+	[/Critical dependency: Accessing import\.meta/]
+];

test/configCases/plugins/define-plugin/index.js

@@ -1,4 +1,7 @@
-/* globals it, should */
+function donotcallme() {
+	expect("asi unsafe call happened").toBe(false);
+}
+
 it("should define FALSE", function() {
 	expect(FALSE).toBe(false);
 	expect(typeof FALSE).toBe("boolean");
@@ -126,6 +129,10 @@ it("should define OBJECT", function() {
 	expect(o.SUB.FUNCTION(10)).toBe(11);
 });
 it("should define OBJECT.SUB.CODE", function() {
+	(donotcallme)
+	OBJECT;
+	(donotcallme)
+	OBJECT.SUB;
 	expect(typeof OBJECT.SUB.CODE).toBe("number");
 	expect(OBJECT.SUB.CODE).toBe(3);
 	if (OBJECT.SUB.CODE !== 3) require("fail");
@@ -148,6 +155,8 @@ it("should define OBJECT.SUB.STRING", function() {
 	})(OBJECT.SUB);
 });
 it("should define ARRAY", function() {
+	(donotcallme)
+	ARRAY;
 	expect(Array.isArray(ARRAY)).toBeTruthy();
 	expect(ARRAY).toHaveLength(2);
 });

types.d.ts

@@ -3824,7 +3824,7 @@ declare class JavascriptParser extends Parser {
 	parseCalculatedString(expression?: any): any;
 	evaluate(source?: any): BasicEvaluatedExpression;
 	getComments(range?: any): any;
-	isAsiPosition(pos?: any): any;
+	isAsiPosition(pos: number): boolean;
 	isStatementLevelExpression(expr?: any): boolean;
 	getTagData(name?: any, tag?: any): any;
 	tagVariable(name?: any, tag?: any, data?: any): void;