fix(lsp): provide diagnostics for unresolved modules (#8872)

1 files changed, 48 insertions, 14 deletions

diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs
index 7cf6aca37..26f38ef38 100644
--- a/cli/lsp/analysis.rs
+++ b/cli/lsp/analysis.rs
@@ -86,7 +86,6 @@ pub fn references_to_diagnostics(
         severity: Some(lsp_types::DiagnosticSeverity::Warning),
         code: Some(lsp_types::NumberOrString::String(code)),
         code_description: None,
-        // TODO(@kitsonk) this won't make sense for every diagnostic
         source: Some("deno-lint".to_string()),
         message,
         related_information: None,
@@ -100,12 +99,13 @@ pub fn references_to_diagnostics(
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub struct Dependency {
   pub is_dynamic: bool,
-  pub maybe_code: Option<ResolvedImport>,
-  pub maybe_type: Option<ResolvedImport>,
+  pub maybe_code: Option<ResolvedDependency>,
+  pub maybe_code_specifier_range: Option<Range>,
+  pub maybe_type: Option<ResolvedDependency>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum ResolvedImport {
+pub enum ResolvedDependency {
   Resolved(ModuleSpecifier),
   Err(String),
 }
@@ -114,7 +114,7 @@ pub fn resolve_import(
   specifier: &str,
   referrer: &ModuleSpecifier,
   maybe_import_map: &Option<ImportMap>,
-) -> ResolvedImport {
+) -> ResolvedDependency {
   let maybe_mapped = if let Some(import_map) = maybe_import_map {
     if let Ok(maybe_specifier) =
       import_map.resolve(specifier, referrer.as_str())
@@ -132,13 +132,13 @@ pub fn resolve_import(
   } else {
     match ModuleSpecifier::resolve_import(specifier, referrer.as_str()) {
       Ok(resolved) => resolved,
-      Err(err) => return ResolvedImport::Err(err.to_string()),
+      Err(err) => return ResolvedDependency::Err(err.to_string()),
     }
   };
   let referrer_scheme = referrer.as_url().scheme();
   let specifier_scheme = specifier.as_url().scheme();
   if referrer_scheme == "https" && specifier_scheme == "http" {
-    return ResolvedImport::Err(
+    return ResolvedDependency::Err(
       "Modules imported via https are not allowed to import http modules."
         .to_string(),
     );
@@ -147,10 +147,10 @@ pub fn resolve_import(
     && !(specifier_scheme == "https" || specifier_scheme == "http")
     && !remapped
   {
-    return ResolvedImport::Err("Remote modules are not allowed to import local modules.  Consider using a dynamic import instead.".to_string());
+    return ResolvedDependency::Err("Remote modules are not allowed to import local modules.  Consider using a dynamic import instead.".to_string());
   }
 
-  ResolvedImport::Resolved(specifier)
+  ResolvedDependency::Resolved(specifier)
 }
 
 // TODO(@kitsonk) a lot of this logic is duplicated in module_graph.rs in
@@ -160,7 +160,7 @@ pub fn analyze_dependencies(
   source: &str,
   media_type: &MediaType,
   maybe_import_map: &Option<ImportMap>,
-) -> Option<(HashMap<String, Dependency>, Option<ResolvedImport>)> {
+) -> Option<(HashMap<String, Dependency>, Option<ResolvedDependency>)> {
   let specifier_str = specifier.to_string();
   let source_map = Rc::new(swc_common::SourceMap::default());
   let mut maybe_type = None;
@@ -222,7 +222,21 @@ pub fn analyze_dependencies(
         | swc_ecmascript::dep_graph::DependencyKind::ImportType => {
           dep.maybe_type = Some(resolved_import)
         }
-        _ => dep.maybe_code = Some(resolved_import),
+        _ => {
+          dep.maybe_code_specifier_range = Some(Range {
+            start: Position {
+              line: (desc.specifier_line - 1) as u32,
+              character: desc.specifier_col as u32,
+            },
+            end: Position {
+              line: (desc.specifier_line - 1) as u32,
+              character: (desc.specifier_col
+                + desc.specifier.chars().count()
+                + 2) as u32,
+            },
+          });
+          dep.maybe_code = Some(resolved_import);
+        }
       }
       if maybe_resolved_type_import.is_some() && dep.maybe_type.is_none() {
         dep.maybe_type = maybe_resolved_type_import;
@@ -293,27 +307,47 @@ mod tests {
       actual.get("https://cdn.skypack.dev/react").cloned(),
       Some(Dependency {
         is_dynamic: false,
-        maybe_code: Some(ResolvedImport::Resolved(
+        maybe_code: Some(ResolvedDependency::Resolved(
           ModuleSpecifier::resolve_url("https://cdn.skypack.dev/react")
             .unwrap()
         )),
-        maybe_type: Some(ResolvedImport::Resolved(
+        maybe_type: Some(ResolvedDependency::Resolved(
           ModuleSpecifier::resolve_url(
             "https://deno.land/x/types/react/index.d.ts"
           )
           .unwrap()
         )),
+        maybe_code_specifier_range: Some(Range {
+          start: Position {
+            line: 8,
+            character: 27,
+          },
+          end: Position {
+            line: 8,
+            character: 58,
+          }
+        }),
       })
     );
     assert_eq!(
       actual.get("https://deno.land/x/oak@v6.3.2/mod.ts").cloned(),
       Some(Dependency {
         is_dynamic: false,
-        maybe_code: Some(ResolvedImport::Resolved(
+        maybe_code: Some(ResolvedDependency::Resolved(
           ModuleSpecifier::resolve_url("https://deno.land/x/oak@v6.3.2/mod.ts")
             .unwrap()
         )),
         maybe_type: None,
+        maybe_code_specifier_range: Some(Range {
+          start: Position {
+            line: 5,
+            character: 11,
+          },
+          end: Position {
+            line: 5,
+            character: 50,
+          }
+        }),
       })
     );
   }