Repository: denoland/deno
Pull request: fix(lsp): ignore a few more diagnostics for ambient modules

Missed a few resolution errors


Changed files:
filename: cli/lsp/diagnostics.rs
patch: @@ -1532,6 +1532,60 @@ fn relative_specifier(specifier: &Url, referrer: &Url) -> String {
   }
 }
 
+fn maybe_ambient_import_specifier(
+  diagnostic: &DenoDiagnostic,
+) -> Option<String> {
+  match diagnostic {
+    DenoDiagnostic::NoCache(url) | DenoDiagnostic::NoLocal(url) => {
+      Some(url.to_string())
+    }
+    DenoDiagnostic::ResolutionError(err) => {
+      maybe_ambient_specifier_resolution_err(err)
+    }
+    _ => None,
+  }
+}
+
+fn maybe_ambient_specifier_resolution_err(
+  err: &ResolutionError,
+) -> Option<String> {
+  match err {
+    ResolutionError::InvalidDowngrade { .. }
+    | ResolutionError::InvalidJsrHttpsTypesImport { .. }
+    | ResolutionError::InvalidLocalImport { .. } => None,
+    ResolutionError::InvalidSpecifier { error, .. } => match error {
+      SpecifierError::InvalidUrl(..) => None,
+      SpecifierError::ImportPrefixMissing { specifier, .. } => {
+        Some(specifier.to_string())
+      }
+    },
+    ResolutionError::ResolverError { error, .. } => match &**error {
+      ResolveError::Specifier(specifier_error) => match specifier_error {
+        SpecifierError::InvalidUrl(..) => None,
+        SpecifierError::ImportPrefixMissing { specifier, .. } => {
+          Some(specifier.to_string())
+        }
+      },
+      ResolveError::ImportMap(import_map_error) => {
+        match import_map_error.as_kind() {
+          ImportMapErrorKind::UnmappedBareSpecifier(spec, _) => {
+            Some(spec.clone())
+          }
+          ImportMapErrorKind::JsonParse(_)
+          | ImportMapErrorKind::ImportMapNotObject
+          | ImportMapErrorKind::ImportsFieldNotObject
+          | ImportMapErrorKind::ScopesFieldNotObject
+          | ImportMapErrorKind::ScopePrefixNotObject(_)
+          | ImportMapErrorKind::BlockedByNullEntry(_)
+          | ImportMapErrorKind::SpecifierResolutionFailure { .. }
+          | ImportMapErrorKind::SpecifierBacktracksAbovePrefix { .. } => None,
+        }
+      }
+      ResolveError::Other(..) => None,
+    },
+  }
+}
+
 fn diagnose_resolution(
   snapshot: &language_server::StateSnapshot,
   dependency_key: &str,
@@ -1672,16 +1726,14 @@ fn diagnose_resolution(
     }
     // The specifier resolution resulted in an error, so we want to issue a
     // diagnostic for that.
-    Resolution::Err(err) => match &**err {
-      ResolutionError::InvalidSpecifier {
-        error: SpecifierError::ImportPrefixMissing { .. },
-        ..
-      } => {
+    Resolution::Err(err) => {
+      if maybe_ambient_specifier_resolution_err(err).is_none() {
+        diagnostics.push(DenoDiagnostic::ResolutionError(*err.clone()))
+      } else {
         deferred_diagnostics
           .push(DenoDiagnostic::ResolutionError(*err.clone()));
       }
-      _ => diagnostics.push(DenoDiagnostic::ResolutionError(*err.clone())),
-    },
+    }
     _ => (),
   }
   (diagnostics, deferred_diagnostics)
@@ -1773,25 +1825,12 @@ fn diagnose_dependency(
   deferred_diagnostics.extend(
     deferred
       .iter()
-      .filter_map(|diag| match diag {
-        DenoDiagnostic::NoCache(url) | DenoDiagnostic::NoLocal(url) => Some(
-          Box::new(
-            import_ranges
-              .iter()
-              .map(|range| (url.to_string(), diag.to_lsp_diagnostic(range))),
-          ) as Box<dyn Iterator<Item = (String, lsp::Diagnostic)>>,
-        ),
-        DenoDiagnostic::ResolutionError(
-          ResolutionError::InvalidSpecifier {
-            error: SpecifierError::ImportPrefixMissing { specifier, .. },
-            ..
-          },
-        ) => {
-          Some(Box::new(import_ranges.iter().map(|range| {
-            (specifier.clone(), diag.to_lsp_diagnostic(range))
-          })))
-        }
-        _ => None,
+      .filter_map(|diag| {
+        maybe_ambient_import_specifier(diag).map(|spec| {
+          import_ranges
+            .iter()
+            .map(move |range| (spec.clone(), diag.to_lsp_diagnostic(range)))
+        })
       })
       .flatten(),
   );
@@ -1817,18 +1856,8 @@ fn diagnose_dependency(
         .map(|diag| diag.to_lsp_diagnostic(&range)),
     );
     deferred_diagnostics.extend(Box::new(deferred.iter().filter_map(|diag| {
-      match diag {
-        DenoDiagnostic::NoCache(url) | DenoDiagnostic::NoLocal(url) => {
-          Some((url.to_string(), diag.to_lsp_diagnostic(&range)))
-        }
-        DenoDiagnostic::ResolutionError(
-          ResolutionError::InvalidSpecifier {
-            error: SpecifierError::ImportPrefixMissing { specifier, .. },
-            ..
-          },
-        ) => Some((specifier.clone(), diag.to_lsp_diagnostic(&range))),
-        _ => None,
-      }
+      maybe_ambient_import_specifier(diag)
+        .map(|spec| (spec, diag.to_lsp_diagnostic(&range)))
     })));
   }
 }

filename: tests/specs/npm/dynamic_npm_resolution_failure/main.out
patch: @@ -1,8 +1,8 @@
 [UNORDERED_START]
 Download http://localhost:4260/chalk
 Download http://localhost:4260/@denotest%2fdep-cannot-parse
-[UNORDERED_END]
 Download http://localhost:4260/chalk/chalk-5.0.1.tgz
+[UNORDERED_END]
 Hi
 TypeError: Error in @denotest/dep-cannot-parse@1.0.0 parsing version requirement for dependency "@denotest/esm-basic": "unknown-scheme:unknown"
     0: Invalid version requirement