Skip to content

Commit

Permalink
chore(lsp): use 'install' terminology for jsr and npm packages (#25119)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn authored Aug 21, 2024
1 parent 7139337 commit 76990df
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 41 deletions.
42 changes: 24 additions & 18 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,9 @@ impl DiagnosticsState {
if diagnostic.code
== Some(lsp::NumberOrString::String("no-cache".to_string()))
|| diagnostic.code
== Some(lsp::NumberOrString::String("no-cache-jsr".to_string()))
== Some(lsp::NumberOrString::String("not-installed-jsr".to_string()))
|| diagnostic.code
== Some(lsp::NumberOrString::String("no-cache-npm".to_string()))
== Some(lsp::NumberOrString::String("not-installed-npm".to_string()))
{
no_cache_diagnostics.push(diagnostic.clone());
}
Expand Down Expand Up @@ -991,9 +991,9 @@ pub enum DenoDiagnostic {
/// A remote module was not found in the cache.
NoCache(ModuleSpecifier),
/// A remote jsr package reference was not found in the cache.
NoCacheJsr(PackageReq, ModuleSpecifier),
NotInstalledJsr(PackageReq, ModuleSpecifier),
/// A remote npm package reference was not found in the cache.
NoCacheNpm(PackageReq, ModuleSpecifier),
NotInstalledNpm(PackageReq, ModuleSpecifier),
/// A local module was not found on the local file system.
NoLocal(ModuleSpecifier),
/// The specifier resolved to a remote specifier that was redirected to
Expand All @@ -1018,8 +1018,8 @@ impl DenoDiagnostic {
Self::InvalidAttributeType(_) => "invalid-attribute-type",
Self::NoAttributeType => "no-attribute-type",
Self::NoCache(_) => "no-cache",
Self::NoCacheJsr(_, _) => "no-cache-jsr",
Self::NoCacheNpm(_, _) => "no-cache-npm",
Self::NotInstalledJsr(_, _) => "not-installed-jsr",
Self::NotInstalledNpm(_, _) => "not-installed-npm",
Self::NoLocal(_) => "no-local",
Self::Redirect { .. } => "redirect",
Self::ResolutionError(err) => {
Expand Down Expand Up @@ -1100,17 +1100,22 @@ impl DenoDiagnostic {
}),
..Default::default()
},
"no-cache" | "no-cache-jsr" | "no-cache-npm" => {
"no-cache" | "not-installed-jsr" | "not-installed-npm" => {
let data = diagnostic
.data
.clone()
.ok_or_else(|| anyhow!("Diagnostic is missing data"))?;
let data: DiagnosticDataSpecifier = serde_json::from_value(data)?;
let title = if matches!(
code.as_str(),
"not-installed-jsr" | "not-installed-npm"
) {
format!("Install \"{}\" and its dependencies.", data.specifier)
} else {
format!("Cache \"{}\" and its dependencies.", data.specifier)
};
lsp::CodeAction {
title: format!(
"Cache \"{}\" and its dependencies.",
data.specifier
),
title,
kind: Some(lsp::CodeActionKind::QUICKFIX),
diagnostics: Some(vec![diagnostic.clone()]),
command: Some(lsp::Command {
Expand Down Expand Up @@ -1216,8 +1221,8 @@ impl DenoDiagnostic {
match code.as_str() {
"import-map-remap"
| "no-cache"
| "no-cache-jsr"
| "no-cache-npm"
| "not-installed-jsr"
| "not-installed-npm"
| "no-attribute-type"
| "redirect"
| "import-node-prefix-missing" => true,
Expand Down Expand Up @@ -1255,8 +1260,8 @@ impl DenoDiagnostic {
Self::InvalidAttributeType(assert_type) => (lsp::DiagnosticSeverity::ERROR, format!("The module is a JSON module and expected an attribute type of \"json\". Instead got \"{assert_type}\"."), None),
Self::NoAttributeType => (lsp::DiagnosticSeverity::ERROR, "The module is a JSON module and not being imported with an import attribute. Consider adding `with { type: \"json\" }` to the import statement.".to_string(), None),
Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: {specifier}"), Some(json!({ "specifier": specifier }))),
Self::NoCacheJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing jsr package: {}", pkg_req), Some(json!({ "specifier": specifier }))),
Self::NoCacheNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: {}", pkg_req), Some(json!({ "specifier": specifier }))),
Self::NotInstalledJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("JSR package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
Self::NotInstalledNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("NPM package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
Self::NoLocal(specifier) => {
let maybe_sloppy_resolution = SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)).resolve(specifier, ResolutionMode::Execution);
let data = maybe_sloppy_resolution.as_ref().map(|res| {
Expand Down Expand Up @@ -1410,7 +1415,8 @@ fn diagnose_resolution(
JsrPackageReqReference::from_specifier(specifier)
{
let req = pkg_ref.into_inner().req;
diagnostics.push(DenoDiagnostic::NoCacheJsr(req, specifier.clone()));
diagnostics
.push(DenoDiagnostic::NotInstalledJsr(req, specifier.clone()));
} else if let Ok(pkg_ref) =
NpmPackageReqReference::from_specifier(specifier)
{
Expand All @@ -1419,7 +1425,7 @@ fn diagnose_resolution(
let req = pkg_ref.into_inner().req;
if !npm_resolver.is_pkg_req_folder_cached(&req) {
diagnostics
.push(DenoDiagnostic::NoCacheNpm(req, specifier.clone()));
.push(DenoDiagnostic::NotInstalledNpm(req, specifier.clone()));
}
}
} else if let Some(module_name) = specifier.as_str().strip_prefix("node:")
Expand All @@ -1445,7 +1451,7 @@ fn diagnose_resolution(
// check that a @types/node package exists in the resolver
let types_node_req = PackageReq::from_str("@types/node").unwrap();
if !npm_resolver.is_pkg_req_folder_cached(&types_node_req) {
diagnostics.push(DenoDiagnostic::NoCacheNpm(
diagnostics.push(DenoDiagnostic::NotInstalledNpm(
types_node_req,
ModuleSpecifier::parse("npm:@types/node").unwrap(),
));
Expand Down
4 changes: 2 additions & 2 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1667,9 +1667,9 @@ impl Inner {
if diagnostic.code
== Some(NumberOrString::String("no-cache".to_string()))
|| diagnostic.code
== Some(NumberOrString::String("no-cache-jsr".to_string()))
== Some(NumberOrString::String("not-installed-jsr".to_string()))
|| diagnostic.code
== Some(NumberOrString::String("no-cache-npm".to_string()))
== Some(NumberOrString::String("not-installed-npm".to_string()))
{
includes_no_cache = true;
}
Expand Down
43 changes: 22 additions & 21 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5221,9 +5221,9 @@ fn lsp_code_actions_deno_cache_jsr() {
"end": { "line": 1, "character": 49 },
},
"severity": 1,
"code": "no-cache-jsr",
"code": "not-installed-jsr",
"source": "deno",
"message": "Uncached or missing jsr package: @denotest/add@1",
"message": "JSR package \"@denotest/add@1\" is not installed or doesn't exist.",
"data": { "specifier": "jsr:@denotest/add@1" },
}],
"version": 1,
Expand All @@ -5244,9 +5244,9 @@ fn lsp_code_actions_deno_cache_jsr() {
"end": { "line": 1, "character": 49 },
},
"severity": 1,
"code": "no-cache-jsr",
"code": "not-installed-jsr",
"source": "deno",
"message": "Uncached or missing jsr package: @denotest/add@1",
"message": "JSR package \"@denotest/add@1\" is not installed or doesn't exist.",
"data": { "specifier": "jsr:@denotest/add@1" },
}],
"only": ["quickfix"],
Expand All @@ -5256,17 +5256,17 @@ fn lsp_code_actions_deno_cache_jsr() {
assert_eq!(
res,
json!([{
"title": "Cache \"jsr:@denotest/add@1\" and its dependencies.",
"title": "Install \"jsr:@denotest/add@1\" and its dependencies.",
"kind": "quickfix",
"diagnostics": [{
"range": {
"start": { "line": 1, "character": 28 },
"end": { "line": 1, "character": 49 },
},
"severity": 1,
"code": "no-cache-jsr",
"code": "not-installed-jsr",
"source": "deno",
"message": "Uncached or missing jsr package: @denotest/add@1",
"message": "JSR package \"@denotest/add@1\" is not installed or doesn't exist.",
"data": { "specifier": "jsr:@denotest/add@1" },
}],
"command": {
Expand Down Expand Up @@ -5710,9 +5710,9 @@ fn lsp_code_actions_deno_cache_npm() {
"end": { "line": 0, "character": 29 }
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: chalk",
"message": "NPM package \"chalk\" is not installed or doesn't exist.",
"data": { "specifier": "npm:chalk" }
}],
"version": 1
Expand All @@ -5737,9 +5737,9 @@ fn lsp_code_actions_deno_cache_npm() {
"end": { "line": 0, "character": 29 }
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: chalk",
"message": "NPM package \"chalk\" is not installed or doesn't exist.",
"data": { "specifier": "npm:chalk" }
}],
"only": ["quickfix"]
Expand All @@ -5749,17 +5749,17 @@ fn lsp_code_actions_deno_cache_npm() {
assert_eq!(
res,
json!([{
"title": "Cache \"npm:chalk\" and its dependencies.",
"title": "Install \"npm:chalk\" and its dependencies.",
"kind": "quickfix",
"diagnostics": [{
"range": {
"start": { "line": 0, "character": 18 },
"end": { "line": 0, "character": 29 }
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: chalk",
"message": "NPM package \"chalk\" is not installed or doesn't exist.",
"data": { "specifier": "npm:chalk" }
}],
"command": {
Expand Down Expand Up @@ -5812,9 +5812,9 @@ fn lsp_code_actions_deno_cache_all() {
"end": { "line": 2, "character": 37 },
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: chalk",
"message": "NPM package \"chalk\" is not installed or doesn't exist.",
"data": { "specifier": "npm:chalk" },
},
],
Expand Down Expand Up @@ -5900,9 +5900,9 @@ fn lsp_code_actions_deno_cache_all() {
"end": { "line": 2, "character": 37 },
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: chalk",
"message": "NPM package \"chalk\" is not installed or doesn't exist.",
"data": { "specifier": "npm:chalk" },
},
],
Expand Down Expand Up @@ -8956,7 +8956,8 @@ fn lsp_completions_node_builtin() {
.diagnostics
.into_iter()
.filter(|d| {
d.code == Some(lsp::NumberOrString::String("no-cache-npm".to_string()))
d.code
== Some(lsp::NumberOrString::String("not-installed-npm".to_string()))
})
.collect::<Vec<_>>();

Expand All @@ -8972,9 +8973,9 @@ fn lsp_completions_node_builtin() {
"specifier": "npm:@types/node",
},
"severity": 1,
"code": "no-cache-npm",
"code": "not-installed-npm",
"source": "deno",
"message": "Uncached or missing npm package: @types/node"
"message": "NPM package \"@types/node\" is not installed or doesn't exist."
}
])
);
Expand Down

0 comments on commit 76990df

Please sign in to comment.