Skip to content

Commit d53f57a

Browse files
oxc-botBoshen
oxc-bot
authored and
Boshen
committed
feat(minifier): remove unused assignment expression
1 parent ce5d074 commit d53f57a

File tree

11 files changed

+268
-173
lines changed

11 files changed

+268
-173
lines changed

‎crates/oxc_ast/src/ast_impl/js.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -983,6 +983,21 @@ impl<'a> AssignmentTargetMaybeDefault<'a> {
983983
_ => None,
984984
}
985985
}
986+
987+
/// Returns mut identifier bound by this assignment target.
988+
pub fn identifier_mut(&mut self) -> Option<&mut IdentifierReference<'a>> {
989+
match self {
990+
AssignmentTargetMaybeDefault::AssignmentTargetIdentifier(id) => Some(id),
991+
Self::AssignmentTargetWithDefault(target) => {
992+
if let AssignmentTarget::AssignmentTargetIdentifier(id) = &mut target.binding {
993+
Some(id)
994+
} else {
995+
None
996+
}
997+
}
998+
_ => None,
999+
}
1000+
}
9861001
}
9871002

9881003
impl Statement<'_> {

‎crates/oxc_minifier/src/ctx.rs‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ impl<'a> oxc_ecmascript::is_global_reference::IsGlobalReference<'a> for Ctx<'a,
5050
self.scoping()
5151
.get_reference(reference_id)
5252
.symbol_id()
53-
.and_then(|symbol_id| self.state.symbol_values.get_constant_value(symbol_id))
53+
.and_then(|symbol_id| self.state.symbol_values.get_symbol_value(symbol_id))
54+
.filter(|sv| sv.write_references_count == 0)
55+
.and_then(|sv| sv.initialized_constant.as_ref())
5456
.cloned()
5557
}
5658
}
@@ -164,7 +166,23 @@ impl<'a> Ctx<'a, '_> {
164166
false
165167
}
166168

167-
pub fn init_value(&mut self, symbol_id: SymbolId, constant: ConstantValue<'a>) {
169+
pub fn init_value(&mut self, symbol_id: SymbolId, constant: Option<ConstantValue<'a>>) {
170+
let mut exported = false;
171+
if self.scoping.current_scope_id() == self.scoping().root_scope_id() {
172+
for ancestor in self.ancestors() {
173+
if ancestor.is_export_named_declaration()
174+
|| ancestor.is_export_all_declaration()
175+
|| ancestor.is_export_default_declaration()
176+
{
177+
exported = true;
178+
}
179+
}
180+
}
181+
182+
let for_statement_init = self.ancestors().nth(1).is_some_and(|ancestor| {
183+
ancestor.is_parent_of_for_statement_init() || ancestor.is_parent_of_for_statement_left()
184+
});
185+
168186
let mut read_references_count = 0;
169187
let mut write_references_count = 0;
170188
for r in self.scoping().get_resolved_references(symbol_id) {
@@ -177,8 +195,14 @@ impl<'a> Ctx<'a, '_> {
177195
}
178196

179197
let scope_id = self.scoping.current_scope_id();
180-
let symbol_value =
181-
SymbolValue { constant, read_references_count, write_references_count, scope_id };
198+
let symbol_value = SymbolValue {
199+
initialized_constant: constant,
200+
exported,
201+
for_statement_init,
202+
read_references_count,
203+
write_references_count,
204+
scope_id,
205+
};
182206
self.state.symbol_values.init_value(symbol_id, symbol_value);
183207
}
184208

‎crates/oxc_minifier/src/peephole/inline.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use oxc_ast::ast::*;
2-
use oxc_ecmascript::constant_evaluation::ConstantEvaluation;
2+
use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, ConstantValue};
33
use oxc_span::GetSpan;
44

