Skip to content

Commit c557854

Browse files
sapphi-redsapphi-red
committed
feat(ecmascript): support MayHaveSideEffects for Statements (#13402)
Added MayHaveSideEffects for Statements.
1 parent 3e902a0 commit c557854

File tree

5 files changed

+344
-24
lines changed

5 files changed

+344
-24
lines changed

‎crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs‎ renamed to ‎crates/oxc_ecmascript/src/side_effects/expressions.rs‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,7 @@ use crate::{
55
to_primitive::ToPrimitive,
66
};
77

8-
use super::{PropertyReadSideEffects, context::MayHaveSideEffectsContext};
9-
10-
/// Returns true if subtree changes application state.
11-
///
12-
/// This trait assumes the following:
13-
/// - `.toString()`, `.valueOf()`, and `[Symbol.toPrimitive]()` are side-effect free.
14-
/// - This is mainly to assume `ToPrimitive` is side-effect free.
15-
/// - Note that the builtin `Array::toString` has a side-effect when a value contains a Symbol as `ToString(Symbol)` throws an error. Maybe we should revisit this assumption and remove it.
16-
/// - For example, `"" == [Symbol()]` returns an error, but this trait returns `false`.
17-
/// - Errors thrown when creating a String or an Array that exceeds the maximum length does not happen.
18-
/// - TDZ errors does not happen.
19-
///
20-
/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94)
21-
pub trait MayHaveSideEffects<'a> {
22-
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool;
23-
}
24-
25-
impl<'a, T: MayHaveSideEffects<'a>> MayHaveSideEffects<'a> for Option<T> {
26-
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
27-
self.as_ref().is_some_and(|t| t.may_have_side_effects(ctx))
28-
}
29-
}
8+
use super::{MayHaveSideEffects, PropertyReadSideEffects, context::MayHaveSideEffectsContext};
309

3110
impl<'a> MayHaveSideEffects<'a> for Expression<'a> {
3211
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {

‎crates/oxc_ecmascript/src/side_effects/mod.rs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
mod context;
2-
mod may_have_side_effects;
2+
mod expressions;
3+
mod statements;
34

45
pub use context::{MayHaveSideEffectsContext, PropertyReadSideEffects};
5-
pub use may_have_side_effects::MayHaveSideEffects;
6+
7+
/// Returns true if subtree changes application state.
8+
///
9+
/// This trait assumes the following:
10+
/// - `.toString()`, `.valueOf()`, and `[Symbol.toPrimitive]()` are side-effect free.
11+
/// - This is mainly to assume `ToPrimitive` is side-effect free.
12+
/// - Note that the builtin `Array::toString` has a side-effect when a value contains a Symbol as `ToString(Symbol)` throws an error. Maybe we should revisit this assumption and remove it.
13+
/// - For example, `"" == [Symbol()]` returns an error, but this trait returns `false`.
14+
/// - Errors thrown when creating a String or an Array that exceeds the maximum length does not happen.
15+
/// - TDZ errors does not happen.
16+
///
17+
/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94)
18+
pub trait MayHaveSideEffects<'a> {
19+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool;
20+
}
21+
22+
impl<'a, T: MayHaveSideEffects<'a>> MayHaveSideEffects<'a> for Option<T> {
23+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
24+
self.as_ref().is_some_and(|t| t.may_have_side_effects(ctx))
25+
}
26+
}

