Skip to content

Commit

Permalink
fix(lsp): rewrite imports for 'Move to a new file' action (#27427)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn authored Dec 20, 2024
1 parent 3c147d6 commit feb94d0
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 33 deletions.
16 changes: 11 additions & 5 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,26 +603,32 @@ fn try_reverse_map_package_json_exports(
/// For a set of tsc changes, can them for any that contain something that looks
/// like an import and rewrite the import specifier to include the extension
pub fn fix_ts_import_changes(
referrer: &ModuleSpecifier,
resolution_mode: ResolutionMode,
changes: &[tsc::FileTextChanges],
language_server: &language_server::Inner,
) -> Result<Vec<tsc::FileTextChanges>, AnyError> {
let import_mapper = language_server.get_ts_response_import_mapper(referrer);
let mut r = Vec::new();
for change in changes {
let Ok(referrer) = ModuleSpecifier::parse(&change.file_name) else {
continue;
};
let referrer_doc = language_server.get_asset_or_document(&referrer).ok();
let resolution_mode = referrer_doc
.as_ref()
.map(|d| d.resolution_mode())
.unwrap_or(ResolutionMode::Import);
let import_mapper =
language_server.get_ts_response_import_mapper(&referrer);
let mut text_changes = Vec::new();
for text_change in &change.text_changes {
let lines = text_change.new_text.split('\n');

let new_lines: Vec<String> = lines
.map(|line| {
// This assumes that there's only one import per line.
if let Some(captures) = IMPORT_SPECIFIER_RE.captures(line) {
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, resolution_mode)
.check_unresolved_specifier(specifier, &referrer, resolution_mode)
{
line.replace(specifier, &new_specifier)
} else {
Expand Down
7 changes: 7 additions & 0 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,13 @@ impl AssetOrDocument {
pub fn document_lsp_version(&self) -> Option<i32> {
self.document().and_then(|d| d.maybe_lsp_version())
}

pub fn resolution_mode(&self) -> ResolutionMode {
match self {
AssetOrDocument::Asset(_) => ResolutionMode::Import,
AssetOrDocument::Document(d) => d.resolution_mode(),
}
}
}

type ModuleResult = Result<deno_graph::JsModule, deno_graph::ModuleGraphError>;
Expand Down
44 changes: 16 additions & 28 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1855,20 +1855,12 @@ impl Inner {
}

let changes = if code_action_data.fix_id == "fixMissingImport" {
fix_ts_import_changes(
&code_action_data.specifier,
maybe_asset_or_doc
.as_ref()
.and_then(|d| d.document())
.map(|d| d.resolution_mode())
.unwrap_or(ResolutionMode::Import),
&combined_code_actions.changes,
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
LspError::internal_error()
})?
fix_ts_import_changes(&combined_code_actions.changes, self).map_err(
|err| {
error!("Unable to remap changes: {:#}", err);
LspError::internal_error()
},
)?
} else {
combined_code_actions.changes
};
Expand Down Expand Up @@ -1912,20 +1904,16 @@ 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,
asset_or_doc
.document()
.map(|d| d.resolution_mode())
.unwrap_or(ResolutionMode::Import),
&refactor_edit_info.edits,
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
LspError::internal_error()
})?
if kind_suffix == ".rewrite.function.returnType"
|| kind_suffix == ".move.newFile"
{
refactor_edit_info.edits =
fix_ts_import_changes(&refactor_edit_info.edits, self).map_err(
|err| {
error!("Unable to remap changes: {:#}", err);
LspError::internal_error()
},
)?
}
code_action.edit = refactor_edit_info.to_workspace_edit(self)?;
code_action
Expand Down
113 changes: 113 additions & 0 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6066,6 +6066,119 @@ fn lsp_jsr_code_action_missing_declaration() {
);
}

#[test]
fn lsp_jsr_code_action_move_to_new_file() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.build();
let temp_dir = context.temp_dir();
let file = source_file(
temp_dir.path().join("file.ts"),
r#"
import { someFunction } from "jsr:@denotest/types-file";
export const someValue = someFunction();
"#,
);
let mut client = context.new_lsp_command().build();
client.initialize_default();
client.write_request(
"workspace/executeCommand",
json!({
"command": "deno.cache",
"arguments": [[], file.url()],
}),
);
client.did_open_file(&file);
let list = client
.write_request_with_res_as::<Option<lsp::CodeActionResponse>>(
"textDocument/codeAction",
json!({
"textDocument": { "uri": file.url() },
"range": {
"start": { "line": 2, "character": 19 },
"end": { "line": 2, "character": 28 },
},
"context": { "diagnostics": [] },
}),
)
.unwrap();
let action = list
.iter()
.find_map(|c| match c {
lsp::CodeActionOrCommand::CodeAction(a)
if &a.title == "Move to a new file" =>
{
Some(a)
}
_ => None,
})
.unwrap();
let res = client.write_request("codeAction/resolve", json!(action));
assert_eq!(
res,
json!({
"title": "Move to a new file",
"kind": "refactor.move.newFile",
"edit": {
"documentChanges": [
{
"textDocument": { "uri": file.url(), "version": 1 },
"edits": [
{
"range": {
"start": { "line": 1, "character": 6 },
"end": { "line": 2, "character": 0 },
},
"newText": "",
},
{
"range": {
"start": { "line": 2, "character": 0 },
"end": { "line": 3, "character": 4 },
},
"newText": "",
},
],
},
{
"kind": "create",
"uri": file.url().join("someValue.ts").unwrap(),
"options": {
"ignoreIfExists": true,
},
},
{
"textDocument": {
"uri": file.url().join("someValue.ts").unwrap(),
"version": null,
},
"edits": [
{
"range": {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 0 },
},
"newText": "import { someFunction } from \"jsr:@denotest/types-file\";\n\nexport const someValue = someFunction();\n",
},
],
},
],
},
"isPreferred": false,
"data": {
"specifier": file.url(),
"range": {
"start": { "line": 2, "character": 19 },
"end": { "line": 2, "character": 28 },
},
"refactorName": "Move to a new file",
"actionName": "Move to a new file",
},
}),
);
}

#[test]
fn lsp_code_actions_deno_cache_npm() {
let context = TestContextBuilder::new().use_temp_cwd().build();
Expand Down

0 comments on commit feb94d0

Please sign in to comment.