fix(lsp): handle import mapped `node:` specifier (#19956)

Closes https://github.com/denoland/vscode_deno/issues/805

9 files changed, 81 insertions, 68 deletions

diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 1ac5934ff..d987279de 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -1104,18 +1104,6 @@ impl Documents {
           continue;
         }
       }
-      if let Some(module_name) = specifier.strip_prefix("node:") {
-        if deno_node::is_builtin_node_module(module_name) {
-          // return itself for node: specifiers because during type checking
-          // we resolve to the ambient modules in the @types/node package
-          // rather than deno_std/node
-          results.push(Some((
-            ModuleSpecifier::parse(&specifier).unwrap(),
-            MediaType::Dts,
-          )));
-          continue;
-        }
-      }
       if specifier.starts_with("asset:") {
         if let Ok(specifier) = ModuleSpecifier::parse(&specifier) {
           let media_type = MediaType::from_specifier(&specifier);
@@ -1221,6 +1209,7 @@ impl Documents {
           }
         }
       }
+
       hasher.finish()
     }
 
@@ -1290,9 +1279,10 @@ impl Documents {
         options.document_preload_limit,
       );
       self.resolver_config_hash = new_resolver_config_hash;
-    }
 
-    self.dirty = true;
+      self.dirty = true;
+      self.calculate_dependents_if_dirty();
+    }
   }
 
   fn refresh_dependencies(
@@ -1416,12 +1406,11 @@ impl Documents {
 
       fn analyze_doc(&mut self, specifier: &ModuleSpecifier, doc: &Document) {
         self.analyzed_specifiers.insert(specifier.clone());
-        for (name, dependency) in doc.dependencies() {
-          if !self.has_node_builtin_specifier && name.starts_with("node:") {
-            self.has_node_builtin_specifier = true;
-          }
-
+        for dependency in doc.dependencies().values() {
           if let Some(dep) = dependency.get_code() {
+            if !self.has_node_builtin_specifier && dep.scheme() == "node" {
+              self.has_node_builtin_specifier = true;
+            }
             self.add(dep, specifier);
           }
           if let Some(dep) = dependency.get_type() {
@@ -1484,6 +1473,15 @@ impl Documents {
     specifier: &ModuleSpecifier,
     maybe_node_resolver: Option<&Arc<NodeResolver>>,
   ) -> Option<(ModuleSpecifier, MediaType)> {
+    if let Some(module_name) = specifier.as_str().strip_prefix("node:") {
+      if deno_node::is_builtin_node_module(module_name) {
+        // return itself for node: specifiers because during type checking
+        // we resolve to the ambient modules in the @types/node package
+        // rather than deno_std/node
+        return Some((specifier.clone(), MediaType::Dts));
+      }
+    }
+
     if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) {
       return node_resolve_npm_req_ref(npm_ref, maybe_node_resolver);
     }
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 3ac9610b3..2fef8cfa0 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -1290,7 +1290,7 @@ impl Inner {
     })
   }
 
-  fn refresh_documents_config(&mut self) {
+  async fn refresh_documents_config(&mut self) {
     self.documents.update_config(UpdateDocumentConfigOptions {
       enabled_urls: self.config.enabled_urls(),
       document_preload_limit: self
@@ -1303,6 +1303,10 @@ impl Inner {
       npm_registry_api: self.npm.api.clone(),
       npm_resolution: self.npm.resolution.clone(),
     });
+
+    // refresh the npm specifiers because it might have discovered
+    // a @types/node package and now's a good time to do that anyway
+    self.refresh_npm_specifiers().await;
   }
 
   async fn shutdown(&self) -> LspResult<()> {
@@ -1447,7 +1451,7 @@ impl Inner {
     }
 
     self.recreate_npm_services_if_necessary().await;
-    self.refresh_documents_config();
+    self.refresh_documents_config().await;
 
     self.diagnostics_server.invalidate_all();
     self.send_diagnostics_update();
@@ -1566,8 +1570,7 @@ impl Inner {
 
     if touched {
       self.recreate_npm_services_if_necessary().await;
-      self.refresh_documents_config();
-      self.refresh_npm_specifiers().await;
+      self.refresh_documents_config().await;
       self.diagnostics_server.invalidate_all();
       self.ts_server.restart(self.snapshot()).await;
       self.send_diagnostics_update();
@@ -3007,7 +3010,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
 
     {
       let mut ls = self.0.write().await;
-      ls.refresh_documents_config();
+      ls.refresh_documents_config().await;
       ls.diagnostics_server.invalidate_all();
       ls.send_diagnostics_update();
     }
@@ -3075,7 +3078,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
         .map(|d| d.is_diagnosable())
         .unwrap_or(false)
       {
-        ls.refresh_documents_config();
+        ls.refresh_documents_config().await;
         ls.send_diagnostics_update();
         ls.send_testing_update();
       }
@@ -3165,7 +3168,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
 
     if self.refresh_specifiers_from_client().await {
       let mut ls = self.0.write().await;
-      ls.refresh_documents_config();
+      ls.refresh_documents_config().await;
       ls.diagnostics_server.invalidate_all();
       ls.send_diagnostics_update();
     }
diff --git a/cli/tests/integration/coverage_tests.rs b/cli/tests/integration/coverage_tests.rs
index 0ac5974a6..f256d0121 100644
--- a/cli/tests/integration/coverage_tests.rs
+++ b/cli/tests/integration/coverage_tests.rs
@@ -38,10 +38,9 @@ fn no_tests() {
 #[test]
 fn error_if_invalid_cache() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
-  let deno_dir = context.deno_dir();
-  let deno_dir_path = deno_dir.path();
-  let tempdir = TempDir::new();
-  let tempdir = tempdir.path().join("cov");
+  let temp_dir_path = context.temp_dir().path();
+  let other_temp_dir = TempDir::new();
+  let other_tempdir = other_temp_dir.path().join("cov");
 
   let invalid_cache_path = util::testdata_path().join("coverage/invalid_cache");
   let mod_before_path = util::testdata_path()
@@ -54,8 +53,8 @@ fn error_if_invalid_cache() {
     .join(&invalid_cache_path)
     .join("mod.test.ts");
 
-  let mod_temp_path = deno_dir_path.join("mod.ts");
-  let mod_test_temp_path = deno_dir_path.join("mod.test.ts");
+  let mod_temp_path = temp_dir_path.join("mod.ts");
+  let mod_test_temp_path = temp_dir_path.join("mod.test.ts");
 
   // Write the initial mod.ts file
   std::fs::copy(mod_before_path, &mod_temp_path).unwrap();
@@ -68,7 +67,7 @@ fn error_if_invalid_cache() {
     .args_vec(vec![
       "test".to_string(),
       "--quiet".to_string(),
-      format!("--coverage={}", tempdir),
+      format!("--coverage={}", other_tempdir),
     ])
     .run();
 
@@ -80,7 +79,7 @@ fn error_if_invalid_cache() {
 
   let output = context
     .new_command()
-    .args_vec(vec!["coverage".to_string(), format!("{}/", tempdir)])
+    .args_vec(vec!["coverage".to_string(), format!("{}/", other_tempdir)])
     .run();
 
   output.assert_exit_code(1);
@@ -94,7 +93,7 @@ fn error_if_invalid_cache() {
 
 fn run_coverage_text(test_name: &str, extension: &str) {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
@@ -164,7 +163,7 @@ fn run_coverage_text(test_name: &str, extension: &str) {
 #[test]
 fn multifile_coverage() {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
@@ -231,7 +230,7 @@ fn multifile_coverage() {
 
 fn no_snaps_included(test_name: &str, extension: &str) {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
@@ -279,7 +278,7 @@ fn no_snaps_included(test_name: &str, extension: &str) {
 
 fn no_tests_included(test_name: &str, extension: &str) {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
@@ -328,7 +327,7 @@ fn no_tests_included(test_name: &str, extension: &str) {
 #[test]
 fn no_npm_cache_coverage() {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
@@ -373,7 +372,7 @@ fn no_npm_cache_coverage() {
 #[test]
 fn no_transpiled_lines() {
   let context = TestContext::default();
-  let tempdir = context.deno_dir();
+  let tempdir = context.temp_dir();
   let tempdir = tempdir.path().join("cov");
 
   let output = context
diff --git a/cli/tests/integration/init_tests.rs b/cli/tests/integration/init_tests.rs
index d92bfd371..e3f146c2a 100644
--- a/cli/tests/integration/init_tests.rs
+++ b/cli/tests/integration/init_tests.rs
@@ -7,9 +7,7 @@ use util::TestContextBuilder;
 #[test]
 fn init_subcommand_without_dir() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
-  let deno_dir = context.deno_dir();
-
-  let cwd = deno_dir.path();
+  let cwd = context.temp_dir().path();
 
   let output = context.new_command().args("init").split_output().run();
 
@@ -59,8 +57,7 @@ fn init_subcommand_without_dir() {
 #[test]
 fn init_subcommand_with_dir_arg() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
-  let deno_dir = context.deno_dir();
-  let cwd = deno_dir.path();
+  let cwd = context.temp_dir().path();
 
   let output = context
     .new_command()
@@ -117,8 +114,7 @@ fn init_subcommand_with_dir_arg() {
 #[test]
 fn init_subcommand_with_quiet_arg() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
-  let deno_dir = context.deno_dir();
-  let cwd = deno_dir.path();
+  let cwd = context.temp_dir().path();
 
   let output = context
     .new_command()
diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs
index dc68ab514..15ede69ea 100644
--- a/cli/tests/integration/lsp_tests.rs
+++ b/cli/tests/integration/lsp_tests.rs
@@ -611,6 +611,39 @@ fn lsp_import_map_config_file_auto_discovered_symlink() {
 }
 
 #[test]
+fn lsp_import_map_node_specifiers() {
+  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+
+  temp_dir.write("deno.json", r#"{ "imports": { "fs": "node:fs" } }"#);
+
+  // cache @types/node
+  context
+    .new_command()
+    .args("cache npm:@types/node")
+    .run()
+    .skip_output_check()
+    .assert_exit_code(0);
+
+  let mut client = context.new_lsp_command().build();
+  client.initialize(|builder| {
+    builder.set_config("./deno.json");
+  });
+
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("a.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": "import fs from \"fs\";\nconsole.log(fs);"
+    }
+  }));
+  assert_eq!(diagnostics.all(), vec![]);
+
+  client.shutdown();
+}
+
+#[test]
 fn lsp_deno_task() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
   let temp_dir = context.temp_dir();
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index f1ba0cdfa..e14da4aca 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -1524,7 +1524,6 @@ fn auto_discover_lock_file() {
 fn peer_deps_with_copied_folders_and_lockfile() {
   let context = TestContextBuilder::for_npm()
     .use_sync_npm_download()
-    .use_separate_deno_dir() // the "npm" folder means something in the deno dir, so use a separate folder
     .use_copy_temp_dir("npm/peer_deps_with_copied_folders")
     .cwd("npm/peer_deps_with_copied_folders")
     .build();
@@ -1906,7 +1905,6 @@ fn reload_info_not_found_cache_but_exists_remote() {
 fn binary_package_with_optional_dependencies() {
   let context = TestContextBuilder::for_npm()
     .use_sync_npm_download()
-    .use_separate_deno_dir() // the "npm" folder means something in the deno dir, so use a separate folder
     .use_copy_temp_dir("npm/binary_package")
     .cwd("npm/binary_package")
     .build();
diff --git a/cli/tools/check.rs b/cli/tools/check.rs
index aa4b9db0a..75ac2dc96 100644
--- a/cli/tools/check.rs
+++ b/cli/tools/check.rs
@@ -268,7 +268,7 @@ fn get_check_hash(
     }
   }
 
-  // Check if any of the top level npm pckages have changed. We could go
+  // Check if any of the top level npm packages have changed. We could go
   // further and check all the individual npm packages, but that's
   // probably overkill.
   let mut package_reqs = package_reqs.into_iter().collect::<Vec<_>>();
diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs
index bdc5efbe0..f9c546c6f 100644
--- a/test_util/src/builders.rs
+++ b/test_util/src/builders.rs
@@ -29,7 +29,6 @@ use crate::TempDir;
 pub struct TestContextBuilder {
   use_http_server: bool,
   use_temp_cwd: bool,
-  use_separate_deno_dir: bool,
   use_symlinked_temp_dir: bool,
   /// Copies the files at the specified directory in the "testdata" directory
   /// to the temp folder and runs the test from there. This is useful when
@@ -77,15 +76,6 @@ impl TestContextBuilder {
     self
   }
 
-  /// By default, the temp_dir and the deno_dir will be shared.
-  /// In some cases, that might cause an issue though, so calling
-  /// this will use a separate directory for the deno dir and the
-  /// temp directory.
-  pub fn use_separate_deno_dir(mut self) -> Self {
-    self.use_separate_deno_dir = true;
-    self
-  }
-
   /// Copies the files at the specified directory in the "testdata" directory
   /// to the temp folder and runs the test from there. This is useful when
   /// the test creates files in the testdata directory (ex. a node_modules folder)
@@ -127,11 +117,7 @@ impl TestContextBuilder {
       .clone()
       .unwrap_or_else(std::env::temp_dir);
     let deno_dir = TempDir::new_in(&temp_dir_path);
-    let temp_dir = if self.use_separate_deno_dir {
-      TempDir::new_in(&temp_dir_path)
-    } else {
-      deno_dir.clone()
-    };
+    let temp_dir = TempDir::new_in(&temp_dir_path);
     let temp_dir = if self.use_symlinked_temp_dir {
       TempDir::new_symlinked(temp_dir)
     } else {
@@ -535,10 +521,11 @@ impl TestCommandOutput {
     &self.testdata_dir
   }
 
-  pub fn skip_output_check(&self) {
+  pub fn skip_output_check(&self) -> &Self {
     *self.asserted_combined.borrow_mut() = true;
     *self.asserted_stdout.borrow_mut() = true;
     *self.asserted_stderr.borrow_mut() = true;
+    self
   }
 
   pub fn skip_exit_code_check(&self) {
diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs
index 59f67e3d9..81ebd3c47 100644
--- a/test_util/src/lsp.rs
+++ b/test_util/src/lsp.rs
@@ -6,7 +6,6 @@ use crate::PathRef;
 use crate::TestContext;
 use crate::TestContextBuilder;
 
-use super::new_deno_dir;
 use super::TempDir;
 
 use anyhow::Result;
@@ -524,7 +523,7 @@ impl LspClientBuilder {
   }
 
   pub fn build_result(&self) -> Result<LspClient> {
-    let deno_dir = new_deno_dir();
+    let deno_dir = self.context.as_ref().unwrap().deno_dir().clone();
     let mut command = Command::new(&self.deno_exe);
     command
       .env("DENO_DIR", deno_dir.path())