Repository: swc-project/swc (rust)

Issue:
SWC Compress option incorrectly splits Return keyword from rest of the expression when preserving an inline comment
### Describe the bug

In swc's "Compress" step, an inline comment near some part of a conditionally assigned variable that is used in a `return` statement, results in bad compilation - the `return` keyword is incorrectly split from the rest of the final line expression (and without enclosing brackets to preserve the logic together).

This results in a bug in the compiled code, since a return on its own line is interpreted with ASI (automatic semi-colon insertion) as an `undefined` return by browser engines.

### Input code

```typescript
export const fn = () => {
  let varA;
  if (condCheck) {
    // a bad comment
    varA = "a";
  } else {
    varA = "b";
  }
  return objCreator({
    varA,
  });
};
```


### Config

```json
{
  "jsc": {
    "externalHelpers": true,
    "parser": {
      "tsx": true,
      "syntax": "typescript",
    },
    "preserveAllComments": true,
    "target": "es5",
    "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
      }
    }
  },
}
```


### Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.3.100&code=H4sIAAAAAAAAA0WM0QqDMAxF3%2FsVF59aGPgBxcHwS1qNuE3bETMZDP%2Fd1G3sIRByzgm9HpkFXU6LYEhoYB2aM94GmEiwBr543a8DrEp9O1J3dwcG6hoBMfSazzMlOY6l0DdVqEq3gaaFvv4PxQ%2FSYZInJ%2BR4a5mCZLZ%2F81Qc583mzQ4flsvfpgAAAA%3D%3D&config=H4sIAAAAAAAAA32UsW7jMAyG9z5F4LnrLd2KAkXfQlAsylFPFg2RSmMUefdStpPrObSXwOFH%2FpRIit9Ph0PzSW3zcviWT%2FkDF4acbPyAOEAmAZwLPM9wsJkg353FwnT5z0VMNCa21drwOAC1OQzczPR608kgOmd4jfEN%2Bx4SrxKxzR1w1QD6swQ3fUjBj7%2Bzt9hXKfplE6vElpumt5HuR5tYxi9aHVnsR8QINu0QY8mExNDNVVkLtxijHQjM2WZFpZ7U5kCopaiwMDgzZBxUnlzggNIWhTqwzrToQEEhQ8vhDFqY5JKwRHI95T4TdnAsXTd1fBUNZxuLZSUnXKaWyGkV1RMGYuNL0ko4w40azHAp7joyeJOBS06PcZ8Y0kZP%2FgJIBaIlSrYHTXfy8DJPW9F%2BNzIkLyPLo8IjqrdM0ElRTQheqWytDGQOWjczuNJCrWyrHWfBG%2BWj4MCA9zIrijR9BW5PWtL6vtErQPprvTZVMzD3V7jB64PYwe9yS9YHbPHoLZ%2B2KY39EeNOgh74hG7HQVrBuI2zbInLsM1LciCjAU51KTSBxyUgD4DRxGkxPsyGPA9RNF3E4781sThcn26%2FsoGvP%2Ft0hTb1BQAA

### SWC Info output

```
export var fn = function() {
    var varA;
    return // a bad comment
    varA = condCheck ? "a" : "b", objCreator({
        varA: varA
    });
};
```

### Expected behavior

Possibly some variant of:

```
export var fn = function() {
    var varA;
    return (
    // a bad comment
    varA = condCheck ? "a" : "b", objCreator({
        varA: varA
    })
   )
};
```

Where the `return` and the remaining expression logic is preserved together.

### Actual behavior

SWC output:

```
export var fn = function() {
    var varA;
    return // a bad comment
    varA = condCheck ? "a" : "b", objCreator({
        varA: varA
    });
};
```

Again, this results in a `fn` that when invoked, simply returns `undefined` (which breaks the original code logic, which returned the result of `objCreator`). 

### Version

1.3.100

### Additional context

There may be a simpler repro instance, but this is the simplest I've narrowed to so far.

Pull request:
fix(es/codegen): Emit extra paren when return assign and assign targe…
…t is ident

<!-- Note: CI script will automatically rebase your PR so please do not rebase unless required -->

<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. -->

**Description:**

<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->

**BREAKING CHANGE:**

<!--
If this PR introduces a breaking change, it must contain a notice for it to be included in the CHANGELOG. Add description or remove entirely if not breaking.

You may need to update `MIGRATION.md` for the breaking changes.
-->

**Related issue (if exists):**
Closes #8412 

