fix(check): regression where config "types" entries caused type checking errors (#18124)

Closes #18117
Closes #18121 (this is just over 10ms faster in a directory one up from
the root folder)

cc @nayeemrmn

cli/args/config_file.rs

@@ -185,10 +185,16 @@ fn parse_compiler_options(
 
   for (key, value) in compiler_options.iter() {
     let key = key.as_str();
-    if IGNORED_COMPILER_OPTIONS.contains(&key) {
-      items.push(key.to_string());
-    } else {
-      filtered.insert(key.to_string(), value.to_owned());
+    // We don't pass "types" entries to typescript via the compiler
+    // options and instead provide those to tsc as "roots". This is
+    // because our "types" behavior is at odds with how TypeScript's
+    // "types" works.
+    if key != "types" {
+      if IGNORED_COMPILER_OPTIONS.contains(&key) {
+        items.push(key.to_string());
+      } else {
+        filtered.insert(key.to_string(), value.to_owned());
+      }
     }
   }
   let value = serde_json::to_value(filtered)?;

cli/build.rs

@@ -177,16 +177,6 @@ mod ts {
       })
     }
 
-    #[op]
-    fn op_cwd() -> String {
-      "cache:///".into()
-    }
-
-    #[op]
-    fn op_exists() -> bool {
-      false
-    }
-
     #[op]
     fn op_is_node_file() -> bool {
       false
@@ -254,8 +244,6 @@ mod ts {
     let tsc_extension = Extension::builder("deno_tsc")
       .ops(vec![
         op_build_info::decl(),
-        op_cwd::decl(),
-        op_exists::decl(),
         op_is_node_file::decl(),
         op_load::decl(),
         op_script_version::decl(),

cli/lsp/documents.rs

@@ -818,7 +818,7 @@ pub struct Documents {
   resolver_config_hash: u64,
   /// Any imports to the context supplied by configuration files. This is like
   /// the imports into the a module graph in CLI.
-  imports: Arc<HashMap<ModuleSpecifier, GraphImport>>,
+  imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
   /// A resolver that takes into account currently loaded import map and JSX
   /// settings.
   resolver: CliGraphResolver,
@@ -851,6 +851,14 @@ impl Documents {
     }
   }
 
+  pub fn module_graph_imports(&self) -> impl Iterator<Item = &ModuleSpecifier> {
+    self
+      .imports
+      .values()
+      .flat_map(|i| i.dependencies.values())
+      .flat_map(|value| value.get_type().or_else(|| value.get_code()))
+  }
+
   /// "Open" a document from the perspective of the editor, meaning that
   /// requests for information from the document will come from the in-memory
   /// representation received from the language server client, versus reading
@@ -946,7 +954,6 @@ impl Documents {
 
   /// Return `true` if the specifier can be resolved to a document.
   pub fn exists(&self, specifier: &ModuleSpecifier) -> bool {
-    // keep this fast because it's used by op_exists, which is a hot path in tsc
     let specifier = self.specifier_resolver.resolve(specifier);
     if let Some(specifier) = specifier {
       if self.open_docs.contains_key(&specifier) {
@@ -1239,7 +1246,7 @@ impl Documents {
           })
           .collect()
       } else {
-        HashMap::new()
+        IndexMap::new()
       },
     );
 

cli/lsp/language_server.rs

@@ -97,7 +97,6 @@ pub struct StateSnapshot {
   pub cache_metadata: cache::CacheMetadata,
   pub documents: Documents,
   pub maybe_import_map: Option<Arc<ImportMap>>,
-  pub root_uri: Option<Url>,
   pub maybe_npm_resolver: Option<NpmPackageResolver>,
 }
 
@@ -576,7 +575,6 @@ impl Inner {
       documents: self.documents.clone(),
       maybe_import_map: self.maybe_import_map.clone(),
       maybe_npm_resolver: Some(self.npm_resolver.snapshotted()),
-      root_uri: self.config.root_uri.clone(),
     })
   }
 

cli/lsp/tsc.rs

@@ -109,8 +109,7 @@ impl TsServer {
         while let Some((req, state_snapshot, tx, token)) = rx.recv().await {
           if !started {
             // TODO(@kitsonk) need to reflect the debug state of the lsp here
-            start(&mut ts_runtime, false, &state_snapshot)
-              .expect("could not start tsc");
+            start(&mut ts_runtime, false).unwrap();
             started = true;
           }
           let value = request(&mut ts_runtime, state_snapshot, req, token);
@@ -2659,24 +2658,6 @@ struct SpecifierArgs {
   specifier: String,
 }
 
-#[op]
-fn op_exists(state: &mut OpState, args: SpecifierArgs) -> bool {
-  let state = state.borrow_mut::<State>();
-  // we don't measure the performance of op_exists anymore because as of TS 4.5
-  // it is noisy with all the checking for custom libs, that we can't see the
-  // forrest for the trees as well as it compounds any lsp performance
-  // challenges, opening a single document in the editor causes some 3k worth
-  // of op_exists requests... :omg:
-  let specifier = match state.normalize_specifier(&args.specifier) {
-    Ok(url) => url,
-    // sometimes tsc tries to query invalid specifiers, especially when
-    // something else isn't quite right, so instead of bubbling up the error
-    // back to tsc, we simply swallow it and say the file doesn't exist
-    Err(_) => return false,
-  };
-  state.state_snapshot.documents.exists(&specifier)
-}
-
 #[op]
 fn op_is_cancelled(state: &mut OpState) -> bool {
   let state = state.borrow_mut::<State>();
@@ -2768,15 +2749,45 @@ fn op_script_names(state: &mut OpState) -> Vec<String> {
   let state = state.borrow_mut::<State>();
   let documents = &state.state_snapshot.documents;
   let open_docs = documents.documents(true, true);
-
-  let mut result = Vec::with_capacity(open_docs.len() + 1);
+  let mut result = Vec::new();
+  let mut seen = HashSet::new();
 
   if documents.has_injected_types_node_package() {
     // ensure this is first so it resolves the node types first
-    result.push("asset:///node_types.d.ts".to_string());
+    let specifier = "asset:///node_types.d.ts";
+    result.push(specifier.to_string());
+    seen.insert(specifier);
+  }
+
+  // inject these next because they're global
+  for import in documents.module_graph_imports() {
+    if seen.insert(import.as_str()) {
+      result.push(import.to_string());
+    }
+  }
+
+  // finally include the documents and all their dependencies
+  for doc in &open_docs {
+    let specifier = doc.specifier();
+    if seen.insert(specifier.as_str()) {
+      result.push(specifier.to_string());
+    }
+  }
+
+  // and then all their dependencies (do this after to avoid exists calls)
+  for doc in &open_docs {
+    for dep in doc.dependencies().values() {
+      if let Some(specifier) = dep.get_type().or_else(|| dep.get_code()) {
+        if seen.insert(specifier.as_str()) {
+          // only include dependencies we know to exist otherwise typescript will error
+          if documents.exists(specifier) {
+            result.push(specifier.to_string());
+          }
+        }
+      }
+    }
   }
 
-  result.extend(open_docs.into_iter().map(|d| d.specifier().to_string()));
   result
 }
 
@@ -2812,7 +2823,6 @@ fn js_runtime(performance: Arc<Performance>) -> JsRuntime {
 fn init_extension(performance: Arc<Performance>) -> Extension {
   Extension::builder("deno_tsc")
     .ops(vec![
-      op_exists::decl(),
       op_is_cancelled::decl(),
       op_is_node_file::decl(),
       op_load::decl(),
@@ -2832,16 +2842,8 @@ fn init_extension(performance: Arc<Performance>) -> Extension {
 
 /// Instruct a language server runtime to start the language server and provide
 /// it with a minimal bootstrap configuration.
-fn start(
-  runtime: &mut JsRuntime,
-  debug: bool,
-  state_snapshot: &StateSnapshot,
-) -> Result<(), AnyError> {
-  let root_uri = state_snapshot
-    .root_uri
-    .clone()
-    .unwrap_or_else(|| Url::parse("cache:///").unwrap());
-  let init_config = json!({ "debug": debug, "rootUri": root_uri });
+fn start(runtime: &mut JsRuntime, debug: bool) -> Result<(), AnyError> {
+  let init_config = json!({ "debug": debug });
   let init_src = format!("globalThis.serverInit({init_config});");
 
   runtime.execute_script(&located_script_name!(), &init_src)?;
@@ -3495,8 +3497,7 @@ mod tests {
     let location = temp_dir.path().join("deps");
     let state_snapshot = Arc::new(mock_state_snapshot(sources, &location));
     let mut runtime = js_runtime(Default::default());
-    start(&mut runtime, debug, &state_snapshot)
-      .expect("could not start server");
+    start(&mut runtime, debug).unwrap();
     let ts_config = TsConfig::new(config);
     assert_eq!(
       request(
@@ -4038,34 +4039,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn test_op_exists() {
-    let temp_dir = TempDir::new();
-    let (mut rt, state_snapshot, _) = setup(
-      &temp_dir,
-      false,
-      json!({
-        "target": "esnext",
-        "module": "esnext",
-        "lib": ["deno.ns", "deno.window"],
-        "noEmit": true,
-      }),
-      &[],
-    );
-    let performance = Arc::new(Performance::default());
-    let state = State::new(state_snapshot, performance);
-    let op_state = rt.op_state();
-    let mut op_state = op_state.borrow_mut();
-    op_state.put(state);
-    let actual = op_exists::call(
-      &mut op_state,
-      SpecifierArgs {
-        specifier: "/error/unknown:something/index.d.ts".to_string(),
-      },
-    );
-    assert!(!actual);
-  }
-
   #[test]
   fn test_completion_entry_filter_text() {
     let fixture = CompletionEntry {

cli/tests/integration/check_tests.rs

@@ -26,14 +26,14 @@ itest!(check_random_extension {
 });
 
 itest!(check_all {
-  args: "check --quiet --all check/check_all.ts",
-  output: "check/check_all.out",
+  args: "check --quiet --all check/all/check_all.ts",
+  output: "check/all/check_all.out",
   http_server: true,
   exit_code: 1,
 });
 
 itest!(check_all_local {
-  args: "check --quiet check/check_all.ts",
+  args: "check --quiet check/all/check_all.ts",
   output_str: Some(""),
   http_server: true,
 });
@@ -233,11 +233,18 @@ fn ts_no_recheck_on_redirect() {
 }
 
 itest!(check_dts {
-  args: "check --quiet check/check_dts.d.ts",
-  output: "check/check_dts.out",
+  args: "check --quiet check/dts/check_dts.d.ts",
+  output: "check/dts/check_dts.out",
   exit_code: 1,
 });
 
+itest!(check_types_dts {
+  args: "check main.ts",
+  cwd: Some("check/types_dts/"),
+  output: "check/types_dts/main.out",
+  exit_code: 0,
+});
+
 itest!(package_json_basic {
   args: "check main.ts",
   output: "package_json/basic/main.check.out",

cli/tests/integration/run_tests.rs

@@ -826,15 +826,15 @@ itest!(config {
 
 itest!(config_types {
   args:
-    "run --reload --quiet --config run/config_types/tsconfig.json run/config_types/main.ts",
+    "run --reload --quiet --check=all --config run/config_types/tsconfig.json run/config_types/main.ts",
   output: "run/config_types/main.out",
 });
 
 itest!(config_types_remote {
-    http_server: true,
-    args: "run --reload --quiet --config run/config_types/remote.tsconfig.json run/config_types/main.ts",
-    output: "run/config_types/main.out",
-  });
+  http_server: true,
+  args: "run --reload --quiet --check=all --config run/config_types/remote.tsconfig.json run/config_types/main.ts",
+  output: "run/config_types/main.out",
+});
 
 itest!(empty_typescript {
   args: "run --reload --check run/empty.ts",

cli/tests/testdata/check/types_dts/deno.json

@@ -0,0 +1,7 @@
+{
+  "compilerOptions": {
+    "types": [
+      "./types.d.ts"
+    ]
+  }
+}

cli/tests/testdata/check/types_dts/main.out

@@ -0,0 +1 @@
+Check file:///[WILDCARD]/check/types_dts/main.ts

cli/tests/testdata/check/types_dts/main.ts

@@ -0,0 +1,3 @@
+console.log("Hello world!");
+
+test.test();

cli/tests/testdata/check/types_dts/types.d.ts

@@ -0,0 +1,3 @@
+declare class test {
+  static test(): void;
+}

cli/tools/check.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use std::collections::HashSet;
 use std::sync::Arc;
 
 use deno_ast::MediaType;
@@ -98,7 +99,6 @@ pub fn check(
     debug: options.debug,
     graph: graph.clone(),
     hash_data,
-    maybe_config_specifier: options.maybe_config_specifier,
     maybe_npm_resolver: Some(npm_resolver.clone()),
     maybe_tsbuildinfo,
     root_names,
@@ -230,6 +230,41 @@ fn get_tsc_roots(
   graph: &ModuleGraph,
   check_js: bool,
 ) -> Vec<(ModuleSpecifier, MediaType)> {
+  fn maybe_get_check_entry(
+    module: &deno_graph::Module,
+    check_js: bool,
+  ) -> Option<(ModuleSpecifier, MediaType)> {
+    match module {
+      Module::Esm(module) => match module.media_type {
+        MediaType::TypeScript
+        | MediaType::Tsx
+        | MediaType::Mts
+        | MediaType::Cts
+        | MediaType::Dts
+        | MediaType::Dmts
+        | MediaType::Dcts
+        | MediaType::Jsx => Some((module.specifier.clone(), module.media_type)),
+        MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => {
+          if check_js || has_ts_check(module.media_type, &module.source) {
+            Some((module.specifier.clone(), module.media_type))
+          } else {
+            None
+          }
+        }
+        MediaType::Json
+        | MediaType::Wasm
+        | MediaType::TsBuildInfo
+        | MediaType::SourceMap
+        | MediaType::Unknown => None,
+      },
+      Module::External(_)
+      | Module::Node(_)
+      | Module::Npm(_)
+      | Module::Json(_) => None,
+    }
+  }
+
+  // todo(https://github.com/denoland/deno_graph/pull/253/): pre-allocate this
   let mut result = Vec::new();
   if graph.has_node_specifier {
     // inject a specifier that will resolve node types
@@ -238,33 +273,45 @@ fn get_tsc_roots(
       MediaType::Dts,
     ));
   }
-  result.extend(graph.modules().filter_map(|module| match module {
-    Module::Esm(module) => match module.media_type {
-      MediaType::TypeScript
-      | MediaType::Tsx
-      | MediaType::Mts
-      | MediaType::Cts
-      | MediaType::Dts
-      | MediaType::Dmts
-      | MediaType::Dcts
-      | MediaType::Jsx => Some((module.specifier.clone(), module.media_type)),
-      MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => {
-        if check_js || has_ts_check(module.media_type, &module.source) {
-          Some((module.specifier.clone(), module.media_type))
-        } else {
-          None
+
+  let mut seen_roots =
+    HashSet::with_capacity(graph.imports.len() + graph.roots.len());
+
+  // put in the global types first so that they're resolved before anything else
+  for import in graph.imports.values() {
+    for dep in import.dependencies.values() {
+      let specifier = dep.get_type().or_else(|| dep.get_code());
+      if let Some(specifier) = &specifier {
+        if seen_roots.insert(*specifier) {
+          let maybe_entry = graph
+            .get(specifier)
+            .and_then(|m| maybe_get_check_entry(m, check_js));
+          if let Some(entry) = maybe_entry {
+            result.push(entry);
+          }
         }
       }
-      MediaType::Json
-      | MediaType::Wasm
-      | MediaType::TsBuildInfo
-      | MediaType::SourceMap
-      | MediaType::Unknown => None,
-    },
-    Module::External(_)
-    | Module::Node(_)
-    | Module::Npm(_)
-    | Module::Json(_) => None,
+    }
+  }
+
+  // then the roots
+  for root in &graph.roots {
+    if let Some(module) = graph.get(root) {
+      if seen_roots.insert(root) {
+        if let Some(entry) = maybe_get_check_entry(module, check_js) {
+          result.push(entry);
+        }
+      }
+    }
+  }
+
+  // now the rest
+  result.extend(graph.modules().filter_map(|module| {
+    if seen_roots.contains(module.specifier()) {
+      None
+    } else {
+      maybe_get_check_entry(module, check_js)
+    }
   }));
   result
 }

cli/tsc/99_main_compiler.js

@@ -20,9 +20,6 @@ delete Object.prototype.__proto__;
   let logDebug = false;
   let logSource = "JS";
 
-  /** @type {string=} */
-  let cwd;
-
   // The map from the normalized specifier to the original.
   // TypeScript normalizes the specifier in its internal processing,
   // but the original specifier is needed when looking up the source from the runtime.
@@ -349,6 +346,7 @@ delete Object.prototype.__proto__;
   // analysis in Rust operates on fully resolved URLs,
   // it makes sense to use the same scheme here.
   const ASSETS_URL_PREFIX = "asset:///";
+  const CACHE_URL_PREFIX = "cache:///";
 
   /** Diagnostics that are intentionally ignored when compiling TypeScript in
    * Deno, as they provide misleading or incorrect information. */
@@ -447,8 +445,9 @@ delete Object.prototype.__proto__;
       if (logDebug) {
         debug(`host.fileExists("${specifier}")`);
       }
-      specifier = normalizedToOriginalMap.get(specifier) ?? specifier;
-      return ops.op_exists({ specifier });
+      // this is used by typescript to find the libs path
+      // so we can completely ignore it
+      return false;
     },
     readFile(specifier) {
       if (logDebug) {
@@ -527,7 +526,7 @@ delete Object.prototype.__proto__;
       if (logDebug) {
         debug(`host.getCurrentDirectory()`);
       }
-      return cwd ?? ops.op_cwd();
+      return CACHE_URL_PREFIX;
     },
     getCanonicalFileName(fileName) {
       return fileName;
@@ -1177,13 +1176,12 @@ delete Object.prototype.__proto__;
     }
   }
 
-  /** @param {{ debug: boolean; rootUri?: string; }} init */
-  function serverInit({ debug: debugFlag, rootUri }) {
+  /** @param {{ debug: boolean; }} init */
+  function serverInit({ debug: debugFlag }) {
     if (hasStarted) {
       throw new Error("The language server has already been initialized.");
     }
     hasStarted = true;
-    cwd = rootUri;
     languageService = ts.createLanguageService(host, documentRegistry);
     setLogDebug(debugFlag, "TSLS");
     debug("serverInit()");

cli/tsc/mod.rs

@@ -352,7 +352,6 @@ pub struct Request {
   pub debug: bool,
   pub graph: Arc<ModuleGraph>,
   pub hash_data: Vec<Vec<u8>>,
-  pub maybe_config_specifier: Option<ModuleSpecifier>,
   pub maybe_npm_resolver: Option<NpmPackageResolver>,
   pub maybe_tsbuildinfo: Option<String>,
   /// A vector of strings that represent the root/entry point modules for the
@@ -374,7 +373,6 @@ pub struct Response {
 struct State {
   hash_data: Vec<Vec<u8>>,
   graph: Arc<ModuleGraph>,
-  maybe_config_specifier: Option<ModuleSpecifier>,
   maybe_tsbuildinfo: Option<String>,
   maybe_response: Option<RespondArgs>,
   maybe_npm_resolver: Option<NpmPackageResolver>,
@@ -386,7 +384,6 @@ impl State {
   pub fn new(
     graph: Arc<ModuleGraph>,
     hash_data: Vec<Vec<u8>>,
-    maybe_config_specifier: Option<ModuleSpecifier>,
     maybe_npm_resolver: Option<NpmPackageResolver>,
     maybe_tsbuildinfo: Option<String>,
     root_map: HashMap<String, ModuleSpecifier>,
@@ -395,7 +392,6 @@ impl State {
     State {
       hash_data,
       graph,
-      maybe_config_specifier,
       maybe_npm_resolver,
       maybe_tsbuildinfo,
       maybe_response: None,
@@ -406,8 +402,7 @@ impl State {
 }
 
 fn normalize_specifier(specifier: &str) -> Result<ModuleSpecifier, AnyError> {
-  resolve_url_or_path(&specifier.replace(".d.ts.d.ts", ".d.ts"))
-    .map_err(|err| err.into())
+  resolve_url_or_path(specifier).map_err(|err| err.into())
 }
 
 #[derive(Debug, Deserialize)]
@@ -429,17 +424,6 @@ fn op_create_hash(s: &mut OpState, args: Value) -> Result<Value, AnyError> {
   Ok(json!({ "hash": hash }))
 }
 
-#[op]
-fn op_cwd(s: &mut OpState) -> Result<String, AnyError> {
-  let state = s.borrow_mut::<State>();
-  if let Some(config_specifier) = &state.maybe_config_specifier {
-    let cwd = config_specifier.join("./")?;
-    Ok(cwd.to_string())
-  } else {
-    Ok("cache:///".to_string())
-  }
-}
-
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct EmitArgs {
@@ -465,27 +449,6 @@ fn op_emit(state: &mut OpState, args: EmitArgs) -> bool {
   true
 }
 
-#[derive(Debug, Deserialize)]
-struct ExistsArgs {
-  /// The fully qualified specifier that should be loaded.
-  specifier: String,
-}
-
-#[op]
-fn op_exists(state: &mut OpState, args: ExistsArgs) -> bool {
-  let state = state.borrow_mut::<State>();
-  let graph = &state.graph;
-  if let Ok(specifier) = normalize_specifier(&args.specifier) {
-    if specifier.scheme() == "asset" || specifier.scheme() == "data" {
-      true
-    } else {
-      graph.get(&specifier).is_some()
-    }
-  } else {
-    false
-  }
-}
-
 #[derive(Debug, Deserialize)]
 struct LoadArgs {
   /// The fully qualified specifier that should be loaded.
@@ -866,7 +829,6 @@ pub fn exec(request: Request) -> Result<Response, AnyError> {
         state.put(State::new(
           request.graph.clone(),
           request.hash_data.clone(),
-          request.maybe_config_specifier.clone(),
           request.maybe_npm_resolver.clone(),
           request.maybe_tsbuildinfo.clone(),
           root_map.clone(),
@@ -912,10 +874,8 @@ pub fn exec(request: Request) -> Result<Response, AnyError> {
 
 fn get_tsc_ops() -> Vec<deno_core::OpDecl> {
   vec![
-    op_cwd::decl(),
     op_create_hash::decl(),
     op_emit::decl(),
-    op_exists::decl(),
     op_is_node_file::decl(),
     op_load::decl(),
     op_resolve::decl(),
@@ -982,7 +942,6 @@ mod tests {
       Arc::new(graph),
       hash_data,
       None,
-      None,
       maybe_tsbuildinfo,
       HashMap::new(),
       HashMap::new(),
@@ -1024,7 +983,6 @@ mod tests {
       debug: false,
       graph: Arc::new(graph),
       hash_data,
-      maybe_config_specifier: None,
       maybe_npm_resolver: None,
       maybe_tsbuildinfo: None,
       root_names: vec![(specifier.clone(), MediaType::TypeScript)],