From a45a40533eb25c5a12df98189338320f8aa6dcc4 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 23 Jul 2024 19:39:14 +0100 Subject: [PATCH] fix(lsp): rewrite import for 'infer return type' action (#24685) --- Cargo.lock | 4 +- cli/lsp/analysis.rs | 8 +-- cli/lsp/language_server.rs | 18 ++++++- cli/lsp/tsc.rs | 2 +- tests/integration/lsp_tests.rs | 98 ++++++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 912f81bafe..b38172a069 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7541,9 +7541,9 @@ dependencies = [ [[package]] name = "v8" -version = "0.98.0" +version = "0.98.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "feb252d5be11c32cb4755d66d58db30ff30af5f1f17183e83ff54383a402c5f6" +checksum = "d03f42deef61349d31ae100e7bcdcc5d9293c1126cb8aff8fd56ba3cba18340b" dependencies = [ "bindgen", "bitflags 2.5.0", diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 8480f6e1de..a6a69cbf0e 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -73,8 +73,9 @@ static PREFERRED_FIXES: Lazy> = .collect() }); -static IMPORT_SPECIFIER_RE: Lazy = - lazy_regex::lazy_regex!(r#"\sfrom\s+["']([^"']*)["']"#); +static IMPORT_SPECIFIER_RE: Lazy = lazy_regex::lazy_regex!( + r#"\sfrom\s+["']([^"']*)["']|import\s*\(\s*["']([^"']*)["']\s*\)"# +); const SUPPORTED_EXTENSIONS: &[&str] = &[ ".ts", ".tsx", ".js", ".jsx", ".mjs", ".mts", ".cjs", ".cts", ".d.ts", @@ -528,7 +529,8 @@ pub fn fix_ts_import_changes( .map(|line| { // This assumes that there's only one import per line. if let Some(captures) = IMPORT_SPECIFIER_RE.captures(line) { - let specifier = captures.get(1).unwrap().as_str(); + let specifier = + captures.iter().skip(1).find_map(|s| s).unwrap().as_str(); if let Some(new_specifier) = import_mapper.check_unresolved_specifier(specifier, referrer) { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index aa1b4c282f..9680c63f9d 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1810,7 +1810,10 @@ impl Inner { LspError::internal_error() })?; code_action - } else if kind.as_str().starts_with(CodeActionKind::REFACTOR.as_str()) { + } else if let Some(kind_suffix) = kind + .as_str() + .strip_prefix(CodeActionKind::REFACTOR.as_str()) + { let mut code_action = params; let action_data: refactor::RefactorCodeActionData = from_value(data) .map_err(|err| { @@ -1819,7 +1822,7 @@ impl Inner { })?; let asset_or_doc = self.get_asset_or_document(&action_data.specifier)?; let line_index = asset_or_doc.line_index(); - let refactor_edit_info = self + let mut refactor_edit_info = self .ts_server .get_edits_for_refactor( self.snapshot(), @@ -1841,6 +1844,17 @@ impl Inner { asset_or_doc.scope().cloned(), ) .await?; + if kind_suffix == ".rewrite.function.returnType" { + refactor_edit_info.edits = fix_ts_import_changes( + &action_data.specifier, + &refactor_edit_info.edits, + &self.get_ts_response_import_mapper(&action_data.specifier), + ) + .map_err(|err| { + error!("Unable to remap changes: {:#}", err); + LspError::internal_error() + })? + } code_action.edit = refactor_edit_info.to_workspace_edit(self)?; code_action } else { diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 6c808a5e40..4592edad40 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2944,7 +2944,7 @@ pub fn file_text_changes_to_workspace_edit( #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct RefactorEditInfo { - edits: Vec, + pub edits: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub rename_location: Option, } diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 8034bb6837..cfcf72ddce 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -7373,6 +7373,104 @@ fn lsp_npm_completions_auto_import_and_quick_fix_no_import_map() { client.shutdown(); } +#[test] +fn lsp_infer_return_type() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.write("deno.json", json!({}).to_string()); + let types_file = source_file( + temp_dir.path().join("types.d.ts"), + r#" + export interface SomeInterface { + someField: number; + } + declare global { + export function someFunction(): SomeInterface; + } + "#, + ); + let file = source_file( + temp_dir.path().join("file.ts"), + r#" + function foo() { + return someFunction(); + } + foo(); + "#, + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { "uri": file.uri() }, + "range": { + "start": { "line": 1, "character": 15 }, + "end": { "line": 1, "character": 18 }, + }, + "context": { + "diagnostics": [], + "only": ["refactor.rewrite.function.returnType"], + } + }), + ); + assert_eq!( + &res, + &json!([ + { + "title": "Infer function return type", + "kind": "refactor.rewrite.function.returnType", + "isPreferred": false, + "data": { + "specifier": file.uri(), + "range": { + "start": { "line": 1, "character": 15 }, + "end": { "line": 1, "character": 18 }, + }, + "refactorName": "Infer function return type", + "actionName": "Infer function return type", + }, + } + ]), + ); + let code_action = res.as_array().unwrap().first().unwrap(); + let res = client.write_request("codeAction/resolve", code_action); + assert_eq!( + &res, + &json!({ + "title": "Infer function return type", + "kind": "refactor.rewrite.function.returnType", + "isPreferred": false, + "data": { + "specifier": file.uri(), + "range": { + "start": { "line": 1, "character": 15 }, + "end": { "line": 1, "character": 18 }, + }, + "refactorName": "Infer function return type", + "actionName": "Infer function return type", + }, + "edit": { + "documentChanges": [ + { + "textDocument": { "uri": file.uri(), "version": null }, + "edits": [ + { + "range": { + "start": { "line": 1, "character": 20 }, + "end": { "line": 1, "character": 20 }, + }, + "newText": format!(": import(\"{}\").SomeInterface", types_file.uri()), + }, + ], + }, + ], + }, + }), + ); + client.shutdown(); +} + // Regression test for https://github.com/denoland/deno/issues/23895. #[test] fn lsp_npm_types_nested_js_dts() {