Patch:
diff --git a/crates/swc_ecma_codegen/src/lib.rs b/crates/swc_ecma_codegen/src/lib.rs
index 082bc94702..1277d2d7e7 100644
--- a/crates/swc_ecma_codegen/src/lib.rs
+++ b/crates/swc_ecma_codegen/src/lib.rs
@@ -2885,9 +2885,7 @@ where
     }
 
     fn has_leading_comment(&self, arg: &Expr) -> bool {
-        if let Some(cmt) = self.comments {
-            let span = arg.span();
-
+        fn span_has_leading_comment(cmt: &dyn Comments, span: Span) -> bool {
             let lo = span.lo;
 
             // see #415
@@ -2903,36 +2901,32 @@ where
                     return true;
                 }
             }
+
+            false
+        }
+
+        let cmt = if let Some(cmt) = self.comments {
+            if span_has_leading_comment(cmt, arg.span()) {
+                return true;
+            }
+
+            cmt
         } else {
             return false;
-        }
+        };
 
         match arg {
-            Expr::Call(c) => match &c.callee {
-                Callee::Super(callee) => {
-                    if let Some(cmt) = self.comments {
-                        let lo = callee.span.lo;
-
-                        if cmt.has_leading(lo) {
-                            return true;
-                        }
-                    }
-                }
-                Callee::Import(callee) => {
-                    if let Some(cmt) = self.comments {
-                        let lo = callee.span.lo;
+            Expr::Call(c) => {
+                let has_leading = match &c.callee {
+                    Callee::Super(callee) => span_has_leading_comment(cmt, callee.span),
+                    Callee::Import(callee) => span_has_leading_comment(cmt, callee.span),
+                    Callee::Expr(callee) => self.has_leading_comment(callee),
+                };
 
-                        if cmt.has_leading(lo) {
-                            return true;
-                        }
-                    }
-                }
-                Callee::Expr(callee) => {
-                    if self.has_leading_comment(callee) {
-                        return true;
-                    }
+                if has_leading {
+                    return true;
                 }
-            },
+            }
 
             Expr::Member(m) => {
                 if self.has_leading_comment(&m.obj) {
@@ -2941,12 +2935,8 @@ where
             }
 
             Expr::SuperProp(m) => {
-                if let Some(cmt) = self.comments {
-                    let lo = m.span.lo;
-
-                    if cmt.has_leading(lo) {
-                        return true;
-                    }
+                if span_has_leading_comment(cmt, m.span) {
+                    return true;
                 }
             }
 
@@ -2971,18 +2961,29 @@ where
             }
 
             Expr::Assign(e) => {
-                if let Some(cmt) = self.comments {
-                    let lo = e.span.lo;
+                let lo = e.span.lo;
 
-                    if cmt.has_leading(lo) {
-                        return true;
-                    }
+                if cmt.has_leading(lo) {
+                    return true;
                 }
 
-                if let Some(e) = e.left.as_expr() {
-                    if self.has_leading_comment(e) {
-                        return true;
-                    }
+                let has_leading = match &e.left {
+                    PatOrExpr::Expr(e) => self.has_leading_comment(e),
+
+                    PatOrExpr::Pat(p) => match &**p {
+                        Pat::Expr(e) => self.has_leading_comment(e),
+                        Pat::Ident(i) => span_has_leading_comment(cmt, i.span),
+                        Pat::Array(a) => span_has_leading_comment(cmt, a.span),
+                        Pat::Object(o) => span_has_leading_comment(cmt, o.span),
+                        // TODO: remove after #8333
+                        Pat::Rest(r) => span_has_leading_comment(cmt, r.span),
+                        Pat::Assign(a) => span_has_leading_comment(cmt, a.span),
+                        Pat::Invalid(_) => false,
+                    },
+                };
+
+                if has_leading {
+                    return true;
                 }
             }
 
@@ -3015,7 +3016,7 @@ where
 
         keyword!("return");
 
-        if let Some(ref arg) = n.arg {
+        if let Some(arg) = n.arg.as_deref() {
             let need_paren = n
                 .arg
                 .as_deref()


Test patch:
diff --git a/crates/swc/tests/fixture/issues-8xxx/8412/input/.swcrc b/crates/swc/tests/fixture/issues-8xxx/8412/input/.swcrc
new file mode 100644
index 0000000000..8a5d2ce2de
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8412/input/.swcrc
@@ -0,0 +1,16 @@
+{
+    "jsc": {
+        "externalHelpers": true,
+        "parser": {
+            "tsx": true,
+            "syntax": "typescript"
+        },
+        "preserveAllComments": true,
+        "target": "es5",
+        "minify": {
+            "compress": {
+                "defaults": true
+            }
+        }
+    }
+}
diff --git a/crates/swc/tests/fixture/issues-8xxx/8412/input/index.js b/crates/swc/tests/fixture/issues-8xxx/8412/input/index.js
new file mode 100644
index 0000000000..aa891c4d39
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8412/input/index.js
@@ -0,0 +1,12 @@
+export const fn = () => {
+    let varA;
+    if (condCheck) {
+        // a bad comment
+        varA = "a";
+    } else {
+        varA = "b";
+    }
+    return objCreator({
+        varA,
+    });
+};
diff --git a/crates/swc/tests/fixture/issues-8xxx/8412/output/index.js b/crates/swc/tests/fixture/issues-8xxx/8412/output/index.js
new file mode 100644
index 0000000000..94988db673
--- /dev/null
+++ b/crates/swc/tests/fixture/issues-8xxx/8412/output/index.js
@@ -0,0 +1,7 @@
+export var fn = function() {
+    var varA;
+    return(// a bad comment
+    varA = condCheck ? "a" : "b", objCreator({
+        varA: varA
+    }));
+};