fix: rewrite AssignmentTargetProperty

crates/rolldown/src/finalizer/impl_visit_mut_for_finalizer.rs

@@ -4,6 +4,7 @@ use oxc::{
     ast::{self, SimpleAssignmentTarget},
     VisitMut,
   },
+  span::Span,
 };
 use rolldown_common::{ExportsKind, ModuleId, SymbolRef, WrapKind};
 use rolldown_oxc::{Dummy, ExpressionExt, IntoIn, StatementExt, TakeIn};
@@ -278,13 +279,14 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
       if ident.name != canonical_name {
         ident.name = canonical_name.clone();
       }
-      *ident.symbol_id.get_mut() = None;
+      ident.symbol_id.get_mut().take();
     } else {
       // Some `BindingIdentifier`s constructed by bundler don't have `SymbolId` and we just ignore them.
     }
   }
 
   fn visit_identifier_reference(&mut self, ident: &mut ast::IdentifierReference) {
+    // This ensure all `IdentifierReference`s are processed
     debug_assert!(
       self.is_global_identifier_reference(ident) || ident.reference_id.get().is_none(),
       "{} doesn't get processed in {}",
@@ -341,11 +343,13 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
 
   fn visit_object_property(&mut self, prop: &mut ast::ObjectProperty<'ast>) {
     // Ensure `{ a }` would be rewritten to `{ a: a$1 }` instead of `{ a$1 }`
-    match prop.value {
-      ast::Expression::Identifier(ref id_ref) if prop.shorthand => {
-        if let Some(expr) = self.generate_finalized_expr_for_reference(id_ref, true) {
+    match &mut prop.value {
+      ast::Expression::Identifier(id_ref) if prop.shorthand => {
+        if let Some(expr) = self.generate_finalized_expr_for_reference(id_ref, false) {
           prop.value = expr;
           prop.shorthand = false;
+        } else {
+          id_ref.reference_id.get_mut().take();
         }
       }
       _ => {}
@@ -371,6 +375,7 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
               ident.name = canonical_name.clone();
               prop.shorthand = false;
             }
+            ident.symbol_id.get_mut().take();
           }
         }
         // Ensure `const { a = 1 } = ...;` will be rewritten to `const { a: a$1 = 1 } = ...` instead of `const { a$1 = 1 } = ...`
@@ -388,6 +393,7 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
               ident.name = canonical_name.clone();
               prop.shorthand = false;
             }
+            ident.symbol_id.get_mut().take();
           }
         }
         _ => {
@@ -438,6 +444,53 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
     }
   }
 
+  fn visit_assignment_target_property(
+    &mut self,
+    property: &mut ast::AssignmentTargetProperty<'ast>,
+  ) {
+    if let ast::AssignmentTargetProperty::AssignmentTargetPropertyIdentifier(prop) = property {
+      if let Some(target) =
+        self.generate_finalized_simple_assignment_target_for_reference(&prop.binding)
+      {
+        *property = ast::AssignmentTargetProperty::AssignmentTargetPropertyProperty(
+          ast::AssignmentTargetPropertyProperty {
+            name: ast::PropertyKey::Identifier(
+              self.snippet.id_name(prop.binding.name.clone()).into_in(self.alloc),
+            ),
+            binding: if let Some(init) = prop.init.take() {
+              ast::AssignmentTargetMaybeDefault::AssignmentTargetWithDefault(
+                ast::AssignmentTargetWithDefault {
+                  binding: ast::AssignmentTarget::SimpleAssignmentTarget(target),
+                  init,
+                  span: Span::default(),
+                }
+                .into_in(self.alloc),
+              )
+            } else {
+              ast::AssignmentTargetMaybeDefault::AssignmentTarget(
+                ast::AssignmentTarget::SimpleAssignmentTarget(target),
+              )
+            },
+            span: Span::default(),
+          }
+          .into_in(self.alloc),
+        );
+      } else {
+        prop.binding.reference_id.get_mut().take();
+      }
+    }
+
+    // visit children
+    match property {
+      ast::AssignmentTargetProperty::AssignmentTargetPropertyIdentifier(ident) => {
+        self.visit_assignment_target_property_identifier(ident);
+      }
+      ast::AssignmentTargetProperty::AssignmentTargetPropertyProperty(prop) => {
+        self.visit_assignment_target_property_property(prop);
+      }
+    }
+  }
+
   fn visit_simple_assignment_target(&mut self, target: &mut SimpleAssignmentTarget<'ast>) {
     self.rewrite_simple_assignment_target(target);
 

crates/rolldown/src/finalizer/rename.rs

@@ -53,6 +53,45 @@ where
     None
   }
 
