feat(ecmascript): complete may_have_side_effects

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

@@ -327,6 +327,10 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
                     if matches!(name, "Object" | "Number" | "Boolean" | "String")
                         && self.is_global_reference(right_ident)
                     {
+                        let left_ty = ValueType::from(left);
+                        if left_ty.is_undetermined() {
+                            return None;
+                        }
                         return Some(ConstantValue::Boolean(
                             name == "Object" && ValueType::from(left).is_object(),
                         ));

crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs

@@ -2,16 +2,23 @@ use oxc_ast::ast::*;
 
 /// Returns true if subtree changes application state.
 ///
+/// This trait assumes the following:
+/// - `.toString()`, `.valueOf()`, and `[Symbol.toPrimitive]()` are side-effect free.
+/// - Errors thrown when creating a String or an Array that exceeds the maximum length does not happen.
+/// - TDZ errors does not happen.
+///
 /// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94)
 pub trait MayHaveSideEffects {
     fn is_global_reference(&self, ident: &IdentifierReference<'_>) -> bool;
 
     fn expression_may_have_side_effects(&self, e: &Expression<'_>) -> bool {
         match e {
-            // Reference read can have a side effect.
             Expression::Identifier(ident) => match ident.name.as_str() {
-                "NaN" | "Infinity" | "undefined" => !self.is_global_reference(ident),
-                _ => true,
+                "NaN" | "Infinity" | "undefined" => false,
+                // Reading global variables may have a side effect.
+                // NOTE: It should also return true when the reference might refer to a reference value created by a with statement
+                // NOTE: we ignore TDZ errors
+                _ => self.is_global_reference(ident),
             },
             Expression::NumericLiteral(_)
             | Expression::BooleanLiteral(_)
@@ -22,11 +29,13 @@ pub trait MayHaveSideEffects {
             | Expression::MetaProperty(_)
             | Expression::ThisExpression(_)
             | Expression::ArrowFunctionExpression(_)
-            | Expression::FunctionExpression(_) => false,
+            | Expression::FunctionExpression(_)
+            | Expression::Super(_) => false,
             Expression::TemplateLiteral(template) => {
                 template.expressions.iter().any(|e| self.expression_may_have_side_effects(e))
             }
             Expression::UnaryExpression(e) => self.unary_expression_may_have_side_effects(e),
+            Expression::LogicalExpression(e) => self.logical_expression_may_have_side_effects(e),
             Expression::ParenthesizedExpression(e) => {
                 self.expression_may_have_side_effects(&e.expression)
             }
@@ -43,10 +52,9 @@ pub trait MayHaveSideEffects {
                 .properties
                 .iter()
                 .any(|property| self.object_property_kind_may_have_side_effects(property)),
-            Expression::ArrayExpression(e) => e
-                .elements
-                .iter()
-                .any(|element| self.array_expression_element_may_have_side_effects(element)),
+            Expression::ArrayExpression(e) => self.array_expression_may_have_side_effects(e),
+            Expression::ClassExpression(e) => self.class_may_have_side_effects(e),
+            // NOTE: private in can throw `TypeError`
             _ => true,
         }
     }
@@ -56,6 +64,9 @@ pub trait MayHaveSideEffects {
         fn is_simple_unary_operator(operator: UnaryOperator) -> bool {
             operator != UnaryOperator::Delete
         }
+        if e.operator == UnaryOperator::Typeof && matches!(&e.argument, Expression::Identifier(_)) {
+            return false;
+        }
         if is_simple_unary_operator(e.operator) {
             return self.expression_may_have_side_effects(&e.argument);
         }
@@ -71,14 +82,32 @@ pub trait MayHaveSideEffects {
             || self.expression_may_have_side_effects(&e.right)
     }
 
+    fn logical_expression_may_have_side_effects(&self, e: &LogicalExpression<'_>) -> bool {
+        self.expression_may_have_side_effects(&e.left)
+            || self.expression_may_have_side_effects(&e.right)
+    }
+
+    fn array_expression_may_have_side_effects(&self, e: &ArrayExpression<'_>) -> bool {
+        e.elements
+            .iter()
+            .any(|element| self.array_expression_element_may_have_side_effects(element))
+    }
+
     fn array_expression_element_may_have_side_effects(
         &self,
         e: &ArrayExpressionElement<'_>,
     ) -> bool {
         match e {
-            ArrayExpressionElement::SpreadElement(e) => {
-                self.expression_may_have_side_effects(&e.argument)
-            }
+            ArrayExpressionElement::SpreadElement(e) => match &e.argument {
+                Expression::ArrayExpression(arr) => {
+                    self.array_expression_may_have_side_effects(arr)
+                }
+                Expression::StringLiteral(_) => false,
+                Expression::TemplateLiteral(t) => {
+                    t.expressions.iter().any(|e| self.expression_may_have_side_effects(e))
+                }
+                _ => true,
+            },
             match_expression!(ArrayExpressionElement) => {
                 self.expression_may_have_side_effects(e.to_expression())
             }
@@ -89,9 +118,16 @@ pub trait MayHaveSideEffects {
     fn object_property_kind_may_have_side_effects(&self, e: &ObjectPropertyKind<'_>) -> bool {
         match e {
             ObjectPropertyKind::ObjectProperty(o) => self.object_property_may_have_side_effects(o),
-            ObjectPropertyKind::SpreadProperty(e) => {
-                self.expression_may_have_side_effects(&e.argument)
-            }
+            ObjectPropertyKind::SpreadProperty(e) => match &e.argument {
+                Expression::ArrayExpression(arr) => {
+                    self.array_expression_may_have_side_effects(arr)
+                }
+                Expression::StringLiteral(_) => false,
+                Expression::TemplateLiteral(t) => {
+                    t.expressions.iter().any(|e| self.expression_may_have_side_effects(e))
+                }
+                _ => true,
+            },
         }
     }
 
@@ -108,4 +144,29 @@ pub trait MayHaveSideEffects {
             }
         }
     }
+
+    fn class_may_have_side_effects(&self, class: &Class<'_>) -> bool {
+        class.body.body.iter().any(|element| self.class_element_may_have_side_effects(element))
+    }
+
+    fn class_element_may_have_side_effects(&self, e: &ClassElement<'_>) -> bool {
+        match e {
+            // TODO: check side effects inside the block
+            ClassElement::StaticBlock(block) => !block.body.is_empty(),
+            ClassElement::MethodDefinition(e) => {
+                e.r#static && self.property_key_may_have_side_effects(&e.key)
+            }
+            ClassElement::PropertyDefinition(e) => {
+                e.r#static
+                    && (self.property_key_may_have_side_effects(&e.key)
+                        || e.value
+                            .as_ref()
+                            .is_some_and(|v| self.expression_may_have_side_effects(v)))
+            }
+            ClassElement::AccessorProperty(e) => {
+                e.r#static && self.property_key_may_have_side_effects(&e.key)
+            }
+            ClassElement::TSIndexSignature(_) => false,
+        }
+    }
 }

crates/oxc_minifier/README.md

@@ -29,6 +29,8 @@ The compressor is responsible for rewriting statements and expressions for minim
   - Examples that breaks this assumption: `(() => { console.log(v); let v; })()`
 - `with` statement is not used
   - Examples that breaks this assumption: `with (Math) { console.log(PI); }`
+- `.toString()`, `.valueOf()`, `[Symbol.toPrimitive]()` are side-effect free
+  - Examples that breaks this assumption: `{ toString() { console.log('sideeffect') } }`
 - Errors thrown when creating a String or an Array that exceeds the maximum length can disappear or moved
   - Examples that breaks this assumption: `try { new Array(Number(2n**53n)) } catch { console.log('log') }`
 

crates/oxc_minifier/src/peephole/minimize_conditional_expression.rs

@@ -609,7 +609,7 @@ mod test {
         test("var x; (x && false) && y()", "var x; x && !1");
         test("(x && true) && y()", "x && y()");
         test("(x && false) && y()", "x && !1");
-        test("var x; (x || true) && y()", "var x; x || !0, y()");
+        test("var x; (x || true) && y()", "var x; y()");
         test("var x; (x || false) && y()", "var x; x && y()");
 
         test("(x || true) && y()", "x || !0, y()");

crates/oxc_minifier/src/peephole/statement_fusion.rs

@@ -113,7 +113,7 @@ mod test {
         // Never fuse a statement into a block that contains let/const/class declarations, or you risk
         // colliding variable names. (unless the AST is normalized).
         test("a; {b;}", "a,b");
-        test("a; {b; var a = 1;}", "a, b; var a = 1;");
+        test("a; {b; var a = 1;}", "b; var a = 1;");
         test_same("a; { b; let a = 1; }");
         test("a; { b; const a = 1; }", "a; { b; let a = 1; }");
         test_same("a; { b; class a {} }");

crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs

@@ -1385,7 +1385,7 @@ mod test {
     #[test]
     fn test_fold_arrow_function_return() {
         test("const foo = () => { return 'baz' }", "const foo = () => 'baz'");
-        test("const foo = () => { foo; return 'baz' }", "const foo = () => (foo, 'baz')");
+        test("const foo = () => { foo.foo; return 'baz' }", "const foo = () => (foo.foo, 'baz')");
     }
 
     #[test]