Repository: swc-project/swc (rust)

Issue:
Compression misplaces function call when variable storing return value is used for logical assignment
### Describe the bug

When a return value of a function is assigned to a `let` variable, and that variable is used behind a logical assignment, compression tries to remove that intermediate variable and move the function call directly after the logical assignment.

This changes the logic such that before compilation the function would have always been called, but after compilation it's moved behind the logical assignment and will be called conditionally.

Since the function can have a side-effect that should always run, this is incorrect behavior.

### Input code

```typescript
const myArr = [];
// function with side effect
function foo(arr) {
    arr.push('foo');
    return 'foo';
}
let a;

if (Math.random() > 0.5) {
    a = true;
}

// the function call below should always run
// regardless of whether `a` is `undefined`
let b = foo(myArr);

// const seems to keep this line here instead of
// moving it behind the logitcal nullish assignment
// const b = foo(myArr);

a ??= b;

console.log(a);
console.log(myArr);
```


### Config

```json
{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": false
    },
    "target": "es2022",
    "loose": false,
    "minify": {
      "compress": {
        "arguments": false,
        "arrows": true,
        "booleans": true,
        "booleans_as_integers": false,
        "collapse_vars": true,
        "comparisons": true,
        "computed_props": true,
        "conditionals": true,
        "dead_code": true,
        "directives": true,
        "drop_console": false,
        "drop_debugger": true,
        "evaluate": true,
        "expression": false,
        "hoist_funs": false,
        "hoist_props": true,
        "hoist_vars": false,
        "if_return": true,
        "join_vars": true,
        "keep_classnames": false,
        "keep_fargs": true,
        "keep_fnames": false,
        "keep_infinity": false,
        "loops": true,
        "negate_iife": true,
        "properties": true,
        "reduce_funcs": false,
        "reduce_vars": false,
        "side_effects": true,
        "switches": true,
        "typeofs": true,
        "unsafe": false,
        "unsafe_arrows": false,
        "unsafe_comps": false,
        "unsafe_Function": false,
        "unsafe_math": false,
        "unsafe_symbols": false,
        "unsafe_methods": false,
        "unsafe_proto": false,
        "unsafe_regexp": false,
        "unsafe_undefined": false,
        "unused": true,
        "const_to_let": true,
        "pristine_globals": true,
        "passes": 2
      },
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true
}
```


### Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.3.74&code=H4sIAAAAAAAAA2WQTU7DMBCF9z7F2zXZpGxYRaXiAJygQsoknsQWjl35h6hC3J1xJFokvBrPzHvvs6fgU8Z6e40RJ1zee3U8Yi5%2ByjZ4bDYbJKtZ3VtzCA3F2OJLQY6U3bUk0xxkcGj7vRk5l%2Bixt3r1rRxnUK%2BUndG8UTZdJK%2FD2rR4wVP3fPcSghwLV0nFyIYfKBM5h5Fd2JBMKE6D3Ea3hFh8XY68UNSOU0KYsRkWdcRAA2zCULzm2XrWww4zSlJ9yP5uga4G0%2F4TiXlNyAEfzFchELETHcSMYWWDSUtAFazh0%2FoFVuzYWK93XhcWm4UVvjhnkwGlZBe%2Fss%2BPkH%2FxhPP5hFGquhAcd%2BLTkIz%2B3n%2FXfwDJh74ktAEAAA%3D%3D&config=H4sIAAAAAAAAA32UO3LjMAyGe5%2FCozrFjostcoDtcgYOTYIyvXxoCNCxJuO7L0TJjjeG1En48AMkAOJrt993ZzTd%2B%2F6LP%2Fln0AWhPP7ZgmMifWVLByZqNMUP1L3d6Rkn5HRAaKbbTDrSpQdqKjz8OhwWRRdyRrgrFlv0ybvxOafJcSiA%2BGRjK4esERLh%2F%2FqFlfw5ASr12X7MOYBOG0RpVD4R9FCkwCaHoAcEddFFiDKdVBePWUoxwUpg1VDyIPJkPfmcOOcrtaCtMtmCgHwBQ%2F4CkoxzsSwhX0%2B4T8MWjrXvW59%2FqOGiQ9Uk5IRrawmfVoh6yh5JuZqkEs5wpQYzXIr7U%2BmdKkC1pFfdOfu00pO%2FAFyBoBGTjiDFbR6O52lN7TaVPjkeWRoFzvMt3TJBz0VV3juhslNloJCXulnAVgNTZY10nAWvlA%2B9BQXO8awIofHTkzlJSWkcIDsBcH%2B1k6ZqBurxClf49CA28B%2B%2BJckDtnhETad1imM85rCRIAKdst1w4FZQXseFt8R1WOc1WeDRACu6VGzgdQnwA6CsQtuXL7PBz4Mjqj7ko7gmBp7z1sTDYrs9lnPUqf9eAvN%2B3i0OXcy2Nrhs%2Fqnp877%2B3X073Vfz4zadx4%2B7cjrJ7vYPdLfRTEUGAAA%3D

