From 87ddd1f04d2b71aadb3a8a5fb3e4ac15988947c1 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 18 Sep 2023 17:39:28 +0100 Subject: [PATCH] fix(lsp): restore tsc's quick fix ordering (#20545) --- cli/lsp/analysis.rs | 34 ++---- cli/tests/integration/lsp_tests.rs | 188 ++++++++++++++--------------- 2 files changed, 107 insertions(+), 115 deletions(-) diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 808f09dec0..c1def60128 100644 --- a/cli/lsp/analysis.rs +++ b/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) { - (CodeActionKind::Deno(a), CodeActionKind::Deno(b)) => { - 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 + let (tsc, rest): (Vec<_>, Vec<_>) = self + .actions .into_iter() - .map(|i| match i { - CodeActionKind::Tsc(c, _) => lsp::CodeActionOrCommand::CodeAction(c), + .partition(|a| matches!(a, CodeActionKind::Tsc(..))); + 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() } diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 9390f8f86a..5499c8a6ee 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/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", - "source": "deno-lint", - }, - { - "title": "Disable no-await-in-sync-fn for this line", + "title": "Disable prefer-const 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": { - "specifier": "file:///a/file00.ts", - "fixId": "fixMissingImport" + "edit": { + "documentChanges": [{ + "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": { - "documentChanges": [{ - "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" - }] - }] + "data": { + "specifier": "file:///a/file00.ts", + "fixId": "fixMissingImport" } }]) ); @@ -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",