Repository: swc-project/swc (rust)

Issue:
Comments on types are stripped
### Describe the bug

Comments are stripped due to unknown reasons, against expectation.

### Input code

```typescript
// FILE bad.ts:

/* istanbul ignore next */
type Z = number;
x = 1;


// FILE good.ts
/* istanbul ignore next */
x = 1;
```


### Config

```json
{
  "jsc": {
    "transform": {}
  },
  "swcrc": true
}
```


### Playground link

https://play.swc.rs/?version=1.2.117&code=H4sIAAAAAAAAAzXMOwrAIBBF0d5VvFoCklqykXQahjigY4hjPruPTdpz4ToLbhok9gzepZ4EoUdhnXEO%2Bh6EFQukl0inx7AuWy2FRKGJGzILTbiDbmkA4Y8h1otwBeGWzDMWszcfPlBa620AAAA%3D&config=H4sIAAAAAAAAA6tWys3My0yrVLJKS8wpTtVRyipOVrKqVipILCpOLQKxiivzShIrlKyUSioLUouTizILSpRqa2sBrzFAMDkAAAA%3D

### Expected behavior

`bad.ts` should emit:

```
/* istanbul ignore next */
x = 1;
```

but does emit:

```
x = 1;
```

dropping the istanbul comment

### Version

1.2.117

### Additional context

_No response_

Pull request:
feat(swc): Add comment preservation
<!-- 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. -->


## Problem

Generated code will drop comments in a variety of situations. Some of these situations may include:

- Comments that precede types
- Comments that precede / trail nodes that are transformed / moved / removed


The JS Ecosystem is (unfortunately) reliant on maintaining comment continuity between source and compiled code. There are many existing tools that use comments as annotations for various functionality, the most entrenched probably being `istanbul` coverage annotations. Not maintaining good parity between source and compiled output causes adopting SWC for test runners to be non-tenable for a lot of customers.


## Solution

Implement a "comment fixer" visitor, `dropped_comments_preserver`, and introduce a feature flag `preserve_dropped_comments` into the jsc configuration object.

### Overview of changes

Given the way SWC internals track comments, the comment fixer makes a best effort to preserve comments that may have otherwise been dropped. The output may not be 1:1 with what babel or other transpilers output, but it will preserve all expected functionality from existing js tooling where possible.

When `preserve_dropped_comments` is enabled, all trailing and leading comments will be shifted to the first non-dummy span that exists in the ast. This guarantees that all existing comments will be transported to the compiled output, orphaned comments will _precede_ comments that would otherwise be preserved, and comments that would be preserved remain in a consistent position.


---

I tried my best to organize things as I thought they should be according to the current standards of the repo. Open to any and all suggestions 🙏  would _really_ like to support this feature as it is currently blocking us over here at Walmart from adopting the SWC jest transformer fully. The gains are too good to ignore.

<!-- 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): #2964 **

---


Closes #2964


Patch:
diff --git a/crates/swc/src/config/mod.rs b/crates/swc/src/config/mod.rs
index 21353072c6..217b8272a7 100644
--- a/crates/swc/src/config/mod.rs
+++ b/crates/swc/src/config/mod.rs
@@ -65,6 +65,7 @@ use swc_ecma_visit::{Fold, VisitMutWith};
 use self::util::BoolOrObject;
 use crate::{
     builder::PassBuilder,
+    dropped_comments_preserver::dropped_comments_preserver,
     plugin::{PluginConfig, PluginContext},
     SwcImportResolver,
 };
@@ -296,6 +297,7 @@ impl Options {
             minify: mut js_minify,
             experimental,
             lints,
+            preserve_all_comments,
             ..
         } = config.jsc;
 
