Repository: denoland/deno
Pull request: fix(lsp): directly use file referrer when loading document

Fixes #24592.

On this line: https://github.com/denoland/deno/blob/v1.45.5/cli/lsp/documents.rs#L1467, the `referrer` won't yield the correct file referrer [later](https://github.com/denoland/deno/blob/v1.45.5/cli/lsp/documents.rs#L1185) if it's a remote specifier which isn't directly the key to a document. Passing around `file_referrer` is the intended solution anyway and seems to work. I don't know how to reproduce this for a test.

Changed files:
filename: cli/lsp/completions.rs
patch: @@ -858,8 +858,8 @@ mod tests {
         .global()
         .set(&specifier, HashMap::default(), source.as_bytes())
         .expect("could not cache file");
-      let document =
-        documents.get_or_load(&specifier, &temp_dir.uri().join("$").unwrap());
+      let document = documents
+        .get_or_load(&specifier, Some(&temp_dir.uri().join("$").unwrap()));
       assert!(document.is_some(), "source could not be setup");
     }
     documents

filename: cli/lsp/diagnostics.rs
patch: @@ -1367,21 +1367,20 @@ fn diagnose_resolution(
   let mut diagnostics = vec![];
   match resolution {
     Resolution::Ok(resolved) => {
+      let file_referrer = referrer_doc.file_referrer();
       let specifier = &resolved.specifier;
-      let managed_npm_resolver = snapshot
-        .resolver
-        .maybe_managed_npm_resolver(referrer_doc.file_referrer());
+      let managed_npm_resolver =
+        snapshot.resolver.maybe_managed_npm_resolver(file_referrer);
       for (_, headers) in snapshot
         .resolver
-        .redirect_chain_headers(specifier, referrer_doc.file_referrer())
+        .redirect_chain_headers(specifier, file_referrer)
       {
         if let Some(message) = headers.get("x-deno-warning") {
           diagnostics.push(DenoDiagnostic::DenoWarn(message.clone()));
         }
       }
-      if let Some(doc) = snapshot
-        .documents
-        .get_or_load(specifier, referrer_doc.specifier())
+      if let Some(doc) =
+        snapshot.documents.get_or_load(specifier, file_referrer)
       {
         if let Some(headers) = doc.maybe_headers() {
           if let Some(message) = headers.get("x-deno-warning") {

filename: cli/lsp/documents.rs
patch: @@ -1180,11 +1180,10 @@ impl Documents {
   pub fn get_or_load(
     &self,
     specifier: &ModuleSpecifier,
-    referrer: &ModuleSpecifier,
+    file_referrer: Option<&ModuleSpecifier>,
   ) -> Option<Arc<Document>> {
-    let file_referrer = self.get_file_referrer(referrer);
     let specifier =
-      self.resolve_document_specifier(specifier, file_referrer.as_deref())?;
+      self.resolve_document_specifier(specifier, file_referrer)?;
     if let Some(document) = self.open_docs.get(&specifier) {
       Some(document.clone())
     } else {
@@ -1193,7 +1192,7 @@ impl Documents {
         &self.resolver,
         &self.config,
         &self.cache,
-        file_referrer.as_deref(),
+        file_referrer,
       )
     }
   }
@@ -1464,7 +1463,7 @@ impl Documents {
       specifier = s;
       media_type = Some(mt);
     }
-    let Some(doc) = self.get_or_load(&specifier, referrer) else {
+    let Some(doc) = self.get_or_load(&specifier, file_referrer) else {
       let media_type =
         media_type.unwrap_or_else(|| MediaType::from_specifier(&specifier));
       return Some((specifier, media_type));

filename: cli/lsp/language_server.rs
patch: @@ -1333,8 +1333,9 @@ impl Inner {
     {
       return Ok(None);
     }
-    let document =
-      file_referrer.and_then(|r| self.documents.get_or_load(&specifier, &r));
+    let document = self
+      .documents
+      .get_or_load(&specifier, file_referrer.as_ref());
     let Some(document) = document else {
       return Ok(None);
     };
@@ -1448,7 +1449,7 @@ impl Inner {
     {
       let dep_doc = dep
         .get_code()
-        .and_then(|s| self.documents.get_or_load(s, &specifier));
+        .and_then(|s| self.documents.get_or_load(s, file_referrer));
       let dep_maybe_types_dependency =
         dep_doc.as_ref().map(|d| d.maybe_types_dependency());
       let value = match (dep.maybe_code.is_none(), dep.maybe_type.is_none(), &dep_maybe_types_dependency) {

filename: cli/lsp/tsc.rs
patch: @@ -2043,12 +2043,10 @@ impl DocumentSpan {
     let target_asset_or_doc =
       language_server.get_maybe_asset_or_document(&target_specifier)?;
     let target_line_index = target_asset_or_doc.line_index();
-    let file_referrer = language_server
-      .documents
-      .get_file_referrer(&target_specifier);
+    let file_referrer = target_asset_or_doc.file_referrer();
     let target_uri = language_server
       .url_map
-      .normalize_specifier(&target_specifier, file_referrer.as_deref())
+      .normalize_specifier(&target_specifier, file_referrer)
       .ok()?;
     let (target_range, target_selection_range) =
       if let Some(context_span) = &self.context_span {
@@ -2092,10 +2090,10 @@ impl DocumentSpan {
       language_server.get_maybe_asset_or_document(&specifier)?;
     let line_index = asset_or_doc.line_index();
     let range = self.text_span.to_range(line_index);
-    let file_referrer = language_server.documents.get_file_referrer(&specifier);
+    let file_referrer = asset_or_doc.file_referrer();
     let mut target = language_server
       .url_map
-      .normalize_specifier(&specifier, file_referrer.as_deref())
+      .normalize_specifier(&specifier, file_referrer)
       .ok()?
       .into_url();
     target.set_fragment(Some(&format!(
@@ -2153,10 +2151,10 @@ impl NavigateToItem {
     let asset_or_doc =
       language_server.get_asset_or_document(&specifier).ok()?;
     let line_index = asset_or_doc.line_index();
-    let file_referrer = language_server.documents.get_file_referrer(&specifier);
+    let file_referrer = asset_or_doc.file_referrer();
     let uri = language_server
       .url_map
-      .normalize_specifier(&specifier, file_referrer.as_deref())
+      .normalize_specifier(&specifier, file_referrer)
       .ok()?;
     let range = self.text_span.to_range(line_index);
     let location = lsp::Location {
@@ -4232,14 +4230,10 @@ impl State {
   }
 
   fn get_document(&self, specifier: &ModuleSpecifier) -> Option<Arc<Document>> {
-    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, &ModuleSpecifier::parse("file:///").unwrap())
-    }
+    self
+      .state_snapshot
+      .documents
+      .get_or_load(specifier, self.last_scope.as_ref())
   }
 
   fn get_asset_or_document(
@@ -4561,7 +4555,7 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
           specifier,
           doc.file_referrer(),
         )?;
-        let types_doc = documents.get_or_load(&types, specifier)?;
+        let types_doc = documents.get_or_load(&types, doc.file_referrer())?;
         Some(types_doc.specifier().clone())
       })();
       // If there is a types dep, use that as the root instead. But if the doc