fix(check/lsp): correctly resolve compilerOptions.types (#27686)

Fixes https://github.com/denoland/deno/issues/27062

In the LSP we were passing `npm` specifiers to TSC as roots, but TSC
needs fully resolved specifiers (like the actual file path).

In `deno check` we were often excluding the specifiers entirely from the
roots.

In both cases, we need to resolve the specifiers fully and then pass
them to tsc

cli/lsp/tsc.rs

@@ -73,6 +73,7 @@ use super::documents::Document;
 use super::documents::DocumentsFilter;
 use super::language_server;
 use super::language_server::StateSnapshot;
+use super::logging::lsp_log;
 use super::performance::Performance;
 use super::performance::PerformanceMark;
 use super::refactor::RefactorCodeActionData;
@@ -4695,7 +4696,24 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
       .graph_imports_by_referrer(scope)
     {
       for specifier in specifiers {
-        script_names.insert(specifier.to_string());
+        if let Ok(req_ref) =
+          deno_semver::npm::NpmPackageReqReference::from_specifier(specifier)
+        {
+          let Some((resolved, _)) =
+            state.state_snapshot.resolver.npm_to_file_url(
+              &req_ref,
+              scope,
+              ResolutionMode::Import,
+              Some(scope),
+            )
+          else {
+            lsp_log!("failed to resolve {req_ref} to file URL");
+            continue;
+          };
+          script_names.insert(resolved.to_string());
+        } else {
+          script_names.insert(specifier.to_string());
+        }
       }
     }
   }

cli/tools/check.rs

@@ -13,6 +13,7 @@ use deno_graph::Module;
 use deno_graph::ModuleError;
 use deno_graph::ModuleGraph;
 use deno_graph::ModuleLoadError;
+use deno_semver::npm::NpmPackageNvReference;
 use deno_terminal::colors;
 use once_cell::sync::Lazy;
 use regex::Regex;
