feat(es/transformer): Implement object rest spread transform

[kdy1]

Summary

Implements ES2018 object rest spread transformation in the new swc_ecma_transformer architecture.

Changes

• Implemented ObjectRest transformer for rest patterns in destructuring
• Implemented ObjectSpread transformer for spread expressions in object literals
• Added support for nested patterns and complex destructuring cases
• Integrated with Assumptions for configuration options:
  • object_rest_no_symbols: Use loose mode without symbol handling
  • set_spread_properties: Use Object.assign-like semantics
  • pure_getters: Assume getters have no side effects

Implementation Details

The implementation follows the esbuild lowering algorithm and handles:

• Nested object/array rest patterns
• Computed property keys (both pure and impure)
• Function parameters with rest patterns
• Catch clause patterns
• For-in/for-of loop patterns
• Assignment expressions with rest patterns
• Export declarations with rest patterns

Test Plan

✅ All tests passing:

• cargo test -p swc_ecma_compat_es2018 (0 tests in this package)
• cargo test -p swc_ecma_transforms_compat --test es2018_object_rest_spread (72 passed, 6 ignored)

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1f45ab5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Pull Request Review: ES2018 Object Rest Spread Transformation

Summary

This PR implements ES2018 object rest/spread transformation in the new swc_ecma_transformer architecture. The implementation follows the esbuild lowering algorithm and migrates code from swc_ecma_compat_es2018 to the new transformer system.

Code Quality & Architecture ✅

Strengths:

1. Well-structured code: The implementation uses a clean trait-based design (RestOutput trait) to adapt the algorithm for different contexts (declarations vs expressions)
2. Clear separation of concerns: Split into ObjectRest and ObjectSpread transformers with dedicated responsibilities
3. Good documentation: Links to esbuild source code and inline comments explaining complex logic
4. Comprehensive test coverage: 72 tests passing demonstrates thorough testing

Design Patterns:

• The RestLowerer<O: RestOutput> generic design is elegant and allows code reuse between declaration and expression contexts
• Pattern matching is used effectively throughout
• Proper use of Rust idioms (mem::take, drain, etc.)

Potential Issues & Concerns

1. ⚠️ Missing Fast-Path Optimization

The original code in swc_ecma_compat_es2018/src/object_rest.rs had a #[fast_path(RestVisitor)] macro on line 335, but this optimization appears to be removed in the new implementation at crates/swc_ecma_transformer/src/es2018/object_rest_spread.rs:336.

Impact: This could cause unnecessary traversal of AST nodes that don't contain rest patterns, potentially affecting performance.

Recommendation: Verify if this was intentional or if the fast-path optimization should be re-added.

2. 🔍 Security: Expression Evaluation Order

The implementation carefully handles RHS-before-LHS semantics (lines 343-345, 531-532), which is critical for correct behavior. This looks correct. ✅

3. 🔍 Computed Keys with Side Effects

Lines 127-140 handle impure computed keys by pre-evaluating them. The logic appears sound:

• Checks if computed keys are pure using is_pure_expr
• Creates temp variables for impure keys
• This prevents multiple evaluations of side-effectful expressions ✅

4. ⚠️ Potential Edge Case: Empty Object Null Check

Lines 249-258 handle the null check for rest patterns. The logic is subtle:

let is_fresh_source = captured_keys.is_empty();
if rest_idx > 0 || is_fresh_source {
    self.out.assign(obj.into(), init_expr);
}

Question: When rest_idx == 0 and !is_fresh_source, the code skips emitting the destructuring. While the comment explains this is for already-checked sources, ensure this doesn't miss null/undefined checks in edge cases.

5. 📝 Code Consistency

The is_pure_expr function (lines 1107-1118) checks for pure expressions. Consider:

• Adding tests for Symbol member access edge cases
• Template literals with no expressions are considered pure ✅

Performance Considerations

Positive:

1. Pre-allocation: Uses Vec::with_capacity appropriately (e.g., line 340, 585)
2. Efficient iteration: Uses iterators and drain to avoid unnecessary clones
3. Lazy evaluation: Only processes patterns that need transformation

Concerns:

1. Missing fast-path: As mentioned above, the removed #[fast_path(RestVisitor)] could impact performance
2. Recursive calls: visit_object can recurse through split_object_pattern - ensure stack safety for deeply nested patterns (though this is unlikely in practice)

Best Practices Compliance ✅

According to AGENTS.md:

• ✅ Comments and documentation in English
• ✅ Unit tests included (72 passing tests)
• ✅ Code follows Rust conventions
• ✅ Uses Cow<str> and &str appropriately for Atoms

Test Coverage

According to the PR description:

• ✅ 72 tests passing
• ✅ 6 tests ignored (expected)
• ✅ Covers nested patterns, computed keys, function parameters, catch clauses, for loops, assignment expressions, and export declarations

Recommendation: Verify the 6 ignored tests - are they legitimately unsupported features or bugs to fix later?

Security Considerations

1. ✅ No command injection risks: Pure AST transformation
2. ✅ No unsafe code: Uses safe Rust throughout
3. ✅ Side-effect handling: Properly handles expressions with side effects
4. ✅ Null/undefined checks: Maintains JavaScript semantics for destructuring null/undefined

Integration Issues

CI Failure: Cargo Shear ❌

The PR has a failing "Cargo shear" check. This needs to be investigated and fixed before merge.

Recommendation:

• Check if the new dependency swc_ecma_transformer added to swc_ecma_compat_es2018/Cargo.toml is causing issues
• Ensure no circular dependencies were introduced

