Adopt define_method for Map/Set/WeakMap/WeakSet prototype builtins

[pmatos]

Refactoring goal

Continue the define_method migration started in #197 / commit 1d1152d (Adopt define_method for Array.prototype builtins). collections.rs still hand-rolled the two-step create_function + insert_builtin dance at 40 method call sites across setup_map/set/weakmap/weakset_prototype (plus WeakRef and FinalizationRegistry), even though Interpreter::define_method — whose own doc comment says it "replaces the two-step create_function + insert_builtin dance repeated at every builtin call site" — already captures exactly that pattern.

This was surfaced as the single highest impact × safety opportunity by an improve-codebase-architecture audit (top finding in 3 of 4 subsystem explorers). Switching collections.rs over removes ~200 lines of net duplication and brings the file in line with the one established convention.

What changed

• 40 let x = self.create_function(JsFunction::native("name", len, |…| {…})); cell.borrow_mut().insert_builtin("name", x); sites → one-line self.define_method(target, "name", len, |…| {…}) calls.
• Reused iterator functions keep their let-binding so aliasing is preserved: Map.prototype.entries is reused for [@@iterator]; Set.prototype.values is reused for keys and [@@iterator].
• Deliberately untouched (not the plain method-install shape): the size/species getters, symbol-keyed installs, the constructor data property, and the static Map.groupBy.

define_method installs the same writable / non-enumerable / configurable own property the two-step dance did, so behavior is unchanged. The large line delta is mechanical re-indentation (cargo fmt de-indents each closure body one level once the wrapper is gone) — the same shape as the Array migration's diff.

Tests that validate it

Written before the refactor (TDD) and left in place as a regression guard:

collection_prototype_methods_have_correctly_shaped_builtins (src/interpreter/tests.rs) — a JS-level characterization test (public seam, not internals) pinning, for every migrated method:

• typeof === "function", correct .name and .length;
• the writable:true / enumerable:false / configurable:true descriptor (Object.getOwnPropertyDescriptor);
• the well-known aliasing (Map.prototype[@@iterator] === entries; Set.prototype.keys === values === [@@iterator]);
• functional smoke of Map/Set/WeakMap/WeakSet get/set/has/delete/size/forEach/iteration and Set union/intersection/isSubsetOf.

Verification

Check	Result
cargo test --release	197/197 pass (incl. new test + smoke oracle)
cargo clippy --release -- -D warnings	clean
cargo fmt --check	clean
run-custom-tests.py	3/3
test262 built-ins/{Map,Set,WeakMap,WeakSet,WeakRef,FinalizationRegistry}	1772/1772 (100%), 0 regressions
Full test262 suite	0 regressions attributable to this change

The full-suite run reports 22 baseline deltas, but they are entirely in intl402/DateTimeFormat / Temporal toLocaleString (ICU-data-sensitive locale formatting) and reproduce identically with this change stashed — pre-existing environment/baseline drift, not caused by this refactor, which touches no Intl code.

🤖 Generated with Claude Code