Repository: denoland/deno
Pull request: fix(lsp): correct scope attribution for injected @types/node

Fixes #24401.

Changed files:
filename: cli/lsp/documents.rs
patch: @@ -1246,9 +1246,13 @@ impl Documents {
     &self,
     specifiers: &[String],
     referrer: &ModuleSpecifier,
+    file_referrer: Option<&ModuleSpecifier>,
   ) -> Vec<Option<(ModuleSpecifier, MediaType)>> {
     let document = self.get(referrer);
-    let file_referrer = document.as_ref().and_then(|d| d.file_referrer());
+    let file_referrer = document
+      .as_ref()
+      .and_then(|d| d.file_referrer())
+      .or(file_referrer);
     let dependencies = document.as_ref().map(|d| d.dependencies());
     let mut results = Vec::new();
     for specifier in specifiers {

filename: cli/lsp/language_server.rs
patch: @@ -321,7 +321,7 @@ impl LanguageServer {
       inner.resolver.did_cache();
       inner.refresh_npm_specifiers().await;
       inner.diagnostics_server.invalidate_all();
-      inner.project_changed([], false);
+      inner.project_changed([], true);
       inner
         .ts_server
         .cleanup_semantic_cache(inner.snapshot())

filename: cli/lsp/tsc.rs
patch: @@ -4203,8 +4203,7 @@ struct State {
   response_tx: Option<oneshot::Sender<Result<String, AnyError>>>,
   state_snapshot: Arc<StateSnapshot>,
   specifier_map: Arc<TscSpecifierMap>,
-  root_referrers: HashMap<ModuleSpecifier, ModuleSpecifier>,
-  last_referrer: Option<ModuleSpecifier>,
+  last_scope: Option<ModuleSpecifier>,
   token: CancellationToken,
   pending_requests: Option<UnboundedReceiver<Request>>,
   mark: Option<PerformanceMark>,
@@ -4223,27 +4222,21 @@ impl State {
       response_tx: None,
       state_snapshot,
       specifier_map,
-      root_referrers: Default::default(),
-      last_referrer: None,
+      last_scope: None,
       token: Default::default(),
       mark: None,
       pending_requests: Some(pending_requests),
     }
   }
 
   fn get_document(&self, specifier: &ModuleSpecifier) -> Option<Arc<Document>> {
-    if let Some(referrer) = self.root_referrers.get(specifier) {
-      self
-        .state_snapshot
-        .documents
-        .get_or_load(specifier, referrer)
-    } else if let Some(referrer) = &self.last_referrer {
+    if let Some(scope) = &self.last_scope {
+      self.state_snapshot.documents.get_or_load(specifier, scope)
+    } else {
       self
         .state_snapshot
         .documents
-        .get_or_load(specifier, referrer)
-    } else {
-      self.state_snapshot.documents.get(specifier)
+        .get_or_load(specifier, &ModuleSpecifier::parse("file:///").unwrap())
     }
   }
 
@@ -4422,6 +4415,7 @@ async fn op_poll_requests(
   state.response_tx = Some(response_tx);
   let id = state.last_id;
   state.last_id += 1;
+  state.last_scope.clone_from(&scope);
   let mark = state
     .performance
     .mark_with_args(format!("tsc.host.{}", request.method()), &request);
@@ -4447,7 +4441,7 @@ fn op_resolve_inner(
   let specifiers = state
     .state_snapshot
     .documents
-    .resolve(&args.specifiers, &referrer)
+    .resolve(&args.specifiers, &referrer, state.last_scope.as_ref())
     .into_iter()
     .map(|o| {
       o.map(|(s, mt)| {
@@ -4458,7 +4452,6 @@ fn op_resolve_inner(
       })
     })
     .collect();
-  state.last_referrer = Some(referrer);
   state.performance.measure(mark);
   Ok(specifiers)
 }
@@ -4471,7 +4464,7 @@ fn op_respond(
 ) {
   let state = state.borrow_mut::<State>();
   state.performance.measure(state.mark.take().unwrap());
-  state.last_referrer = None;
+  state.last_scope = None;
   let response = if !error.is_empty() {
     Err(anyhow!("tsc error: {error}"))
   } else {
@@ -4526,17 +4519,13 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
 
   // inject these next because they're global
   for (scope, script_names) in &mut result.by_scope {
-    for (referrer, specifiers) in state
+    for (_, specifiers) in state
       .state_snapshot
       .resolver
       .graph_imports_by_referrer(scope)
     {
       for specifier in specifiers {
         script_names.insert(specifier.to_string());
-        state
-          .root_referrers
-          .entry(specifier.clone())
-          .or_insert(referrer.clone());
       }
     }
   }

filename: tests/integration/lsp_tests.rs
patch: @@ -8606,6 +8606,69 @@ fn lsp_completions_node_specifier() {
   client.shutdown();
 }
 
+#[test]
+fn lsp_completions_node_specifier_node_modules_dir() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    temp_dir.path().join("deno.json"),
+    json!({
+      "nodeModulesDir": true,
+    })
+    .to_string(),
+  );
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("file.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": "import fs from \"node:fs\";\n",
+    },
+  }));
+  client.write_request(
+    "workspace/executeCommand",
+    json!({
+      "command": "deno.cache",
+      "arguments": [[], temp_dir.uri().join("file.ts").unwrap()],
+    }),
+  );
+  client.write_notification(
+    "textDocument/didChange",
+    json!({
+      "textDocument": {
+        "uri": temp_dir.uri().join("file.ts").unwrap(),
+        "version": 2,
+      },
+      "contentChanges": [
+        {
+          "range": {
+            "start": { "line": 1, "character": 0 },
+            "end": { "line": 1, "character": 0 },
+          },
+          "text": "fs.",
+        },
+      ],
+    }),
+  );
+  let list = client.get_completion_list(
+    temp_dir.uri().join("file.ts").unwrap(),
+    (1, 3),
+    json!({
+      "triggerKind": 2,
+      "triggerCharacter": ".",
+    }),
+  );
+  assert!(!list.is_incomplete);
+  assert!(list.items.iter().any(|i| i.label == "writeFile"));
+  assert!(list.items.iter().any(|i| i.label == "writeFileSync"));
+  client.shutdown();
+}
+
 #[test]
 fn lsp_completions_registry() {
   let context = TestContextBuilder::new()