fix(lsp): relative completions for bare import-mapped specifiers (denoland#26137)

Author: nayeemrmn

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<Vec<lsp::CompletionItem>> {
+) -> Option<CompletionList> {
   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"]

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/[email protected]/",
+        "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,39 @@ fn lsp_import_map_import_completions() {
     })
   );
 
-  client.write_notification(
-    "textDocument/didChange",
+  let res = client.get_completion(
+    &uri,
+    (2, 31),
+    json!({ "triggerKind": 2, "triggerCharacter": "/" }),
+  );
+  assert_eq!(
+    json!(res),
     json!({
-      "textDocument": {
-        "uri": uri,
-        "version": 2
-      },
-      "contentChanges": [
+      "isIncomplete": false,
+      "items": [
         {
-          "range": {
-            "start": { "line": 1, "character": 20 },
-            "end": { "line": 1, "character": 20 }
+          "label": "b.ts",
+          "kind": 17,
+          "detail": "(local)",
+          "sortText": "1",
+          "filterText": "/~/b.ts",
+          "textEdit": {
+            "range": {
+              "start": { "line": 2, "character": 28 },
+              "end": { "line": 2, "character": 31 },
+            },
+            "newText": "/~/b.ts",
           },
-          "text": "/~/"
-        }
-      ]
+          "commitCharacters": ["\"", "'"],
+        },
+      ],
     }),
   );
 
   let res = client.get_completion(
-    uri,
-    (1, 23),
-    json!({
-      "triggerKind": 2,
-      "triggerCharacter": "/"
-    }),
+    &uri,
+    (3, 32),
+    json!({ "triggerKind": 2, "triggerCharacter": "/" }),
   );
   assert_eq!(
     json!(res),
@@ -1472,18 +1488,18 @@ fn lsp_import_map_import_completions() {
           "kind": 17,
           "detail": "(local)",
           "sortText": "1",
-          "filterText": "/~/b.ts",
+          "filterText": "lib/b.ts",
           "textEdit": {
             "range": {
-              "start": { "line": 1, "character": 20 },
-              "end": { "line": 1, "character": 23 }
+              "start": { "line": 3, "character": 28 },
+              "end": { "line": 3, "character": 32 },
             },
-            "newText": "/~/b.ts"
+            "newText": "lib/b.ts",
           },
           "commitCharacters": ["\"", "'"],
-        }
-      ]
-    })
+        },
+      ],
+    }),
   );
 
   client.shutdown();