fix(lsp): restore tsc's quick fix ordering (#20545)

cli/lsp/analysis.rs

@@ -848,31 +848,23 @@ impl CodeActionCollection {
 
   /// Move out the code actions and return them as a `CodeActionResponse`.
   pub fn get_response(self) -> lsp::CodeActionResponse {
-    let mut actions = self.actions;
-
     // Prefer TSC fixes first, then Deno fixes, then Deno lint fixes.
-    actions.sort_by(|a, b| match (a, b) {
+    let (tsc, rest): (Vec<_>, Vec<_>) = self
-      (CodeActionKind::Deno(a), CodeActionKind::Deno(b)) => {
+      .actions
-        a.title.cmp(&b.title)
-      }
-      (CodeActionKind::DenoLint(a), CodeActionKind::DenoLint(b)) => {
-        a.title.cmp(&b.title)
-      }
-      (CodeActionKind::Tsc(a, _), CodeActionKind::Tsc(b, _)) => {
-        a.title.cmp(&b.title)
-      }
-      (CodeActionKind::Tsc(_, _), _) => Ordering::Less,
-      (CodeActionKind::Deno(_), CodeActionKind::Tsc(_, _)) => Ordering::Greater,
-      (CodeActionKind::Deno(_), CodeActionKind::DenoLint(_)) => Ordering::Less,
-      (CodeActionKind::DenoLint(_), _) => Ordering::Greater,
-    });
-
-    actions
       .into_iter()
-      .map(|i| match i {
+      .partition(|a| matches!(a, CodeActionKind::Tsc(..)));
-        CodeActionKind::Tsc(c, _) => lsp::CodeActionOrCommand::CodeAction(c),
+    let (deno, deno_lint): (Vec<_>, Vec<_>) = rest
+      .into_iter()
+      .partition(|a| matches!(a, CodeActionKind::Deno(_)));
+
+    tsc
+      .into_iter()
+      .chain(deno)
+      .chain(deno_lint)
+      .map(|k| match k {
         CodeActionKind::Deno(c) => lsp::CodeActionOrCommand::CodeAction(c),
         CodeActionKind::DenoLint(c) => lsp::CodeActionOrCommand::CodeAction(c),
+        CodeActionKind::Tsc(c, _) => lsp::CodeActionOrCommand::CodeAction(c),
       })
       .collect()
   }

cli/tests/integration/lsp_tests.rs

@@ -4045,24 +4045,6 @@ fn lsp_code_actions() {
   assert_eq!(
     res,
     json!([{
-      "title": "Add all missing 'async' modifiers",
-      "kind": "quickfix",
-      "diagnostics": [{
-        "range": {
-          "start": { "line": 1, "character": 2 },
-          "end": { "line": 1, "character": 7 }
-        },
-        "severity": 1,
-        "code": 1308,
-        "source": "deno-ts",
-        "message": "'await' expressions are only allowed within async functions and at the top levels of modules.",
-        "relatedInformation": []
-      }],
-      "data": {
-        "specifier": "file:///a/file.ts",
-        "fixId": "fixAwaitInSyncFunction"
-      }
-    }, {
       "title": "Add async modifier to containing function",
       "kind": "quickfix",
       "diagnostics": [{
@@ -4097,6 +4079,24 @@ fn lsp_code_actions() {
           }]
         }]
       }
+    }, {
+      "title": "Add all missing 'async' modifiers",
+      "kind": "quickfix",
+      "diagnostics": [{
+        "range": {
+          "start": { "line": 1, "character": 2 },
+          "end": { "line": 1, "character": 7 }
+        },
+        "severity": 1,
+        "code": 1308,
+        "source": "deno-ts",
+        "message": "'await' expressions are only allowed within async functions and at the top levels of modules.",
+        "relatedInformation": []
+      }],
+      "data": {
+        "specifier": "file:///a/file.ts",
+        "fixId": "fixAwaitInSyncFunction"
+      }
     }])
   );
   let res = client
@@ -4253,11 +4253,7 @@ fn test_lsp_code_actions_ordering() {
         "source": "deno",
       },
       {
-        "title": "Disable no-await-in-sync-fn for the entire file",
+        "title": "Disable prefer-const for this line",
-        "source": "deno-lint",
-      },
-      {
-        "title": "Disable no-await-in-sync-fn for this line",
         "source": "deno-lint",
       },
       {
@@ -4265,11 +4261,15 @@ fn test_lsp_code_actions_ordering() {
         "source": "deno-lint",
       },
       {
-        "title": "Disable prefer-const for this line",
+        "title": "Ignore lint errors for the entire file",
         "source": "deno-lint",
       },
       {
-        "title": "Ignore lint errors for the entire file",
+        "title": "Disable no-await-in-sync-fn for this line",
+        "source": "deno-lint",
+      },
+      {
+        "title": "Disable no-await-in-sync-fn for the entire file",
         "source": "deno-lint",
       },
       {
@@ -4543,7 +4543,7 @@ export class DuckConfig {
   assert_eq!(
     res,
     json!([{
-      "title": "Add all missing imports",
+      "title": "Add import from \"./file02.ts\"",
       "kind": "quickfix",
       "diagnostics": [{
         "range": {
@@ -4555,9 +4555,20 @@ export class DuckConfig {
         "source": "deno-ts",
         "message": "Cannot find name 'DuckConfigOptions'."
       }],
-      "data": {
+      "edit": {
-        "specifier": "file:///a/file00.ts",
+        "documentChanges": [{
-        "fixId": "fixMissingImport"
+          "textDocument": {
+            "uri": "file:///a/file00.ts",
+            "version": 1
+          },
+          "edits": [{
+            "range": {
+              "start": { "line": 0, "character": 0 },
+              "end": { "line": 0, "character": 0 }
+            },
+            "newText": "import { DuckConfigOptions } from \"./file02.ts\";\n\n"
+          }]
+        }]
       }
     }, {
       "title": "Add import from \"./file01.ts\"",
@@ -4588,7 +4599,7 @@ export class DuckConfig {
         }]
       }
     }, {
-      "title": "Add import from \"./file02.ts\"",
+      "title": "Add all missing imports",
       "kind": "quickfix",
       "diagnostics": [{
         "range": {
@@ -4600,20 +4611,9 @@ export class DuckConfig {
         "source": "deno-ts",
         "message": "Cannot find name 'DuckConfigOptions'."
       }],
-      "edit": {
+      "data": {
-        "documentChanges": [{
+        "specifier": "file:///a/file00.ts",
-          "textDocument": {
+        "fixId": "fixMissingImport"
-            "uri": "file:///a/file00.ts",
-            "version": 1
-          },
-          "edits": [{
-            "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 }
-            },
-            "newText": "import { DuckConfigOptions } from \"./file02.ts\";\n\n"
-          }]
-        }]
       }
     }])
   );
@@ -8185,31 +8185,6 @@ fn lsp_code_actions_ignore_lint() {
   assert_eq!(
     res,
     json!([{
-      "title": "Disable prefer-const for the entire file",
-      "kind": "quickfix",
-      "diagnostics": [{
-        "range": {
-          "start": { "line": 1, "character": 5 },
-          "end": { "line": 1, "character": 12 }
-        },
-        "severity": 1,
-        "code": "prefer-const",
-        "source": "deno-lint",
-        "message": "'message' is never reassigned\nUse 'const' instead",
-        "relatedInformation": []
-      }],
-      "edit": {
-        "changes": {
-          "file:///a/file.ts": [{
-            "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 }
-            },
-            "newText": "// deno-lint-ignore-file prefer-const\n"
-          }]
-        }
-      }
-    }, {
       "title": "Disable prefer-const for this line",
       "kind": "quickfix",
       "diagnostics": [{
@@ -8234,6 +8209,31 @@ fn lsp_code_actions_ignore_lint() {
           }]
         }
       }
+    }, {
+      "title": "Disable prefer-const for the entire file",
+      "kind": "quickfix",
+      "diagnostics": [{
+        "range": {
+          "start": { "line": 1, "character": 5 },
+          "end": { "line": 1, "character": 12 }
+        },
+        "severity": 1,
+        "code": "prefer-const",
+        "source": "deno-lint",
+        "message": "'message' is never reassigned\nUse 'const' instead",
+        "relatedInformation": []
+      }],
+      "edit": {
+        "changes": {
+          "file:///a/file.ts": [{
+            "range": {
+              "start": { "line": 0, "character": 0 },
+              "end": { "line": 0, "character": 0 }
+            },
+            "newText": "// deno-lint-ignore-file prefer-const\n"
+          }]
+        }
+      }
     }, {
       "title": "Ignore lint errors for the entire file",
       "kind": "quickfix",
@@ -8312,31 +8312,6 @@ console.log(snake_case);
   assert_eq!(
     res,
     json!([{
-      "title": "Disable prefer-const for the entire file",
-      "kind": "quickfix",
-      "diagnostics": [{
-        "range": {
-          "start": { "line": 3, "character": 5 },
-          "end": { "line": 3, "character": 15 }
-        },
-        "severity": 1,
-        "code": "prefer-const",
-        "source": "deno-lint",
-        "message": "'snake_case' is never reassigned\nUse 'const' instead",
-        "relatedInformation": []
-      }],
-      "edit": {
-        "changes": {
-          "file:///a/file.ts": [{
-            "range": {
-              "start": { "line": 1, "character": 34 },
-              "end": { "line": 1, "character": 34 }
-            },
-            "newText": " prefer-const"
-          }]
-        }
-      }
-    }, {
       "title": "Disable prefer-const for this line",
       "kind": "quickfix",
       "diagnostics": [{
@@ -8361,6 +8336,31 @@ console.log(snake_case);
           }]
         }
       }
+    }, {
+      "title": "Disable prefer-const for the entire file",
+      "kind": "quickfix",
+      "diagnostics": [{
+        "range": {
+          "start": { "line": 3, "character": 5 },
+          "end": { "line": 3, "character": 15 }
+        },
+        "severity": 1,
+        "code": "prefer-const",
+        "source": "deno-lint",
+        "message": "'snake_case' is never reassigned\nUse 'const' instead",
+        "relatedInformation": []
+      }],
+      "edit": {
+        "changes": {
+          "file:///a/file.ts": [{
+            "range": {
+              "start": { "line": 1, "character": 34 },
+              "end": { "line": 1, "character": 34 }
+            },
+            "newText": " prefer-const"
+          }]
+        }
+      }
     }, {
       "title": "Ignore lint errors for the entire file",
       "kind": "quickfix",