From fcd9bbe8eeec7d418b47544b6699e405c4d83826 Mon Sep 17 00:00:00 2001 From: HasanAlrimawi <141642411+HasanAlrimawi@users.noreply.github.com> Date: Wed, 24 Jul 2024 18:12:42 +0300 Subject: [PATCH] fix: update lsp error message of 'relative import path' to 'use deno add' for npm/jsr packages (#24524) --- cli/lsp/diagnostics.rs | 21 +++++- tests/integration/lsp_tests.rs | 125 +++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 3 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 53a6fd7d31c25a..899dc9681ad751 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -36,6 +36,7 @@ use deno_core::unsync::spawn_blocking; use deno_core::unsync::JoinHandle; use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; +use deno_graph::source::ResolveError; use deno_graph::Resolution; use deno_graph::ResolutionError; use deno_graph::SpecifierError; @@ -48,6 +49,7 @@ use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use import_map::ImportMap; +use import_map::ImportMapError; use log::error; use std::collections::HashMap; use std::collections::HashSet; @@ -1272,12 +1274,25 @@ impl DenoDiagnostic { (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, sloppy_resolution), data) }, Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{from}\" was redirected to \"{to}\"."), Some(json!({ "specifier": from, "redirect": to }))), - Self::ResolutionError(err) => ( + Self::ResolutionError(err) => { + let mut message; + message = enhanced_resolution_error_message(err); + if let deno_graph::ResolutionError::ResolverError {error, ..} = err{ + if let ResolveError::Other(resolve_error, ..) = (*error).as_ref() { + if let Some(ImportMapError::UnmappedBareSpecifier(specifier, _)) = resolve_error.downcast_ref::() { + if specifier.chars().next().unwrap_or('\0') == '@'{ + let hint = format!("\nHint: Use [deno add {}] to add the dependency.", specifier); + message.push_str(hint.as_str()); + } + } + } + } + ( lsp::DiagnosticSeverity::ERROR, - enhanced_resolution_error_message(err), + message, graph_util::get_resolution_error_bare_node_specifier(err) .map(|specifier| json!({ "specifier": specifier })) - ), + )}, Self::InvalidNodeSpecifier(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Unknown Node built-in module: {}", specifier.path()), None), Self::BareNodeSpecifier(specifier) => (lsp::DiagnosticSeverity::WARNING, format!("\"{}\" is resolved to \"node:{}\". If you want to use a built-in Node module, add a \"node:\" prefix.", specifier, specifier), Some(json!({ "specifier": specifier }))), }; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index cfcf72ddceef5b..e8904942de79ce 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -171,6 +171,131 @@ fn lsp_triple_slash_types() { client.shutdown(); } +#[test] +fn unadded_dependency_message_with_import_map() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write( + "import_map.json", + json!({ + "imports": { + + } + }) + .to_string(), + ); + temp_dir.write( + "deno.json", + json!({ + "importMap": "import_map.json".to_string(), + }) + .to_string(), + ); + temp_dir.write( + "file.ts", + r#" + import * as x from "@std/fs"; + "#, + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [[], temp_dir.uri().join("file.ts").unwrap()], + }), + ); + + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": temp_dir.read_to_string("file.ts"), + } + })); + // expected lsp_messages don't include the file path + let mut expected_lsp_messages = Vec::from(["`x` is never used\nIf this is intentional, prefix it with an underscore like `_x`", + "'x' is declared but its value is never read.", + "Relative import path \"@std/fs\" not prefixed with / or ./ or ../ and not in import map from \" Hint: Use [deno add @std/fs] to add the dependency."]); + expected_lsp_messages.sort(); + let all_diagnostics = diagnostics.all(); + let mut correct_lsp_messages = all_diagnostics + .iter() + .map(|d| d.message.as_str()) + .collect::>(); + correct_lsp_messages.sort(); + let part1 = correct_lsp_messages[1].split("file").collect::>()[0]; + let part2 = correct_lsp_messages[1].split('\n').collect::>()[1]; + let file_path_removed_from_message = format!("{} {}", part1, part2); + correct_lsp_messages[1] = file_path_removed_from_message.as_str(); + assert_eq!(correct_lsp_messages, expected_lsp_messages); + client.shutdown(); +} + +#[test] +fn unadded_dependency_message() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write( + "deno.json", + json!({ + "imports": { + + } + }) + .to_string(), + ); + temp_dir.write( + "file.ts", + r#" + import * as x from "@std/fs"; + "#, + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [[], temp_dir.uri().join("file.ts").unwrap()], + }), + ); + + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": temp_dir.read_to_string("file.ts"), + } + })); + // expected lsp_messages don't include the file path + let mut expected_lsp_messages = Vec::from(["`x` is never used\nIf this is intentional, prefix it with an underscore like `_x`", + "'x' is declared but its value is never read.", + "Relative import path \"@std/fs\" not prefixed with / or ./ or ../ and not in import map from \" Hint: Use [deno add @std/fs] to add the dependency."]); + expected_lsp_messages.sort(); + let all_diagnostics = diagnostics.all(); + let mut correct_lsp_messages = all_diagnostics + .iter() + .map(|d| d.message.as_str()) + .collect::>(); + correct_lsp_messages.sort(); + let part1 = correct_lsp_messages[1].split("file").collect::>()[0]; + let part2 = correct_lsp_messages[1].split('\n').collect::>()[1]; + let file_path_removed_from_message = format!("{} {}", part1, part2); + correct_lsp_messages[1] = file_path_removed_from_message.as_str(); + assert_eq!(correct_lsp_messages, expected_lsp_messages); + client.shutdown(); +} + #[test] fn lsp_import_map() { let context = TestContextBuilder::new().use_temp_cwd().build();