fix(check): filter resolution errors for bare ambient modules (#30690)

cli/graph_util.rs

@@ -23,6 +23,7 @@ use deno_graph::ModuleGraph;
 use deno_graph::ModuleGraphError;
 use deno_graph::ModuleLoadError;
 use deno_graph::ResolutionError;
+use deno_graph::SpecifierError;
 use deno_graph::WorkspaceFastCheckOption;
 use deno_graph::source::Loader;
 use deno_graph::source::ResolveError;
@@ -43,6 +44,7 @@ use deno_resolver::npm::DenoInNpmPackageChecker;
 use deno_runtime::deno_permissions::PermissionsContainer;
 use deno_semver::SmallStackString;
 use deno_semver::jsr::JsrDepPackageReq;
+use import_map::ImportMapErrorKind;
 use indexmap::IndexMap;
 use node_resolver::errors::NodeJsErrorCode;
 use sys_traits::FsMetadata;
@@ -74,6 +76,7 @@ use crate::util::progress_bar::ProgressBar;
 pub struct GraphValidOptions<'a> {
   pub check_js: CheckJsOption<'a>,
   pub kind: GraphKind,
+  pub will_type_check: bool,
   /// Whether to exit the process for integrity check errors such as
   /// lockfile checksum mismatches and JSR integrity failures.
   /// Otherwise, surfaces integrity errors as errors.
@@ -106,6 +109,7 @@ pub fn graph_valid(
     GraphWalkErrorsOptions {
       check_js: options.check_js,
       kind: options.kind,
+      will_type_check: options.will_type_check,
       allow_unknown_media_types: options.allow_unknown_media_types,
       allow_unknown_jsr_exports: options.allow_unknown_jsr_exports,
     },
@@ -129,6 +133,7 @@ pub fn graph_valid(
 pub struct GraphWalkErrorsOptions<'a> {
   pub check_js: CheckJsOption<'a>,
   pub kind: GraphKind,
+  pub will_type_check: bool,
   pub allow_unknown_media_types: bool,
   pub allow_unknown_jsr_exports: bool,
 }
@@ -145,6 +150,7 @@ pub fn graph_walk_errors<'a>(
     sys: &CliSys,
     graph_kind: GraphKind,
     allow_unknown_media_types: bool,
+    will_type_check: bool,
     error: &ModuleGraphError,
   ) -> bool {
     if (graph_kind == GraphKind::TypesOnly || allow_unknown_media_types)
@@ -157,8 +163,7 @@ pub fn graph_walk_errors<'a>(
     }
 
     // surface these as typescript diagnostics instead
-    graph_kind.include_types()
+    will_type_check && has_module_graph_error_for_tsc_diagnostic(sys, error)
-      && has_module_graph_error_for_tsc_diagnostic(sys, error)
   }
 
   graph
@@ -177,6 +182,7 @@ pub fn graph_walk_errors<'a>(
         sys,
         graph.graph_kind(),
         options.allow_unknown_media_types,
+        options.will_type_check,
         &error,
       ) {
         log::debug!("Ignoring: {}", error);
@@ -269,14 +275,16 @@ pub fn module_error_for_tsc_diagnostic<'a>(
   }
 }
 
-pub struct ModuleNotFoundNodeResolutionErrorRef<'a> {
+#[derive(Debug)]
+pub struct ResolutionErrorRef<'a> {
   pub specifier: &'a str,
