feat(lsp): diagnostics for deno types and triple-slash refs (#10699)

Fixes #9823

cli/lsp/analysis.rs

@@ -140,6 +140,7 @@ pub struct Dependency {
   pub maybe_code: Option<ResolvedDependency>,
   pub maybe_code_specifier_range: Option<Range>,
   pub maybe_type: Option<ResolvedDependency>,
+  pub maybe_type_specifier_range: Option<Range>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -259,13 +260,24 @@ pub fn analyze_dependencies(
 
   // Parse leading comments for supported triple slash references.
   for comment in parsed_module.get_leading_comments().iter() {
-    if let Some(ts_reference) = parse_ts_reference(&comment.text) {
+    if let Some((ts_reference, span)) = parse_ts_reference(&comment) {
+      let loc = parsed_module.source_map.lookup_char_pos(span.lo);
       match ts_reference {
         TypeScriptReference::Path(import) => {
           let dep = dependencies.entry(import.clone()).or_default();
           let resolved_import =
             resolve_import(&import, specifier, maybe_import_map);
           dep.maybe_code = Some(resolved_import);
+          dep.maybe_code_specifier_range = Some(Range {
+            start: Position {
+              line: (loc.line - 1) as u32,
+              character: loc.col_display as u32,
+            },
+            end: Position {
+              line: (loc.line - 1) as u32,
+              character: (loc.col_display + import.chars().count() + 2) as u32,
+            },
+          });
         }
         TypeScriptReference::Types(import) => {
           let resolved_import =
@@ -273,11 +285,20 @@ pub fn analyze_dependencies(
           if media_type == &MediaType::JavaScript
             || media_type == &MediaType::Jsx
           {
-            maybe_type = Some(resolved_import)
+            maybe_type = Some(resolved_import.clone());
-          } else {
-            let dep = dependencies.entry(import).or_default();
-            dep.maybe_type = Some(resolved_import);
           }
+          let dep = dependencies.entry(import.clone()).or_default();
+          dep.maybe_type = Some(resolved_import);
+          dep.maybe_type_specifier_range = Some(Range {
+            start: Position {
+              line: (loc.line - 1) as u32,
+              character: loc.col_display as u32,
+            },
+            end: Position {
+              line: (loc.line - 1) as u32,
+              character: (loc.col_display + import.chars().count() + 2) as u32,
+            },
+          });
         }
       }
     }
@@ -294,7 +315,13 @@ pub fn analyze_dependencies(
     let maybe_resolved_type_dependency =
       // Check for `@deno-types` pragmas that affect the import
       if let Some(comment) = desc.leading_comments.last() {
-        parse_deno_types(&comment.text).as_ref().map(|deno_types| resolve_import(deno_types, specifier, maybe_import_map))
+        parse_deno_types(&comment).as_ref().map(|(deno_types, span)| {
+          (
+            resolve_import(deno_types, specifier, maybe_import_map),
+            deno_types.clone(),
+            parsed_module.source_map.lookup_char_pos(span.lo)
+          )
+        })
       } else {
         None
       };
@@ -304,6 +331,17 @@ pub fn analyze_dependencies(
     match desc.kind {
       swc_ecmascript::dep_graph::DependencyKind::ExportType
       | swc_ecmascript::dep_graph::DependencyKind::ImportType => {
+        dep.maybe_type_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_type = Some(resolved_import)
       }
       _ => {
@@ -321,8 +359,22 @@ pub fn analyze_dependencies(
         dep.maybe_code = Some(resolved_import);
       }
     }
-    if maybe_resolved_type_dependency.is_some() && dep.maybe_type.is_none() {
+    if dep.maybe_type.is_none() {
-      dep.maybe_type = maybe_resolved_type_dependency;
+      if let Some((resolved_dependency, specifier, loc)) =
+        maybe_resolved_type_dependency
+      {
+        dep.maybe_type_specifier_range = Some(Range {
+          start: Position {
+            line: (loc.line - 1) as u32,
+            character: (loc.col_display + 1) as u32,
+          },
+          end: Position {
+            line: (loc.line - 1) as u32,
+            character: (loc.col_display + 1 + specifier.chars().count()) as u32,
+          },
+        });
+        dep.maybe_type = Some(resolved_dependency);
+      }
     }
   }
 
@@ -723,6 +775,16 @@ mod tests {
             character: 58,
           }
         }),
+        maybe_type_specifier_range: Some(Range {
+          start: Position {
+            line: 7,
+            character: 20,
+          },
+          end: Position {
+            line: 7,
+            character: 62,
+          }
+        })
       })
     );
     assert_eq!(
@@ -743,6 +805,7 @@ mod tests {
             character: 50,
           }
         }),
+        maybe_type_specifier_range: None,
       })
     );
   }

