diff --git a/crates/oxc_ecmascript/src/side_effects/expressions.rs b/crates/oxc_ecmascript/src/side_effects/expressions.rs index 81c6c09ce79bc..c3bf04ca2b29e 100644 --- a/crates/oxc_ecmascript/src/side_effects/expressions.rs +++ b/crates/oxc_ecmascript/src/side_effects/expressions.rs @@ -297,7 +297,10 @@ fn is_pure_call(name: &str) -> bool { fn is_pure_constructor(name: &str) -> bool { matches!(name, "Set" | "Map" | "WeakSet" | "WeakMap" | "ArrayBuffer" | "Date" | "Boolean" | "Error" | "EvalError" | "RangeError" | "ReferenceError" - | "SyntaxError" | "TypeError" | "URIError" | "Number" | "Object" | "String") + | "SyntaxError" | "TypeError" | "URIError" | "Number" | "Object" | "String" + | "Int8Array" | "Uint8Array" | "Uint8ClampedArray" | "Int16Array" | "Uint16Array" + | "Int32Array" | "Uint32Array" | "Float32Array" | "Float64Array" + | "BigInt64Array" | "BigUint64Array") } /// Whether the name matches any known global constructors. @@ -641,12 +644,18 @@ impl<'a> MayHaveSideEffects<'a> for ObjectPropertyKind<'a> { fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool { match self { ObjectPropertyKind::ObjectProperty(o) => o.may_have_side_effects(ctx), - ObjectPropertyKind::SpreadProperty(e) => match &e.argument { - Expression::ArrayExpression(arr) => arr.may_have_side_effects(ctx), - Expression::StringLiteral(_) => false, - Expression::TemplateLiteral(t) => t.may_have_side_effects(ctx), - _ => true, - }, + ObjectPropertyKind::SpreadProperty(e) => { + if ctx.property_read_side_effects() == PropertyReadSideEffects::None { + e.argument.may_have_side_effects(ctx) + } else { + match &e.argument { + Expression::ArrayExpression(arr) => arr.may_have_side_effects(ctx), + Expression::StringLiteral(_) => false, + Expression::TemplateLiteral(t) => t.may_have_side_effects(ctx), + _ => true, + } + } + } } } } @@ -701,7 +710,9 @@ impl<'a> MayHaveSideEffects<'a> for ClassElement<'a> { block.body.iter().any(|stmt| stmt.may_have_side_effects(ctx)) } ClassElement::MethodDefinition(e) => { - !e.decorators.is_empty() || e.key.may_have_side_effects(ctx) + !e.decorators.is_empty() + || e.key.may_have_side_effects(ctx) + || e.value.params.items.iter().any(|item| !item.decorators.is_empty()) } ClassElement::PropertyDefinition(e) => { !e.decorators.is_empty() @@ -710,7 +721,9 @@ impl<'a> MayHaveSideEffects<'a> for ClassElement<'a> { && e.value.as_ref().is_some_and(|v| v.may_have_side_effects(ctx))) } ClassElement::AccessorProperty(e) => { - !e.decorators.is_empty() || e.key.may_have_side_effects(ctx) + !e.decorators.is_empty() + || e.key.may_have_side_effects(ctx) + || e.value.as_ref().is_some_and(|init| init.may_have_side_effects(ctx)) } ClassElement::TSIndexSignature(_) => false, } @@ -767,7 +780,17 @@ impl<'a> MayHaveSideEffects<'a> for ComputedMemberExpression<'a> { !integer_index_property_access_may_have_side_effects(&self.object, b, ctx) }) } - _ => true, + _ => { + // Non-literal keys (e.g. `obj[expr]`) may trigger toString/valueOf on the key, + // which is a side effect. But if property read side effects are disabled, + // only check the key expression and object for their own side effects. + if ctx.property_read_side_effects() == PropertyReadSideEffects::None { + self.expression.may_have_side_effects(ctx) + || self.object.may_have_side_effects(ctx) + } else { + true + } + } } } } diff --git a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs index e7e1275d60a12..7f0c75e41a41c 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs @@ -73,6 +73,20 @@ fn test(source_text: &str, expected: bool) { test_with_ctx(source_text, &ctx, expected); } +#[track_caller] +fn test_ts(source_text: &str, expected: bool) { + let ctx = Ctx::default(); + let allocator = Allocator::default(); + let ret = Parser::new(&allocator, source_text, SourceType::tsx()).parse(); + assert!(!ret.panicked, "{source_text}"); + assert!(ret.errors.is_empty(), "{source_text}"); + + let Some(Statement::ExpressionStatement(stmt)) = &ret.program.body.first() else { + panic!("should have a expression statement body: {source_text}"); + }; + assert_eq!(stmt.expression.may_have_side_effects(&ctx), expected, "{source_text}"); +} + #[track_caller] fn test_with_global_variables( source_text: &str, @@ -751,8 +765,17 @@ fn test_class_expression() { test("(class { static a = foo() })", true); test("(class { accessor [foo()]; })", true); test("(class { static accessor [foo()]; })", true); + // AccessorProperty with value + test("(class { accessor a = 1; })", false); + test("(class { accessor a = foo(); })", true); + test("(class { static accessor a = 1; })", false); + test("(class { static accessor a = foo(); })", true); test("(class { #x; static { #x in {}; } })", false); test("(class { #x; static { #x in foo(); } })", true); + // MethodDefinition with parameter decorators (TypeScript feature) + test_ts("(class { a(@foo x) {} })", true); + test_ts("(class { a(@foo x, @bar y) {} })", true); + test_ts("(class { a(x) {} })", false); } #[test] @@ -913,6 +936,21 @@ fn test_new_expressions() { test("new Number", false); test("new Object", false); test("new String", false); + + // TypedArray constructors + test("new Int8Array", false); + test("new Uint8Array", false); + test("new Uint8ClampedArray", false); + test("new Int16Array", false); + test("new Uint16Array", false); + test("new Int32Array", false); + test("new Uint32Array", false); + test("new Float32Array", false); + test("new Float64Array", false); + test("new BigInt64Array", false); + test("new BigUint64Array", false); + // DataView requires an ArrayBuffer argument; calling without one throws + test("new DataView", true); } // `PF` in @@ -1149,10 +1187,23 @@ fn test_property_read_side_effects_support() { test_with_ctx("foo[0]", &none_ctx, false); test_with_ctx("foo[0n]", &none_ctx, false); test_with_ctx("foo[bar()]", &none_ctx, true); + // Non-literal keys: when property_read_side_effects is None, + // only the key expression and object are checked for side effects + test_with_ctx("foo[bar]", &all_ctx, true); + test_with_ctx("foo[bar]", &none_ctx, false); + test_with_ctx("foo[bar()]", &all_ctx, true); + test_with_ctx("foo[bar()]", &none_ctx, true); // bar() itself has side effects test_with_ctx("foo.#bar", &all_ctx, true); test_with_ctx("foo.#bar", &none_ctx, false); test_with_ctx("({ bar } = foo)", &all_ctx, true); // test_with_ctx("({ bar } = foo)", &none_ctx, false); + + // ObjectExpression spread: when property_read_side_effects is None, + // spread delegates to the argument's own side effects + test_with_ctx("({...foo})", &all_ctx, true); + test_with_ctx("({...foo})", &none_ctx, false); + test_with_ctx("({...foo()})", &all_ctx, true); + test_with_ctx("({...foo()})", &none_ctx, true); // foo() itself has side effects } #[test]