From 94b588ce6624ae2f2ed648c7b1a479a10513542f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 11 Oct 2024 07:40:17 +0100 Subject: [PATCH] fix(lsp): relative completions for bare import-mapped specifiers (#26137) --- cli/lsp/completions.rs | 130 ++++++++++++++++----------------- tests/integration/lsp_tests.rs | 92 +++++++++++++---------- 2 files changed, 117 insertions(+), 105 deletions(-) diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index a040be5691..1590743b2b 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -200,15 +200,11 @@ pub async fn get_import_completions( { // completions for import map specifiers Some(lsp::CompletionResponse::List(completion_list)) - } else if text.starts_with("./") - || text.starts_with("../") - || text.starts_with('/') + } else if let Some(completion_list) = + get_local_completions(specifier, &text, &range, resolver) { // completions for local relative modules - Some(lsp::CompletionResponse::List(CompletionList { - is_incomplete: false, - items: get_local_completions(specifier, &text, &range, resolver)?, - })) + Some(lsp::CompletionResponse::List(completion_list)) } else if !text.is_empty() { // completion of modules from a module registry or cache check_auto_config_registry( @@ -363,15 +359,15 @@ fn get_local_completions( text: &str, range: &lsp::Range, resolver: &LspResolver, -) -> Option> { +) -> Option { if base.scheme() != "file" { return None; } - let parent = base.join(text).ok()?.join(".").ok()?; + let parent = &text[..text.char_indices().rfind(|(_, c)| *c == '/')?.0 + 1]; let resolved_parent = resolver .as_graph_resolver(Some(base)) .resolve( - parent.as_str(), + parent, &Range { specifier: base.clone(), start: deno_graph::Position::zeroed(), @@ -381,62 +377,62 @@ fn get_local_completions( ) .ok()?; let resolved_parent_path = url_to_file_path(&resolved_parent).ok()?; - let raw_parent = - &text[..text.char_indices().rfind(|(_, c)| *c == '/')?.0 + 1]; if resolved_parent_path.is_dir() { let cwd = std::env::current_dir().ok()?; - let items = std::fs::read_dir(resolved_parent_path).ok()?; - Some( - items - .filter_map(|de| { - let de = de.ok()?; - let label = de.path().file_name()?.to_string_lossy().to_string(); - let entry_specifier = resolve_path(de.path().to_str()?, &cwd).ok()?; - if entry_specifier == *base { - return None; - } - let full_text = format!("{raw_parent}{label}"); - let text_edit = Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: *range, - new_text: full_text.clone(), - })); - let filter_text = Some(full_text); - match de.file_type() { - Ok(file_type) if file_type.is_dir() => Some(lsp::CompletionItem { - label, - kind: Some(lsp::CompletionItemKind::FOLDER), - detail: Some("(local)".to_string()), - filter_text, - sort_text: Some("1".to_string()), - text_edit, - commit_characters: Some( - IMPORT_COMMIT_CHARS.iter().map(|&c| c.into()).collect(), - ), - ..Default::default() - }), - Ok(file_type) if file_type.is_file() => { - if is_importable_ext(&de.path()) { - Some(lsp::CompletionItem { - label, - kind: Some(lsp::CompletionItemKind::FILE), - detail: Some("(local)".to_string()), - filter_text, - sort_text: Some("1".to_string()), - text_edit, - commit_characters: Some( - IMPORT_COMMIT_CHARS.iter().map(|&c| c.into()).collect(), - ), - ..Default::default() - }) - } else { - None - } + let entries = std::fs::read_dir(resolved_parent_path).ok()?; + let items = entries + .filter_map(|de| { + let de = de.ok()?; + let label = de.path().file_name()?.to_string_lossy().to_string(); + let entry_specifier = resolve_path(de.path().to_str()?, &cwd).ok()?; + if entry_specifier == *base { + return None; + } + let full_text = format!("{parent}{label}"); + let text_edit = Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: *range, + new_text: full_text.clone(), + })); + let filter_text = Some(full_text); + match de.file_type() { + Ok(file_type) if file_type.is_dir() => Some(lsp::CompletionItem { + label, + kind: Some(lsp::CompletionItemKind::FOLDER), + detail: Some("(local)".to_string()), + filter_text, + sort_text: Some("1".to_string()), + text_edit, + commit_characters: Some( + IMPORT_COMMIT_CHARS.iter().map(|&c| c.into()).collect(), + ), + ..Default::default() + }), + Ok(file_type) if file_type.is_file() => { + if is_importable_ext(&de.path()) { + Some(lsp::CompletionItem { + label, + kind: Some(lsp::CompletionItemKind::FILE), + detail: Some("(local)".to_string()), + filter_text, + sort_text: Some("1".to_string()), + text_edit, + commit_characters: Some( + IMPORT_COMMIT_CHARS.iter().map(|&c| c.into()).collect(), + ), + ..Default::default() + }) + } else { + None } - _ => None, } - }) - .collect(), - ) + _ => None, + } + }) + .collect(); + Some(CompletionList { + is_incomplete: false, + items, + }) } else { None } @@ -921,11 +917,11 @@ mod tests { }, }, &Default::default(), - ); - assert!(actual.is_some()); - let actual = actual.unwrap(); - assert_eq!(actual.len(), 3); - for item in actual { + ) + .unwrap(); + assert!(!actual.is_incomplete); + assert_eq!(actual.items.len(), 3); + for item in actual.items { match item.text_edit { Some(lsp::CompletionTextEdit::Edit(text_edit)) => { assert!(["./b", "./f.mjs", "./g.json"] diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 19ea89a54b..0f2d437558 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -1342,6 +1342,7 @@ fn lsp_import_map_import_completions() { "/#/": "./src/", "fs": "https://example.com/fs/index.js", "std/": "https://example.com/std@0.123.0/", + "lib/": "./lib/", }, "scopes": { "file:///": { @@ -1364,17 +1365,18 @@ fn lsp_import_map_import_completions() { "uri": uri, "languageId": "typescript", "version": 1, - "text": "import * as a from \"/~/b.ts\";\nimport * as b from \"\"" - } + "text": r#" + import * as b from ""; + import * as b from "/~/"; + import * as b from "lib/"; + "#, + }, })); let res = client.get_completion( &uri, - (1, 20), - json!({ - "triggerKind": 2, - "triggerCharacter": "\"" - }), + (1, 28), + json!({ "triggerKind": 2, "triggerCharacter": "\"" }), ); assert_eq!( json!(res), @@ -1409,6 +1411,13 @@ fn lsp_import_map_import_completions() { "sortText": "std", "insertText": "std", "commitCharacters": ["\"", "'"], + }, { + "label": "lib", + "kind": 19, + "detail": "(import map)", + "sortText": "lib", + "insertText": "lib", + "commitCharacters": ["\"", "'"], }, { "label": "fs", "kind": 17, @@ -1435,32 +1444,10 @@ fn lsp_import_map_import_completions() { }) ); - client.write_notification( - "textDocument/didChange", - json!({ - "textDocument": { - "uri": uri, - "version": 2 - }, - "contentChanges": [ - { - "range": { - "start": { "line": 1, "character": 20 }, - "end": { "line": 1, "character": 20 } - }, - "text": "/~/" - } - ] - }), - ); - let res = client.get_completion( - uri, - (1, 23), - json!({ - "triggerKind": 2, - "triggerCharacter": "/" - }), + &uri, + (2, 31), + json!({ "triggerKind": 2, "triggerCharacter": "/" }), ); assert_eq!( json!(res), @@ -1475,15 +1462,44 @@ fn lsp_import_map_import_completions() { "filterText": "/~/b.ts", "textEdit": { "range": { - "start": { "line": 1, "character": 20 }, - "end": { "line": 1, "character": 23 } + "start": { "line": 2, "character": 28 }, + "end": { "line": 2, "character": 31 }, }, - "newText": "/~/b.ts" + "newText": "/~/b.ts", }, "commitCharacters": ["\"", "'"], - } - ] - }) + }, + ], + }), + ); + + let res = client.get_completion( + &uri, + (3, 32), + json!({ "triggerKind": 2, "triggerCharacter": "/" }), + ); + assert_eq!( + json!(res), + json!({ + "isIncomplete": false, + "items": [ + { + "label": "b.ts", + "kind": 17, + "detail": "(local)", + "sortText": "1", + "filterText": "lib/b.ts", + "textEdit": { + "range": { + "start": { "line": 3, "character": 28 }, + "end": { "line": 3, "character": 32 }, + }, + "newText": "lib/b.ts", + }, + "commitCharacters": ["\"", "'"], + }, + ], + }), ); client.shutdown();