Suggestions for Improvement

1. Add back fast-path optimization if it was unintentionally removed
2. Fix CI failure - Cargo shear needs to pass
3. Add inline examples to complex functions like split_object_pattern and lower_object_rest
4. Document the 6 ignored tests - add comments explaining why they're ignored
5. Performance testing: Consider benchmarking this against the old implementation to ensure no regressions

Minor Nits

1. Line 161-164: The unreachable!() after pop() assumes the pattern is there - consider a more descriptive error
2. Lines 168-169: #[cfg(swc_ast_unknown)] usage is good for future compatibility
3. The code is very long (1214 lines) - consider splitting into submodules if it grows further

Overall Assessment

Rating: 8/10 - Strong implementation with good architecture and test coverage, but needs CI fix and fast-path consideration.

Before Merge:

1. ✅ Fix Cargo shear CI failure (REQUIRED)
2. ⚠️ Investigate removed fast-path optimization
3. ⚠️ Document ignored tests
4. ⚠️ Add performance benchmarks

Approval Recommendation:

APPROVE with required changes - Fix the CI failure, then this is ready to merge. The fast-path optimization should be investigated but doesn't block the PR if it was intentionally removed.

Great work on this substantial refactoring! The code quality is high and the transformation logic appears sound.

[Copilot]

Pull request overview

This PR implements ES2018 object rest/spread transformation in the new swc_ecma_transformer architecture, migrating functionality from the older swc_ecma_compat_es2018 package. The implementation follows the esbuild lowering algorithm and handles complex destructuring scenarios including nested patterns, computed property keys, and various statement contexts.

Key changes include:

• New ObjectRest and ObjectSpread transformers with comprehensive pattern handling
• Integration with the assumptions system for configuration (object_rest_no_symbols, set_spread_properties, pure_getters)
• Removal of old implementation files and creation of a compatibility adapter that delegates to the new architecture

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/swc_ecma_transformer/src/es2018/object_rest_spread.rs	Core implementation of object rest/spread transformation with 1200+ lines of pattern lowering logic
crates/swc_ecma_transformer/src/es2018/mod.rs	Integration layer that wires up the transformers with configuration from assumptions
crates/swc_ecma_transformer/src/lib.rs	Updates hook chain to pass assumptions to ES2018 transformers
crates/swc_ecma_transformer/src/hook_utils.rs	Adds boilerplate hook methods for various statement types needed by the transformer
crates/swc_ecma_preset_env/src/lib.rs	Updates to enable object rest/spread via new transformer instead of old pass-based approach
crates/swc_ecma_compat_es2018/src/object_rest_spread.rs	Replaces implementation with thin adapter that creates Options and delegates to new transformer
crates/swc_ecma_compat_es2018/src/object_rest.rs	Deleted - functionality moved to transformer package
crates/swc_ecma_compat_es2018/src/object_spread.rs	Deleted - functionality moved to transformer package
crates/swc_ecma_compat_es2018/src/lib.rs	Removes deleted module references
crates/swc_ecma_compat_es2018/Cargo.toml	Adds dependency on swc_ecma_transformer
Cargo.lock	Updates lockfile with new dependency

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Potential panic: accessing var_decl.decls[0] without checking if the array is empty. While for-in/for-of statements typically require at least one declarator, defensive programming would suggest checking is_empty() or using get(0) to avoid panics in edge cases or malformed AST inputs.

[Copilot]

The try_into() conversion can fail silently here. When a pattern cannot be converted to an AssignTarget, the assignment is dropped without any error or warning. This could lead to incorrect transformations where assignments are silently skipped.

Consider either:

1. Logging or handling the error case
2. Using unwrap_or_else with a fallback (as done in pat_to_assign_target_pat on line 1094)
3. Documenting why silent failures are acceptable in this context

Suggested change

	));
	));
	} else {
	eprintln!(
	"Warning: Failed to convert pattern to AssignTarget in ExprOutput::assign at {:?}",
	pat.span()
	);

[Copilot]

The documentation comment shows an incorrect transformation. The example states:

function foo(a, { b, ...c}, d = b, ...e) {}

transforms to:

function foo(a, _1, _2, ..._3) {
  let [{ b, ...c}, d = b, e] = [_1, _2, _3];
}

However, the pattern destructuring for the rest parameter ...e should be ...e not just e in the array pattern. The correct output should be:

let [{ b, ...c}, d = b, ...e] = [_1, _2, _3];

or without the array wrapper since rest parameters cannot be in array destructuring on the left side.

Suggested change

	/// let [{ b, ...c}, d = b, e] = [_1, _2, _3];
	/// let [{ b, ...c}, d = b, ...e] = [_1, _2, _3];

[github-actions]

Binary Sizes

File	Size
swc.linux-x64-gnu.node	31M (31807816 bytes)

Commit: 0449f16

[codspeed-hq]

CodSpeed Performance Report

Merging #11356 will degrade performances by 11.66%

_{Comparing kdy1/port-object-rest-spread (1f45ab5) with main (c388e87)¹}

Summary

❌ 5 regressions
✅ 133 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
❌	es/full-target/es2016	616.4 µs	653 µs	-5.61%
❌	es/full-target/es2018	550.9 µs	588.1 µs	-6.34%
❌	es/full-target/es2017	588.1 µs	643 µs	-8.54%
❌	es/target/es2018	322.7 µs	354.5 µs	-8.95%
❌	es2018_object_rest_spread	291.5 µs	330 µs	-11.66%

Footnotes

1. No successful run was found on main (fb4a68c) during the generation of this report, so c388e87 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