mirror of
https://github.com/denoland/deno.git
synced 2025-01-03 04:48:52 -05:00
fix(lsp): provide diagnostics for import assertions (#13105)
Fixes: #13099
This commit is contained in:
parent
0f53b82cd2
commit
e28fb70aee
6 changed files with 230 additions and 25 deletions
|
@ -384,31 +384,51 @@ pub struct CodeActionCollection {
|
||||||
impl CodeActionCollection {
|
impl CodeActionCollection {
|
||||||
pub(crate) fn add_deno_fix_action(
|
pub(crate) fn add_deno_fix_action(
|
||||||
&mut self,
|
&mut self,
|
||||||
|
specifier: &ModuleSpecifier,
|
||||||
diagnostic: &lsp::Diagnostic,
|
diagnostic: &lsp::Diagnostic,
|
||||||
) -> Result<(), AnyError> {
|
) -> Result<(), AnyError> {
|
||||||
if let Some(data) = diagnostic.data.clone() {
|
if let Some(lsp::NumberOrString::String(code)) = &diagnostic.code {
|
||||||
let fix_data: DenoFixData = serde_json::from_value(data)?;
|
if code == "no-assert-type" {
|
||||||
let title = if matches!(&diagnostic.code, Some(lsp::NumberOrString::String(code)) if code == "no-cache-data")
|
let code_action = lsp::CodeAction {
|
||||||
{
|
title: "Insert import assertion.".to_string(),
|
||||||
"Cache the data URL and its dependencies.".to_string()
|
kind: Some(lsp::CodeActionKind::QUICKFIX),
|
||||||
} else {
|
diagnostics: Some(vec![diagnostic.clone()]),
|
||||||
format!("Cache \"{}\" and its dependencies.", fix_data.specifier)
|
edit: Some(lsp::WorkspaceEdit {
|
||||||
};
|
changes: Some(HashMap::from([(
|
||||||
let code_action = lsp::CodeAction {
|
specifier.clone(),
|
||||||
title,
|
vec![lsp::TextEdit {
|
||||||
kind: Some(lsp::CodeActionKind::QUICKFIX),
|
new_text: " assert { type: \"json\" }".to_string(),
|
||||||
diagnostics: Some(vec![diagnostic.clone()]),
|
range: lsp::Range {
|
||||||
edit: None,
|
start: diagnostic.range.end,
|
||||||
command: Some(lsp::Command {
|
end: diagnostic.range.end,
|
||||||
title: "".to_string(),
|
},
|
||||||
command: "deno.cache".to_string(),
|
}],
|
||||||
arguments: Some(vec![json!([fix_data.specifier])]),
|
)])),
|
||||||
}),
|
..Default::default()
|
||||||
is_preferred: None,
|
}),
|
||||||
disabled: None,
|
..Default::default()
|
||||||
data: None,
|
};
|
||||||
};
|
self.actions.push(CodeActionKind::Deno(code_action));
|
||||||
self.actions.push(CodeActionKind::Deno(code_action));
|
} else if let Some(data) = diagnostic.data.clone() {
|
||||||
|
let fix_data: DenoFixData = serde_json::from_value(data)?;
|
||||||
|
let title = if code == "no-cache-data" {
|
||||||
|
"Cache the data URL and its dependencies.".to_string()
|
||||||
|
} else {
|
||||||
|
format!("Cache \"{}\" and its dependencies.", fix_data.specifier)
|
||||||
|
};
|
||||||
|
let code_action = lsp::CodeAction {
|
||||||
|
title,
|
||||||
|
kind: Some(lsp::CodeActionKind::QUICKFIX),
|
||||||
|
diagnostics: Some(vec![diagnostic.clone()]),
|
||||||
|
command: Some(lsp::Command {
|
||||||
|
title: "".to_string(),
|
||||||
|
command: "deno.cache".to_string(),
|
||||||
|
arguments: Some(vec![json!([fix_data.specifier])]),
|
||||||
|
}),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
self.actions.push(CodeActionKind::Deno(code_action));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
@ -9,6 +9,7 @@ use super::tsc;
|
||||||
|
|
||||||
use crate::diagnostics;
|
use crate::diagnostics;
|
||||||
|
|
||||||
|
use deno_ast::MediaType;
|
||||||
use deno_core::anyhow::anyhow;
|
use deno_core::anyhow::anyhow;
|
||||||
use deno_core::error::AnyError;
|
use deno_core::error::AnyError;
|
||||||
use deno_core::resolve_url;
|
use deno_core::resolve_url;
|
||||||
|
@ -439,6 +440,8 @@ fn diagnose_dependency(
|
||||||
diagnostics: &mut Vec<lsp::Diagnostic>,
|
diagnostics: &mut Vec<lsp::Diagnostic>,
|
||||||
documents: &Documents,
|
documents: &Documents,
|
||||||
resolved: &deno_graph::Resolved,
|
resolved: &deno_graph::Resolved,
|
||||||
|
is_dynamic: bool,
|
||||||
|
maybe_assert_type: Option<&str>,
|
||||||
) {
|
) {
|
||||||
match resolved {
|
match resolved {
|
||||||
Some(Ok((specifier, range))) => {
|
Some(Ok((specifier, range))) => {
|
||||||
|
@ -453,6 +456,34 @@ fn diagnose_dependency(
|
||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
if doc.media_type() == MediaType::Json {
|
||||||
|
match maybe_assert_type {
|
||||||
|
// The module has the correct assertion type, no diagnostic
|
||||||
|
Some("json") => (),
|
||||||
|
// The dynamic import statement is missing an assertion type, which
|
||||||
|
// we might not be able to statically detect, therefore we will
|
||||||
|
// not provide a potentially incorrect diagnostic.
|
||||||
|
None if is_dynamic => (),
|
||||||
|
// The module has an incorrect assertion type, diagnostic
|
||||||
|
Some(assert_type) => diagnostics.push(lsp::Diagnostic {
|
||||||
|
range: documents::to_lsp_range(range),
|
||||||
|
severity: Some(lsp::DiagnosticSeverity::ERROR),
|
||||||
|
code: Some(lsp::NumberOrString::String("invalid-assert-type".to_string())),
|
||||||
|
source: Some("deno".to_string()),
|
||||||
|
message: format!("The module is a JSON module and expected an assertion type of \"json\". Instead got \"{}\".", assert_type),
|
||||||
|
..Default::default()
|
||||||
|
}),
|
||||||
|
// The module is missing an assertion type, diagnostic
|
||||||
|
None => diagnostics.push(lsp::Diagnostic {
|
||||||
|
range: documents::to_lsp_range(range),
|
||||||
|
severity: Some(lsp::DiagnosticSeverity::ERROR),
|
||||||
|
code: Some(lsp::NumberOrString::String("no-assert-type".to_string())),
|
||||||
|
source: Some("deno".to_string()),
|
||||||
|
message: "The module is a JSON module and not being imported with an import assertion. Consider adding `assert { type: \"json\" }` to the import statement.".to_string(),
|
||||||
|
..Default::default()
|
||||||
|
}),
|
||||||
|
}
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
let (code, message) = match specifier.scheme() {
|
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)),
|
"file" => (Some(lsp::NumberOrString::String("no-local".to_string())), format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier)),
|
||||||
|
@ -508,11 +539,15 @@ async fn generate_deps_diagnostics(
|
||||||
&mut diagnostics,
|
&mut diagnostics,
|
||||||
&snapshot.documents,
|
&snapshot.documents,
|
||||||
&dependency.maybe_code,
|
&dependency.maybe_code,
|
||||||
|
dependency.is_dynamic,
|
||||||
|
dependency.maybe_assert_type.as_deref(),
|
||||||
);
|
);
|
||||||
diagnose_dependency(
|
diagnose_dependency(
|
||||||
&mut diagnostics,
|
&mut diagnostics,
|
||||||
&snapshot.documents,
|
&snapshot.documents,
|
||||||
&dependency.maybe_type,
|
&dependency.maybe_type,
|
||||||
|
dependency.is_dynamic,
|
||||||
|
dependency.maybe_assert_type.as_deref(),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
diagnostics_vec.push((
|
diagnostics_vec.push((
|
||||||
|
|
|
@ -1181,7 +1181,10 @@ impl Inner {
|
||||||
"deno-lint" => matches!(&d.code, Some(_)),
|
"deno-lint" => matches!(&d.code, Some(_)),
|
||||||
"deno" => match &d.code {
|
"deno" => match &d.code {
|
||||||
Some(NumberOrString::String(code)) => {
|
Some(NumberOrString::String(code)) => {
|
||||||
code == "no-cache" || code == "no-cache-data"
|
matches!(
|
||||||
|
code.as_str(),
|
||||||
|
"no-cache" | "no-cache-data" | "no-assert-type"
|
||||||
|
)
|
||||||
}
|
}
|
||||||
_ => false,
|
_ => false,
|
||||||
},
|
},
|
||||||
|
@ -1241,7 +1244,7 @@ impl Inner {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Some("deno") => code_actions
|
Some("deno") => code_actions
|
||||||
.add_deno_fix_action(diagnostic)
|
.add_deno_fix_action(&specifier, diagnostic)
|
||||||
.map_err(|err| {
|
.map_err(|err| {
|
||||||
error!("{}", err);
|
error!("{}", err);
|
||||||
LspError::internal_error()
|
LspError::internal_error()
|
||||||
|
|
|
@ -344,6 +344,72 @@ fn lsp_import_map_data_url() {
|
||||||
shutdown(&mut client);
|
shutdown(&mut client);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn lsp_import_assertions() {
|
||||||
|
let mut client = init("initialize_params_import_map.json");
|
||||||
|
client
|
||||||
|
.write_notification(
|
||||||
|
"textDocument/didOpen",
|
||||||
|
json!({
|
||||||
|
"textDocument": {
|
||||||
|
"uri": "file:///a/test.json",
|
||||||
|
"languageId": "json",
|
||||||
|
"version": 1,
|
||||||
|
"text": "{\"a\":1}"
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let mut diagnostics = did_open(
|
||||||
|
&mut client,
|
||||||
|
json!({
|
||||||
|
"textDocument": {
|
||||||
|
"uri": "file:///a/a.ts",
|
||||||
|
"languageId": "typescript",
|
||||||
|
"version": 1,
|
||||||
|
"text": "import a from \"./test.json\";\n\nconsole.log(a);\n"
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
let last = diagnostics.pop().unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
json!(last.diagnostics),
|
||||||
|
json!([
|
||||||
|
{
|
||||||
|
"range": {
|
||||||
|
"start": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 14
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"severity": 1,
|
||||||
|
"code": "no-assert-type",
|
||||||
|
"source": "deno",
|
||||||
|
"message": "The module is a JSON module and not being imported with an import assertion. Consider adding `assert { type: \"json\" }` to the import statement."
|
||||||
|
}
|
||||||
|
])
|
||||||
|
);
|
||||||
|
|
||||||
|
let (maybe_res, maybe_err) = client
|
||||||
|
.write_request(
|
||||||
|
"textDocument/codeAction",
|
||||||
|
load_fixture("code_action_params_import_assertion.json"),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert!(maybe_err.is_none());
|
||||||
|
assert_eq!(
|
||||||
|
maybe_res,
|
||||||
|
Some(load_fixture("code_action_response_import_assertion.json"))
|
||||||
|
);
|
||||||
|
shutdown(&mut client);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn lsp_hover() {
|
fn lsp_hover() {
|
||||||
let mut client = init("initialize_params.json");
|
let mut client = init("initialize_params.json");
|
||||||
|
|
38
cli/tests/testdata/lsp/code_action_params_import_assertion.json
vendored
Normal file
38
cli/tests/testdata/lsp/code_action_params_import_assertion.json
vendored
Normal file
|
@ -0,0 +1,38 @@
|
||||||
|
{
|
||||||
|
"textDocument": {
|
||||||
|
"uri": "file:///a/a.ts"
|
||||||
|
},
|
||||||
|
"range": {
|
||||||
|
"start": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 14
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"context": {
|
||||||
|
"diagnostics": [
|
||||||
|
{
|
||||||
|
"range": {
|
||||||
|
"start": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 14
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"severity": 1,
|
||||||
|
"code": "no-assert-type",
|
||||||
|
"source": "deno",
|
||||||
|
"message": "The module is a JSON module and not being imported with an import assertion. Consider adding `assert { type: \"json\" }` to the import statement."
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"only": [
|
||||||
|
"quickfix"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
43
cli/tests/testdata/lsp/code_action_response_import_assertion.json
vendored
Normal file
43
cli/tests/testdata/lsp/code_action_response_import_assertion.json
vendored
Normal file
|
@ -0,0 +1,43 @@
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"title": "Insert import assertion.",
|
||||||
|
"kind": "quickfix",
|
||||||
|
"diagnostics": [
|
||||||
|
{
|
||||||
|
"range": {
|
||||||
|
"start": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 14
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"severity": 1,
|
||||||
|
"code": "no-assert-type",
|
||||||
|
"source": "deno",
|
||||||
|
"message": "The module is a JSON module and not being imported with an import assertion. Consider adding `assert { type: \"json\" }` to the import statement."
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"edit": {
|
||||||
|
"changes": {
|
||||||
|
"file:///a/a.ts": [
|
||||||
|
{
|
||||||
|
"range": {
|
||||||
|
"start": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 0,
|
||||||
|
"character": 27
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"newText": " assert { type: \"json\" }"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]
|
Loading…
Reference in a new issue