@@ -261,6 +262,8 @@ impl TypeChecker {
       maybe_check_hash,
     } = get_tsc_roots(
       &self.sys,
+      &self.npm_resolver,
+      &self.node_resolver,
       &graph,
       check_js,
       check_state_hash(&self.npm_resolver),
@@ -373,8 +376,11 @@ struct TscRoots {
 /// redirects resolved. We need to include all the emittable files in
 /// the roots, so they get type checked and optionally emitted,
 /// otherwise they would be ignored if only imported into JavaScript.
+#[allow(clippy::too_many_arguments)]
 fn get_tsc_roots(
   sys: &CliSys,
+  npm_resolver: &CliNpmResolver,
+  node_resolver: &CliNodeResolver,
   graph: &ModuleGraph,
   check_js: bool,
   npm_cache_state_hash: Option<u64>,
@@ -457,6 +463,7 @@ fn get_tsc_roots(
         if let Some(hasher) = hasher {
           hasher.write_str(module.specifier.as_str());
         }
+
         None
       }
     }
@@ -493,17 +500,33 @@ fn get_tsc_roots(
   let mut pending = VecDeque::new();
 
   // put in the global types first so that they're resolved before anything else
-  let get_import_specifiers = || {
-    graph
-      .imports
+  for (referrer, import) in graph.imports.iter() {
+    for specifier in import
+      .dependencies
       .values()
-      .flat_map(|i| i.dependencies.values())
       .filter_map(|dep| dep.get_type().or_else(|| dep.get_code()))
-  };
-  for specifier in get_import_specifiers() {
-    let specifier = graph.resolve(specifier);
-    if seen.insert(specifier) {
-      pending.push_back((specifier, false));
+    {
+      let specifier = graph.resolve(specifier);
+      if seen.insert(specifier) {
+        if let Ok(nv_ref) = NpmPackageNvReference::from_specifier(specifier) {
+          let Some(resolved) =
+            resolve_npm_nv_ref(npm_resolver, node_resolver, &nv_ref, referrer)
+          else {
+            result.missing_diagnostics.push(
+              tsc::Diagnostic::from_missing_error(
+                specifier,
+                None,
+                maybe_additional_sloppy_imports_message(sys, specifier),
+              ),
+            );
+            continue;
+          };
+          let mt = MediaType::from_specifier(&resolved);
+          result.roots.push((resolved, mt));
+        } else {
+          pending.push_back((specifier, false));
+        }
+      }
     }
   }
 
@@ -624,6 +647,29 @@ fn get_tsc_roots(
   result
 }
 
+fn resolve_npm_nv_ref(
+  npm_resolver: &CliNpmResolver,
+  node_resolver: &CliNodeResolver,
+  nv_ref: &NpmPackageNvReference,
+  referrer: &ModuleSpecifier,
+) -> Option<ModuleSpecifier> {
+  let pkg_dir = npm_resolver
+    .as_managed()
+    .unwrap()
+    .resolve_pkg_folder_from_deno_module(nv_ref.nv())
+    .ok()?;
+  let resolved = node_resolver
+    .resolve_package_subpath_from_deno_module(
+      &pkg_dir,
+      nv_ref.sub_path(),
+      Some(referrer),
+      node_resolver::ResolutionMode::Import,
+      node_resolver::NodeResolutionKind::Types,
+    )
+    .ok()?;
+  Some(resolved)
+}
+
 /// Matches the `@ts-check` pragma.
 static TS_CHECK_RE: Lazy<Regex> =
   lazy_regex::lazy_regex!(r#"(?i)^\s*@ts-check(?:\s+|$)"#);

tests/integration/lsp_tests.rs

@@ -17296,3 +17296,52 @@ fn wildcard_augment() {
   let diagnostics = client.did_open_file(&source);
   assert_eq!(diagnostics.all().len(), 0);
 }
+
+#[test]
+fn compiler_options_types() {
+  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  let mut client = context.new_lsp_command().build();
+  let temp = context.temp_dir();
+  let temp_dir = temp.path();
+  let source = source_file(
+    temp_dir.join("index.ts"),
+    r#"
+      const foo = [1];
+      foo.augmented();
+    "#,
+  );
+
+  let deno_json = json!({
+    "imports": {
+      "@denotest/augments-global": "npm:@denotest/augments-global@1"
+    },
+    "compilerOptions": { "types": ["@denotest/augments-global"] },
+  });
+
+  temp.write("deno.json", deno_json.to_string());
+
+  client.initialize_default();
+
+  for node_modules_dir in ["none", "auto", "manual"] {
+    let mut deno_json = deno_json.clone();
+    deno_json["nodeModulesDir"] = json!(node_modules_dir);
+    temp.write("deno.json", deno_json.to_string());
+    context
+      .new_command()
+      .args("install")
+      .run()
+      .skip_output_check()
+      .assert_exit_code(0);
+    client.did_change_watched_files(json!({
+      "changes": [{
+        "uri": temp.url().join("deno.json").unwrap(),
+        "type": 2,
+      }],
+    }));
+
+    let diagnostics = client.did_open_file(&source);
+    eprintln!("{:#?}", diagnostics.all());
+    assert_eq!(diagnostics.all().len(), 0);
+    client.did_close_file(&source);
+  }
+}

tests/registry/npm/@denotest/augments-global/1.0.0/index.d.ts

@@ -0,0 +1 @@
+import "./other.d.ts";

tests/registry/npm/@denotest/augments-global/1.0.0/other.d.ts

@@ -0,0 +1,6 @@
+export {}
+declare global {
+  interface Array<T> {
+    augmented(): void
+  }
+}

tests/registry/npm/@denotest/augments-global/1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "@denotest/augments-global",
+  "version": "1.0.0",
+  "types": "./index.d.ts"
+}

tests/specs/check/compiler_options_types/__test__.jsonc

@@ -0,0 +1,53 @@
+{
+  "tempDir": true,
+  "tests": {
+    "node_modules_dir_none": {
+      "steps": [
+        {
+          "args": "run -A ./set_node_modules_dir.ts none",
+          "output": ""
+        },
+        {
+          "args": "install",
+          "output": "[WILDCARD]"
+        },
+        {
+          "args": "check ./main.ts",
+          "output": "Check [WILDCARD]main.ts\n"
+        }
+      ]
+    },
+    "node_modules_dir_auto": {
+      "steps": [
+        {
+          "args": "run -A ./set_node_modules_dir.ts auto",
+          "output": ""
+        },
+        {
+          "args": "install",
+          "output": "[WILDCARD]"
+        },
+        {
+          "args": "check ./main.ts",
+          "output": "Check [WILDCARD]main.ts\n"
+        }
+      ]
+    },
+    "node_modules_dir_manual": {
+      "steps": [
+        {
+          "args": "run -A ./set_node_modules_dir.ts auto",
+          "output": ""
+        },
+        {
+          "args": "install",
+          "output": "[WILDCARD]"
+        },
+        {
+          "args": "check ./main.ts",
+          "output": "Check [WILDCARD]main.ts\n"
+        }
+      ]
+    }
+  }
+}

tests/specs/check/compiler_options_types/deno.json

@@ -0,0 +1,6 @@
+{
+  "imports": {
+    "@denotest/augments-global": "npm:@denotest/augments-global@1"
+  },
+  "compilerOptions": { "types": ["@denotest/augments-global"] }
+}

tests/specs/check/compiler_options_types/main.ts

@@ -0,0 +1,2 @@
+const foo = [1];
+foo.augmented();

tests/specs/check/compiler_options_types/set_node_modules_dir.ts

@@ -0,0 +1,8 @@
+if (Deno.args.length !== 1) {
+  console.error("Usage: set_node_modules_dir.ts <setting>");
+  Deno.exit(1);
+}
+const setting = Deno.args[0].trim();
+const denoJson = JSON.parse(Deno.readTextFileSync("./deno.json"));
+denoJson["nodeModulesDir"] = setting;
+Deno.writeTextFileSync("./deno.json", JSON.stringify(denoJson, null, 2));

tests/util/server/src/lsp.rs

@@ -900,6 +900,20 @@ impl LspClient {
     self.read_diagnostics()
   }
 
+  pub fn did_close_file(&mut self, file: &SourceFile) {
+    self.did_close(json!({
+        "textDocument": file.identifier(),
+    }))
+  }
+
+  pub fn did_close(&mut self, params: Value) {
+    self.did_close_raw(params);
+  }
+
+  pub fn did_close_raw(&mut self, params: Value) {
+    self.write_notification("textDocument/didClose", params);
+  }
+
   pub fn did_open_raw(&mut self, params: Value) {
     self.write_notification("textDocument/didOpen", params);
   }