55
use crate::ctx::Ctx;
@@ -10,11 +10,12 @@ impl<'a> PeepholeOptimizations {
1010
pub fn init_symbol_value(&self, decl: &VariableDeclarator<'a>, ctx: &mut Ctx<'a, '_>) {
1111
let BindingPatternKind::BindingIdentifier(ident) = &decl.id.kind else { return };
1212
let Some(symbol_id) = ident.symbol_id.get() else { return };
13-
// Skip if not `const` variable.
14-
if !ctx.scoping().symbol_flags(symbol_id).is_const_variable() {
13+
// Skip for `var` declarations, due to TDZ problems.
14+
if decl.kind.is_var() {
1515
return;
1616
}
17-
let Some(value) = decl.init.evaluate_value(ctx) else { return };
17+
let value =
18+
decl.init.as_ref().map_or(Some(ConstantValue::Undefined), |e| e.evaluate_value(ctx));
1819
ctx.init_value(symbol_id, value);
1920
}
2021

@@ -33,7 +34,11 @@ impl<'a> PeepholeOptimizations {
3334
if symbol_value.write_references_count > 0 {
3435
return;
3536
}
36-
*expr = ctx.value_to_expr(expr.span(), symbol_value.constant.clone());
37+
if symbol_value.for_statement_init {
38+
return;
39+
}
40+
let Some(cv) = &symbol_value.initialized_constant else { return };
41+
*expr = ctx.value_to_expr(expr.span(), cv.clone());
3742
ctx.state.changed = true;
3843
}
3944
}

