Skip to content

Commit

Permalink
feat: should rewrite symbols in complex patterns correctly (#193)
Browse files Browse the repository at this point in the history
<!-- Thank you for contributing! -->

### Description

https://github.com/rolldown-rs/rolldown/blob/626ea15c90e6cb1dc4a87ae8bc92be7c04599aca/crates/rolldown/tests/fixtures/deconflict/decl_complex_patterns/artifacts.snap

<!-- Please insert your description here and provide especially info about the "what" this PR is solving -->

### Test Plan

<!-- e.g. is there anything you'd like reviewers to focus on? -->

---
  • Loading branch information
hyf0 authored Nov 9, 2023
1 parent 113e0c9 commit e267018
Show file tree
Hide file tree
Showing 13 changed files with 183 additions and 9 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"Renamer",
"rolldown",
"rollup",
"rustc"
"rustc",
"smallvec"
],
"git.ignoreLimitWarning": true,
"json.schemas": [
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/rolldown/src/bundler/graph/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ impl<'graph> Linker<'graph> {
&linking_infos[importer.id],
info,
) {
MatchImportKind::NotFound => panic!(""),
MatchImportKind::NotFound => panic!("info {info:#?}"),
MatchImportKind::PotentiallyAmbiguous(
symbol_ref,
mut potentially_ambiguous_symbol_refs,
Expand Down
59 changes: 58 additions & 1 deletion crates/rolldown/src/bundler/renderer/impl_visit.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use oxc::{
ast::{ast::ExportDefaultDeclarationKind, Visit},
ast::{ast, ast::ExportDefaultDeclarationKind, Visit},
span::GetSpan,
};
use rolldown_common::ExportsKind;
use rolldown_oxc::BindingIdentifierExt;
use rolldown_utils::MagicStringExt;

use crate::bundler::{module::Module, renderer::RenderControl};

Expand Down Expand Up @@ -169,4 +171,59 @@ impl<'ast, 'r> Visit<'ast> for AstRenderer<'r> {
_ => {}
}
}

fn visit_object_pattern(&mut self, pat: &ast::ObjectPattern) {
// visit children
for prop in &pat.properties {
match &prop.value.kind {
// Rewrite `const { a } = obj;`` to `const { a: a$1 } = obj;`
ast::BindingPatternKind::BindingIdentifier(ident) if prop.shorthand => {
self.visit_property_key(&prop.key);

match self.need_to_rename((self.ctx.module.id, ident.expect_symbol_id()).into()) {
Some(new_name) if new_name != &ident.name => {
self.ctx.source.overwrite(
ident.span.start,
ident.span.end,
format!("{}: {new_name}", ident.name),
);
}
_ => {}
}
}
// Rewrite `const { a = 1 } = obj;`` to `const { a: a$1 = 1 } = obj;`
ast::BindingPatternKind::AssignmentPattern(assign_pat)
if prop.shorthand
&& matches!(assign_pat.left.kind, ast::BindingPatternKind::BindingIdentifier(_)) =>
{
let ast::BindingPatternKind::BindingIdentifier(ident) = &assign_pat.left.kind else {
unreachable!()
};
match self.need_to_rename((self.ctx.module.id, ident.expect_symbol_id()).into()) {
Some(new_name) if new_name != &ident.name => {
self.ctx.source.overwrite(
ident.span.start,
ident.span.end,
format!("{}: {new_name}", ident.name),
);
}
_ => {}
}
self.visit_expression(&assign_pat.right);
}
_ => {
self.visit_binding_property(prop);
}
}
}
if let Some(rest) = &pat.rest {
self.visit_rest_element(rest);
}
}

fn visit_assignment_pattern(&mut self, pat: &ast::AssignmentPattern) {
// Visit children
self.visit_binding_pattern(&pat.left);
self.visit_expression(&pat.right);
}
}
9 changes: 4 additions & 5 deletions crates/rolldown/src/bundler/visitors/scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rolldown_common::{
ExportsKind, ImportKind, ImportRecord, ImportRecordId, LocalExport, LocalOrReExport, ModuleId,
ModuleType, NamedImport, ReExport, Specifier, StmtInfo, StmtInfos, SymbolRef,
};
use rolldown_oxc::BindingIdentifierExt;
use rolldown_oxc::{BindingIdentifierExt, BindingPatternExt};
use rustc_hash::FxHashMap;

use crate::bundler::utils::{ast_scope::AstScope, ast_symbol::AstSymbol};
Expand Down Expand Up @@ -192,14 +192,13 @@ impl<'a> Scanner<'a> {
if let Some(decl) = decl.declaration.as_ref() {
match decl {
oxc::ast::ast::Declaration::VariableDeclaration(var_decl) => {
var_decl.declarations.iter().for_each(|decl| match &decl.id.kind {
oxc::ast::ast::BindingPatternKind::BindingIdentifier(id) => {
var_decl.declarations.iter().for_each(|decl| {
decl.id.binding_identifiers().into_iter().for_each(|id| {
self.result.named_exports.insert(
id.name.clone(),
LocalExport { referenced: (self.idx, id.expect_symbol_id()).into() }.into(),
);
}
_ => unimplemented!(),
});
});
}
oxc::ast::ast::Declaration::FunctionDeclaration(fn_decl) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import assert from 'node:assert'
import * as main from './dist/main.mjs'

assert.equal(main.a, 'a1')
assert.equal(main.b, 'b1')
assert.equal(main.c, 'c1')
assert.equal(main.d, 'd1')
assert.equal(main.e, 'e1')
assert.equal(main.a2, 'a2')
assert.equal(main.b2, 'b2')
assert.equal(main.c2, 'c2')
assert.equal(main.d2, 'd2')
assert.equal(main.e2, 'e2')
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
source: crates/rolldown/tests/common/case.rs
expression: content
input_file: crates/rolldown/tests/fixtures/deconflict/decl_complex_patterns
---
# Assets

## main.mjs

```js
import { default as assert_default } from "assert";
// names.js
const [a] = ['a2']
const { b } = { b: 'b2' }
const { b: c } = { b: 'c2' }
const { d = '' } = { d: 'd2' }
const { d: e = '' } = { d: 'e2' }
// main.js
const [a$1] = ['a1']
const { b: b$1 } = { b: 'b1' }
const { b: c$1 } = { b: 'c1' }
const { d: d$1 = '' } = { d: 'd1' }
const { d: e$1 = '' } = { d: 'e1' }
assert_default.equal(a$1, 'a1')
assert_default.equal(a, 'a2')
assert_default.equal(b$1, 'b1')
assert_default.equal(b, 'b2')
assert_default.equal(c$1, 'c1')
assert_default.equal(c, 'c2')
assert_default.equal(d$1, 'd1')
assert_default.equal(d, 'd2')
assert_default.equal(e$1, 'e1')
assert_default.equal(e, 'e2')
export { a$1 as a, a as a2, b$1 as b, b as b2, c$1 as c, c as c2, d$1 as d, d as d2, e$1 as e, e as e2 };
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import assert from 'assert'
import { a as a2, b as b2, c as c2, d as d2, e as e2 } from './names.js'
export const [a] = ['a1']
export const { b } = { b: 'b1' }
export const { b: c } = { b: 'c1' }
export const { d = '' } = { d: 'd1' }
export const { d: e = '' } = { d: 'e1' }
export { a2, b2, c2, d2, e2 }

assert.equal(a, 'a1')
assert.equal(a2, 'a2')
assert.equal(b, 'b1')
assert.equal(b2, 'b2')
assert.equal(c, 'c1')
assert.equal(c2, 'c2')
assert.equal(d, 'd1')
assert.equal(d2, 'd2')
assert.equal(e, 'e1')
assert.equal(e2, 'e2')

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const [a] = ['a2']
export const { b } = { b: 'b2' }
export const { b: c } = { b: 'c2' }
export const { d = '' } = { d: 'd2' }
export const { d: e = '' } = { d: 'e2' }

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
1 change: 1 addition & 0 deletions crates/rolldown_oxc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ repository.workspace = true

[dependencies]
oxc = { workspace = true }
smallvec = { workspace = true }
rolldown_utils = { path = "../rolldown_utils" }
31 changes: 31 additions & 0 deletions crates/rolldown_oxc/src/ext/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use oxc::allocator::Box;
use oxc::ast::ast;
use oxc::semantic::SymbolId;
use smallvec::SmallVec;
pub trait BindingIdentifierExt {
fn expect_symbol_id(&self) -> SymbolId;
}
Expand All @@ -10,3 +12,32 @@ impl BindingIdentifierExt for ast::BindingIdentifier {
self.symbol_id.get().unwrap_or_else(|| panic!("fail get symbol id from {self:?}"))
}
}

pub trait BindingPatternExt {
fn binding_identifiers(&self) -> smallvec::SmallVec<[&Box<ast::BindingIdentifier>; 1]>;
}

impl BindingPatternExt for ast::BindingPattern<'_> {
fn binding_identifiers(&self) -> smallvec::SmallVec<[&Box<ast::BindingIdentifier>; 1]> {
let mut queue = vec![&self.kind];
let mut ret = SmallVec::default();
while let Some(binding_kind) = queue.pop() {
match binding_kind {
ast::BindingPatternKind::BindingIdentifier(id) => {
ret.push(id);
}
ast::BindingPatternKind::ArrayPattern(arr_pat) => {
queue.extend(arr_pat.elements.iter().flatten().map(|pat| &pat.kind).rev());
}
ast::BindingPatternKind::ObjectPattern(obj_pat) => {
queue.extend(obj_pat.properties.iter().map(|prop| &prop.value.kind).rev());
}
//
ast::BindingPatternKind::AssignmentPattern(assign_pat) => {
queue.push(&assign_pat.left.kind);
}
};
}
ret
}
}
2 changes: 1 addition & 1 deletion crates/rolldown_oxc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ mod compiler;
mod ext;

pub use compiler::{OxcCompiler, OxcProgram};
pub use ext::BindingIdentifierExt;
pub use ext::{BindingIdentifierExt, BindingPatternExt};

0 comments on commit e267018

Please sign in to comment.