Skip to content

Commit 850230b

Browse files
authored
fix(es/minifier): preserve non-canonical arguments access (#12052)
**Description:** Computed `arguments` properties are currently parsed or cast directly as numeric indices. Non-canonical names and invalid indices can therefore be mistaken for parameter positions and replaced incorrectly. This PR resolves `arguments` slots through the shared canonical-index parser and ECMAScript number-to-string conversion before substituting parameters. An access is optimized only when its property denotes the same canonical argument index. **Related issue (if exists):** - Closes: #12066 --- - #12043 - #12047 - #12050 - #12048 - #12051 - **#12052** (current) - #12053 - #12054 - #12055 - #12056 - #12057 - #12058 - #12049 - #12059 - #12060
1 parent 3ce9e16 commit 850230b

6 files changed

Lines changed: 74 additions & 59 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
swc_core: patch
3+
swc_ecma_minifier: patch
4+
---
5+
6+
fix(es/minifier): Preserve non-canonical `arguments` property accesses.

crates/swc_ecma_minifier/src/compress/optimize/arguments.rs

Lines changed: 42 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ use std::iter::repeat_with;
22

33
use swc_common::{util::take::Take, DUMMY_SP};
44
use swc_ecma_ast::*;
5-
use swc_ecma_utils::{find_pat_ids, is_valid_prop_ident, private_ident};
5+
use swc_ecma_utils::{
6+
find_pat_ids, is_valid_prop_ident,
7+
number::{parse_canonical_index, ToJsString},
8+
private_ident,
9+
};
610
use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith};
711

812
use super::Optimizer;
@@ -179,64 +183,17 @@ impl VisitMut for ArgReplacer<'_> {
179183

180184
n.visit_mut_children_with(self);
181185

182-
if let Expr::Member(MemberExpr {
183-
obj,
184-
prop: MemberProp::Computed(c),
185-
..
186-
}) = n
187-
{
188-
match &**obj {
189-
Expr::Ident(Ident { sym, .. }) if &**sym == "arguments" => {
190-
match &*c.expr {
191-
Expr::Lit(Lit::Str(Str { value, .. })) => {
192-
let Some(value) = value.as_str() else {
193-
return;
194-
};
195-
let idx = value.parse::<usize>();
196-
let idx = match idx {
197-
Ok(v) => v,
198-
_ => return,
199-
};
200-
201-
self.inject_params_if_required(idx);
202-
203-
if let Some(param) = self.params.get(idx) {
204-
if let Pat::Ident(i) = &param.pat {
205-
self.changed = true;
206-
report_change!(
207-
"arguments: Replacing access to arguments to normal \
208-
reference"
209-
);
210-
*n = i.id.clone().into();
211-
}
212-
}
213-
}
214-
Expr::Lit(Lit::Num(Number { value, .. })) => {
215-
if value.fract() != 0.0 {
216-
// We ignores non-integer values.
217-
return;
218-
}
219-
220-
let idx = value.round() as i64 as usize;
221-
222-
self.inject_params_if_required(idx);
223-
224-
//
225-
if let Some(param) = self.params.get(idx) {
226-
if let Pat::Ident(i) = &param.pat {
227-
report_change!(
228-
"arguments: Replacing access to arguments to normal \
229-
reference"
230-
);
231-
self.changed = true;
232-
*n = i.id.clone().into();
233-
}
234-
}
235-
}
236-
_ => {}
237-
}
238-
}
239-
_ => (),
186+
let Some(idx) = argument_access_index(n) else {
187+
return;
188+
};
189+
190+
self.inject_params_if_required(idx);
191+
192+
if let Some(param) = self.params.get(idx) {
193+
if let Pat::Ident(i) = &param.pat {
194+
self.changed = true;
195+
report_change!("arguments: Replacing access to arguments to normal reference");
196+
*n = i.id.clone().into();
240197
}
241198
}
242199
}
@@ -266,3 +223,29 @@ impl VisitMut for ArgReplacer<'_> {
266223
}
267224
}
268225
}
226+
227+
/// Returns an `arguments` index only when the property key is already in its
228+
/// canonical non-negative integer spelling.
229+
fn argument_access_index(expr: &Expr) -> Option<usize> {
230+
let Expr::Member(MemberExpr {
231+
obj,
232+
prop: MemberProp::Computed(computed),
233+
..
234+
}) = expr
235+
else {
236+
return None;
237+
};
238+
let Expr::Ident(Ident { sym, .. }) = &**obj else {
239+
return None;
240+
};
241+
242+
if &**sym != "arguments" {
243+
return None;
244+
}
245+
246+
match &*computed.expr {
247+
Expr::Lit(Lit::Str(Str { value, .. })) => parse_canonical_index(value.as_str()?),
248+
Expr::Lit(Lit::Num(Number { value, .. })) => parse_canonical_index(&value.to_js_string()),
249+
_ => None,
250+
}
251+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"arguments": true,
3+
"defaults": true,
4+
"passes": 2
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
undefined,undefined,undefined,undefined,zero,one
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
(function (zero, one) {
2+
console.log([
3+
arguments["01"],
4+
arguments[-1],
5+
arguments[1.5],
6+
arguments[1e21],
7+
arguments[-0],
8+
arguments[1],
9+
].map(String).join(","));
10+
})("zero", "one");
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
!function(zero, one) {
2+
console.log([
3+
arguments["01"],
4+
arguments[-1],
5+
arguments[1.5],
6+
arguments[1e21],
7+
zero,
8+
one
9+
].map(String).join(","));
10+
}("zero", "one");

0 commit comments

Comments
 (0)