Do not make Object.defineProperty/ies a side effect by default

src/ast/Entity.ts

@@ -12,5 +12,10 @@ export interface WritableEntity extends Entity {
 	 * expression of this node is reassigned as well.
 	 */
 	deoptimizePath(path: ObjectPath): void;
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
+
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors?: boolean
+	): boolean;
 }

src/ast/nodes/ArrayExpression.ts

@@ -60,8 +60,12 @@ export default class ArrayExpression extends NodeBase {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
 	}
 
 	hasEffectsWhenCalledAtPath(

src/ast/nodes/Identifier.ts

@@ -144,13 +144,17 @@ export default class Identifier extends NodeBase implements PatternNode {
 		);
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
 		return (
 			!this.variable ||
 			(path.length > 0
 				? this.getVariableRespectingTDZ()
 				: this.variable
-			).hasEffectsWhenAssignedAtPath(path, context)
+			).hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors)
 		);
 	}
 

src/ast/nodes/ObjectExpression.ts

@@ -79,8 +79,12 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
 	}
 
 	hasEffectsWhenCalledAtPath(

src/ast/nodes/shared/ClassNode.ts

@@ -86,8 +86,12 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
 	}
 
 	hasEffectsWhenCalledAtPath(

src/ast/nodes/shared/Expression.ts

@@ -52,7 +52,11 @@ export class ExpressionEntity implements WritableEntity {
 		return true;
 	}
 
-	hasEffectsWhenAssignedAtPath(_path: ObjectPath, _context: HasEffectsContext): boolean {
+	hasEffectsWhenAssignedAtPath(
+		_path: ObjectPath,
+		_context: HasEffectsContext,
+		_ignoreAccessors?: boolean
+	): boolean {
 		return true;
 	}
 

src/ast/nodes/shared/ObjectEntity.ts

@@ -302,22 +302,31 @@ export class ObjectEntity extends ExpressionEntity {
 		return false;
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
 		const [key, ...subPath] = path;
 		if (path.length > 1) {
 			if (typeof key !== 'string') {
 				return true;
 			}
 			const expressionAtPath = this.getMemberExpression(key);
 			if (expressionAtPath) {
-				return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context);
+				return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context, ignoreAccessors);
 			}
 			if (this.prototypeExpression) {
-				return this.prototypeExpression.hasEffectsWhenAssignedAtPath(path, context);
+				return this.prototypeExpression.hasEffectsWhenAssignedAtPath(
+					path,
+					context,
+					ignoreAccessors
+				);
 			}
 			return true;
 		}
 
+		if (ignoreAccessors) return false;
 		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') {

src/ast/nodes/shared/knownGlobals.ts

@@ -5,7 +5,11 @@ import type { ObjectPath } from '../../utils/PathTracker';
 const ValueProperties = Symbol('Value Properties');
 
 interface ValueDescription {
-	pure: boolean;
+	// Denotes a proper function except for the side effects listed explicitly
+	function: boolean;
+	// This assumes that mutation itself cannot have side effects and that this
+	// does not trigger setters or getters
+	mutatesArg1: boolean;
 }
 
 interface GlobalDescription {
@@ -14,8 +18,8 @@ interface GlobalDescription {
 	__proto__: null;
 }
 
-const PURE: ValueDescription = { pure: true };
-const IMPURE: ValueDescription = { pure: false };
+const PURE: ValueDescription = { function: true, mutatesArg1: false };
+const IMPURE: ValueDescription = { function: false, mutatesArg1: false };
 
 // We use shortened variables to reduce file size here
 /* OBJECT */
@@ -30,6 +34,12 @@ const PF: GlobalDescription = {
 	[ValueProperties]: PURE
 };
 
+/* FUNCTION THAT MUTATES FIRST ARG WITHOUT TRIGGERING ACCESSORS */
+const MUTATES_ARG_WITHOUT_ACCESSOR: GlobalDescription = {
+	__proto__: null,
+	[ValueProperties]: { function: true, mutatesArg1: true }
+};
+
 /* CONSTRUCTOR */
 const C: GlobalDescription = {
 	__proto__: null,
@@ -173,6 +183,11 @@ const knownGlobals: GlobalDescription = {
 		__proto__: null,
 		[ValueProperties]: PURE,
 		create: PF,
+		// Technically those can throw in certain situations, but we ignore this as
+		// code that relies on this will hopefully wrap this in a try-catch, which
+		// deoptimizes everything anyway
+		defineProperty: MUTATES_ARG_WITHOUT_ACCESSOR,
+		defineProperties: MUTATES_ARG_WITHOUT_ACCESSOR,
 		getOwnPropertyDescriptor: PF,
 		getOwnPropertyNames: PF,
 		getOwnPropertySymbols: PF,
@@ -847,7 +862,7 @@ for (const global of ['window', 'global', 'self', 'globalThis']) {
 	knownGlobals[global] = knownGlobals;
 }
 
-function getGlobalAtPath(path: ObjectPath): ValueDescription | null {
+export function getGlobalAtPath(path: ObjectPath): ValueDescription | null {
 	let currentGlobal: GlobalDescription | null = knownGlobals;
 	for (const pathSegment of path) {
 		if (typeof pathSegment !== 'string') {
@@ -860,15 +875,3 @@ function getGlobalAtPath(path: ObjectPath): ValueDescription | null {
 	}
 	return currentGlobal[ValueProperties];
 }
-
-export function isPureGlobal(path: ObjectPath): boolean {
-	const globalAtPath = getGlobalAtPath(path);
-	return globalAtPath !== null && globalAtPath.pure;
-}
-
-export function isGlobalMember(path: ObjectPath): boolean {
-	if (path.length === 1) {
-		return path[0] === 'undefined' || getGlobalAtPath(path) !== null;
-	}
-	return getGlobalAtPath(path.slice(0, -1)) !== null;
-}

src/ast/variables/GlobalVariable.ts

@@ -1,15 +1,32 @@
-import { isGlobalMember, isPureGlobal } from '../nodes/shared/knownGlobals';
+import { CallOptions } from '../CallOptions';
+import { HasEffectsContext } from '../ExecutionContext';
+import { getGlobalAtPath } from '../nodes/shared/knownGlobals';
+import { UNKNOWN_PATH } from '../utils/PathTracker';
 import type { ObjectPath } from '../utils/PathTracker';
 import Variable from './Variable';
 
 export default class GlobalVariable extends Variable {
 	isReassigned = true;
 
 	hasEffectsWhenAccessedAtPath(path: ObjectPath): boolean {
-		return !isGlobalMember([this.name, ...path]);
+		if (path.length === 0) {
+			return this.name !== 'undefined' && getGlobalAtPath([this.name]) === null;
+		}
+		return getGlobalAtPath([this.name, ...path].slice(0, -1)) === null;
 	}
 
-	hasEffectsWhenCalledAtPath(path: ObjectPath): boolean {
-		return !isPureGlobal([this.name, ...path]);
+	hasEffectsWhenCalledAtPath(
+		path: ObjectPath,
+		callOptions: CallOptions,
+		context: HasEffectsContext
+	): boolean {
+		const globalAtPath = getGlobalAtPath([this.name, ...path]);
+		return (
+			globalAtPath === null ||
+			!globalAtPath.function ||
+			(globalAtPath.mutatesArg1 &&
+				(!callOptions.args.length ||
+					callOptions.args[0].hasEffectsWhenAssignedAtPath(UNKNOWN_PATH, context, true)))
+		);
 	}
 }

src/ast/variables/LocalVariable.ts

@@ -150,13 +150,17 @@ export default class LocalVariable extends Variable {
 			this.init.hasEffectsWhenAccessedAtPath(path, context))!;
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
 		if (this.included) return true;
 		if (path.length === 0) return false;
 		if (this.isReassigned) return true;
 		return (this.init &&
 			!context.assigned.trackEntityAtPathAndGetIfTracked(path, this) &&
-			this.init.hasEffectsWhenAssignedAtPath(path, context))!;
+			this.init.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors))!;
 	}
 
 	hasEffectsWhenCalledAtPath(

src/ast/variables/ThisVariable.ts

@@ -73,10 +73,14 @@ export default class ThisVariable extends LocalVariable {
 		);
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAssignedAtPath(
+		path: ObjectPath,
+		context: HasEffectsContext,
+		ignoreAccessors: boolean
+	): boolean {
 		return (
 			this.getInit(context).hasEffectsWhenAssignedAtPath(path, context) ||
-			super.hasEffectsWhenAssignedAtPath(path, context)
+			super.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors)
 		);
 	}
 

test/form/samples/object-define-property/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'allows globals to have parameter mutation side effects'
+};

test/form/samples/object-define-property/_expected.js

@@ -0,0 +1,7 @@
+const retained1 = {};
+Object.defineProperty(retained1, 'foo', { value: true });
+console.log(retained1);
+
+const retained2 = {};
+Object.defineProperties(retained2, { bar: { value: true } });
+console.log(retained2);

test/form/samples/object-define-property/main.js

@@ -0,0 +1,20 @@
+const removed = {};
+Object.defineProperty(removed, 'foo', { value: true });
+Object.defineProperties(removed, { bar: { value: true } });
+
+const retained1 = {};
+Object.defineProperty(retained1, 'foo', { value: true });
+console.log(retained1);
+
+const retained2 = {};
+Object.defineProperties(retained2, { bar: { value: true } });
+console.log(retained2);
+
+const removed2 = [];
+Object.defineProperty(removed2, 'foo', { value: true });
+
+class removed3 {}
+Object.defineProperty(removed3, 'foo', { value: true });
+
+function removed4() {}
+Object.defineProperty(removed4, 'foo', { value: true });