cli/lsp/diagnostics.rs

@@ -1,13 +1,16 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 
 use super::analysis;
+use super::documents::DocumentCache;
 use super::language_server;
+use super::sources::Sources;
 use super::tsc;
 
 use crate::diagnostics;
 use crate::media_type::MediaType;
 use crate::tokio_util::create_basic_runtime;
 
+use analysis::ResolvedDependency;
 use deno_core::error::anyhow;
 use deno_core::error::AnyError;
 use deno_core::resolve_url;
@@ -392,6 +395,64 @@ async fn generate_ts_diagnostics(
   Ok(diagnostics_vec)
 }
 
+fn diagnose_dependency(
+  diagnostics: &mut Vec<lsp::Diagnostic>,
+  documents: &DocumentCache,
+  sources: &Sources,
+  maybe_dependency: &Option<ResolvedDependency>,
+  maybe_range: &Option<lsp::Range>,
+) {
+  if let (Some(dep), Some(range)) = (maybe_dependency, *maybe_range) {
+    match dep {
+      analysis::ResolvedDependency::Err(err) => {
+        diagnostics.push(lsp::Diagnostic {
+          range,
+          severity: Some(lsp::DiagnosticSeverity::Error),
+          code: Some(err.as_code()),
+          code_description: None,
+          source: Some("deno".to_string()),
+          message: err.to_string(),
+          related_information: None,
+          tags: None,
+          data: None,
+        })
+      }
+      analysis::ResolvedDependency::Resolved(specifier) => {
+        if !(documents.contains_key(&specifier)
+          || sources.contains_key(&specifier))
+        {
+          let (code, message) = match specifier.scheme() {
+            "file" => (Some(lsp::NumberOrString::String("no-local".to_string())), format!("Unable to load a local module: \"{}\".\n  Please check the file path.", specifier)),
+            "data" => (Some(lsp::NumberOrString::String("no-cache-data".to_string())), "Uncached data URL.".to_string()),
+            "blob" => (Some(lsp::NumberOrString::String("no-cache-blob".to_string())), "Uncached blob URL.".to_string()),
+            _ => (Some(lsp::NumberOrString::String("no-cache".to_string())), format!("Uncached or missing remote URL: \"{}\".", specifier)),
+          };
+          diagnostics.push(lsp::Diagnostic {
+            range,
+            severity: Some(lsp::DiagnosticSeverity::Error),
+            code,
+            source: Some("deno".to_string()),
+            message,
+            data: Some(json!({ "specifier": specifier })),
+            ..Default::default()
+          });
+        } else if sources.contains_key(&specifier) {
+          if let Some(message) = sources.get_maybe_warning(&specifier) {
+            diagnostics.push(lsp::Diagnostic {
+              range,
+              severity: Some(lsp::DiagnosticSeverity::Warning),
+              code: Some(lsp::NumberOrString::String("deno-warn".to_string())),
+              source: Some("deno".to_string()),
+              message,
+              ..Default::default()
+            })
+          }
+        }
+      }
+    }
+  }
+}
+
 /// Generate diagnostics for dependencies of a module, attempting to resolve
 /// dependencies on the local file system or in the DENO_DIR cache.
 async fn generate_deps_diagnostics(
@@ -417,54 +478,20 @@ async fn generate_deps_diagnostics(
         let mut diagnostics = Vec::new();
         if let Some(dependencies) = documents.dependencies(specifier) {
           for (_, dependency) in dependencies {
-            // TODO(@kitsonk) add diagnostics for maybe_type dependencies
+            diagnose_dependency(
-            if let (Some(code), Some(range)) =
+              &mut diagnostics,
-              (dependency.maybe_code, dependency.maybe_code_specifier_range)
+              &documents,
-            {
+              &sources,
-              match code {
+              &dependency.maybe_code,
-                analysis::ResolvedDependency::Err(err) => diagnostics.push(lsp::Diagnostic {
+              &dependency.maybe_code_specifier_range,
-                  range,
+            );
-                  severity: Some(lsp::DiagnosticSeverity::Error),
+            diagnose_dependency(
-                  code: Some(err.as_code()),
+              &mut diagnostics,
-                  code_description: None,
+              &documents,
-                  source: Some("deno".to_string()),
+              &sources,
-                  message: err.to_string(),
+              &dependency.maybe_type,
-                  related_information: None,
+              &dependency.maybe_type_specifier_range,
-                  tags: None,
+            );
-                  data: None,
-                }),
-                analysis::ResolvedDependency::Resolved(specifier) => {
-                  if !(documents.contains_key(&specifier) || sources.contains_key(&specifier)) {
-                    let (code, message) = match specifier.scheme() {
-                      "file" => (Some(lsp::NumberOrString::String("no-local".to_string())), format!("Unable to load a local module: \"{}\".\n  Please check the file path.", specifier)),
-                      "data" => (Some(lsp::NumberOrString::String("no-cache-data".to_string())), "Uncached data URL.".to_string()),
-                      "blob" => (Some(lsp::NumberOrString::String("no-cache-blob".to_string())), "Uncached blob URL.".to_string()),
-                      _ => (Some(lsp::NumberOrString::String("no-cache".to_string())), format!("Uncached or missing remote URL: \"{}\".", specifier)),
-                    };
-                    diagnostics.push(lsp::Diagnostic {
-                      range,
-                      severity: Some(lsp::DiagnosticSeverity::Error),
-                      code,
-                      source: Some("deno".to_string()),
-                      message,
-                      data: Some(json!({ "specifier": specifier })),
-                      ..Default::default()
-                    });
-                  } else if sources.contains_key(&specifier) {
-                    if let Some(message) = sources.get_maybe_warning(&specifier) {
-                      diagnostics.push(lsp::Diagnostic {
-                        range,
-                        severity: Some(lsp::DiagnosticSeverity::Warning),
-                        code: Some(lsp::NumberOrString::String("deno-warn".to_string())),
-                        source: Some("deno".to_string()),
-                        message,
-                        ..Default::default()
-                      })
-                    }
-                  }
-                },
-              }
-            }
           }
         }
         diagnostics_vec.push((specifier.clone(), version, diagnostics));

cli/module_graph.rs

@@ -52,6 +52,9 @@ use std::result;
 use std::sync::Arc;
 use std::sync::Mutex;
 use std::time::Instant;
+use swc_common::comments::Comment;
+use swc_common::BytePos;
+use swc_common::Span;
 
 lazy_static::lazy_static! {
   /// Matched the `@deno-types` pragma.
@@ -188,35 +191,48 @@ pub enum TypeScriptReference {
   Types(String),
 }
 
+fn match_to_span(comment: &Comment, m: &regex::Match) -> Span {
+  Span {
+    lo: comment.span.lo + BytePos((m.start() + 1) as u32),
+    hi: comment.span.lo + BytePos((m.end() + 1) as u32),
+    ctxt: comment.span.ctxt,
+  }
+}
+
 /// Determine if a comment contains a triple slash reference and optionally
 /// return its kind and value.
-pub fn parse_ts_reference(comment: &str) -> Option<TypeScriptReference> {
+pub fn parse_ts_reference(
-  if !TRIPLE_SLASH_REFERENCE_RE.is_match(comment) {
+  comment: &Comment,
+) -> Option<(TypeScriptReference, Span)> {
+  if !TRIPLE_SLASH_REFERENCE_RE.is_match(&comment.text) {
     None
-  } else if let Some(captures) = PATH_REFERENCE_RE.captures(comment) {
+  } else if let Some(captures) = PATH_REFERENCE_RE.captures(&comment.text) {
-    Some(TypeScriptReference::Path(
+    let m = captures.get(1).unwrap();
-      captures.get(1).unwrap().as_str().to_string(),
+    Some((
+      TypeScriptReference::Path(m.as_str().to_string()),
+      match_to_span(comment, &m),
     ))
   } else {
-    TYPES_REFERENCE_RE.captures(comment).map(|captures| {
+    TYPES_REFERENCE_RE.captures(&comment.text).map(|captures| {
-      TypeScriptReference::Types(captures.get(1).unwrap().as_str().to_string())
+      let m = captures.get(1).unwrap();
+      (
+        TypeScriptReference::Types(m.as_str().to_string()),
+        match_to_span(comment, &m),
+      )
     })
   }
 }
 
 /// Determine if a comment contains a `@deno-types` pragma and optionally return
 /// its value.
-pub fn parse_deno_types(comment: &str) -> Option<String> {
+pub fn parse_deno_types(comment: &Comment) -> Option<(String, Span)> {
-  if let Some(captures) = DENO_TYPES_RE.captures(comment) {
+  let captures = DENO_TYPES_RE.captures(&comment.text)?;
-    if let Some(m) = captures.get(1) {
+  if let Some(m) = captures.get(1) {
-      Some(m.as_str().to_string())
+    Some((m.as_str().to_string(), match_to_span(comment, &m)))
-    } else if let Some(m) = captures.get(2) {
+  } else if let Some(m) = captures.get(2) {
-      Some(m.as_str().to_string())
+    Some((m.as_str().to_string(), match_to_span(comment, &m)))
-    } else {
-      panic!("unreachable");
-    }
   } else {
-    None
+    unreachable!();
   }
 }
 
@@ -327,7 +343,7 @@ impl Module {
 
     // parse out any triple slash references
     for comment in parsed_module.get_leading_comments().iter() {
-      if let Some(ts_reference) = parse_ts_reference(&comment.text) {
+      if let Some((ts_reference, _)) = parse_ts_reference(&comment) {
         let location = parsed_module.get_location(&comment.span);
         match ts_reference {
           TypeScriptReference::Path(import) => {
@@ -392,7 +408,7 @@ impl Module {
       // Parse out any `@deno-types` pragmas and modify dependency
       let maybe_type = if !desc.leading_comments.is_empty() {
         let comment = desc.leading_comments.last().unwrap();
-        if let Some(deno_types) = parse_deno_types(&comment.text).as_ref() {
+        if let Some((deno_types, _)) = parse_deno_types(&comment).as_ref() {
           Some(self.resolve_import(deno_types, Some(location.clone()))?)
         } else {
           None

cli/tests/integration_tests_lsp.rs

@@ -1398,6 +1398,11 @@ fn lsp_code_actions_deno_cache() {
       }
     }))
     .unwrap();
+  let (id, method, _) = client.read_request::<Value>().unwrap();
+  assert_eq!(method, "workspace/configuration");
+  client
+    .write_response(id, json!({ "enable": true }))
+    .unwrap();
   let (method, _) = client.read_notification::<Value>().unwrap();
   assert_eq!(method, "textDocument/publishDiagnostics");
   let (method, _) = client.read_notification::<Value>().unwrap();
@@ -1851,6 +1856,46 @@ fn lsp_diagnostics_warn() {
   shutdown(&mut client);
 }
 
+#[test]
+fn lsp_diagnostics_deno_types() {
+  let mut client = init("initialize_params.json");
+  client
+    .write_notification(
+      "textDocument/didOpen",
+      load_fixture("did_open_params_deno_types.json"),
+    )
+    .unwrap();
+  let (id, method, _) = client.read_request::<Value>().unwrap();
+  assert_eq!(method, "workspace/configuration");
+  client
+    .write_response(id, json!({ "enable": true }))
+    .unwrap();
+  let (maybe_res, maybe_err) = client
+    .write_request::<_, _, Value>(
+      "textDocument/documentSymbol",
+      json!({
+        "textDocument": {
+          "uri": "file:///a/file.ts"
+        }
+      }),
+    )
+    .unwrap();
+  assert!(maybe_res.is_some());
+  assert!(maybe_err.is_none());
+  let (method, _) = client.read_notification::<Value>().unwrap();
+  assert_eq!(method, "textDocument/publishDiagnostics");
+  let (method, _) = client.read_notification::<Value>().unwrap();
+  assert_eq!(method, "textDocument/publishDiagnostics");
+  let (method, maybe_params) = client
+    .read_notification::<lsp::PublishDiagnosticsParams>()
+    .unwrap();
+  assert_eq!(method, "textDocument/publishDiagnostics");
+  assert!(maybe_params.is_some());
+  let params = maybe_params.unwrap();
+  assert_eq!(params.diagnostics.len(), 5);
+  shutdown(&mut client);
+}
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct PerformanceAverage {

cli/tests/lsp/diagnostics_deno_types.json

@@ -0,0 +1,101 @@
+{
+  "uri": "file:///a/file.ts",
+  "diagnostics": [
+    {
+      "range": {
+        "start": {
+          "line": 0,
+          "character": 21
+        },
+        "end": {
+          "line": 0,
+          "character": 51
+        }
+      },
+      "severity": 1,
+      "code": "no-cache",
+      "source": "deno",
+      "message": "Uncached or missing remote URL: \"https://example.com/a/b.d.ts\".",
+      "data": {
+        "specifier": "https://example.com/a/b.d.ts"
+      }
+    },
+    {
+      "range": {
+        "start": {
+          "line": 7,
+          "character": 19
+        },
+        "end": {
+          "line": 7,
+          "character": 47
+        }
+      },
+      "severity": 1,
+      "code": "no-cache",
+      "source": "deno",
+      "message": "Uncached or missing remote URL: \"https://example.com/a/e.js\".",
+      "data": {
+        "specifier": "https://example.com/a/e.js"
+      }
+    },
+    {
+      "range": {
+        "start": {
+          "line": 6,
+          "character": 16
+        },
+        "end": {
+          "line": 6,
+          "character": 44
+        }
+      },
+      "severity": 1,
+      "code": "no-cache",
+      "source": "deno",
+      "message": "Uncached or missing remote URL: \"https://example.com/a/e.d.ts\".",
+      "data": {
+        "specifier": "https://example.com/a/e.d.ts"
+      }
+    },
+    {
+      "range": {
+        "start": {
+          "line": 4,
+          "character": 19
+        },
+        "end": {
+          "line": 4,
+          "character": 47
+        }
+      },
+      "severity": 1,
+      "code": "no-cache",
+      "source": "deno",
+      "message": "Uncached or missing remote URL: \"https://example.com/a/d.js\".",
+      "data": {
+        "specifier": "https://example.com/a/d.js"
+      }
+    },
+    {
+      "range": {
+        "start": {
+          "line": 3,
+          "character": 15
+        },
+        "end": {
+          "line": 3,
+          "character": 43
+        }
+      },
+      "severity": 1,
+      "code": "no-cache",
+      "source": "deno",
+      "message": "Uncached or missing remote URL: \"https://example.com/a/d.d.ts\".",
+      "data": {
+        "specifier": "https://example.com/a/d.d.ts"
+      }
+    }
+  ],
+  "version": 1
+}

cli/tests/lsp/did_open_params_deno_types.json

@@ -0,0 +1,8 @@
+{
+  "textDocument": {
+    "uri": "file:///a/file.ts",
+    "languageId": "typescript",
+    "version": 1,
+    "text": "/// <reference types=\"https://example.com/a/b.d.ts\" />\n/// <reference path=\"https://example.com/a/c.ts\"\n\n// @deno-types=https://example.com/a/d.d.ts\nimport * as d from \"https://example.com/a/d.js\";\n\n// @deno-types=\"https://example.com/a/e.d.ts\"\nimport * as e from \"https://example.com/a/e.js\";\n\nconsole.log(d, e);\n"
+  }
+}