-  pub maybe_range: Option<&'a deno_graph::Range>,
+  pub range: &'a deno_graph::Range,
+  pub is_module_not_found: bool,
 }
 
 pub fn resolution_error_for_tsc_diagnostic(
   error: &ResolutionError,
-) -> Option<ModuleNotFoundNodeResolutionErrorRef<'_>> {
+) -> Option<ResolutionErrorRef<'_>> {
   fn is_module_not_found_code(code: NodeJsErrorCode) -> bool {
     match code {
       NodeJsErrorCode::ERR_INVALID_MODULE_SPECIFIER
@@ -295,28 +303,63 @@ pub fn resolution_error_for_tsc_diagnostic(
   }
 
   match error {
+    ResolutionError::InvalidDowngrade { .. }
+    | ResolutionError::InvalidJsrHttpsTypesImport { .. }
+    | ResolutionError::InvalidLocalImport { .. } => None,
+    ResolutionError::InvalidSpecifier { error, range } => match error {
+      SpecifierError::InvalidUrl(..) => None,
+      SpecifierError::ImportPrefixMissing { specifier, .. } => {
+        Some(ResolutionErrorRef {
+          specifier,
+          range,
+          is_module_not_found: false,
+        })
+      }
+    },
     ResolutionError::ResolverError {
       error,
       specifier,
       range,
     } => match error.as_ref() {
+      ResolveError::Specifier(error) => match error {
+        SpecifierError::InvalidUrl(..) => None,
+        SpecifierError::ImportPrefixMissing { specifier, .. } => {
+          Some(ResolutionErrorRef {
+            specifier,
+            range,
+            is_module_not_found: false,
+          })
+        }
+      },
+      ResolveError::ImportMap(error) => match error.as_kind() {
+        ImportMapErrorKind::JsonParse(_)
+        | ImportMapErrorKind::ImportMapNotObject
+        | ImportMapErrorKind::ImportsFieldNotObject
+        | ImportMapErrorKind::ScopesFieldNotObject
+        | ImportMapErrorKind::ScopePrefixNotObject(_)
+        | ImportMapErrorKind::BlockedByNullEntry(_)
+        | ImportMapErrorKind::SpecifierResolutionFailure { .. }
+        | ImportMapErrorKind::SpecifierBacktracksAbovePrefix { .. } => None,
+        ImportMapErrorKind::UnmappedBareSpecifier(specifier, _) => {
+          Some(ResolutionErrorRef {
+            specifier,
+            range,
+            is_module_not_found: false,
+          })
+        }
+      },
       ResolveError::Other(error) => {
         let is_module_not_found_error = downcast_ref_deno_resolve_error(error)
           .and_then(|err| err.maybe_node_code())
           .map(is_module_not_found_code)
           .unwrap_or(false);
-        if is_module_not_found_error {
+        is_module_not_found_error.then(|| ResolutionErrorRef {
-          Some(ModuleNotFoundNodeResolutionErrorRef {
           specifier,
-            maybe_range: Some(range),
+          range,
+          is_module_not_found: true,
         })
-        } else {
-          None
       }
-      }
-      ResolveError::Specifier(_) | ResolveError::ImportMap(_) => None,
     },
-    _ => None,
   }
 }
 
@@ -951,16 +994,18 @@ impl ModuleGraphBuilder {
     allow_unknown_media_types: bool,
     allow_unknown_jsr_exports: bool,
   ) -> Result<(), JsErrorBox> {
+    let will_type_check = self.cli_options.type_check_mode().is_true();
     graph_valid(
       graph,
       &self.sys,
       roots,
       GraphValidOptions {
-        kind: if self.cli_options.type_check_mode().is_true() {
+        kind: if will_type_check {
           GraphKind::All
         } else {
           GraphKind::CodeOnly
         },
+        will_type_check,
         check_js: CheckJsOption::Custom(
           self.compiler_options_resolver.as_ref(),
         ),

cli/lsp/language_server.rs

@@ -409,6 +409,7 @@ impl LanguageServer {
         &roots,
         graph_util::GraphValidOptions {
           kind: GraphKind::All,
+          will_type_check: true,
           check_js: CheckJsOption::False,
           exit_integrity_errors: false,
           allow_unknown_media_types: true,

cli/tools/doc.rs

@@ -160,6 +160,7 @@ pub async fn doc(
         GraphWalkErrorsOptions {
           check_js: CheckJsOption::False,
           kind: GraphKind::TypesOnly,
+          will_type_check: false,
           allow_unknown_media_types: false,
           allow_unknown_jsr_exports: false,
         },

cli/tsc/diagnostics.rs

@@ -10,8 +10,12 @@ use deno_core::serde::Serialize;
 use deno_core::serde::Serializer;
 use deno_core::sourcemap::SourceMap;
 use deno_graph::ModuleGraph;
+use deno_graph::ResolutionError;
+use deno_resolver::graph::enhanced_resolution_error_message;
 use deno_terminal::colors;
 
+use crate::graph_util::resolution_error_for_tsc_diagnostic;
+
 const MAX_SOURCE_LINE_LENGTH: usize = 150;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -186,6 +190,34 @@ impl Diagnostic {
     }
   }
 
+  pub fn maybe_from_resolution_error(error: &ResolutionError) -> Option<Self> {
+    let error_ref = resolution_error_for_tsc_diagnostic(error)?;
+    if error_ref.is_module_not_found {
+      return Some(Self::from_missing_error(
+        error_ref.specifier,
+        Some(error_ref.range),
+        None,
+      ));
+    }
+    Some(Self {
+      category: DiagnosticCategory::Error,
+      code: 2307,
+      start: Some(Position::from_deno_graph(error_ref.range.range.start)),
+      end: Some(Position::from_deno_graph(error_ref.range.range.end)),
+      original_source_start: None, // will be applied later
+      message_text: Some(enhanced_resolution_error_message(error)),
+      message_chain: None,
+      source: None,
+      source_line: None,
+      file_name: Some(error_ref.range.specifier.to_string()),
+      related_information: None,
+      reports_deprecated: None,
+      reports_unnecessary: None,
+      other: Default::default(),
+      missing_specifier: Some(error_ref.specifier.to_string()),
+    })
+  }
+
   /// If this diagnostic should be included when it comes from a remote module.
   pub fn include_when_remote(&self) -> bool {
     /// TS6133: value is declared but its value is never read (noUnusedParameters and noUnusedLocals)

cli/type_checker.rs

@@ -36,7 +36,6 @@ use crate::cache::TypeCheckCache;
 use crate::graph_util::BuildFastCheckGraphOptions;
 use crate::graph_util::ModuleGraphBuilder;
 use crate::graph_util::module_error_for_tsc_diagnostic;
-use crate::graph_util::resolution_error_for_tsc_diagnostic;
 use crate::node::CliNodeResolver;
 use crate::npm::CliNpmResolver;
 use crate::sys::CliSys;
@@ -792,7 +791,9 @@ impl<'a> GraphWalker<'a> {
               }
             }
           }
-
+          if dep.is_dynamic {
+            continue;
+          }
           // only surface the code error if there's no type
           let dep_to_check_error = if dep.maybe_type.is_none() {
             &dep.maybe_code
@@ -801,16 +802,10 @@ impl<'a> GraphWalker<'a> {
           };
           if let deno_graph::Resolution::Err(resolution_error) =
             dep_to_check_error
-            && let Some(err) =
+            && let Some(diagnostic) =
-              resolution_error_for_tsc_diagnostic(resolution_error)
+              tsc::Diagnostic::maybe_from_resolution_error(resolution_error)
           {
-            self
+            self.missing_diagnostics.push(diagnostic);
-              .missing_diagnostics
-              .push(tsc::Diagnostic::from_missing_error(
-                err.specifier,
-                err.maybe_range,
-                None,
-              ));
           }
         }
       }

tests/registry/jsr/@denotest/multiple-exports/0.7.0_meta.json

@@ -2,7 +2,6 @@
   "exports": {
     "./add": "./add.ts",
     "./subtract": "./subtract.ts",
-    "./data-json": "./data.json",
+    "./data-json": "./data.json"
-    "./multiply": "./multiply.ts"
   }
 }

tests/registry/jsr/@denotest/multiple-exports/0.7.1_meta.json

@@ -2,6 +2,7 @@
   "exports": {
     "./add": "./add.ts",
     "./subtract": "./subtract.ts",
-    "./data-json": "./data.json"
+    "./data-json": "./data.json",
+    "./multiply": "./multiply.ts"
   }
 }

tests/specs/check/ambient_modules/__test__.jsonc

@@ -14,9 +14,5 @@
     "args": "run bar.ts",
     "output": "run.out",
     "exitCode": 1
-  }, {
-    "args": "check bare_specifier.ts",
-    "output": "bare_specifier.out",
-    "exitCode": 1
   }]
 }

tests/specs/check/ambient_modules/bare_specifier.out

@@ -1,3 +0,0 @@
-error: Relative import path "foo" not prefixed with / or ./ or ../
-  hint: If you want to use the npm package, try running `deno add npm:foo`
-    at file:///[WILDLINE]bare_specifier.ts:1:8

tests/specs/check/ambient_modules/bare_specifier.ts

@@ -1 +0,0 @@
-import "foo";

tests/specs/check/ambient_modules/foo.ts

@@ -1,4 +1,5 @@
 /// <reference types="./modules.d.ts" />
+import type {} from "bare.svg";
 import logo from "/logo.svg";
 import "./global.css";
 import styles from "./styles.module.css";

tests/specs/check/bare_specifier_not_found/check_import_map.out

@@ -1,3 +1,5 @@
-error: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ and not in import map from "file:///[WILDLINE]/main.ts"
+[WILDCARD]TS2307 [ERROR]: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ and not in import map from "file:///[WILDLINE]/main.ts"
   hint: If you want to use a JSR or npm package, try running `deno add jsr:@scope/BAZ` or `deno add npm:@scope/BAZ`
     at file:///[WILDLINE]
+
+error: Type checking failed.

tests/specs/check/bare_specifier_not_found/check_no_import_map.out

@@ -1,3 +1,5 @@
-error: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../
+[WILDCARD]TS2307 [ERROR]: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../
   hint: If you want to use a JSR or npm package, try running `deno add jsr:@scope/BAZ` or `deno add npm:@scope/BAZ`
     at file:///[WILDLINE]
+
+error: Type checking failed.

tests/specs/check/with_bare_import/095_cache_with_bare_import.ts.out

@@ -1,3 +1,5 @@
-[WILDCARD]error: Relative import path "foo" not prefixed with / or ./ or ../
+[WILDCARD]TS2307 [ERROR]: Relative import path "foo" not prefixed with / or ./ or ../
   hint: If you want to use the npm package, try running `deno add npm:foo`
     at file:///[WILDCARD]/095_cache_with_bare_import.ts:[WILDCARD]
+
+error: Type checking failed.

tests/specs/install/jsr_exports_uses_locked/deno.lock

@@ -10,7 +10,7 @@
       "integrity": "3b2e675c1ad7fba2a45bc251992e01aff08a3c974ac09079b11e6a5b95d4bfcb"
     },
     "@denotest/multiple-exports@0.7.0": {
-      "integrity": "e2dff9c6b3b083d80b869f14e12be35178baeb065e08a1fb48d30dc6afbd43a8",
+      "integrity": "efe9748a0c0939c7ac245fee04acc0c42bd7a61874ff71a360c4543e4f5f6b36",
       "dependencies": [
         "jsr:@denotest/add",
         "jsr:@denotest/subtract"

tests/specs/install/jsr_exports_uses_locked/install.out

@@ -3,7 +3,6 @@ Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0_meta.json
 Download http://127.0.0.1:4250/@denotest/multiple-exports/meta.json
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/add.ts
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/subtract.ts
-Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/multiply.ts
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/data.json
 Download http://127.0.0.1:4250/@denotest/add/meta.json
 Download http://127.0.0.1:4250/@denotest/subtract/meta.json

tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/deno.lock

@@ -11,14 +11,14 @@
       "integrity": "3b2e675c1ad7fba2a45bc251992e01aff08a3c974ac09079b11e6a5b95d4bfcb"
     },
     "@denotest/multiple-exports@0.7.0": {
-      "integrity": "e2dff9c6b3b083d80b869f14e12be35178baeb065e08a1fb48d30dc6afbd43a8",
+      "integrity": "efe9748a0c0939c7ac245fee04acc0c42bd7a61874ff71a360c4543e4f5f6b36",
       "dependencies": [
         "jsr:@denotest/add",
         "jsr:@denotest/subtract"
       ]
     },
     "@denotest/multiple-exports@0.7.1": {
-      "integrity": "1554a62f7434dbbeebc5dfc8eb0c690fb1eddb721579a12e918b4c4027b77f02",
+      "integrity": "84cf61094aa4b553522a114637fbc08f6cc9cd007892b7b111e8919e1b3ade63",
       "dependencies": [
         "jsr:@denotest/add",
         "jsr:@denotest/subtract"

tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/install.out

@@ -3,6 +3,7 @@ Download http://127.0.0.1:4250/@denotest/multiple-exports/meta.json
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/add.ts
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/data.json
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/subtract.ts
+Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/multiply.ts
 Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1_meta.json
 Download http://127.0.0.1:4250/@denotest/add/meta.json
 Download http://127.0.0.1:4250/@denotest/subtract/meta.json

tests/specs/run/error_type_definitions/error_type_definitions.ts.out

@@ -1,3 +1,8 @@
-[WILDCARD]error: Failed resolving types. Relative import path "baz" not prefixed with / or ./ or ../
+[WILDCARD]TS2307 [ERROR]: Relative import path "baz" not prefixed with / or ./ or ../
   hint: If you want to use the npm package, try running `deno add npm:baz`
     at [WILDCARD]/type_definitions/bar.d.ts:[WILDCARD]
+
+error: Type checking failed.
+
+  info: The program failed type-checking, but it still might work correctly.
+  hint: Re-run with --no-check to skip type-checking.