From dd6d19e12051fac2ea5639f621501f4710a1b8e1 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 3 Jul 2024 20:25:42 +0100 Subject: [PATCH] fix(lsp): correct scope attribution for injected @types/node (#24404) --- cli/lsp/documents.rs | 6 +++- cli/lsp/language_server.rs | 2 +- cli/lsp/tsc.rs | 31 ++++++----------- tests/integration/lsp_tests.rs | 63 ++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 05f9be718ae3fb..0d9cd4fbbf161e 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1246,9 +1246,13 @@ impl Documents { &self, specifiers: &[String], referrer: &ModuleSpecifier, + file_referrer: Option<&ModuleSpecifier>, ) -> Vec> { let document = self.get(referrer); - let file_referrer = document.as_ref().and_then(|d| d.file_referrer()); + let file_referrer = document + .as_ref() + .and_then(|d| d.file_referrer()) + .or(file_referrer); let dependencies = document.as_ref().map(|d| d.dependencies()); let mut results = Vec::new(); for specifier in specifiers { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index c4f3789caf5e4e..25782b95c4f12a 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -321,7 +321,7 @@ impl LanguageServer { inner.resolver.did_cache(); inner.refresh_npm_specifiers().await; inner.diagnostics_server.invalidate_all(); - inner.project_changed([], false); + inner.project_changed([], true); inner .ts_server .cleanup_semantic_cache(inner.snapshot()) diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 4ef0d4e64e8012..bab9766eb4285e 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4203,8 +4203,7 @@ struct State { response_tx: Option>>, state_snapshot: Arc, specifier_map: Arc, - root_referrers: HashMap, - last_referrer: Option, + last_scope: Option, token: CancellationToken, pending_requests: Option>, mark: Option, @@ -4223,8 +4222,7 @@ impl State { response_tx: None, state_snapshot, specifier_map, - root_referrers: Default::default(), - last_referrer: None, + last_scope: None, token: Default::default(), mark: None, pending_requests: Some(pending_requests), @@ -4232,18 +4230,13 @@ impl State { } fn get_document(&self, specifier: &ModuleSpecifier) -> Option> { - if let Some(referrer) = self.root_referrers.get(specifier) { - self - .state_snapshot - .documents - .get_or_load(specifier, referrer) - } else if let Some(referrer) = &self.last_referrer { + if let Some(scope) = &self.last_scope { + self.state_snapshot.documents.get_or_load(specifier, scope) + } else { self .state_snapshot .documents - .get_or_load(specifier, referrer) - } else { - self.state_snapshot.documents.get(specifier) + .get_or_load(specifier, &ModuleSpecifier::parse("file:///").unwrap()) } } @@ -4422,6 +4415,7 @@ async fn op_poll_requests( state.response_tx = Some(response_tx); let id = state.last_id; state.last_id += 1; + state.last_scope.clone_from(&scope); let mark = state .performance .mark_with_args(format!("tsc.host.{}", request.method()), &request); @@ -4447,7 +4441,7 @@ fn op_resolve_inner( let specifiers = state .state_snapshot .documents - .resolve(&args.specifiers, &referrer) + .resolve(&args.specifiers, &referrer, state.last_scope.as_ref()) .into_iter() .map(|o| { o.map(|(s, mt)| { @@ -4458,7 +4452,6 @@ fn op_resolve_inner( }) }) .collect(); - state.last_referrer = Some(referrer); state.performance.measure(mark); Ok(specifiers) } @@ -4471,7 +4464,7 @@ fn op_respond( ) { let state = state.borrow_mut::(); state.performance.measure(state.mark.take().unwrap()); - state.last_referrer = None; + state.last_scope = None; let response = if !error.is_empty() { Err(anyhow!("tsc error: {error}")) } else { @@ -4526,17 +4519,13 @@ fn op_script_names(state: &mut OpState) -> ScriptNames { // inject these next because they're global for (scope, script_names) in &mut result.by_scope { - for (referrer, specifiers) in state + for (_, specifiers) in state .state_snapshot .resolver .graph_imports_by_referrer(scope) { for specifier in specifiers { script_names.insert(specifier.to_string()); - state - .root_referrers - .entry(specifier.clone()) - .or_insert(referrer.clone()); } } } diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index e05e182cbf63c1..d0df5e6e8aae23 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -8606,6 +8606,69 @@ fn lsp_completions_node_specifier() { client.shutdown(); } +#[test] +fn lsp_completions_node_specifier_node_modules_dir() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write( + temp_dir.path().join("deno.json"), + json!({ + "nodeModulesDir": true, + }) + .to_string(), + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": "import fs from \"node:fs\";\n", + }, + })); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [[], temp_dir.uri().join("file.ts").unwrap()], + }), + ); + client.write_notification( + "textDocument/didChange", + json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "version": 2, + }, + "contentChanges": [ + { + "range": { + "start": { "line": 1, "character": 0 }, + "end": { "line": 1, "character": 0 }, + }, + "text": "fs.", + }, + ], + }), + ); + let list = client.get_completion_list( + temp_dir.uri().join("file.ts").unwrap(), + (1, 3), + json!({ + "triggerKind": 2, + "triggerCharacter": ".", + }), + ); + assert!(!list.is_incomplete); + assert!(list.items.iter().any(|i| i.label == "writeFile")); + assert!(list.items.iter().any(|i| i.label == "writeFileSync")); + client.shutdown(); +} + #[test] fn lsp_completions_registry() { let context = TestContextBuilder::new()