fix(ecmascript): treat collection constructor with variable arg as side-effectful

Dunqing · claude · Dunqing · commit 74c9ddd8766c · 2026-03-14T23:23:47.000+08:00
`new Map(x)`, `new Set(x)`, `new WeakMap(x)`, `new WeakSet(x)` were
incorrectly considered side-effect-free when the argument is a variable
reference. These constructors iterate their argument via
`Symbol.iterator`, which can execute arbitrary user code.

Now only provably safe arguments are considered pure: no arguments,
`null`, `undefined`, or array literals (for Map/WeakMap, entries must
also be array literals). This matches esbuild and Rollup behavior.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/crates/oxc_ecmascript/src/side_effects/expressions.rs b/crates/oxc_ecmascript/src/side_effects/expressions.rs
@@ -295,14 +295,50 @@ fn is_pure_call(name: &str) -> bool {
 
 #[rustfmt::skip]
 fn is_pure_constructor(name: &str) -> bool {
-    matches!(name, "Set" | "Map" | "WeakSet" | "WeakMap" | "ArrayBuffer" | "Date"
+    matches!(name, "ArrayBuffer" | "Date"
             | "Boolean" | "Error" | "EvalError" | "RangeError" | "ReferenceError"
             | "SyntaxError" | "TypeError" | "URIError" | "Number" | "Object" | "String"
             | "Int8Array" | "Uint8Array" | "Uint8ClampedArray" | "Int16Array" | "Uint16Array"
             | "Int32Array" | "Uint32Array" | "Float32Array" | "Float64Array"
             | "BigInt64Array" | "BigUint64Array")
 }
 
+/// Whether a collection constructor (`Map`, `Set`, `WeakMap`, `WeakSet`) call is pure.
+///
+/// These constructors iterate their argument via `Symbol.iterator`, which can have side effects
+/// when the argument is a variable reference (custom iterators, proxies, etc.).
+/// Only provably safe arguments are considered pure:
+/// - No arguments: `new Set()`, `new Map()`
+/// - `null` or `undefined`: `new Set(null)`, `new Map(undefined)`
+/// - Array literals: `new Set([1,2,3])`, `new Map([[k,v]])`
+///
+/// Following esbuild and Rollup behavior.
+/// See <https://github.com/oxc-project/oxc/issues/XXXX>
+fn is_pure_collection_constructor(name: &str, args: &[Argument<'_>]) -> bool {
+    if !matches!(name, "Set" | "Map" | "WeakSet" | "WeakMap") {
+        return false;
+    }
+    match args.first() {
+        // No arguments: always pure
+        None => true,
+        Some(arg) => match arg.as_expression() {
+            Some(Expression::NullLiteral(_)) => true,
+            Some(Expression::Identifier(id)) if id.name == "undefined" => true,
+            Some(Expression::ArrayExpression(arr)) => {
+                // For Map/WeakMap, each element must also be an array literal (key-value pair)
+                if matches!(name, "Map" | "WeakMap") {
+                    arr.elements
+                        .iter()
+                        .all(|el| matches!(el, ArrayExpressionElement::ArrayExpression(_)))
+                } else {
+                    true
+                }
+            }
+            _ => false,
+        },
+    }
+}
+
 /// Whether the name matches any known global constructors.
 ///
 /// <https://tc39.es/ecma262/multipage/global-object.html#sec-constructor-properties-of-the-global-object>
@@ -998,7 +1034,9 @@ impl<'a> MayHaveSideEffects<'a> for NewExpression<'a> {
         if let Expression::Identifier(ident) = &self.callee
             && ctx.is_global_reference(ident)
             && let name = ident.name.as_str()
-            && (is_pure_constructor(name) || (name == "RegExp" && is_valid_regexp(&self.arguments)))
+            && (is_pure_constructor(name)
+                || (name == "RegExp" && is_valid_regexp(&self.arguments))
+                || is_pure_collection_constructor(name, &self.arguments))
         {
             return self.arguments.iter().any(|e| e.may_have_side_effects(ctx));
         }
diff --git a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs
@@ -957,6 +957,36 @@ fn test_new_expressions() {
     test("new BigUint64Array", false);
     // DataView requires an ArrayBuffer argument; calling without one throws
     test("new DataView", true);
+
+    // Collection constructors iterate their argument via Symbol.iterator,
+    // so they are only pure with no args, null, undefined, or array literals.
+    // null/undefined — pure
+    test("new Set(null)", false);
+    test("new Map(null)", false);
+    test("new WeakSet(null)", false);
+    test("new WeakMap(null)", false);
+    test("new Set(undefined)", false);
+    test("new Map(undefined)", false);
+    // Array literal — pure
+    test("new Set([])", false);
+    test("new Set([1, 2, 3])", false);
+    test("new Map([])", false);
+    test("new Map([[1, 2], [3, 4]])", false);
+    test("new WeakSet([])", false);
+    test("new WeakMap([])", false);
+    test("new WeakMap([[{}, 1]])", false);
+    // Variable reference — NOT pure (could have custom Symbol.iterator)
+    test("new Set(x)", true);
+    test("new Map(x)", true);
+    test("new WeakSet(x)", true);
+    test("new WeakMap(x)", true);
+    // Non-array literals — NOT pure
+    test("new Set(false)", true);
+    test("new Map({})", true);
+    // Map with non-array entries — NOT pure
+    test("new Map([x])", true);
+    test("new Map([x, []])", true);
+    test("new Map([[], x])", true);
 }
 
 // `PF` in <https://github.com/rollup/rollup/blob/master/src/ast/nodes/shared/knownGlobals.ts>
