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

6 files changed, 166 insertions, 21 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,
+          }
+        }),
       })
     );
   }
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index 1d0a1fac9..c468fb0fa 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -2,6 +2,7 @@
 
 use super::analysis::get_lint_references;
 use super::analysis::references_to_diagnostics;
+use super::analysis::ResolvedDependency;
 use super::language_server::StateSnapshot;
 use super::memory_cache::FileId;
 use super::tsc;
@@ -9,6 +10,7 @@ use super::tsc;
 use crate::diagnostics;
 use crate::media_type::MediaType;
 
+use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::serde_json::Value;
@@ -19,6 +21,7 @@ use std::mem;
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub enum DiagnosticSource {
+  Deno,
   Lint,
   TypeScript,
 }
@@ -261,3 +264,76 @@ pub async fn generate_ts_diagnostics(
 
   Ok(diagnostics)
 }
+
+pub async fn generate_dependency_diagnostics(
+  state_snapshot: StateSnapshot,
+  diagnostic_collection: DiagnosticCollection,
+) -> Result<DiagnosticVec, AnyError> {
+  tokio::task::spawn_blocking(move || {
+    let mut diagnostics = Vec::new();
+
+    let file_cache = state_snapshot.file_cache.read().unwrap();
+    let mut sources = if let Ok(sources) = state_snapshot.sources.write() {
+      sources
+    } else {
+      return Err(custom_error("Deadlock", "deadlock locking sources"));
+    };
+    for (specifier, doc_data) in state_snapshot.doc_data.iter() {
+      let file_id = file_cache.lookup(specifier).unwrap();
+      let version = doc_data.version;
+      let current_version = diagnostic_collection.get_version(&file_id);
+      if version != current_version {
+        let mut diagnostic_list = Vec::new();
+        if let Some(dependencies) = &doc_data.dependencies {
+          for (_, dependency) in dependencies.iter() {
+            if let (Some(code), Some(range)) = (
+              &dependency.maybe_code,
+              &dependency.maybe_code_specifier_range,
+            ) {
+              match code.clone() {
+                ResolvedDependency::Err(message) => {
+                  diagnostic_list.push(lsp_types::Diagnostic {
+                    range: *range,
+                    severity: Some(lsp_types::DiagnosticSeverity::Error),
+                    code: None,
+                    code_description: None,
+                    source: Some("deno".to_string()),
+                    message,
+                    related_information: None,
+                    tags: None,
+                    data: None,
+                  })
+                }
+                ResolvedDependency::Resolved(specifier) => {
+                  if !(state_snapshot.doc_data.contains_key(&specifier) || sources.contains(&specifier)) {
+                    let is_local = specifier.as_url().scheme() == "file";
+                    diagnostic_list.push(lsp_types::Diagnostic {
+                      range: *range,
+                      severity: Some(lsp_types::DiagnosticSeverity::Error),
+                      code: None,
+                      code_description: None,
+                      source: Some("deno".to_string()),
+                      message: if is_local {
+                        format!("Unable to load a local module: \"{}\".\n  Please check the file path.", specifier)
+                      } else {
+                        format!("Unable to load the module: \"{}\".\n  If the module exists, running `deno cache {}` should resolve this error.", specifier, specifier)
+                      },
+                      related_information: None,
+                      tags: None,
+                      data: None,
+                    })
+                  }
+                },
+              }
+            }
+          }
+        }
+        diagnostics.push((file_id, version, diagnostic_list))
+      }
+    }
+
+    Ok(diagnostics)
+  })
+  .await
+  .unwrap()
+}
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 7834cab7f..72d1b1ad3 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -178,9 +178,35 @@ impl LanguageServer {
       Ok::<(), AnyError>(())
     };
 
