Skip to content

Commit

Permalink
fix: update lsp error message of 'relative import path' to 'use deno …
Browse files Browse the repository at this point in the history
…add' for npm/jsr packages (#24524)
  • Loading branch information
HasanAlrimawi authored Jul 24, 2024
1 parent 199a8ca commit fcd9bbe
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 3 deletions.
21 changes: 18 additions & 3 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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::<ImportMapError>() {
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 }))),
};
Expand Down
125 changes: 125 additions & 0 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<&str>>();
correct_lsp_messages.sort();
let part1 = correct_lsp_messages[1].split("file").collect::<Vec<_>>()[0];
let part2 = correct_lsp_messages[1].split('\n').collect::<Vec<_>>()[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::<Vec<&str>>();
correct_lsp_messages.sort();
let part1 = correct_lsp_messages[1].split("file").collect::<Vec<_>>()[0];
let part2 = correct_lsp_messages[1].split('\n').collect::<Vec<_>>()[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();
Expand Down

0 comments on commit fcd9bbe

Please sign in to comment.