diff options
Diffstat (limited to 'cli/lsp/diagnostics.rs')
| -rw-r--r-- | cli/lsp/diagnostics.rs | 184 |
1 files changed, 139 insertions, 45 deletions
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 965075a2d..b650d8e55 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -857,24 +857,24 @@ impl DenoDiagnostic { } fn diagnose_resolution( - diagnostics: &mut Vec<lsp::Diagnostic>, + lsp_diagnostics: &mut Vec<lsp::Diagnostic>, snapshot: &language_server::StateSnapshot, resolution: &Resolution, is_dynamic: bool, maybe_assert_type: Option<&str>, + ranges: Vec<lsp::Range>, ) { + let mut diagnostics = vec![]; match resolution { Resolution::Ok(resolved) => { let specifier = &resolved.specifier; - let range = documents::to_lsp_range(&resolved.range); // If the module is a remote module and has a `X-Deno-Warning` header, we // want a warning diagnostic with that message. if let Some(metadata) = snapshot.cache_metadata.get(specifier) { if let Some(message) = metadata.get(&cache::MetadataKey::Warning).cloned() { - diagnostics - .push(DenoDiagnostic::DenoWarn(message).to_lsp_diagnostic(&range)); + diagnostics.push(DenoDiagnostic::DenoWarn(message)); } } if let Some(doc) = snapshot.documents.get(specifier) { @@ -883,13 +883,10 @@ fn diagnose_resolution( // diagnostic that indicates this. This then allows us to issue a code // action to replace the specifier with the final redirected one. if doc_specifier != specifier { - diagnostics.push( - DenoDiagnostic::Redirect { - from: specifier.clone(), - to: doc_specifier.clone(), - } - .to_lsp_diagnostic(&range), - ); + diagnostics.push(DenoDiagnostic::Redirect { + from: specifier.clone(), + to: doc_specifier.clone(), + }); } if doc.media_type() == MediaType::Json { match maybe_assert_type { @@ -900,13 +897,10 @@ fn diagnose_resolution( // not provide a potentially incorrect diagnostic. None if is_dynamic => (), // The module has an incorrect assertion type, diagnostic - Some(assert_type) => diagnostics.push( - DenoDiagnostic::InvalidAssertType(assert_type.to_string()) - .to_lsp_diagnostic(&range), - ), + Some(assert_type) => diagnostics + .push(DenoDiagnostic::InvalidAssertType(assert_type.to_string())), // The module is missing an assertion type, diagnostic - None => diagnostics - .push(DenoDiagnostic::NoAssertType.to_lsp_diagnostic(&range)), + None => diagnostics.push(DenoDiagnostic::NoAssertType), } } } else if let Ok(pkg_ref) = @@ -918,19 +912,15 @@ fn diagnose_resolution( .resolve_pkg_id_from_pkg_req(&pkg_ref.req) .is_err() { - diagnostics.push( - DenoDiagnostic::NoCacheNpm(pkg_ref, specifier.clone()) - .to_lsp_diagnostic(&range), - ); + diagnostics + .push(DenoDiagnostic::NoCacheNpm(pkg_ref, specifier.clone())); } } } else if let Some(module_name) = specifier.as_str().strip_prefix("node:") { if deno_node::resolve_builtin_node_module(module_name).is_err() { - diagnostics.push( - DenoDiagnostic::InvalidNodeSpecifier(specifier.clone()) - .to_lsp_diagnostic(&range), - ); + diagnostics + .push(DenoDiagnostic::InvalidNodeSpecifier(specifier.clone())); } else if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { // check that a @types/node package exists in the resolver let types_node_ref = @@ -939,13 +929,10 @@ fn diagnose_resolution( .resolve_pkg_id_from_pkg_req(&types_node_ref.req) .is_err() { - diagnostics.push( - DenoDiagnostic::NoCacheNpm( - types_node_ref, - ModuleSpecifier::parse("npm:@types/node").unwrap(), - ) - .to_lsp_diagnostic(&range), - ); + diagnostics.push(DenoDiagnostic::NoCacheNpm( + types_node_ref, + ModuleSpecifier::parse("npm:@types/node").unwrap(), + )); } } } else { @@ -958,17 +945,21 @@ fn diagnose_resolution( "blob" => DenoDiagnostic::NoCacheBlob, _ => DenoDiagnostic::NoCache(specifier.clone()), }; - diagnostics.push(deno_diagnostic.to_lsp_diagnostic(&range)); + diagnostics.push(deno_diagnostic); } } // The specifier resolution resulted in an error, so we want to issue a // diagnostic for that. - Resolution::Err(err) => diagnostics.push( - DenoDiagnostic::ResolutionError(*err.clone()) - .to_lsp_diagnostic(&documents::to_lsp_range(err.range())), - ), + Resolution::Err(err) => { + diagnostics.push(DenoDiagnostic::ResolutionError(*err.clone())) + } _ => (), } + for range in ranges { + for diagnostic in &diagnostics { + lsp_diagnostics.push(diagnostic.to_lsp_diagnostic(&range)); + } + } } /// Generate diagnostics related to a dependency. The dependency is analyzed to @@ -1005,17 +996,43 @@ fn diagnose_dependency( diagnose_resolution( diagnostics, snapshot, - &dependency.maybe_code, - dependency.is_dynamic, - dependency.maybe_assert_type.as_deref(), - ); - diagnose_resolution( - diagnostics, - snapshot, - &dependency.maybe_type, + if dependency.maybe_code.is_none() { + &dependency.maybe_type + } else { + &dependency.maybe_code + }, dependency.is_dynamic, dependency.maybe_assert_type.as_deref(), + dependency + .imports + .iter() + .map(|i| documents::to_lsp_range(&i.range)) + .collect(), ); + // TODO(nayeemrmn): This is a crude way of detecting `@deno-types` which has + // a different specifier and therefore needs a separate call to + // `diagnose_resolution()`. It would be much cleaner if that were modelled as + // a separate dependency: https://github.com/denoland/deno_graph/issues/247. + if !dependency.maybe_type.is_none() + && !dependency + .imports + .iter() + .any(|i| dependency.maybe_type.includes(&i.range.start).is_some()) + { + let range = match &dependency.maybe_type { + Resolution::Ok(resolved) => documents::to_lsp_range(&resolved.range), + Resolution::Err(error) => documents::to_lsp_range(error.range()), + Resolution::None => unreachable!(), + }; + diagnose_resolution( + diagnostics, + snapshot, + &dependency.maybe_type, + dependency.is_dynamic, + dependency.maybe_assert_type.as_deref(), + vec![range], + ); + } } /// Generate diagnostics that come from Deno module resolution logic (like @@ -1376,4 +1393,81 @@ let c: number = "a"; }) ); } + + #[tokio::test] + async fn duplicate_diagnostics_for_duplicate_imports() { + let temp_dir = TempDir::new(); + let (snapshot, _) = setup( + &temp_dir, + &[( + "file:///a.ts", + r#" + // @deno-types="bad.d.ts" + import "bad.js"; + import "bad.js"; + "#, + 1, + LanguageId::TypeScript, + )], + None, + ); + let config = mock_config(); + let token = CancellationToken::new(); + let actual = generate_deno_diagnostics(&snapshot, &config, token).await; + assert_eq!(actual.len(), 1); + let (_, _, diagnostics) = actual.first().unwrap(); + assert_eq!( + json!(diagnostics), + json!([ + { + "range": { + "start": { + "line": 2, + "character": 15 + }, + "end": { + "line": 2, + "character": 23 + } + }, + "severity": 1, + "code": "import-prefix-missing", + "source": "deno", + "message": "Relative import path \"bad.js\" not prefixed with / or ./ or ../", + }, + { + "range": { + "start": { + "line": 3, + "character": 15 + }, + "end": { + "line": 3, + "character": 23 + } + }, + "severity": 1, + "code": "import-prefix-missing", + "source": "deno", + "message": "Relative import path \"bad.js\" not prefixed with / or ./ or ../", + }, + { + "range": { + "start": { + "line": 1, + "character": 23 + }, + "end": { + "line": 1, + "character": 33 + } + }, + "severity": 1, + "code": "import-prefix-missing", + "source": "deno", + "message": "Relative import path \"bad.d.ts\" not prefixed with / or ./ or ../", + }, + ]) + ); + } } |