+  /// return `None` if
+  /// - the reference is for a global variable/the reference doesn't have a `SymbolId`
+  /// - the reference doesn't have a `ReferenceId`
+  /// - the canonical name is the same as the original name
+  pub fn generate_finalized_simple_assignment_target_for_reference(
+    &self,
+    id_ref: &IdentifierReference,
+  ) -> Option<ast::SimpleAssignmentTarget<'ast>> {
+    // Some `IdentifierReference`s constructed by bundler don't have `ReferenceId` and we just ignore them.
+    let reference_id = id_ref.reference_id.get()?;
+
+    // we will hit this branch if the reference points to a global variable
+    let symbol_id = self.scope.symbol_id_for(reference_id)?;
+
+    let symbol_ref: SymbolRef = (self.ctx.id, symbol_id).into();
+    let canonical_ref = self.ctx.symbols.par_canonical_ref_for(symbol_ref);
+    let symbol = self.ctx.symbols.get(canonical_ref);
+
+    if let Some(ns_alias) = &symbol.namespace_alias {
+      let canonical_ns_name = self.canonical_name_for(ns_alias.namespace_ref);
+      let prop_name = &ns_alias.property_name;
+      let access_expr =
+        self.snippet.literal_prop_access_member_expr(canonical_ns_name.clone(), prop_name.clone());
+
+      return Some(ast::SimpleAssignmentTarget::MemberAssignmentTarget(
+        access_expr.into_in(self.alloc),
+      ));
+    }
+
+    let canonical_name = self.canonical_name_for(canonical_ref);
+    if id_ref.name != canonical_name {
+      return Some(ast::SimpleAssignmentTarget::AssignmentTargetIdentifier(
+        self.snippet.id_ref(canonical_name.clone()).into_in(self.alloc),
+      ));
+    }
+
+    None
+  }
+
   pub fn try_rewrite_identifier_reference_expr(
     &mut self,
     expr: &mut ast::Expression<'ast>,

crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable/artifacts.snap

@@ -0,0 +1,44 @@
+---
+source: crates/rolldown/tests/common/case.rs
+expression: content
+input_file: crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable
+---
+# Assets
+
+## main.mjs
+
+```js
+import { default as assert } from "assert";
+
+// iterables.js
+let dirty;
+const zeroToFive = {
+	[Symbol.iterator](){
+		return {
+			current:0,
+			last:5,
+			next(){
+				const ret = this.current < this.last ? {
+					done:false,
+					value:this.current++
+				} : {
+					done:true
+				};
+				dirty = this.current;
+				return ret;
+			}
+		};
+	}
+};
+
+// loops.js
+const iterate = iterable => {
+	for (const value of  iterable) 	{
+	}
+};
+
+// main.js
+assert.equal(dirty, undefined);
+iterate(zeroToFive);
+assert.equal(dirty, 5);
+```

crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable/iterables.js

@@ -0,0 +1,20 @@
+export let dirty;
+
+export const zeroToFive = {
+    [Symbol.iterator]() {
+        return {
+            current: 0,
+            last: 5,
+            next() {
+                const ret = this.current < this.last
+                    ? { done: false, value: this.current++ }
+                    : { done: true };
+
+                // assert later
+                dirty = this.current;
+
+                return ret;
+            }
+        };
+    }
+};

crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable/loops.js

@@ -0,0 +1,19 @@
+export const awaitable = async (iterable) => {
+	for await (const value of iterable) {
+	}
+}
+
+// This is equivalent to the above 'awaitable' function.
+export const equivalent = async (iterable) => {
+	const iterator = iterable[Symbol.asyncIterator]()
+  let { done } = await iterator.next()
+  while (!done) {
+    ({ done } = await iterator.next())
+  }
+
+}
+
+export const iterate = iterable => {
+	for (const value of iterable) {
+	}
+}

crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable/main.js

@@ -0,0 +1,7 @@
+import assert from 'assert';
+import { zeroToFive, dirty } from './iterables';
+import { iterate } from './loops';
+
+assert.equal(dirty, undefined);
+iterate(zeroToFive);
+assert.equal(dirty, 5);

crates/rolldown/tests/fixtures/rollup/preserve-for-of-iterable/test.config.json

@@ -0,0 +1,7 @@
+{
+  "input": {
+    "external": [
+      "assert"
+    ]
+  }
+}

crates/rolldown/tests/fixtures/rollup/simplify-with-destructuring/artifacts.snap

@@ -0,0 +1,24 @@
+---
+source: crates/rolldown/tests/common/case.rs
+expression: content
+input_file: crates/rolldown/tests/fixtures/rollup/simplify-with-destructuring
+---
+# Assets
+
+## main.mjs
+
+```js
+import { default as assert } from "assert";
+
+// main.js
+let foo, unused;
+null,{foo} = {
+	foo:'bar'
+};
+assert.strictEqual(foo, 'bar');
+const assign = () => unused = {foo} = {
+	foo:'baz'
+};
+assign();
+assert.strictEqual(foo, 'baz');
+```

crates/rolldown/tests/fixtures/rollup/simplify-with-destructuring/main.js

@@ -0,0 +1,9 @@
+import assert from 'assert';
+
+let foo, unused;
+null, { foo } = { foo: 'bar' };
+assert.strictEqual(foo, 'bar');
+
+const assign = () => unused = { foo } = { foo: 'baz' };
+assign();
+assert.strictEqual(foo, 'baz');

crates/rolldown/tests/fixtures/rollup/simplify-with-destructuring/test.config.json

@@ -0,0 +1,7 @@
+{
+  "input": {
+    "external": [
+      "assert"
+    ]
+  }
+}

packages/rollup-tests/src/status.json

@@ -1,6 +1,6 @@
 {
   "failed": 0,
-  "skipFailed": 655,
+  "skipFailed": 657,
   "skipped": 0,
   "passed": 235
 }

packages/rollup-tests/src/status.md

@@ -1,6 +1,6 @@
 |  | number |
 |----| ---- |
 | failed | 0|
-| skipFailed | 655|
+| skipFailed | 657|
 | skipped | 0|
 | passed | 235|