Make array and object prototype singletons immutable for now (#4142)

* Make array and object prototype singletons immutable for now

* Improve coverage

* Fix incorrect spread element handling

src/ast/nodes/ArrayExpression.ts

@@ -79,10 +79,14 @@ export default class ArrayExpression extends NodeBase {
 		const properties: ObjectProperty[] = [
 			{ key: 'length', kind: 'init', property: UNKNOWN_LITERAL_NUMBER }
 		];
+		let hasSpread = false;
 		for (let index = 0; index < this.elements.length; index++) {
 			const element = this.elements[index];
-			if (element instanceof SpreadElement) {
-				properties.unshift({ key: UnknownInteger, kind: 'init', property: element });
+			if (element instanceof SpreadElement || hasSpread) {
+				if (element) {
+					hasSpread = true;
+					properties.unshift({ key: UnknownInteger, kind: 'init', property: element });
+				}
 			} else if (!element) {
 				properties.push({ key: String(index), kind: 'init', property: UNDEFINED_EXPRESSION });
 			} else {

src/ast/nodes/shared/ArrayPrototype.ts

@@ -148,5 +148,6 @@ export const ARRAY_PROTOTYPE = new ObjectEntity(
 		unshift: METHOD_MUTATES_SELF_RETURNS_NUMBER,
 		values: METHOD_DEOPTS_SELF_RETURNS_UNKNOWN
 	} as unknown as PropertyMap,
-	OBJECT_PROTOTYPE
+	OBJECT_PROTOTYPE,
+	true
 );

src/ast/nodes/shared/ObjectEntity.ts

@@ -50,7 +50,8 @@ export class ObjectEntity extends ExpressionEntity {
 	// and we assume there are no setters or getters
 	constructor(
 		properties: ObjectProperty[] | PropertyMap,
-		private prototypeExpression: ExpressionEntity | null
+		private prototypeExpression: ExpressionEntity | null,
+		private immutable = false
 	) {
 		super();
 		if (Array.isArray(properties)) {
@@ -96,7 +97,7 @@ export class ObjectEntity extends ExpressionEntity {
 	}
 
 	deoptimizePath(path: ObjectPath): void {
-		if (this.hasUnknownDeoptimizedProperty) return;
+		if (this.hasUnknownDeoptimizedProperty || this.immutable) return;
 		const key = path[0];
 		if (path.length === 1) {
 			if (typeof key !== 'string') {
@@ -136,9 +137,6 @@ export class ObjectEntity extends ExpressionEntity {
 		thisParameter: ExpressionEntity,
 		recursionTracker: PathTracker
 	): void {
-		if (path.length === 0) {
-			return;
-		}
 		const [key, ...subPath] = path;
 
 		if (
@@ -171,7 +169,9 @@ export class ObjectEntity extends ExpressionEntity {
 						property.deoptimizeThisOnEventAtPath(event, subPath, thisParameter, recursionTracker);
 					}
 				}
-				this.thisParametersToBeDeoptimized.add(thisParameter);
+				if (!this.immutable) {
+					this.thisParametersToBeDeoptimized.add(thisParameter);
+				}
 				return;
 			}
 			for (const property of relevantUnmatchableProperties) {
@@ -194,7 +194,9 @@ export class ObjectEntity extends ExpressionEntity {
 				property.deoptimizeThisOnEventAtPath(event, subPath, thisParameter, recursionTracker);
 			}
 		}
-		this.thisParametersToBeDeoptimized.add(thisParameter);
+		if (!this.immutable) {
+			this.thisParametersToBeDeoptimized.add(thisParameter);
+		}
 		this.prototypeExpression?.deoptimizeThisOnEventAtPath(
 			event,
 			path,
@@ -283,7 +285,9 @@ export class ObjectEntity extends ExpressionEntity {
 				return false;
 			}
 			for (const getter of this.unmatchableGetters) {
-				if (getter.hasEffectsWhenAccessedAtPath(subPath, context)) return true;
+				if (getter.hasEffectsWhenAccessedAtPath(subPath, context)) {
+					return true;
+				}
 			}
 		} else {
 			for (const getters of Object.values(this.gettersByKey).concat([this.unmatchableGetters])) {
@@ -315,6 +319,7 @@ export class ObjectEntity extends ExpressionEntity {
 		}
 
 		if (this.hasUnknownDeoptimizedProperty) return true;
+		// We do not need to test for unknown properties as in that case, hasUnknownDeoptimizedProperty is true
 		if (typeof key === 'string') {
 			if (this.propertiesAndSettersByKey[key]) {
 				const setters = this.settersByKey[key];
@@ -326,12 +331,8 @@ export class ObjectEntity extends ExpressionEntity {
 				return false;
 			}
 			for (const property of this.unmatchableSetters) {
-				if (property.hasEffectsWhenAssignedAtPath(subPath, context)) return true;
-			}
-		} else {
-			for (const setters of Object.values(this.settersByKey).concat([this.unmatchableSetters])) {
-				for (const setter of setters) {
-					if (setter.hasEffectsWhenAssignedAtPath(subPath, context)) return true;
+				if (property.hasEffectsWhenAssignedAtPath(subPath, context)) {
+					return true;
 				}
 			}
 		}
@@ -399,11 +400,6 @@ export class ObjectEntity extends ExpressionEntity {
 					}
 					if (!propertiesAndGettersByKey[key]) {
 						propertiesAndGettersByKey[key] = [property, ...unmatchablePropertiesAndGetters];
-						if (INTEGER_REG_EXP.test(key)) {
-							for (const integerProperty of unknownIntegerProps) {
-								propertiesAndGettersByKey[key].push(integerProperty);
-							}
-						}
 					}
 				}
 			}
@@ -467,7 +463,7 @@ export class ObjectEntity extends ExpressionEntity {
 			return UNKNOWN_EXPRESSION;
 		}
 		const expression = this.getMemberExpression(key);
-		if (expression !== UNKNOWN_EXPRESSION) {
+		if (!(expression === UNKNOWN_EXPRESSION || this.immutable)) {
 			const expressionsToBeDeoptimized = (this.expressionsToBeDeoptimizedByKey[key] =
 				this.expressionsToBeDeoptimizedByKey[key] || []);
 			expressionsToBeDeoptimized.push(origin);

src/ast/nodes/shared/ObjectPrototype.ts

@@ -15,5 +15,6 @@ export const OBJECT_PROTOTYPE = new ObjectEntity(
 		toString: METHOD_RETURNS_STRING,
 		valueOf: METHOD_RETURNS_UNKNOWN
 	} as unknown as PropertyMap,
-	null
+	null,
+	true
 );

test/form/samples/object-expression/reassign-prop-without-proto/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'correctly deoptimizes when there is no proto'
+};

test/form/samples/object-expression/reassign-prop-without-proto/_expected.js

@@ -0,0 +1,7 @@
+const obj = { __proto__: null };
+
+obj.flag = true;
+
+if (obj.flag) {
+	console.log('mutated');
+}

test/form/samples/object-expression/reassign-prop-without-proto/main.js

@@ -0,0 +1,7 @@
+const obj = { __proto__: null };
+
+obj.flag = true;
+
+if (obj.flag) {
+	console.log('mutated');
+}

test/form/samples/object-expression/unknown-getter-no-side-effect/_config.js

@@ -0,0 +1,4 @@
+module.exports = {
+	description: 'removes unknown getter access without side effect',
+	options: { external: ['external'] }
+};

test/form/samples/object-expression/unknown-getter-no-side-effect/_expected.js

@@ -0,0 +1 @@
+import 'external';

test/form/samples/object-expression/unknown-getter-no-side-effect/main.js

@@ -0,0 +1,7 @@
+import { unknown } from 'external';
+
+const obj = {
+	get [unknown]() {}
+};
+
+obj.prop;

test/form/samples/object-expression/unknown-setter-no-side-effect/_config.js

@@ -0,0 +1,4 @@
+module.exports = {
+	description: 'removes unknown setter access without side effect',
+	options: { external: ['external'] }
+};

test/form/samples/object-expression/unknown-setter-no-side-effect/_expected.js

@@ -0,0 +1 @@
+import 'external';

test/form/samples/object-expression/unknown-setter-no-side-effect/main.js

@@ -0,0 +1,7 @@
+import { unknown } from 'external';
+
+const obj = {
+	set [unknown](value) {}
+};
+
+obj.prop = true;

test/function/samples/array-double-spread/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'correctly deoptimizes when there is no proto'
+};

test/function/samples/array-double-spread/main.js

@@ -0,0 +1,19 @@
+const a = [false, , true];
+const b = [false, , true, ...a, false, , true, ...a];
+
+let count = 0;
+
+b[0] ? count+= 10: count++;
+b[1] ? count+= 10: count++;
+b[2] ? count+= 10: count++;
+b[3] ? count+= 10: count++;
+b[4] ? count+= 10: count++;
+b[5] ? count+= 10: count++;
+b[6] ? count+= 10: count++;
+b[7] ? count+= 10: count++;
+b[8] ? count+= 10: count++;
+b[9] ? count+= 10: count++;
+b[10] ? count+= 10: count++;
+b[11] ? count+= 10: count++;
+
+assert.strictEqual(count, 48);

test/function/samples/modify-this-via-getter/unknown-prop-getter/_config.js

@@ -1,7 +1,7 @@
 module.exports = {
 	description: 'handles unknown getters that modify "this"',
 	context: {
-		require(id) {
+		require() {
 			return { unknown: 'prop' };
 		}
 	},

test/function/samples/object-deep-access-effect/_config.js

@@ -0,0 +1,19 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'throws when an nested property of an unknown object property is accessed',
+	context: {
+		require() {
+			return { unknown: 'prop' };
+		}
+	},
+	options: {
+		external: ['external']
+	},
+	exports({ expectError }) {
+		assert.throws(expectError, {
+			name: 'TypeError',
+			message: "Cannot read property 'prop' of undefined"
+		});
+	}
+};

test/function/samples/object-deep-access-effect/main.js

@@ -0,0 +1,6 @@
+import { unknown } from 'external';
+
+export function expectError() {
+	const obj = {};
+	obj[unknown].prop;
+}

test/function/samples/returned-array-mutation/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'tracks array mutations'
+};

test/function/samples/returned-array-mutation/main.js

@@ -0,0 +1,13 @@
+let push = false;
+
+const getArray = () => {
+	const array = [];
+	if (push) {
+		array.push(true);
+	}
+	return array;
+};
+
+assert.strictEqual(getArray()[0] || false, false);
+push = true;
+assert.strictEqual(getArray()[0] || false, true);