fix(migrate): transfer comments to the separator (#7014)

Author: siketyan

.changeset/grumpy-crews-sneeze.md

@@ -0,0 +1,5 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#6516](https://github.com/biomejs/biome/issues/6516): The `biome migrate` command no longer break the member list with trailing comments.

crates/biome_analyze/src/utils.rs

@@ -1,6 +1,6 @@
 use biome_rowan::{
     AstNode, AstSeparatedElement, AstSeparatedList, Language, SyntaxError, SyntaxNode, SyntaxToken,
-    chain_trivia_pieces,
+    chain_trivia_pieces, trim_trailing_trivia_pieces,
 };
 
 /// Returns `true` if `list` is sorted by `get_key`.
@@ -139,11 +139,17 @@ pub fn fix_separators<'a, L: Language + 'a, N: AstNode<Language = L> + 'a>(
             }
         } else if i != last_index || needs_last_separator {
             // The last node is moved and has no trailing separator.
-            // Thus we build a new separator and remove its trailing whitespaces.
-            if let Some(new_node) = node.clone().trim_trailing_trivia() {
+            // Thus we build a new separator and remove its trailing trivia.
+            *optional_separator = Some(match node.syntax().last_trailing_trivia() {
+                // Transfer the trailing trivia to the separator
+                Some(trivia) => make_separator()
+                    .append_trivia_pieces(trim_trailing_trivia_pieces(trivia.pieces())),
+                _ => make_separator(),
+            });
+
+            if let Some(new_node) = node.clone().with_trailing_trivia_pieces([]) {
                 *node = new_node;
             }
-            *optional_separator = Some(make_separator());
         }
     }
 }

crates/biome_migrate/tests/specs/migrations/ruleMover/issue6516.jsonc

@@ -0,0 +1,131 @@
+{
+  "$schema": "https://biomejs.dev/schemas/1.9.4/schema.json",
+  "vcs": {
+    "enabled": false,
+    "clientKind": "git",
+    "useIgnoreFile": true,
+    "defaultBranch": "beta"
+  },
+  "formatter": {
+    "enabled": false, // TODO: enable when https://github.com/biomejs/biome/issues/5879 is implemented
+    "useEditorconfig": true,
+    "indentStyle": "space",
+    "lineWidth": 120
+  },
+  "files": {
+    "ignoreUnknown": true,
+    // Adding folders to the ignore list is GREAT for performance because it prevents biome from descending into them
+    // and having to verify whether each individual file is ignored
+    "ignore": [
+      "**/*.d.ts",
+      ".github/*",
+      ".vscode/*",
+      "build/*",
+      "coverage/*",
+      "dist/*",
+      "docs/*",
+      "node_modules/*",
+      "public/*",
+      "typedoc/*",
+      "src/data/tms.ts" // this file is just too big
+    ]
+  },
+
+  "organizeImports": { "enabled": false }, // TODO: Enable in Biome 2.0
+  "linter": {
+    "enabled": true,
+    "rules": {
+      "recommended": true, // see https://biomejs.dev/linter/rules/#recommended-rules
+      "correctness": {
+        "noUndeclaredVariables": "off", // handled by TypeScript
+        "noUnusedVariables": "error",
+        "noSwitchDeclarations": "error",
+        "noVoidTypeReturn": "warn", // TODO: Refactor and make this an error
+        "noUnusedImports": "off" // Handled by ESLint until multi-line disable is added (in Biome 2.0)
+      },
+      "style": {
+        "noVar": "error",
+        "useEnumInitializers": "off", // large enums like Moves/Species would make this cumbersome
+        "useBlockStatements": "error",
+        "useConst": "error",
+        "useImportType": "error",
+        "noNonNullAssertion": "off", // TODO: fix all non-null assertions in non-test files and turn this on
+        "noParameterAssign": "off", // TODO: add this?
+        "useExponentiationOperator": "off", // Too typo-prone and easy to mixup with standard multiplication (* vs **)
+        "useDefaultParameterLast": "error",
+        "useSingleVarDeclarator": "off",
+        "useNodejsImportProtocol": "error",
+        "useTemplate": "off", // string concatenation is faster: https://stackoverflow.com/questions/29055518/are-es6-template-literals-faster-than-string-concatenation
+        "noNamespaceImport": "error",
+        "noInferrableTypes": "off", // Explicit > Implicit
+        "useConsistentArrayType": "error", // Standardize on `T[]`
+        "useFilenamingConvention": {
+          "level": "error",
+          "options": {
+            "filenameCases": ["kebab-case"],
+            "requireAscii": true,
+            "strictCase": false
+          }
+        }
+      },
+      "suspicious": {
+        "noDoubleEquals": "error",
+        "noExplicitAny": "off", // TODO: Refactor and make this an error
+        "noAssignInExpressions": "warn",
+        "noPrototypeBuiltins": "error",
+        "noFallthroughSwitchClause": "error", // Prevents accidental automatic fallthroughs in switch cases (use disable comment if needed)
+        "noImplicitAnyLet": "warn", // TODO: Refactor and make this an error
+        "noRedeclare": "error",
+        "noGlobalIsNan": "error",
+        "noAsyncPromiseExecutor": "warn", // TODO: Refactor and make this an error
+        "useDefaultSwitchClauseLast": "warn" // TODO: make error?
+      },
+      "complexity": {
+        "noExcessiveCognitiveComplexity": "info", // TODO: Refactor and make this an error
+        "useLiteralKeys": "off",
+        "noForEach": "off", // Foreach vs for of is not that simple.
+        "noUselessSwitchCase": "off", // Explicit > Implicit
+        "noUselessConstructor": "error",
+        "noBannedTypes": "warn", // TODO: refactor and make an error
+        "useRegexLiterals": "off" // Was broken for some reason; TODO: re-enable
+      },
+      "nursery": {
+        "noCommonJs": "error"
+      }
+    }
+  },
+  "javascript": {
+    "formatter": { "quoteStyle": "double", "arrowParentheses": "always" }
+  },
+  "overrides": [
+    {
+      "include": ["test/**"],
+      "javascript": { "globals": [] },
+      "linter": {
+        "rules": {
+          "performance": {
+            "noDelete": "off" // TODO: evaluate if this is necessary for the test(s) to function
+          },
+          "style": {
+            "noNamespaceImport": "off", // this is required for `vi.spyOn` to work in some tests
+            "noNonNullAssertion": "off" // tests are able to make assumptions about the game state that normally can't be made
+          }
+        }
+      }
+    },
+    {
+      // This section is to improve the bug tester workflow
+      "include": ["src/overrides.ts"],
+      "linter": {
+        "rules": {
+          "correctness": {
+            "noUnusedImports": "off"
+          },
+          "style": {
+            "useImportType": "off"
+          }
+        }
+      }
+    }
+  ]
+}