‎crates/oxc_ecmascript/src/side_effects/statements.rs‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
use oxc_ast::ast::*;
2+
3+
use crate::constant_evaluation::{DetermineValueType, ValueType};
4+
5+
use super::{MayHaveSideEffects, PropertyReadSideEffects, context::MayHaveSideEffectsContext};
6+
7+
impl<'a> MayHaveSideEffects<'a> for Statement<'a> {
8+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
9+
match self {
10+
Statement::BlockStatement(block) => block.may_have_side_effects(ctx),
11+
Statement::DoWhileStatement(do_while) => do_while.may_have_side_effects(ctx),
12+
Statement::ExpressionStatement(expr) => expr.expression.may_have_side_effects(ctx),
13+
Statement::IfStatement(if_stmt) => if_stmt.may_have_side_effects(ctx),
14+
Statement::LabeledStatement(labeled) => labeled.body.may_have_side_effects(ctx),
15+
Statement::ReturnStatement(return_stmt) => {
16+
return_stmt.argument.may_have_side_effects(ctx)
17+
}
18+
Statement::SwitchStatement(switch) => switch.may_have_side_effects(ctx),
19+
Statement::TryStatement(try_stmt) => try_stmt.may_have_side_effects(ctx),
20+
Statement::WhileStatement(while_stmt) => while_stmt.may_have_side_effects(ctx),
21+
Statement::BreakStatement(_)
22+
| Statement::ContinueStatement(_)
23+
| Statement::EmptyStatement(_) => false,
24+
match_declaration!(Statement) => self.to_declaration().may_have_side_effects(ctx),
25+
Statement::ForInStatement(_)
26+
| Statement::ForOfStatement(_)
27+
| Statement::ForStatement(_)
28+
| Statement::ThrowStatement(_)
29+
| Statement::WithStatement(_)
30+
| Statement::DebuggerStatement(_) => true,
31+
#[expect(clippy::match_same_arms)]
32+
match_module_declaration!(Statement) => true,
33+
}
34+
}
35+
}
36+
37+
impl<'a> MayHaveSideEffects<'a> for BlockStatement<'a> {
38+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
39+
self.body.iter().any(|stmt| stmt.may_have_side_effects(ctx))
40+
}
41+
}
42+
43+
impl<'a> MayHaveSideEffects<'a> for DoWhileStatement<'a> {
44+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
45+
self.test.may_have_side_effects(ctx) || self.body.may_have_side_effects(ctx)
46+
}
47+
}
48+
49+
impl<'a> MayHaveSideEffects<'a> for IfStatement<'a> {
50+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
51+
self.test.may_have_side_effects(ctx)
52+
|| self.consequent.may_have_side_effects(ctx)
53+
|| self.alternate.may_have_side_effects(ctx)
54+
}
55+
}
56+
57+
impl<'a> MayHaveSideEffects<'a> for SwitchStatement<'a> {
58+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
59+
self.discriminant.may_have_side_effects(ctx)
60+
|| self.cases.iter().any(|case| {
61+
case.test.may_have_side_effects(ctx)
62+
|| case.consequent.iter().any(|stmt| stmt.may_have_side_effects(ctx))
63+
})
64+
}
65+
}
66+
67+
impl<'a> MayHaveSideEffects<'a> for TryStatement<'a> {
68+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
69+
self.block.may_have_side_effects(ctx)
70+
|| self.handler.as_ref().is_some_and(|catch_clause| {
71+
catch_clause
72+
.param
73+
.as_ref()
74+
.is_some_and(|param| param.pattern.may_have_side_effects(ctx))
75+
|| catch_clause.body.may_have_side_effects(ctx)
76+
})
77+
|| self.finalizer.as_ref().is_some_and(|finalizer| finalizer.may_have_side_effects(ctx))
78+
}
79+
}
80+
81+
impl<'a> MayHaveSideEffects<'a> for WhileStatement<'a> {
82+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
83+
self.test.may_have_side_effects(ctx) || self.body.may_have_side_effects(ctx)
84+
}
85+
}
86+
87+
impl<'a> MayHaveSideEffects<'a> for Declaration<'a> {
88+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
89+
match self {
90+
Declaration::VariableDeclaration(var_decl) => var_decl.may_have_side_effects(ctx),
91+
Declaration::FunctionDeclaration(_) => false,
92+
Declaration::ClassDeclaration(class_decl) => class_decl.may_have_side_effects(ctx),
93+
Declaration::TSEnumDeclaration(_)
94+
| Declaration::TSImportEqualsDeclaration(_)
95+
| Declaration::TSModuleDeclaration(_)
96+
| Declaration::TSInterfaceDeclaration(_)
97+
| Declaration::TSTypeAliasDeclaration(_) => unreachable!(),
98+
}
99+
}
100+
}
101+
102+
impl<'a> MayHaveSideEffects<'a> for VariableDeclaration<'a> {
103+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
104+
if self.kind == VariableDeclarationKind::AwaitUsing {
105+
return true;
106+
}
107+
if self.kind == VariableDeclarationKind::Using {
108+
return self.declarations.iter().any(|decl| {
109+
decl.init.as_ref().is_none_or(|init| {
110+
!matches!(init.value_type(ctx), ValueType::Undefined | ValueType::Null)
111+
|| init.may_have_side_effects(ctx)
112+
})
113+
});
114+
}
115+
self.declarations
116+
.iter()
117+
.any(|decl| decl.id.may_have_side_effects(ctx) || decl.init.may_have_side_effects(ctx))
118+
}
119+
}
120+
121+
impl<'a> MayHaveSideEffects<'a> for BindingPattern<'a> {
122+
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
123+
match &self.kind {
124+
BindingPatternKind::ArrayPattern(array_pattern) => {
125+
ctx.property_read_side_effects() != PropertyReadSideEffects::None
126+
|| array_pattern.elements.iter().any(|el| el.may_have_side_effects(ctx))
127+
}
128+
BindingPatternKind::ObjectPattern(object_pattern) => {
129+
ctx.property_read_side_effects() != PropertyReadSideEffects::None
130+
|| object_pattern.properties.iter().any(|prop| {
131+
prop.key.may_have_side_effects(ctx) || prop.value.may_have_side_effects(ctx)
132+
})
133+
}
134+
BindingPatternKind::AssignmentPattern(assignment_pattern) => {
135+
assignment_pattern.left.may_have_side_effects(ctx)
136+
|| assignment_pattern.right.may_have_side_effects(ctx)
137+
}
138+
BindingPatternKind::BindingIdentifier(_) => false,
139+
}
140+
}
141+
}

