Skip to content

Commit

Permalink
fix(lsp): include "node:" prefix for node builtin auto-imports (#27404)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn authored Dec 17, 2024
1 parent d632ec9 commit 2820ba1
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 12 deletions.
4 changes: 4 additions & 0 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ impl<'a> TsResponseImportMapper<'a> {
}
}

if specifier.scheme() == "node" {
return Some(specifier.to_string());
}

if let Some(jsr_path) = specifier.as_str().strip_prefix(jsr_url().as_str())
{
let mut segments = jsr_path.split('/');
Expand Down
51 changes: 39 additions & 12 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ use deno_core::OpState;
use deno_core::PollEventLoopOptions;
use deno_core::RuntimeOptions;
use deno_path_util::url_to_file_path;
use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::tokio_util::create_basic_runtime;
use indexmap::IndexMap;
Expand Down Expand Up @@ -3411,10 +3412,18 @@ fn parse_code_actions(
additional_text_edits.extend(change.text_changes.iter().map(|tc| {
let mut text_edit = tc.as_text_edit(asset_or_doc.line_index());
if let Some(specifier_rewrite) = &data.specifier_rewrite {
text_edit.new_text = text_edit.new_text.replace(
&specifier_rewrite.old_specifier,
&specifier_rewrite.new_specifier,
);
let specifier_index = text_edit
.new_text
.char_indices()
.find_map(|(b, c)| (c == '\'' || c == '"').then_some(b));
if let Some(i) = specifier_index {
let mut specifier_part = text_edit.new_text.split_off(i);
specifier_part = specifier_part.replace(
&specifier_rewrite.old_specifier,
&specifier_rewrite.new_specifier,
);
text_edit.new_text.push_str(&specifier_part);
}
if let Some(deno_types_specifier) =
&specifier_rewrite.new_deno_types_specifier
{
Expand Down Expand Up @@ -3587,10 +3596,17 @@ impl CompletionEntryDetails {
&mut insert_replace_edit.new_text
}
};
*new_text = new_text.replace(
&specifier_rewrite.old_specifier,
&specifier_rewrite.new_specifier,
);
let specifier_index = new_text
.char_indices()
.find_map(|(b, c)| (c == '\'' || c == '"').then_some(b));
if let Some(i) = specifier_index {
let mut specifier_part = new_text.split_off(i);
specifier_part = specifier_part.replace(
&specifier_rewrite.old_specifier,
&specifier_rewrite.new_specifier,
);
new_text.push_str(&specifier_part);
}
if let Some(deno_types_specifier) =
&specifier_rewrite.new_deno_types_specifier
{
Expand Down Expand Up @@ -3729,7 +3745,7 @@ pub struct CompletionItemData {
#[serde(rename_all = "camelCase")]
struct CompletionEntryDataAutoImport {
module_specifier: String,
file_name: String,
file_name: Option<String>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -3786,9 +3802,20 @@ impl CompletionEntry {
else {
return;
};
if let Ok(normalized) = specifier_map.normalize(&raw.file_name) {
self.auto_import_data =
Some(CompletionNormalizedAutoImportData { raw, normalized });
if let Some(file_name) = &raw.file_name {
if let Ok(normalized) = specifier_map.normalize(file_name) {
self.auto_import_data =
Some(CompletionNormalizedAutoImportData { raw, normalized });
}
} else if SUPPORTED_BUILTIN_NODE_MODULES
.contains(&raw.module_specifier.as_str())
{
if let Ok(normalized) =
resolve_url(&format!("node:{}", &raw.module_specifier))
{
self.auto_import_data =
Some(CompletionNormalizedAutoImportData { raw, normalized });
}
}
}

Expand Down
67 changes: 67 additions & 0 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7960,6 +7960,73 @@ fn lsp_completions_auto_import() {
client.shutdown();
}

#[test]
fn lsp_completions_auto_import_node_builtin() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.build();
let temp_dir = context.temp_dir();
let mut client = context.new_lsp_command().build();
client.initialize_default();
client.did_open(json!({
"textDocument": {
"uri": temp_dir.url().join("file.ts").unwrap(),
"languageId": "typescript",
"version": 1,
"text": r#"
import "npm:@types/node";
pathToFileURL
"#,
}
}));
client.write_request(
"workspace/executeCommand",
json!({
"command": "deno.cache",
"arguments": [[], temp_dir.url().join("file.ts").unwrap()],
}),
);
let list = client.get_completion_list(
temp_dir.url().join("file.ts").unwrap(),
(2, 21),
json!({ "triggerKind": 2 }),
);
assert!(!list.is_incomplete);
let item = list
.items
.iter()
.find(|item| item.label == "pathToFileURL")
.unwrap();
let res = client.write_request("completionItem/resolve", json!(item));
assert_eq!(
res,
json!({
"label": "pathToFileURL",
"labelDetails": {
"description": "node:url",
},
"kind": 3,
"detail": "function pathToFileURL(path: string, options?: PathToFileUrlOptions): URL",
"documentation": {
"kind": "markdown",
"value": "This function ensures that `path` is resolved absolutely, and that the URL\ncontrol characters are correctly encoded when converting into a File URL.\n\n```js\nimport { pathToFileURL } from 'node:url';\n\nnew URL('/foo#1', 'file:'); // Incorrect: file:///foo#1\npathToFileURL('/foo#1'); // Correct: file:///foo#1 (POSIX)\n\nnew URL('/some/path%.c', 'file:'); // Incorrect: file:///some/path%.c\npathToFileURL('/some/path%.c'); // Correct: file:///some/path%.c (POSIX)\n```\n\n*@since* - v10.12.0 \n\n*@param* - path The path to convert to a File URL. \n\n*@return* - The file URL object.",
},
"sortText": "￿16_1",
"additionalTextEdits": [
{
"range": {
"start": { "line": 2, "character": 0 },
"end": { "line": 2, "character": 0 },
},
"newText": " import { pathToFileURL } from \"node:url\";\n",
},
],
}),
);
client.shutdown();
}

#[test]
fn lsp_npm_completions_auto_import_and_quick_fix_no_import_map() {
let context = TestContextBuilder::new()
Expand Down

0 comments on commit 2820ba1

Please sign in to comment.