foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00
Code Issues Packages 1 Wiki Activity

feat(lsp): auto-import completions from byonm dependencies (#26680)

Browse source
This commit is contained in:
Nayeem Rahman 2024-11-06 06:26:46 +00:00 committed by GitHub
parent ef7432c03f
commit 5088b25f23
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 344 additions and 105 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
cli/lsp/analysis.rs
View file

@ -12,7 +12,9 @@ use super::urls::url_to_uri;
use crate::args::jsr_url;
use crate::lsp::search::PackageSearchApi;
use crate::tools::lint::CliLinter;
use crate::util::path::relative_specifier;
use deno_config::workspace::MappedResolution;
use deno_graph::source::ResolutionMode;
use deno_lint::diagnostic::LintDiagnosticRange;
use deno_ast::SourceRange;
@ -228,6 +230,7 @@ pub struct TsResponseImportMapper<'a> {
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
resolver: &'a LspResolver,
tsc_specifier_map: &'a tsc::TscSpecifierMap,
file_referrer: ModuleSpecifier,
}
@ -236,12 +239,14 @@ impl<'a> TsResponseImportMapper<'a> {
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
resolver: &'a LspResolver,
tsc_specifier_map: &'a tsc::TscSpecifierMap,
file_referrer: &ModuleSpecifier,
) -> Self {
Self {
documents,
maybe_import_map,
resolver,
tsc_specifier_map,
file_referrer: file_referrer.clone(),
}
}
@ -387,6 +392,11 @@ impl<'a> TsResponseImportMapper<'a> {
}
}
}
} else if let Some(dep_name) = self
.resolver
.file_url_to_package_json_dep(specifier, Some(&self.file_referrer))
{
return Some(dep_name);
}
// check if the import map has this specifier
@ -457,19 +467,36 @@ impl<'a> TsResponseImportMapper<'a> {
specifier: &str,
referrer: &ModuleSpecifier,
) -> Option<String> {
if let Ok(specifier) = referrer.join(specifier) {
if let Some(specifier) = self.check_specifier(&specifier, referrer) {
return Some(specifier);
}
}
let specifier = specifier.strip_suffix(".js").unwrap_or(specifier);
for ext in SUPPORTED_EXTENSIONS {
let specifier_with_ext = format!("{specifier}{ext}");
if self
.documents
.contains_import(&specifier_with_ext, referrer)
let specifier_stem = specifier.strip_suffix(".js").unwrap_or(specifier);
let specifiers = std::iter::once(Cow::Borrowed(specifier)).chain(
SUPPORTED_EXTENSIONS
.iter()
.map(|ext| Cow::Owned(format!("{specifier_stem}{ext}"))),
);
for specifier in specifiers {
if let Some(specifier) = self
.resolver
.as_graph_resolver(Some(&self.file_referrer))
.resolve(
&specifier,
&deno_graph::Range {
specifier: referrer.clone(),
start: deno_graph::Position::zeroed(),
end: deno_graph::Position::zeroed(),
},
ResolutionMode::Types,
)
.ok()
.and_then(|s| self.tsc_specifier_map.normalize(s.as_str()).ok())
.filter(|s| self.documents.exists(s, Some(&self.file_referrer)))
{
return Some(specifier_with_ext);
if let Some(specifier) = self
.check_specifier(&specifier, referrer)
.or_else(|| relative_specifier(referrer, &specifier))
.filter(|s| !s.contains("/node_modules/"))
{
return Some(specifier);
}
}
}
None
@ -559,8 +586,9 @@ fn try_reverse_map_package_json_exports(
pub fn fix_ts_import_changes(
referrer: &ModuleSpecifier,
changes: &[tsc::FileTextChanges],
import_mapper: &TsResponseImportMapper,
language_server: &language_server::Inner,
) -> Result<Vec<tsc::FileTextChanges>, AnyError> {
let import_mapper = language_server.get_ts_response_import_mapper(referrer);
let mut r = Vec::new();
for change in changes {
let mut text_changes = Vec::new();
@ -605,7 +633,7 @@ pub fn fix_ts_import_changes(
fn fix_ts_import_action<'a>(
referrer: &ModuleSpecifier,
action: &'a tsc::CodeFixAction,
import_mapper: &TsResponseImportMapper,
language_server: &language_server::Inner,
) -> Option<Cow<'a, tsc::CodeFixAction>> {
if !matches!(
action.fix_name.as_str(),
@ -621,6 +649,7 @@ fn fix_ts_import_action<'a>(
let Some(specifier) = specifier else {
return Some(Cow::Borrowed(action));
};
let import_mapper = language_server.get_ts_response_import_mapper(referrer);
if let Some(new_specifier) =
import_mapper.check_unresolved_specifier(specifier, referrer)
{
@ -728,7 +757,7 @@ pub fn ts_changes_to_edit(
}))
}
#[derive(Debug, Deserialize)]
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct CodeActionData {
pub specifier: ModuleSpecifier,
@ -998,11 +1027,8 @@ impl CodeActionCollection {
"The action returned from TypeScript is unsupported.",
));
}
let Some(action) = fix_ts_import_action(
specifier,
action,
&language_server.get_ts_response_import_mapper(specifier),
) else {
let Some(action) = fix_ts_import_action(specifier, action, language_server)
else {
return Ok(());
};
let edit = ts_changes_to_edit(&action.changes, language_server)?;
@ -1051,10 +1077,12 @@ impl CodeActionCollection {
specifier: &ModuleSpecifier,
diagnostic: &lsp::Diagnostic,
) {
let data = Some(json!({
"specifier": specifier,
"fixId": action.fix_id,
}));
let data = action.fix_id.as_ref().map(|fix_id| {
json!(CodeActionData {
specifier: specifier.clone(),
fix_id: fix_id.clone(),
})
});
let title = if let Some(description) = &action.fix_all_description {
description.clone()
} else {

28
cli/lsp/documents.rs
View file

@ -1059,34 +1059,6 @@ impl Documents {
self.cache.is_valid_file_referrer(specifier)
}
/// Return `true` if the provided specifier can be resolved to a document,
/// otherwise `false`.
pub fn contains_import(
&self,
specifier: &str,
referrer: &ModuleSpecifier,
) -> bool {
let file_referrer = self.get_file_referrer(referrer);
let maybe_specifier = self
.resolver
.as_graph_resolver(file_referrer.as_deref())
.resolve(
specifier,
&deno_graph::Range {
specifier: referrer.clone(),
start: deno_graph::Position::zeroed(),
end: deno_graph::Position::zeroed(),
},
ResolutionMode::Types,
)
.ok();
if let Some(import_specifier) = maybe_specifier {
self.exists(&import_specifier, file_referrer.as_deref())
} else {
false
}
}
pub fn resolve_document_specifier(
&self,
specifier: &ModuleSpecifier,

13
cli/lsp/language_server.rs
View file

@ -1837,7 +1837,7 @@ impl Inner {
fix_ts_import_changes(
&code_action_data.specifier,
&combined_code_actions.changes,
&self.get_ts_response_import_mapper(&code_action_data.specifier),
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
@ -1890,7 +1890,7 @@ impl Inner {
refactor_edit_info.edits = fix_ts_import_changes(
&action_data.specifier,
&refactor_edit_info.edits,
&self.get_ts_response_import_mapper(&action_data.specifier),
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
@ -1921,7 +1921,8 @@ impl Inner {
// todo(dsherret): this should probably just take the resolver itself
// as the import map is an implementation detail
.and_then(|d| d.resolver.maybe_import_map()),
self.resolver.as_ref(),
&self.resolver,
&self.ts_server.specifier_map,
file_referrer,
)
}
@ -2284,7 +2285,11 @@ impl Inner {
.into(),
scope.cloned(),
)
.await;
.await
.unwrap_or_else(|err| {
error!("Unable to get completion info from TypeScript: {:#}", err);
None
});
if let Some(completions) = maybe_completion_info {
response = Some(

45
cli/lsp/resolver.rs
View file

@ -74,6 +74,7 @@ struct LspScopeResolver {
pkg_json_resolver: Option<Arc<PackageJsonResolver>>,
redirect_resolver: Option<Arc<RedirectResolver>>,
graph_imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
package_json_deps_by_resolution: Arc<IndexMap<ModuleSpecifier, String>>,
config_data: Option<Arc<ConfigData>>,
}
@ -88,6 +89,7 @@ impl Default for LspScopeResolver {
pkg_json_resolver: None,
redirect_resolver: None,
graph_imports: Default::default(),
package_json_deps_by_resolution: Default::default(),
config_data: None,
}
}
@ -165,6 +167,33 @@ impl LspScopeResolver {
)
})
.unwrap_or_default();
let package_json_deps_by_resolution = (|| {
let node_resolver = node_resolver.as_ref()?;
let package_json = config_data?.maybe_pkg_json()?;
let referrer = package_json.specifier();
let dependencies = package_json.dependencies.as_ref()?;
let result = dependencies
.iter()
.flat_map(|(name, _)| {
let req_ref =
NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?;
let specifier = into_specifier_and_media_type(Some(
node_resolver
.resolve_req_reference(
&req_ref,
&referrer,
NodeResolutionMode::Types,
)
.ok()?,
))
.0;
Some((specifier, name.clone()))
})
.collect();
Some(result)
})();
let package_json_deps_by_resolution =
Arc::new(package_json_deps_by_resolution.unwrap_or_default());
Self {
cjs_tracker: lsp_cjs_tracker,
graph_resolver,
@ -174,6 +203,7 @@ impl LspScopeResolver {
pkg_json_resolver: Some(pkg_json_resolver),
redirect_resolver,
graph_imports,
package_json_deps_by_resolution,
config_data: config_data.cloned(),
}
}
@ -216,6 +246,9 @@ impl LspScopeResolver {
redirect_resolver: self.redirect_resolver.clone(),
pkg_json_resolver: Some(pkg_json_resolver),
graph_imports: self.graph_imports.clone(),
package_json_deps_by_resolution: self
.package_json_deps_by_resolution
.clone(),
config_data: self.config_data.clone(),
})
}
@ -407,6 +440,18 @@ impl LspResolver {
)))
}
pub fn file_url_to_package_json_dep(
&self,
specifier: &ModuleSpecifier,
file_referrer: Option<&ModuleSpecifier>,
) -> Option<String> {
let resolver = self.get_scope_resolver(file_referrer);
resolver
.package_json_deps_by_resolution
.get(specifier)
.cloned()
}
pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool {
fn has_node_modules_dir(specifier: &ModuleSpecifier) -> bool {
// consider any /node_modules/ directory as being in the node_modules

102
cli/lsp/tsc.rs
View file

@ -236,7 +236,7 @@ pub struct TsServer {
performance: Arc<Performance>,
sender: mpsc::UnboundedSender<Request>,
receiver: Mutex<Option<mpsc::UnboundedReceiver<Request>>>,
specifier_map: Arc<TscSpecifierMap>,
pub specifier_map: Arc<TscSpecifierMap>,
inspector_server: Mutex<Option<Arc<InspectorServer>>>,
pending_change: Mutex<Option<PendingChange>>,
}
@ -882,20 +882,22 @@ impl TsServer {
options: GetCompletionsAtPositionOptions,
format_code_settings: FormatCodeSettings,
scope: Option<ModuleSpecifier>,
) -> Option<CompletionInfo> {
) -> Result<Option<CompletionInfo>, AnyError> {
let req = TscRequest::GetCompletionsAtPosition(Box::new((
self.specifier_map.denormalize(&specifier),
position,
options,
format_code_settings,
)));
match self.request(snapshot, req, scope).await {
Ok(maybe_info) => maybe_info,
Err(err) => {
log::error!("Unable to get completion info from TypeScript: {:#}", err);
None
}
}
self
.request::<Option<CompletionInfo>>(snapshot, req, scope)
.await
.map(|mut info| {
if let Some(info) = &mut info {
info.normalize(&self.specifier_map);
}
info
})
}
pub async fn get_completion_details(
@ -3642,6 +3644,12 @@ pub struct CompletionInfo {
}
impl CompletionInfo {
fn normalize(&mut self, specifier_map: &TscSpecifierMap) {
for entry in &mut self.entries {
entry.normalize(specifier_map);
}
}
pub fn as_completion_response(
&self,
line_index: Arc<LineIndex>,
@ -3703,11 +3711,17 @@ pub struct CompletionItemData {
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
struct CompletionEntryDataImport {
struct CompletionEntryDataAutoImport {
module_specifier: String,
file_name: String,
}
#[derive(Debug)]
pub struct CompletionNormalizedAutoImportData {
raw: CompletionEntryDataAutoImport,
normalized: ModuleSpecifier,
}
#[derive(Debug, Default, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct CompletionEntry {
@ -3740,9 +3754,28 @@ pub struct CompletionEntry {
is_import_statement_completion: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
data: Option<Value>,
/// This is not from tsc, we add it for convenience during normalization.
/// Represents `self.data.file_name`, but normalized.
#[serde(skip)]
auto_import_data: Option<CompletionNormalizedAutoImportData>,
}
impl CompletionEntry {
fn normalize(&mut self, specifier_map: &TscSpecifierMap) {
let Some(data) = &self.data else {
return;
};
let Ok(raw) =
serde_json::from_value::<CompletionEntryDataAutoImport>(data.clone())
else {
return;
};
if let Ok(normalized) = specifier_map.normalize(&raw.file_name) {
self.auto_import_data =
Some(CompletionNormalizedAutoImportData { raw, normalized });
}
}
fn get_commit_characters(
&self,
info: &CompletionInfo,
@ -3891,25 +3924,24 @@ impl CompletionEntry {
if let Some(source) = &self.source {
let mut display_source = source.clone();
if let Some(data) = &self.data {
if let Ok(import_data) =
serde_json::from_value::<CompletionEntryDataImport>(data.clone())
if let Some(import_data) = &self.auto_import_data {
if let Some(new_module_specifier) = language_server
.get_ts_response_import_mapper(specifier)
.check_specifier(&import_data.normalized, specifier)
.or_else(|| relative_specifier(specifier, &import_data.normalized))
{
if let Ok(import_specifier) = resolve_url(&import_data.file_name) {
if let Some(new_module_specifier) = language_server
.get_ts_response_import_mapper(specifier)
.check_specifier(&import_specifier, specifier)
.or_else(|| relative_specifier(specifier, &import_specifier))
{
display_source.clone_from(&new_module_specifier);
if new_module_specifier != import_data.module_specifier {
specifier_rewrite =
Some((import_data.module_specifier, new_module_specifier));
}
} else if source.starts_with(jsr_url().as_str()) {
return None;
}
if new_module_specifier.contains("/node_modules/") {
return None;
}
display_source.clone_from(&new_module_specifier);
if new_module_specifier != import_data.raw.module_specifier {
specifier_rewrite = Some((
import_data.raw.module_specifier.clone(),
new_module_specifier,
));
}
} else if source.starts_with(jsr_url().as_str()) {
return None;
}
}
// We want relative or bare (import-mapped or otherwise) specifiers to
@ -4212,6 +4244,13 @@ impl TscSpecifierMap {
return specifier.to_string();
}
let mut specifier = original.to_string();
if specifier.contains("/node_modules/.deno/")
&& !specifier.contains("/node_modules/@types/node/")
{
// The ts server doesn't give completions from files in
// `node_modules/.deno/`. We work around it like this.
specifier = specifier.replace("/node_modules/", "/$node_modules/");
}
let media_type = MediaType::from_specifier(original);
// If the URL-inferred media type doesn't correspond to tsc's path-inferred
// media type, force it to be the same by appending an extension.
@ -4329,7 +4368,7 @@ fn op_is_cancelled(state: &mut OpState) -> bool {
fn op_is_node_file(state: &mut OpState, #[string] path: String) -> bool {
let state = state.borrow::<State>();
let mark = state.performance.mark("tsc.op.op_is_node_file");
let r = match ModuleSpecifier::parse(&path) {
let r = match state.specifier_map.normalize(path) {
Ok(specifier) => state.state_snapshot.resolver.in_node_modules(&specifier),
Err(_) => false,
};
@ -4609,7 +4648,10 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
for doc in &docs {
let specifier = doc.specifier();
let is_open = doc.is_open();
if is_open || specifier.scheme() == "file" {
if is_open
|| (specifier.scheme() == "file"
&& !state.state_snapshot.resolver.in_node_modules(specifier))
{
let script_names = doc
.scope()
.and_then(|s| result.by_scope.get_mut(s))
@ -6035,6 +6077,7 @@ mod tests {
Some(temp_dir.url()),
)
.await
.unwrap()
.unwrap();
assert_eq!(info.entries.len(), 22);
let details = ts_server
@ -6194,6 +6237,7 @@ mod tests {
Some(temp_dir.url()),
)
.await
.unwrap()
.unwrap();
let entry = info
.entries

185
tests/integration/lsp_tests.rs
View file

@ -6628,6 +6628,23 @@ export class DuckConfig {
}]
}]
}
}, {
"title": "Add all missing imports",
"kind": "quickfix",
"diagnostics": [{
"range": {
"start": { "line": 0, "character": 50 },
"end": { "line": 0, "character": 67 }
},
"severity": 1,
"code": 2304,
"source": "deno-ts",
"message": "Cannot find name 'DuckConfigOptions'."
}],
"data": {
"specifier": "file:///a/file00.ts",
"fixId": "fixMissingImport"
}
}, {
"title": "Add import from \"./file01.ts\"",
"kind": "quickfix",
@ -6656,23 +6673,6 @@ export class DuckConfig {
}]
}]
}
}, {
"title": "Add all missing imports",
"kind": "quickfix",
"diagnostics": [{
"range": {
"start": { "line": 0, "character": 50 },
"end": { "line": 0, "character": 67 }
},
"severity": 1,
"code": 2304,
"source": "deno-ts",
"message": "Cannot find name 'DuckConfigOptions'."
}],
"data": {
"specifier": "file:///a/file00.ts",
"fixId": "fixMissingImport"
}
}])
);
let res = client.write_request(
@ -8125,6 +8125,151 @@ fn lsp_npm_completions_auto_import_and_quick_fix_no_import_map() {
client.shutdown();
}
#[test]
fn lsp_npm_auto_import_and_quick_fix_byonm() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.add_npm_env_vars()
.build();
let temp_dir = context.temp_dir();
temp_dir.write("deno.json", json!({}).to_string());
temp_dir.write(
"package.json",
json!({
"dependencies": {
"cowsay": "*",
},
})
.to_string(),
);
context
.new_command()
.args("install")
.run()
.skip_output_check();
temp_dir.write("other.ts", "import \"cowsay\";\n");
let mut client = context.new_lsp_command().build();
client.initialize_default();
let diagnostics = client.did_open(json!({
"textDocument": {
"uri": temp_dir.url().join("file.ts").unwrap(),
"languageId": "typescript",
"version": 1,
"text": "think({ text: \"foo\" });\n",
},
}));
let list = client.get_completion_list(
temp_dir.url().join("file.ts").unwrap(),
(0, 5),
json!({ "triggerKind": 1 }),
);
assert!(!list.is_incomplete);
let item = list
.items
.iter()
.find(|item| item.label == "think")
.unwrap();
let res = client.write_request("completionItem/resolve", item);
assert_eq!(
res,
json!({
"label": "think",
"labelDetails": {
"description": "cowsay",
},
"kind": 3,
"detail": "function think(options: IOptions): string",
"documentation": {
"kind": "markdown",
"value": "\n\n*@param* \noptions ## Face :\nEither choose a mode (set the value as true) **_or_**\nset your own defined eyes and tongue to `e` and `T`.\n- ### `e` : eyes\n- ### `T` : tongue\n\n## Cow :\nEither specify a cow name (e.g. \"fox\") **_or_**\nset the value of `r` to true which selects a random cow.\n- ### `r` : random selection\n- ### `f` : cow name - from `cows` folder\n\n## Modes :\nModes are just ready-to-use faces, here's their list:\n- #### `b` : borg\n- #### `d` : dead \n- #### `g` : greedy\n- #### `p` : paranoia\n- #### `s` : stoned\n- #### `t` : tired\n- #### `w` : youthful\n- #### `y` : wired \n\n*@example* \n```\n// custom cow and face\ncowsay.think({\n text: 'Hello world!',\n e: '^^', // eyes\n T: 'U ', // tongue\n f: 'USA' // name of the cow from `cows` folder\n})\n\n// using a random cow\ncowsay.think({\n text: 'Hello world!',\n e: 'xx', // eyes\n r: true, // random mode - use a random cow.\n})\n\n// using a mode\ncowsay.think({\n text: 'Hello world!',\n y: true, // using y mode - youthful mode\n})\n```",
},
"sortText": "￿16_0",
"additionalTextEdits": [
{
"range": {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 0 },
},
"newText": "import { think } from \"cowsay\";\n\n",
},
],
}),
);
let diagnostics = diagnostics
.messages_with_file_and_source(
temp_dir.url().join("file.ts").unwrap().as_str(),
"deno-ts",
)
.diagnostics;
let res = client.write_request(
"textDocument/codeAction",
json!(json!({
"textDocument": {
"uri": temp_dir.url().join("file.ts").unwrap(),
},
"range": {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 5 },
},
"context": {
"diagnostics": &diagnostics,
"only": ["quickfix"],
},
})),
);
assert_eq!(
res,
json!([
{
"title": "Add import from \"cowsay\"",
"kind": "quickfix",
"diagnostics": &diagnostics,
"edit": {
"documentChanges": [{
"textDocument": {
"uri": temp_dir.url().join("file.ts").unwrap(),
"version": 1,
},
"edits": [{
"range": {
"start": { "line": 0, "character": 0 },
"end": { "line": 0, "character": 0 },
},
"newText": "import { think } from \"cowsay\";\n\n",
}],
}],
},
},
{
"title": "Add missing function declaration 'think'",
"kind": "quickfix",
"diagnostics": &diagnostics,
"edit": {
"documentChanges": [
{
"textDocument": {
"uri": temp_dir.url().join("file.ts").unwrap(),
"version": 1,
},
"edits": [
{
"range": {
"start": { "line": 1, "character": 0 },
"end": { "line": 1, "character": 0 },
},
"newText": "\nfunction think(arg0: { text: string; }) {\n throw new Error(\"Function not implemented.\");\n}\n",
},
],
},
],
},
},
]),
);
client.shutdown();
}
#[test]
fn lsp_completions_node_specifier() {
let context = TestContextBuilder::new().use_temp_cwd().build();
@ -8237,8 +8382,8 @@ fn lsp_infer_return_type() {
let context = TestContextBuilder::new().use_temp_cwd().build();
let temp_dir = context.temp_dir();
temp_dir.write("deno.json", json!({}).to_string());
let types_file = source_file(
temp_dir.path().join("types.d.ts"),
temp_dir.write(
"types.d.ts",
r#"
export interface SomeInterface {
someField: number;
@ -8319,7 +8464,7 @@ fn lsp_infer_return_type() {
"start": { "line": 1, "character": 20 },
"end": { "line": 1, "character": 20 },
},
"newText": format!(": import(\"{}\").SomeInterface", types_file.url()),
"newText": ": import(\"./types.d.ts\").SomeInterface",
},
],
},