Skip to content

Commit

Permalink
fix(lsp): improved npm specifier to import map entry mapping (denolan…
Browse files Browse the repository at this point in the history
…d#22016)

Upgrades to the latest deno_semver
  • Loading branch information
dsherret committed Jan 21, 2024
1 parent e58462d commit fbfeedb
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 29 deletions.
30 changes: 18 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ libz-sys = { version = "1.1", default-features = false }
log = "=0.4.20"
lsp-types = "=0.94.1" # used by tower-lsp and "proposed" feature is unstable in patch releases
memmem = "0.1.1"
monch = "=0.4.3"
monch = "=0.5.0"
notify = "=5.0.0"
num-bigint = { version = "0.4", features = ["rand"] }
once_cell = "1.17.1"
Expand Down
8 changes: 3 additions & 5 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,8 @@ deno_lint = { version = "=0.53.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.15.3"
deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
# todo(dsherret): investigate https://github.com/denoland/deno_semver/commit/98f9174baef199809295077b3b68c9fa58defb9b causing
# lsp_completions_auto_import_and_quick_fix_with_import_map to fail when bumping this version
deno_semver = "=0.5.1"
deno_task_shell = "=0.14.0"
deno_semver = "=0.5.4"
deno_task_shell = "=0.14.3"
eszip = "=0.57.0"
napi_sym.workspace = true

Expand Down Expand Up @@ -99,7 +97,7 @@ flate2.workspace = true
fs3.workspace = true
glob = "0.3.1"
hex.workspace = true
import_map = { version = "=0.18.1", features = ["ext"] }
import_map = { version = "=0.18.2", features = ["ext"] }
indexmap.workspace = true
jsonc-parser = { version = "=0.23.0", features = ["serde"] }
lazy-regex.workspace = true
Expand Down
49 changes: 39 additions & 10 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ use deno_runtime::deno_node::NodeResolver;
use deno_runtime::deno_node::NpmResolver;
use deno_runtime::deno_node::PathClean;
use deno_runtime::permissions::PermissionsContainer;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageReq;
use import_map::ImportMap;
use once_cell::sync::Lazy;
use regex::Regex;
use std::cmp::Ordering;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use tower_lsp::lsp_types as lsp;
use tower_lsp::lsp_types::Position;
Expand Down Expand Up @@ -211,20 +213,31 @@ impl<'a> TsResponseImportMapper<'a> {
if !pkg_reqs.is_empty() {
let sub_path = self.resolve_package_path(specifier);
if let Some(import_map) = self.maybe_import_map {
for pkg_req in &pkg_reqs {
let paths = vec![
concat_npm_specifier("npm:", pkg_req, sub_path.as_deref()),
concat_npm_specifier("npm:/", pkg_req, sub_path.as_deref()),
];
for path in paths {
if let Some(mapped_path) = ModuleSpecifier::parse(&path)
.ok()
.and_then(|s| import_map.lookup(&s, referrer))
let pkg_reqs = pkg_reqs.iter().collect::<HashSet<_>>();
let mut matches = Vec::new();
for entry in import_map.entries_for_referrer(referrer) {
if let Some(value) = entry.raw_value {
if let Ok(package_ref) =
NpmPackageReqReference::from_str(value)
{
return Some(mapped_path);
if pkg_reqs.contains(package_ref.req()) {
let sub_path = sub_path.as_deref().unwrap_or("");
let value_sub_path = package_ref.sub_path().unwrap_or("");
if let Some(key_sub_path) =
sub_path.strip_prefix(value_sub_path)
{
matches
.push(format!("{}{}", entry.raw_key, key_sub_path));
}
}
}
}
}
// select the shortest match
matches.sort_by_key(|a| a.len());
if let Some(matched) = matches.first() {
return Some(matched.to_string());
}
}

// if not found in the import map, return the first pkg req
Expand Down Expand Up @@ -267,6 +280,22 @@ impl<'a> TsResponseImportMapper<'a> {
// a search for the .d.ts file instead
if specifier_path.extension().and_then(|e| e.to_str()) == Some("js") {
search_paths.insert(0, specifier_path.with_extension("d.ts"));
} else if let Some(file_name) =
specifier_path.file_name().and_then(|f| f.to_str())
{
// In some other cases, typescript will provide the .d.ts extension, but the
// export might not have a .d.ts defined. In that case, look for the corresponding
// JavaScript file after not being able to find the .d.ts file.
if let Some(file_stem) = file_name.strip_suffix(".d.ts") {
search_paths
.push(specifier_path.with_file_name(format!("{}.js", file_stem)));
} else if let Some(file_stem) = file_name.strip_suffix(".d.cts") {
search_paths
.push(specifier_path.with_file_name(format!("{}.cjs", file_stem)));
} else if let Some(file_stem) = file_name.strip_suffix(".d.mts") {
search_paths
.push(specifier_path.with_file_name(format!("{}.mjs", file_stem)));
}
}

for search_path in search_paths {
Expand Down
51 changes: 51 additions & 0 deletions cli/tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6450,6 +6450,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
"imports": {
"print_hello": "http:https://localhost:4545/subdir/print_hello.ts",
"chalk": "npm:chalk@~5",
"nested/": "npm:/@denotest/types-exports-subpaths@1/nested/",
"types-exports-subpaths/": "npm:/@denotest/types-exports-subpaths@1/"
}
}"#;
Expand All @@ -6470,6 +6471,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
"import _test1 from 'npm:chalk@^5.0';\n",
"import chalk from 'npm:chalk@~5';\n",
"import chalk from 'npm:chalk@~5';\n",
"import {entryB} from 'npm:@denotest/types-exports-subpaths@1/nested/entry-b';\n",
"import {printHello} from 'print_hello';\n",
"\n",
),
Expand All @@ -6483,6 +6485,7 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
"arguments": [
[
"npm:@denotest/types-exports-subpaths@1/client",
"npm:@denotest/types-exports-subpaths@1/nested/entry-b",
"npm:chalk@^5.0",
"npm:chalk@~5",
"http:https://localhost:4545/subdir/print_hello.ts",
Expand Down Expand Up @@ -6822,6 +6825,54 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() {
}
}])
);

// try auto-import with npm package with sub-path on value side of import map
client.did_open(json!({
"textDocument": {
"uri": "file:https:///a/nested_path.ts",
"languageId": "typescript",
"version": 1,
"text": "entry",
}
}));
let list = client.get_completion_list(
"file:https:///a/nested_path.ts",
(0, 5),
json!({ "triggerKind": 1 }),
);
assert!(!list.is_incomplete);
let item = list
.items
.iter()
.find(|item| item.label == "entryB")
.unwrap();

let res = client.write_request("completionItem/resolve", item);
assert_eq!(
res,
json!({
"label": "entryB",
"labelDetails": {
"description": "nested/entry-b",
},
"kind": 3,
"detail": "function entryB(): \"b\"",
"documentation": {
"kind": "markdown",
"value": ""
},
"sortText": "￿16_0",
"additionalTextEdits": [
{
"range": {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 0 }
},
"newText": "import { entryB } from \"nested/entry-b\";\n\n"
}
]
})
);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export function entryC() {
export function entryA() {
return 12;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export function entryB(): "b";
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function entryB() {
return "b";
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
},
"./entry-a": {
"import": "./dist/entry-a.js"
},
"./nested/entry-b": {
"import": "./dist/entry-b.js"
}
}
}

0 comments on commit fbfeedb

Please sign in to comment.