‎crates/oxc_minifier/src/peephole/minimize_conditions.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,9 @@ mod test {
526526
// In the following test case, we can't remove the duplicate "alert(x);" lines since each "x"
527527
// refers to a different variable.
528528
// We only try removing duplicate statements if the AST is normalized and names are unique.
529-
test_same(
529+
test(
530530
"if (Math.random() < 0.5) { let x = 3; alert(x); } else { let x = 5; alert(x); }",
531+
"if (Math.random() < 0.5) { let x = 3; alert(3); } else { let x = 5; alert(5); }",
531532
);
532533
}
533534

‎crates/oxc_minifier/src/peephole/normalize.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,8 @@ mod test {
454454
#[test]
455455
fn fold_number_nan() {
456456
test("foo(Number.NaN)", "foo(NaN)");
457-
test_same("let Number; foo(Number.NaN)");
457+
test_same("var Number; foo(Number.NaN)");
458+
test_same("let Number; foo((void 0).NaN)");
458459
}
459460

460461
#[test]

‎crates/oxc_minifier/src/peephole/remove_unused_variable_declaration.rs‎

Lines changed: 69 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use oxc_allocator::TakeIn;
12
use oxc_ast::ast::*;
23

34
use crate::{CompressOptionsUnused, ctx::Ctx};
@@ -88,37 +89,47 @@ impl<'a> PeepholeOptimizations {
8889

8990
pub fn remove_unused_assignment_expression(
9091
&self,
91-
_e: &mut Expression<'a>,
92-
_ctx: &mut Ctx<'a, '_>,
92+
e: &mut Expression<'a>,
93+
ctx: &mut Ctx<'a, '_>,
9394
) -> bool {
94-
// let Expression::AssignmentExpression(assign_expr) = e else { return false };
95-
// if matches!(
96-
// ctx.state.options.unused,
97-
// CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign
98-
// ) {
99-
// return false;
100-
// }
101-
// let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) =
102-
// assign_expr.left.as_simple_assignment_target()
103-
// else {
104-
// return false;
105-
// };
106-
// if Self::keep_top_level_var_in_script_mode(ctx) {
107-
// return false;
108-
// }
109-
// let Some(reference_id) = ident.reference_id.get() else { return false };
110-
// let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else {
111-
// return false;
112-
// };
113-
// // Keep error for assigning to `const foo = 1; foo = 2`.
114-
// if ctx.scoping().symbol_flags(symbol_id).is_const_variable() {
115-
// return false;
116-
// }
117-
// if !ctx.scoping().get_resolved_references(symbol_id).all(|r| !r.flags().is_read()) {
118-
// return false;
119-
// }
120-
// *e = assign_expr.right.take_in(ctx.ast);
121-
// state.changed = true;
95+
let Expression::AssignmentExpression(assign_expr) = e else { return false };
96+
if matches!(
97+
ctx.state.options.unused,
98+
CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign
99+
) {
100+
return false;
101+
}
102+
let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) =
103+
assign_expr.left.as_simple_assignment_target()
104+
else {
105+
return false;
106+
};
107+
if Self::keep_top_level_var_in_script_mode(ctx) {
108+
return false;
109+
}
110+
let Some(reference_id) = ident.reference_id.get() else { return false };
111+
let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else {
112+
return false;
113+
};
114+
// Keep error for assigning to `const foo = 1; foo = 2`.
115+
if ctx.scoping().symbol_flags(symbol_id).is_const_variable() {
116+
return false;
117+
}
118+
let Some(symbol_value) = ctx.state.symbol_values.get_symbol_value(symbol_id) else {
119+
return false;
120+
};
121+
// Cannot remove assignment to live bindings: `export let foo; foo = 1;`.
122+
if symbol_value.exported {
123+
return false;
124+
}
125+
if symbol_value.read_references_count > 0 {
126+
return false;
127+
}
128+
if symbol_value.for_statement_init {
129+
return false;
130+
}
131+
*e = assign_expr.right.take_in(ctx.ast);
132+
ctx.state.changed = true;
122133
false
123134
}
124135

@@ -180,33 +191,48 @@ mod test {
180191
}
181192

182193
#[test]
183-
#[ignore]
184194
fn remove_unused_assignment_expression() {
185195
let options = CompressOptions::smallest();
186-
test_options("var x = 1; x = 2;", "", &options);
187-
test_options("var x = 1; x = 2;", "", &options);
188-
test_options("var x = 1; x = foo();", "foo()", &options);
189-
test_same_options("export let foo; foo = 0;", &options);
196+
// Vars are not handled yet due to TDZ.
197+
test_same_options("var x = 1; x = 2;", &options);
198+
test_same_options("var x = 1; x = foo();", &options);
199+
test_same_options("export var foo; foo = 0;", &options);
190200
test_same_options("var x = 1; x = 2, foo(x)", &options);
191201
test_same_options("function foo() { return t = x(); } foo();", &options);
202+
test_same_options("function foo() { var t; return t = x(); } foo();", &options);
203+
test_same_options("function foo(t) { return t = x(); } foo();", &options);
204+
205+
test_options("let x = 1; x = 2;", "", &options);
206+
test_options("let x = 1; x = foo();", "foo()", &options);
207+
test_same_options("export let foo; foo = 0;", &options);
208+
test_same_options("let x = 1; x = 2, foo(x)", &options);
209+
test_same_options("function foo() { return t = x(); } foo();", &options);
192210
test_options(
193-
"function foo() { var t; return t = x(); } foo();",
194-
"function foo() { return x(); } foo();",
211+
"function foo() { let t; return t = x(); } foo();",
212+
"function foo() { return x() } foo()",
195213
&options,
196214
);
197-
test_options(
198-
"function foo(t) { return t = x(); } foo();",
199-
"function foo(t) { return x(); } foo();",
215+
test_same_options("function foo(t) { return t = x(); } foo();", &options);
216+
217+
test_same_options("for(let i;;) foo(i)", &options);
218+
test_same_options("for(let i in []) foo(i)", &options);
219+
220+
test_options("var a; ({ a: a } = {})", "var a; ({ a } = {})", &options);
221+
test_options("var a; b = ({ a: a })", "var a; b = ({ a })", &options);
222+
223+
test_options("let foo = {}; foo = 1", "", &options);
224+
225+
test_same_options(
226+
"let bracketed = !1; for(;;) bracketed = !bracketed, log(bracketed)",
200227
&options,
201228
);
202229

203230
let options = CompressOptions::smallest();
204231
let source_type = SourceType::cjs();
205232
test_same_options_source_type("var x = 1; x = 2;", source_type, &options);
206233
test_same_options_source_type("var x = 1; x = 2, foo(x)", source_type, &options);
207-
test_options_source_type(
208-
"function foo() { var x = 1; x = 2; bar() } foo()",
209-
"function foo() { bar() } foo()",
234+
test_same_options_source_type(
235+
"function foo() { var x = 1; x = 2, bar() } foo()",
210236
source_type,
211237
&options,
212238
);

0 commit comments

Comments
 (0)