‎crates/oxc_minifier/tests/ecmascript/may_have_side_effects_statements.rs‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
use javascript_globals::GLOBALS;
2+
use oxc_allocator::Allocator;
3+
use oxc_ast::ast::{Expression, IdentifierReference, Statement};
4+
use oxc_ecmascript::{
5+
GlobalContext,
6+
side_effects::{MayHaveSideEffects, MayHaveSideEffectsContext},
7+
};
8+
use oxc_minifier::PropertyReadSideEffects;
9+
use oxc_parser::Parser;
10+
use oxc_span::SourceType;
11+
use rustc_hash::FxHashSet;
12+
13+
struct Ctx {
14+
global_variable_names: FxHashSet<&'static str>,
15+
}
16+
17+
impl Default for Ctx {
18+
fn default() -> Self {
19+
Self {
20+
global_variable_names: GLOBALS["builtin"]
21+
.keys()
22+
.copied()
23+
.chain(["arguments", "URL"])
24+
.collect::<FxHashSet<_>>(),
25+
}
26+
}
27+
}
28+
29+
impl<'a> GlobalContext<'a> for Ctx {
30+
fn is_global_reference(&self, ident: &IdentifierReference<'a>) -> bool {
31+
self.global_variable_names.contains(ident.name.as_str())
32+
}
33+
}
34+
35+
impl MayHaveSideEffectsContext<'_> for Ctx {
36+
fn annotations(&self) -> bool {
37+
true
38+
}
39+
40+
fn manual_pure_functions(&self, _callee: &Expression) -> bool {
41+
false
42+
}
43+
44+
fn property_read_side_effects(&self) -> PropertyReadSideEffects {
45+
PropertyReadSideEffects::All
46+
}
47+
48+
fn unknown_global_side_effects(&self) -> bool {
49+
true
50+
}
51+
}
52+
53+
#[track_caller]
54+
fn test(source_text: &str, expected: bool) {
55+
let allocator = Allocator::default();
56+
let ret = Parser::new(&allocator, source_text, SourceType::mjs()).parse();
57+
assert!(!ret.panicked, "{source_text}");
58+
assert!(ret.errors.is_empty(), "{source_text}");
59+
60+
let stmt = ret.program.body.first().unwrap();
61+
assert_eq!(stmt.may_have_side_effects(&Ctx::default()), expected, "{source_text}");
62+
}
63+
64+
#[track_caller]
65+
fn test_in_function(source_text: &str, expected: bool) {
66+
let allocator = Allocator::default();
67+
let ret = Parser::new(&allocator, source_text, SourceType::mjs()).parse();
68+
assert!(!ret.panicked, "{source_text}");
69+
assert!(ret.errors.is_empty(), "{source_text}");
70+
71+
let Some(Statement::FunctionDeclaration(stmt)) = &ret.program.body.first() else {
72+
panic!("should have a function declaration: {source_text}");
73+
};
74+
let stmt = stmt.body.as_ref().expect("should have a body").statements.first().unwrap();
75+
assert_eq!(stmt.may_have_side_effects(&Ctx::default()), expected, "{source_text}");
76+
}
77+
78+
#[test]
79+
fn test_block() {
80+
test("{}", false);
81+
test("{ ; ; }", false);
82+
test("{ foo() }", true);
83+
test("{ ; ; foo() }", true);
84+
}
85+
86+
#[test]
87+
fn test_do_while() {
88+
test("do { foo() } while (true)", true);
89+
test("do {} while (foo())", true);
90+
test("do {} while (true)", false);
91+
}
92+
93+
#[test]
94+
fn test_expr() {
95+
test("1", false);
96+
test("foo()", true);
97+
}
98+
99+
#[test]
100+
fn test_if() {
101+
test("if (foo()) {}", true);
102+
test("if (true) { foo() }", true);
103+
test("if (true) {}", false);
104+
}
105+
106+
#[test]
107+
fn test_labeled() {
108+
test("label: foo()", true);
109+
test("label: 1", false);
110+
}
111+
112+
#[test]
113+
fn test_return() {
114+
test_in_function("function _() { return foo() }", true);
115+
test_in_function("function _() { return 1 }", false);
116+
}
117+
118+
#[test]
119+
fn test_switch() {
120+
test("switch (foo()) {}", true);
121+
test("switch (true) { case foo(): }", true);
122+
test("switch (true) { case true: foo() }", true);
123+
test("switch (true) { case true: true }", false);
124+
}
125+
126+
#[test]
127+
fn test_try() {
128+
test("try { foo() } catch {}", true);
129+
test("try { true } catch ({}) {}", true);
130+
test("try { true } catch { foo() }", true);
131+
test("try { true } finally { foo() }", true);
132+
test("try { true } catch (e) { true } finally { true }", false);
133+
}
134+
135+
#[test]
136+
fn test_while() {
137+
test("while (true) { foo() }", true);
138+
test("while (foo()) {}", true);
139+
test("while (true) {}", false);
140+
}
141+
142+
#[test]
143+
fn test_declarations() {
144+
test("await using a = null", true);
145+
test("await using a = true", true);
146+
test("using a = null", false);
147+
test("using a = void 0", false);
148+
test("using a = null, b = 1", true);
149+
test("using a = void foo()", true);
150+
test("var a = foo()", true);
151+
test("var a = true", false);
152+
test("let a = foo()", true);
153+
test("let a = true", false);
154+
test("const a = foo()", true);
155+
test("const a = true", false);
156+
157+
test("var [a] = []", true);
158+
test("var [a = foo()] = []", true);
159+
test("var [[a] = [foo()]] = []", true);
160+
test("var [a] = foo", true);
161+
test("var {a} = {}", true);
162+
test("var {a = foo()} = {}", true);
163+
test("var {a} = foo", true);
164+
}
165+
166+
#[test]
167+
fn test_others() {
168+
test("for (var a in b) {}", true);
169+
test("for (var a of b) {}", true);
170+
test("for (;;) {}", true);
171+
test("throw 1", true);
172+
test("with (a) {}", true);
173+
test("debugger", true);
174+
175+
test("import 'a'", true);
176+
test("export * from 'a'", true);
177+
test("export { a }", true);
178+
}

‎crates/oxc_minifier/tests/ecmascript/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ mod array_join;
22
mod is_int32_or_uint32;
33
mod is_literal_value;
44
mod may_have_side_effects;
5+
mod may_have_side_effects_statements;
56
mod prop_name;
67
mod to_boolean;
78
mod to_number;

0 commit comments

Comments
 (0)