-    let (lint_res, ts_res) = tokio::join!(lint, ts);
+    let deps = async {
+      if enabled {
+        let diagnostics_collection = self.diagnostics.read().unwrap().clone();
+        let diagnostics = diagnostics::generate_dependency_diagnostics(
+          self.snapshot(),
+          diagnostics_collection,
+        )
+        .await?;
+        {
+          let mut diagnostics_collection = self.diagnostics.write().unwrap();
+          for (file_id, version, diagnostics) in diagnostics {
+            diagnostics_collection.set(
+              file_id,
+              DiagnosticSource::Deno,
+              version,
+              diagnostics,
+            );
+          }
+        }
+        self.publish_diagnostics().await?
+      };
+
+      Ok::<(), AnyError>(())
+    };
+
+    let (lint_res, ts_res, deps_res) = tokio::join!(lint, ts, deps);
     lint_res?;
     ts_res?;
+    deps_res?;
 
     Ok(())
   }
@@ -213,6 +239,11 @@ impl LanguageServer {
               .diagnostics_for(file_id, DiagnosticSource::TypeScript)
               .cloned(),
           );
+          diagnostics.extend(
+            diagnostics_collection
+              .diagnostics_for(file_id, DiagnosticSource::Deno)
+              .cloned(),
+          );
         }
         let specifier = {
           let file_cache = self.file_cache.read().unwrap();
diff --git a/cli/lsp/sources.rs b/cli/lsp/sources.rs
index 63b4ebd99..c6ab87f21 100644
--- a/cli/lsp/sources.rs
+++ b/cli/lsp/sources.rs
@@ -23,7 +23,7 @@ use std::time::SystemTime;
 #[derive(Debug, Clone, Default)]
 struct Metadata {
   dependencies: Option<HashMap<String, analysis::Dependency>>,
-  maybe_types: Option<analysis::ResolvedImport>,
+  maybe_types: Option<analysis::ResolvedDependency>,
   media_type: MediaType,
   source: String,
   version: String,
@@ -255,7 +255,7 @@ impl Sources {
     let dependencies = &metadata.dependencies?;
     let dependency = dependencies.get(specifier)?;
     if let Some(type_dependency) = &dependency.maybe_type {
-      if let analysis::ResolvedImport::Resolved(resolved_specifier) =
+      if let analysis::ResolvedDependency::Resolved(resolved_specifier) =
         type_dependency
       {
         self.resolution_result(resolved_specifier)
@@ -264,7 +264,7 @@ impl Sources {
       }
     } else {
       let code_dependency = &dependency.maybe_code.clone()?;
-      if let analysis::ResolvedImport::Resolved(resolved_specifier) =
+      if let analysis::ResolvedDependency::Resolved(resolved_specifier) =
         code_dependency
       {
         self.resolution_result(resolved_specifier)
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 4cd13f70d..2a0f7d76c 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -1,6 +1,6 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
-use super::analysis::ResolvedImport;
+use super::analysis::ResolvedDependency;
 use super::language_server::StateSnapshot;
 use super::text;
 use super::utils;
@@ -839,9 +839,10 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
             } else if let Some(resolved_import) = &dependency.maybe_code {
               resolved_import.clone()
             } else {
-              ResolvedImport::Err("missing dependency".to_string())
+              ResolvedDependency::Err("missing dependency".to_string())
             };
-          if let ResolvedImport::Resolved(resolved_specifier) = resolved_import
+          if let ResolvedDependency::Resolved(resolved_specifier) =
+            resolved_import
           {
             if state
               .state_snapshot
diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
index 9b08dee93..de9e74d2e 100644
--- a/cli/tsc/99_main_compiler.js
+++ b/cli/tsc/99_main_compiler.js
@@ -128,6 +128,9 @@ delete Object.prototype.__proto__;
     // TS2691: An import path cannot end with a '.ts' extension. Consider
     // importing 'bad-module' instead.
     2691,
+    // TS2792: Cannot find module. Did you mean to set the 'moduleResolution'
+    // option to 'node', or to add aliases to the 'paths' option?
+    2792,
     // TS5009: Cannot find the common subdirectory path for the input files.
     5009,
     // TS5055: Cannot write file