### SWC Info output

_No response_

### Expected behavior

In above input, the output should keep `b = foo(myArr)` like how it does when it's declared with `const` instead of `let`.

### Actual behavior

_No response_

### Version

1.3.74

### Additional context

_No response_

Pull request:
fix(es/minifier): Abort seq inliner if `b` can short-circuit
**Description:**


**Related issue:**

 - Closes #8119

Patch:
diff --git a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs
index 4f7a6e2f1c..771d4a59c0 100644
--- a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs
+++ b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs
@@ -2489,6 +2489,10 @@ impl Optimizer<'_> {
     /// 1, arr[i]`
     //
     fn should_not_check_rhs_of_assign(&self, a: &Mergable, b: &mut AssignExpr) -> Result<bool, ()> {
+        if b.op.may_short_circuit() {
+            return Ok(true);
+        }
+
         if let Some(a_id) = a.id() {
             match a {
                 Mergable::Expr(Expr::Assign(AssignExpr { op: op!("="), .. })) => {}


Test patch:
diff --git a/crates/swc/tests/exec/issues-8xxx/8119/.swcrc b/crates/swc/tests/exec/issues-8xxx/8119/.swcrc
new file mode 100644
index 0000000000..f48b5d4bf0
--- /dev/null
+++ b/crates/swc/tests/exec/issues-8xxx/8119/.swcrc
@@ -0,0 +1,65 @@
+{
+    "jsc": {
+        "parser": {
+            "syntax": "ecmascript",
+            "jsx": false
+        },
+        "target": "es2022",
+        "loose": false,
+        "minify": {
+            "compress": {
+                "arguments": false,
+                "arrows": true,
+                "booleans": true,
+                "booleans_as_integers": false,
+                "collapse_vars": true,
+                "comparisons": true,
+                "computed_props": true,
+                "conditionals": true,
+                "dead_code": true,
+                "directives": true,
+                "drop_console": false,
+                "drop_debugger": true,
+                "evaluate": true,
+                "expression": false,
+                "hoist_funs": false,
+                "hoist_props": true,
+                "hoist_vars": false,
+                "if_return": true,
+                "join_vars": true,
+                "keep_classnames": false,
+                "keep_fargs": true,
+                "keep_fnames": false,
+                "keep_infinity": false,
+                "loops": true,
+                "negate_iife": true,
+                "properties": true,
+                "reduce_funcs": false,
+                "reduce_vars": false,
+                "side_effects": true,
+                "switches": true,
+                "typeofs": true,
+                "unsafe": false,
+                "unsafe_arrows": false,
+                "unsafe_comps": false,
+                "unsafe_Function": false,
+                "unsafe_math": false,
+                "unsafe_symbols": false,
+                "unsafe_methods": false,
+                "unsafe_proto": false,
+                "unsafe_regexp": false,
+                "unsafe_undefined": false,
+                "unused": true,
+                "const_to_let": true,
+                "pristine_globals": true,
+                "passes": 2
+            },
+            "mangle": false
+        }
+    },
+    "module": {
+        "type": "es6"
+    },
+    "minify": false,
+    "isModule": true
+}
\ No newline at end of file
diff --git a/crates/swc/tests/exec/issues-8xxx/8119/1/exec.js b/crates/swc/tests/exec/issues-8xxx/8119/1/exec.js
new file mode 100644
index 0000000000..812e99cbd7
--- /dev/null
+++ b/crates/swc/tests/exec/issues-8xxx/8119/1/exec.js
@@ -0,0 +1,24 @@
+const myArr = [];
+// function with side effect
+function foo(arr) {
+    arr.push('foo');
+    return 'foo';
+}
+let a;
+
+if (Math.random() > 1.00000) {
+    a = true;
+}
+
+// the function call below should always run
+// regardless of whether `a` is `undefined`
+let b = foo(myArr);
+
+// const seems to keep this line here instead of
+// moving it behind the logitcal nullish assignment
+// const b = foo(myArr);
+
+a ??= b;
+
+console.log(a);
+console.log(myArr);
\ No newline at end of file
diff --git a/crates/swc/tests/exec/issues-8xxx/8119/2/exec.js b/crates/swc/tests/exec/issues-8xxx/8119/2/exec.js
new file mode 100644
index 0000000000..8351227178
--- /dev/null
+++ b/crates/swc/tests/exec/issues-8xxx/8119/2/exec.js
@@ -0,0 +1,24 @@
+const myArr = [];
+// function with side effect
+function foo(arr) {
+    arr.push('foo');
+    return 'foo';
+}
+let a;
+
+if (Math.random() > -0.1) {
+    a = true;
+}
+
+// the function call below should always run
+// regardless of whether `a` is `undefined`
+let b = foo(myArr);
+
+// const seems to keep this line here instead of
+// moving it behind the logitcal nullish assignment
+// const b = foo(myArr);
+
+a ??= b;
+
+console.log(a);
+console.log(myArr);
\ No newline at end of file
diff --git a/crates/swc/tests/fixture/issues-8xxx/8119/input/.swcrc b/crates/swc/tests/fixture/issues-8xxx/8119/input/.swcrc
new file mode 100644
index 0000000000..f48b5d4bf0
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8119/input/.swcrc
@@ -0,0 +1,65 @@
+{
+    "jsc": {
+        "parser": {
+            "syntax": "ecmascript",
+            "jsx": false
+        },
+        "target": "es2022",
+        "loose": false,
+        "minify": {
+            "compress": {
+                "arguments": false,
+                "arrows": true,
+                "booleans": true,
+                "booleans_as_integers": false,
+                "collapse_vars": true,
+                "comparisons": true,
+                "computed_props": true,
+                "conditionals": true,
+                "dead_code": true,
+                "directives": true,
+                "drop_console": false,
+                "drop_debugger": true,
+                "evaluate": true,
+                "expression": false,
+                "hoist_funs": false,
+                "hoist_props": true,
+                "hoist_vars": false,
+                "if_return": true,
+                "join_vars": true,
+                "keep_classnames": false,
+                "keep_fargs": true,
+                "keep_fnames": false,
+                "keep_infinity": false,
+                "loops": true,
+                "negate_iife": true,
+                "properties": true,
+                "reduce_funcs": false,
+                "reduce_vars": false,
+                "side_effects": true,
+                "switches": true,
+                "typeofs": true,
+                "unsafe": false,
+                "unsafe_arrows": false,
+                "unsafe_comps": false,
+                "unsafe_Function": false,
+                "unsafe_math": false,
+                "unsafe_symbols": false,
+                "unsafe_methods": false,
+                "unsafe_proto": false,
+                "unsafe_regexp": false,
+                "unsafe_undefined": false,
+                "unused": true,
+                "const_to_let": true,
+                "pristine_globals": true,
+                "passes": 2
+            },
+            "mangle": false
+        }
+    },
+    "module": {
+        "type": "es6"
+    },
+    "minify": false,
+    "isModule": true
+}
\ No newline at end of file
diff --git a/crates/swc/tests/fixture/issues-8xxx/8119/input/1.js b/crates/swc/tests/fixture/issues-8xxx/8119/input/1.js
new file mode 100644
index 0000000000..b7d762a3c3
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8119/input/1.js
@@ -0,0 +1,24 @@
+const myArr = [];
+// function with side effect
+function foo(arr) {
+    arr.push('foo');
+    return 'foo';
+}
+let a;
+
+if (Math.random() > 0.5) {
+    a = true;
+}
+
+// the function call below should always run
+// regardless of whether `a` is `undefined`
+let b = foo(myArr);
+
+// const seems to keep this line here instead of
+// moving it behind the logitcal nullish assignment
+// const b = foo(myArr);
+
+a ??= b;
+
+console.log(a);
+console.log(myArr);
\ No newline at end of file
diff --git a/crates/swc/tests/fixture/issues-8xxx/8119/output/1.js b/crates/swc/tests/fixture/issues-8xxx/8119/output/1.js
new file mode 100644
index 0000000000..e452f14404
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8119/output/1.js
@@ -0,0 +1,10 @@
+let a;
+const myArr = [];
+Math.random() > 0.5 && (a = !0);
+// the function call below should always run
+// regardless of whether `a` is `undefined`
+let b = (myArr.push('foo'), 'foo');
+console.log(// const seems to keep this line here instead of
+// moving it behind the logitcal nullish assignment
+// const b = foo(myArr);
+a ??= b), console.log(myArr);
diff --git a/crates/swc_ecma_minifier/tests/exec.rs b/crates/swc_ecma_minifier/tests/exec.rs
index a02d73b4c2..a29b300159 100644
--- a/crates/swc_ecma_minifier/tests/exec.rs
+++ b/crates/swc_ecma_minifier/tests/exec.rs
@@ -10947,3 +10947,167 @@ fn issue_7274() {
         "#,
     );
 }
+
+#[test]
+fn issue_8119_1() {
+    run_exec_test(
+        r#"
+        const myArr = [];
+        // function with side effect
+        function foo(arr) {
+            arr.push('foo');
+            return 'foo';
+        }
+        let a;
+
+        if (Math.random() > 1.1) {
+            a = true;
+        }
+
+        // the function call below should always run
+        // regardless of whether `a` is `undefined`
+        let b = foo(myArr);
+
+        // const seems to keep this line here instead of
+        // moving it behind the logitcal nullish assignment
+        // const b = foo(myArr);
+
+        a ??= b;
+
+        console.log(a);
+        console.log(myArr);
+        "#,
+        r#"
+        {
+            "arguments": false,
+            "arrows": true,
+            "booleans": true,
+            "booleans_as_integers": false,
+            "collapse_vars": true,
+            "comparisons": true,
+            "computed_props": true,
+            "conditionals": true,
+            "dead_code": true,
+            "directives": true,
+            "drop_console": false,
+            "drop_debugger": true,
+            "evaluate": true,
+            "expression": false,
+            "hoist_funs": false,
+            "hoist_props": true,
+            "hoist_vars": false,
+            "if_return": true,
+            "join_vars": true,
+            "keep_classnames": false,
+            "keep_fargs": true,
+            "keep_fnames": false,
+            "keep_infinity": false,
+            "loops": true,
+            "negate_iife": true,
+            "properties": true,
+            "reduce_funcs": false,
+            "reduce_vars": false,
+            "side_effects": true,
+            "switches": true,
+            "typeofs": true,
+            "unsafe": false,
+            "unsafe_arrows": false,
+            "unsafe_comps": false,
+            "unsafe_Function": false,
+            "unsafe_math": false,
+            "unsafe_symbols": false,
+            "unsafe_methods": false,
+            "unsafe_proto": false,
+            "unsafe_regexp": false,
+            "unsafe_undefined": false,
+            "unused": true,
+            "const_to_let": true,
+            "pristine_globals": true,
+            "passes": 2
+        }
+        "#,
+        false,
+    );
+}
+
+#[test]
+fn issue_8119_2() {
+    run_exec_test(
+        r#"
+        const myArr = [];
+        // function with side effect
+        function foo(arr) {
+            arr.push('foo');
+            return 'foo';
+        }
+        let a;
+
+        if (Math.random() > -0.1) {
+            a = true;
+        }
+
+        // the function call below should always run
+        // regardless of whether `a` is `undefined`
+        let b = foo(myArr);
+
+        // const seems to keep this line here instead of
+        // moving it behind the logitcal nullish assignment
+        // const b = foo(myArr);
+
+        a ??= b;
+
+        console.log(a);
+        console.log(myArr);
+        "#,
+        r#"
+        {
+            "arguments": false,
+            "arrows": true,
+            "booleans": true,
+            "booleans_as_integers": false,
+            "collapse_vars": true,
+            "comparisons": true,
+            "computed_props": true,
+            "conditionals": true,
+            "dead_code": true,
+            "directives": true,
+            "drop_console": false,
+            "drop_debugger": true,
+            "evaluate": true,
+            "expression": false,
+            "hoist_funs": false,
+            "hoist_props": true,
+            "hoist_vars": false,
+            "if_return": true,
+            "join_vars": true,
+            "keep_classnames": false,
+            "keep_fargs": true,
+            "keep_fnames": false,
+            "keep_infinity": false,
+            "loops": true,
+            "negate_iife": true,
+            "properties": true,
+            "reduce_funcs": false,
+            "reduce_vars": false,
+            "side_effects": true,
+            "switches": true,
+            "typeofs": true,
+            "unsafe": false,
+            "unsafe_arrows": false,
+            "unsafe_comps": false,
+            "unsafe_Function": false,
+            "unsafe_math": false,
+            "unsafe_symbols": false,
+            "unsafe_methods": false,
+            "unsafe_proto": false,
+            "unsafe_regexp": false,
+            "unsafe_undefined": false,
+            "unused": true,
+            "const_to_let": true,
+            "pristine_globals": true,
+            "passes": 2
+        }
+        "#,
+        false,
+    );
+}
diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8119/config.json b/crates/swc_ecma_minifier/tests/fixture/issues/8119/config.json
new file mode 100644
index 0000000000..ed751935a8
--- /dev/null
+++ b/crates/swc_ecma_minifier/tests/fixture/issues/8119/config.json
@@ -0,0 +1,47 @@
+{
+    "arguments": false,
+    "arrows": true,
+    "booleans": true,
+    "booleans_as_integers": false,
+    "collapse_vars": true,
+    "comparisons": true,
+    "computed_props": true,
+    "conditionals": true,
+    "dead_code": true,
+    "directives": true,
+    "drop_console": false,
+    "drop_debugger": true,
+    "evaluate": true,
+    "expression": false,
+    "hoist_funs": false,
+    "hoist_props": true,
+    "hoist_vars": false,
+    "if_return": true,
+    "join_vars": true,
+    "keep_classnames": false,
+    "keep_fargs": true,
+    "keep_fnames": false,
+    "keep_infinity": false,
+    "loops": true,
+    "negate_iife": true,
+    "properties": true,
+    "reduce_funcs": false,
+    "reduce_vars": false,
+    "side_effects": true,
+    "switches": true,
+    "typeofs": true,
+    "unsafe": false,
+    "unsafe_arrows": false,
+    "unsafe_comps": false,
+    "unsafe_Function": false,
+    "unsafe_math": false,
+    "unsafe_symbols": false,
+    "unsafe_methods": false,
+    "unsafe_proto": false,
+    "unsafe_regexp": false,
+    "unsafe_undefined": false,
+    "unused": true,
+    "const_to_let": true,
+    "pristine_globals": true,
+    "passes": 2
+}
diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8119/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8119/input.js
new file mode 100644
index 0000000000..b7d762a3c3
--- /dev/null
+++ b/crates/swc_ecma_minifier/tests/fixture/issues/8119/input.js
@@ -0,0 +1,24 @@
+const myArr = [];
+// function with side effect
+function foo(arr) {
+    arr.push('foo');
+    return 'foo';
+}
+let a;
+
+if (Math.random() > 0.5) {
+    a = true;
+}
+
+// the function call below should always run
+// regardless of whether `a` is `undefined`
+let b = foo(myArr);
+
+// const seems to keep this line here instead of
+// moving it behind the logitcal nullish assignment
+// const b = foo(myArr);
+
+a ??= b;
+
+console.log(a);
+console.log(myArr);
\ No newline at end of file
diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8119/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8119/output.js
new file mode 100644
index 0000000000..6ab32f78e5
--- /dev/null
+++ b/crates/swc_ecma_minifier/tests/fixture/issues/8119/output.js
@@ -0,0 +1,10 @@
+let a;
+const myArr = [];
+function foo(arr) {
+    return arr.push('foo'), 'foo';
+}
+Math.random() > 0.5 && (a = !0);
+let b = foo(myArr);
+a ??= b;
+console.log(a);
+console.log(myArr);
diff --git a/crates/swc_ecma_minifier/tests/passing.txt b/crates/swc_ecma_minifier/tests/passing.txt
index f4248cf15a..88fbcc357e 100644
--- a/crates/swc_ecma_minifier/tests/passing.txt
+++ b/crates/swc_ecma_minifier/tests/passing.txt
@@ -744,6 +744,9 @@ hoist_props/issue_3071_2/input.js
 hoist_props/issue_3071_2_toplevel/input.js
 hoist_props/issue_3071_3/input.js
 hoist_props/single_use/input.js
+hoist_props/toplevel_const/input.js
+hoist_props/toplevel_let/input.js
+hoist_props/toplevel_var/input.js
 hoist_vars/issue_2295/input.js
 hoist_vars/regression_toplevel_args/input.js
 hoist_vars/sequences/input.js
diff --git a/crates/swc_ecma_minifier/tests/postponed.txt b/crates/swc_ecma_minifier/tests/postponed.txt
index f1e504b6f7..b990fc50ab 100644
--- a/crates/swc_ecma_minifier/tests/postponed.txt
+++ b/crates/swc_ecma_minifier/tests/postponed.txt
@@ -40,9 +40,6 @@ hoist_props/name_collision_1/input.js
 hoist_props/name_collision_2/input.js
 hoist_props/name_collision_3/input.js
 hoist_props/new_this/input.js
-hoist_props/toplevel_const/input.js
-hoist_props/toplevel_let/input.js
-hoist_props/toplevel_var/input.js
 hoist_props/undefined_key/input.js
 ie8/do_screw_try_catch_undefined/input.js
 ie8/dont_screw_try_catch/input.js