@@ -368,7 +370,11 @@ impl Options {
 
         let regenerator = transform.regenerator.clone();
 
-        let preserve_comments = js_minify.as_ref().map(|v| v.format.comments.clone());
+        let preserve_comments = if preserve_all_comments {
+            Some(BoolOrObject::from(true))
+        } else {
+            js_minify.as_ref().map(|v| v.format.comments.clone())
+        };
 
         if syntax.typescript() {
             transform.legacy_decorator = true;
@@ -498,7 +504,11 @@ impl Options {
                 syntax.jsx()
             ),
             pass,
-            Optional::new(jest::jest(), transform.hidden.jest)
+            Optional::new(jest::jest(), transform.hidden.jest),
+            Optional::new(
+                dropped_comments_preserver(comments.cloned()),
+                preserve_all_comments
+            ),
         );
 
         Ok(BuiltInput {
@@ -1031,6 +1041,9 @@ pub struct JscConfig {
 
     #[serde(default)]
     pub lints: LintConfig,
+
+    #[serde(default)]
+    pub preserve_all_comments: bool,
 }
 
 /// `jsc.experimental` in `.swcrc`
@@ -1567,6 +1580,8 @@ impl Merge for JscConfig {
         self.paths.merge(&from.paths);
         self.minify.merge(&from.minify);
         self.experimental.merge(&from.experimental);
+        self.preserve_all_comments
+            .merge(&from.preserve_all_comments)
     }
 }
 
diff --git a/crates/swc/src/dropped_comments_preserver.rs b/crates/swc/src/dropped_comments_preserver.rs
new file mode 100644
index 0000000000..a1ca76ecde
--- /dev/null
+++ b/crates/swc/src/dropped_comments_preserver.rs
@@ -0,0 +1,138 @@
+use swc_common::{
+    comments::{Comment, Comments, SingleThreadedComments},
+    BytePos, Span, DUMMY_SP,
+};
+use swc_ecma_ast::{Module, Script};
+use swc_ecma_visit::{as_folder, noop_visit_mut_type, Fold, VisitMut, VisitMutWith};
+
+/// Preserves comments that would otherwise be dropped.
+///
+/// If during compilation an ast node associated with
+/// a comment is dropped, the comment will not appear in the final emitted
+/// output. This can create problems in the JavaScript ecosystem, particularly
+/// around instanbul coverage and other tooling that relies on comment
+/// directives.
+///
+/// This transformer shifts orphaned comments to the next closest known span
+/// while making a best-effort to preserve the "general orientation" of
+/// comments.
+
+pub fn dropped_comments_preserver(
+    comments: Option<SingleThreadedComments>,
+) -> impl Fold + VisitMut {
+    as_folder(DroppedCommentsPreserver {
+        comments,
+        is_first_span: true,
+        known_spans: Vec::new(),
+    })
+}
+
+struct DroppedCommentsPreserver {
+    comments: Option<SingleThreadedComments>,
+    is_first_span: bool,
+    known_spans: Vec<Span>,
+}
+
+type CommentEntries = Vec<(BytePos, Vec<Comment>)>;
+
+impl VisitMut for DroppedCommentsPreserver {
+    noop_visit_mut_type!();
+
+    fn visit_mut_module(&mut self, module: &mut Module) {
+        module.visit_mut_children_with(self);
+        self.shift_comments_to_known_spans();
+    }
+
+    fn visit_mut_script(&mut self, script: &mut Script) {
+        script.visit_mut_children_with(self);
+        self.shift_comments_to_known_spans();
+    }
+
+    fn visit_mut_span(&mut self, span: &mut Span) {
+        if span.is_dummy() || self.is_first_span {
+            self.is_first_span = false;
+            return;
+        }
+
+        self.known_spans.push(*span);
+        span.visit_mut_children_with(self)
+    }
+}
+
+impl DroppedCommentsPreserver {
+    fn shift_comments_to_known_spans(&self) {
+        if let Some(comments) = &self.comments {
+            let trailing_comments = self.shift_leading_comments(comments);
+
+            self.shift_trailing_comments(trailing_comments);
+        }
+    }
+
+    /// We'll be shifting all comments to known span positions, so drain the
+    /// current comments first to limit the amount of look ups needed into
+    /// the hashmaps.
+    ///
+    /// This way, we only need to take the comments once, and then add them back
+    /// once.
+    fn collect_existing_comments(&self, comments: &SingleThreadedComments) -> CommentEntries {
+        let (mut leading_comments, mut trailing_comments) = comments.borrow_all_mut();
+        let mut existing_comments: CommentEntries = leading_comments
+            .drain()
+            .chain(trailing_comments.drain())
+            .collect();
+
+        existing_comments.sort_by(|(bp_a, _), (bp_b, _)| bp_a.cmp(bp_b));
+
+        existing_comments
+    }
+
+    /// Shift all comments to known leading positions.
+    /// This prevents trailing comments from ending up associated with
+    /// nodes that will not emit trailing comments, while
+    /// preserving any comments that might show up after all code positions.
+    ///
+    /// This maintains the highest fidelity between existing comment positions
+    /// of pre and post compiled code.
+    fn shift_leading_comments(&self, comments: &SingleThreadedComments) -> CommentEntries {
+        let mut existing_comments = self.collect_existing_comments(comments);
+
+        for span in self.known_spans.iter() {
+            let (comments_to_move, next_byte_positions): (CommentEntries, CommentEntries) =
+                existing_comments
+                    .drain(..)
+                    .partition(|(bp, _)| *bp <= span.lo);
+
+            existing_comments.extend(next_byte_positions);
+
+            let collected_comments = comments_to_move.into_iter().flat_map(|(_, c)| c).collect();
+
+            self.comments
+                .add_leading_comments(span.lo, collected_comments)
+        }
+
+        existing_comments
+    }
+
+    /// These comments trail all known span lo byte positions.
+    /// Therefore, by shifting them to trail the highest known hi position, we
+    /// ensure that any remaining trailing comments are emitted in a
+    /// similar location
+    fn shift_trailing_comments(&self, remaining_comment_entries: CommentEntries) {
+        let last_trailing = self
+            .known_spans
+            .iter()
+            .copied()
+            .fold(
+                DUMMY_SP,
+                |acc, span| if span.hi > acc.hi { span } else { acc },
+            );
+
+        self.comments.add_trailing_comments(
+            last_trailing.hi,
+            remaining_comment_entries
+                .into_iter()
+                .flat_map(|(_, c)| c)
+                .collect(),
+        );
+    }
+}
diff --git a/crates/swc/src/lib.rs b/crates/swc/src/lib.rs
index 33a24fc1a5..cf5f66a8eb 100644
--- a/crates/swc/src/lib.rs
+++ b/crates/swc/src/lib.rs
@@ -170,6 +170,7 @@ use crate::config::{
 
 mod builder;
 pub mod config;
+mod dropped_comments_preserver;
 mod plugin;
 pub mod resolver {
     use std::path::PathBuf;
diff --git a/node-swc/src/types.ts b/node-swc/src/types.ts
index 8a2bb6ddef..6146205170 100644
--- a/node-swc/src/types.ts
+++ b/node-swc/src/types.ts
@@ -518,6 +518,8 @@ export interface JscConfig {
   }
 
   minify?: JsMinifyOptions;
+
+  preserveAllComments?: boolean;
 }
 
 export type JscTarget =


Test patch:
diff --git a/crates/swc/tests/fixture/issue-2964.case-1/input/.swcrc b/crates/swc/tests/fixture/issue-2964.case-1/input/.swcrc
new file mode 100644
index 0000000000..ce3b647bf2
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-1/input/.swcrc
@@ -0,0 +1,16 @@
+{
+  "jsc": {
+    "parser": {
+      "syntax": "typescript",
+      "tsx": false,
+      "decorators": false,
+      "dynamicImport": false
+    },
+    "transform": null,
+    "target": "es5",
+    "loose": false,
+    "externalHelpers": false,
+    "keepClassNames": false,
+    "preserveAllComments": true
+  }
+}
diff --git a/crates/swc/tests/fixture/issue-2964.case-1/input/index.ts b/crates/swc/tests/fixture/issue-2964.case-1/input/index.ts
new file mode 100644
index 0000000000..49bd67572e
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-1/input/index.ts
@@ -0,0 +1,11 @@
+/* istanbul ignore next */
+type Z = number;
+// preserved comment
+const x = 1;
+
+// Stacked Comment
+// Another comment
+type Y = string;
+const a = "";
+
+// trailing comment
diff --git a/crates/swc/tests/fixture/issue-2964.case-1/output/index.ts b/crates/swc/tests/fixture/issue-2964.case-1/output/index.ts
new file mode 100644
index 0000000000..29bd2361bc
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-1/output/index.ts
@@ -0,0 +1,5 @@
+/* istanbul ignore next */ // preserved comment
+var x = 1;
+// Stacked Comment
+// Another comment
+var a = ""; // trailing comment
diff --git a/crates/swc/tests/fixture/issue-2964.case-2/input/.swcrc b/crates/swc/tests/fixture/issue-2964.case-2/input/.swcrc
new file mode 100644
index 0000000000..f80939d14e
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-2/input/.swcrc
@@ -0,0 +1,33 @@
+{
+  "sourceMaps": false,
+  "module": {
+    "type": "commonjs"
+  },
+  "jsc": {
+    "parser": {
+      "syntax": "typescript",
+      "tsx": true,
+      "dynamicImport": true,
+      "decorators": false
+    },
+    "transform": {
+      "react": {
+        "pragma": "React.createElement",
+        "pragmaFrag": "React.Fragment",
+        "throwIfNamespace": true,
+        "development": false,
+        "useBuiltins": false
+      }
+    },
+    "target": "es2015",
+    "loose": false,
+    "externalHelpers": false,
+    "keepClassNames": false,
+    "minify": {
+      "compress": false,
+      "mangle": false
+    },
+    "preserveAllComments": true
+  },
+  "minify": false
+}
\ No newline at end of file
diff --git a/crates/swc/tests/fixture/issue-2964.case-2/input/index.ts b/crates/swc/tests/fixture/issue-2964.case-2/input/index.ts
new file mode 100644
index 0000000000..c964059d2f
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-2/input/index.ts
@@ -0,0 +1,12 @@
+//top comment
+export const noop = () => {};
+/* istanbul ignore next */
+export const badIstanbul = (test: Record<string, unknown>) => {
+    const { value, ...pixelParams } = test;
+    console.log('fail');
+};
+
+/* istanbul ignore next: UI-5137 */
+export const downloadDocument = (): void => {
+    console.log('fail');
+};
\ No newline at end of file
diff --git a/crates/swc/tests/fixture/issue-2964.case-2/output/index.ts b/crates/swc/tests/fixture/issue-2964.case-2/output/index.ts
new file mode 100644
index 0000000000..64cd686ba9
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-2/output/index.ts
@@ -0,0 +1,20 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", {
+    value: true
+});
+exports.downloadDocument = exports.badIstanbul = exports.noop = void 0;
+var swcHelpers = require("@swc/helpers");
+//top comment
+const noop = ()=>{};
+exports.noop = noop;
+var /* istanbul ignore next */ badIstanbul = (test)=>{
+    const { value  } = test, pixelParams = swcHelpers.objectWithoutProperties(test, [
+        "value"
+    ]);
+    console.log('fail');
+};
+exports.badIstanbul = badIstanbul;
+/* istanbul ignore next: UI-5137 */ const downloadDocument = ()=>{
+    console.log('fail');
+};
+exports.downloadDocument = downloadDocument;
diff --git a/crates/swc/tests/fixture/issue-2964.case-3/input/.swcrc b/crates/swc/tests/fixture/issue-2964.case-3/input/.swcrc
new file mode 100644
index 0000000000..456db5699c
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-3/input/.swcrc
@@ -0,0 +1,32 @@
+{
+  "sourceMaps": false,
+  "module": {
+    "type": "commonjs"
+  },
+  "jsc": {
+    "parser": {
+      "syntax": "typescript",
+      "tsx": false,
+      "dynamicImport": true,
+      "decorators": false
+    },
+    "transform": {
+      "react": {
+        "pragma": "React.createElement",
+        "pragmaFrag": "React.Fragment",
+        "throwIfNamespace": true,
+        "development": false,
+        "useBuiltins": false
+      },
+      "hidden": {
+        "jest": true
+      }
+    },
+    "target": "es2015",
+    "loose": false,
+    "externalHelpers": false,
+    "keepClassNames": false,
+    "preserveAllComments": true
+  },
+  "minify": false
+}
diff --git a/crates/swc/tests/fixture/issue-2964.case-3/input/index.ts b/crates/swc/tests/fixture/issue-2964.case-3/input/index.ts
new file mode 100644
index 0000000000..3d60d82360
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-3/input/index.ts
@@ -0,0 +1,7 @@
+// single line comment
+const x = ({y, ...rest}: /*todo: refactor any type*/ any) => {
+    return {
+        y, // another comment
+        z: rest.z // final comment
+    }
+}
diff --git a/crates/swc/tests/fixture/issue-2964.case-3/output/index.ts b/crates/swc/tests/fixture/issue-2964.case-3/output/index.ts
new file mode 100644
index 0000000000..1c8fbfd6c3
--- /dev/null
+++ b/crates/swc/tests/fixture/issue-2964.case-3/output/index.ts
@@ -0,0 +1,14 @@
+"use strict";
+var swcHelpers = require("@swc/helpers");
+// single line comment
+const x = (_param)=>/*todo: refactor any type*/ {
+    var { y  } = _param, rest = swcHelpers.objectWithoutProperties(_param, [
+        "y"
+    ]);
+    return {
+        y,
+        // another comment
+        z: rest.z
+    };
+} // final comment
+;
diff --git a/crates/swc/tests/rust_api.rs b/crates/swc/tests/rust_api.rs
index b957d02eda..a15bd34adf 100644
--- a/crates/swc/tests/rust_api.rs
+++ b/crates/swc/tests/rust_api.rs
@@ -149,6 +149,7 @@ fn shopify_2_same_opt() {
                     experimental: Default::default(),
                     lints: Default::default(),
                     assumptions: Default::default(),
+                    preserve_all_comments: false,
                 },
                 module: None,
                 minify: false,
diff --git a/node-swc/__tests__/preserve_comments.mjs b/node-swc/__tests__/preserve_comments.mjs
new file mode 100644
index 0000000000..cb81809db6
--- /dev/null
+++ b/node-swc/__tests__/preserve_comments.mjs
@@ -0,0 +1,51 @@
+import swc from "../..";
+import path from "path";
+import {fileURLToPath} from "url";
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+
+describe("Should preserve comments", () => {
+    it("Should preserve comments preceding types", async () => {
+        const input = `/*comment*/ type X = number; const x: X = 1`;
+        const result = swc.transformSync(input, {
+            "jsc": {
+                "parser": {
+                    "syntax": "typescript",
+                },
+                "preserveAllComments": true
+            }
+        });
+        expect(result.code).toBe('/*comment*/ var x = 1;\n');
+    });
+
+    it("Should preserve comments preceding shifted functions", () => {
+        const filename = path.resolve(
+            __dirname + "/../tests/issue-2964/input1.ts"
+        );
+
+        const {code} = swc.transformFileSync(filename);
+
+        expect(code).toContain("/* input 1 comment 1 */ var tail =")
+        expect(code).toContain(`// input 1 comment 2\nvar saysHello =`)
+    });
+
+    it("Should not share comments between modules", () => {
+        const filename1 = path.resolve(
+            __dirname + "/../tests/issue-2964/input1.ts"
+        );
+        const filename2 = path.resolve(
+            __dirname + "/../tests/issue-2964/input2.ts"
+        );
+
+        swc.transformFileSync(filename1);
+
+        const result2 = swc.transformFileSync(filename2);
+        const result1 = swc.transformFileSync(filename1);
+
+        expect(result1.code).toMatch("input 1");
+        expect(result1.code).not.toMatch("input 2");
+
+        expect(result2.code).toMatch("input 2");
+        expect(result2.code).not.toMatch("input 1")
+    });
+})
\ No newline at end of file
diff --git a/node-swc/tests/issue-2964/.swcrc b/node-swc/tests/issue-2964/.swcrc
new file mode 100644
index 0000000000..6aac1d3f4f
--- /dev/null
+++ b/node-swc/tests/issue-2964/.swcrc
@@ -0,0 +1,36 @@
+{
+  "sourceMaps": true,
+  "module": {
+    "type": "commonjs"
+  },
+  "jsc": {
+    "preserveAllComments": true,
+    "parser": {
+      "syntax": "typescript",
+      "tsx": false,
+      "dynamicImport": true,
+      "decorators": false
+    },
+    "transform": {
+      "react": {
+        "pragma": "React.createElement",
+        "pragmaFrag": "React.Fragment",
+        "throwIfNamespace": true,
+        "development": false,
+        "useBuiltins": false
+      },
+      "hidden": {
+        "jest": true
+      }
+    },
+    "target": "es3",
+    "loose": false,
+    "externalHelpers": false,
+    "keepClassNames": false,
+    "minify": {
+      "compress": false,
+      "mangle": false
+    }
+  },
+  "minify": false
+}
\ No newline at end of file
diff --git a/node-swc/tests/issue-2964/input1.ts b/node-swc/tests/issue-2964/input1.ts
new file mode 100644
index 0000000000..fdacbd6c8e
--- /dev/null
+++ b/node-swc/tests/issue-2964/input1.ts
@@ -0,0 +1,5 @@
+/* input 1 comment 1 */
+const tail = <T extends Array<unknown>>([_, ...tail]: T) => tail;
+
+// input 1 comment 2
+export const saysHello = () => console.log("hello");
\ No newline at end of file
diff --git a/node-swc/tests/issue-2964/input2.ts b/node-swc/tests/issue-2964/input2.ts
new file mode 100644
index 0000000000..d84c490ecb
--- /dev/null
+++ b/node-swc/tests/issue-2964/input2.ts
@@ -0,0 +1,7 @@
+// input 2 comment 1
+type Z = number;
+const x = 1;
+
+/* input 2 comment 2 */
+type Y = string;
+const a = "";