diff options
| -rw-r--r-- | cli/lsp/analysis.rs | 79 | ||||
| -rw-r--r-- | cli/lsp/diagnostics.rs | 123 | ||||
| -rw-r--r-- | cli/module_graph.rs | 54 | ||||
| -rw-r--r-- | cli/tests/integration_tests_lsp.rs | 45 | ||||
| -rw-r--r-- | cli/tests/lsp/diagnostics_deno_types.json | 101 | ||||
| -rw-r--r-- | cli/tests/lsp/did_open_params_deno_types.json | 8 |
6 files changed, 335 insertions, 75 deletions
diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 19ee60d4c..831ad9b29 100644 --- a/cli/lsp/analysis.rs +++ b/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) - } else { - let dep = dependencies.entry(import).or_default(); - dep.maybe_type = Some(resolved_import); + maybe_type = Some(resolved_import.clone()); } + 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() { - dep.maybe_type = maybe_resolved_type_dependency; + if dep.maybe_type.is_none() { + 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, }) ); } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index ea781abde..9b9035ac5 100644 --- a/cli/lsp/diagnostics.rs +++ b/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 - if let (Some(code), Some(range)) = - (dependency.maybe_code, dependency.maybe_code_specifier_range) - { - match code { - 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() - }) - } - } - }, - } - } + diagnose_dependency( + &mut diagnostics, + &documents, + &sources, + &dependency.maybe_code, + &dependency.maybe_code_specifier_range, + ); + diagnose_dependency( + &mut diagnostics, + &documents, + &sources, + &dependency.maybe_type, + &dependency.maybe_type_specifier_range, + ); } } diagnostics_vec.push((specifier.clone(), version, diagnostics)); diff --git a/cli/module_graph.rs b/cli/module_graph.rs index 8613be1e7..368df0a74 100644 --- a/cli/module_graph.rs +++ b/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: ®ex::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> { - if !TRIPLE_SLASH_REFERENCE_RE.is_match(comment) { +pub fn parse_ts_reference( + 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) { - Some(TypeScriptReference::Path( - captures.get(1).unwrap().as_str().to_string(), + } else if let Some(captures) = PATH_REFERENCE_RE.captures(&comment.text) { + let m = captures.get(1).unwrap(); + Some(( + TypeScriptReference::Path(m.as_str().to_string()), + match_to_span(comment, &m), )) } else { - TYPES_REFERENCE_RE.captures(comment).map(|captures| { - TypeScriptReference::Types(captures.get(1).unwrap().as_str().to_string()) + TYPES_REFERENCE_RE.captures(&comment.text).map(|captures| { + 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> { - if let Some(captures) = DENO_TYPES_RE.captures(comment) { - if let Some(m) = captures.get(1) { - Some(m.as_str().to_string()) - } else if let Some(m) = captures.get(2) { - Some(m.as_str().to_string()) - } else { - panic!("unreachable"); - } +pub fn parse_deno_types(comment: &Comment) -> Option<(String, Span)> { + let captures = DENO_TYPES_RE.captures(&comment.text)?; + if let Some(m) = captures.get(1) { + Some((m.as_str().to_string(), match_to_span(comment, &m))) + } else if let Some(m) = captures.get(2) { + Some((m.as_str().to_string(), match_to_span(comment, &m))) } 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 diff --git a/cli/tests/integration_tests_lsp.rs b/cli/tests/integration_tests_lsp.rs index 76041c502..8e04cbb95 100644 --- a/cli/tests/integration_tests_lsp.rs +++ b/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 { diff --git a/cli/tests/lsp/diagnostics_deno_types.json b/cli/tests/lsp/diagnostics_deno_types.json new file mode 100644 index 000000000..f33945a59 --- /dev/null +++ b/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 +} diff --git a/cli/tests/lsp/did_open_params_deno_types.json b/cli/tests/lsp/did_open_params_deno_types.json new file mode 100644 index 000000000..6f085d045 --- /dev/null +++ b/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" + } +} |