diff options
author | Nayeem Rahman <nayeemrmn99@gmail.com> | 2024-07-03 20:25:42 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-03 20:25:42 +0100 |
commit | dd6d19e12051fac2ea5639f621501f4710a1b8e1 (patch) | |
tree | 3efb02c40416f5ffb1204f66dd4d5afd20166ef3 | |
parent | 3242e2718fdbbdb6d1855c0f694e816f9af3f09c (diff) |
fix(lsp): correct scope attribution for injected @types/node (#24404)
-rw-r--r-- | cli/lsp/documents.rs | 6 | ||||
-rw-r--r-- | cli/lsp/language_server.rs | 2 | ||||
-rw-r--r-- | cli/lsp/tsc.rs | 31 | ||||
-rw-r--r-- | 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 05f9be718..0d9cd4fbb 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<Option<(ModuleSpecifier, MediaType)>> { 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 c4f3789ca..25782b95c 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 4ef0d4e64..bab9766eb 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4203,8 +4203,7 @@ struct State { response_tx: Option<oneshot::Sender<Result<String, AnyError>>>, state_snapshot: Arc<StateSnapshot>, specifier_map: Arc<TscSpecifierMap>, - root_referrers: HashMap<ModuleSpecifier, ModuleSpecifier>, - last_referrer: Option<ModuleSpecifier>, + last_scope: Option<ModuleSpecifier>, token: CancellationToken, pending_requests: Option<UnboundedReceiver<Request>>, mark: Option<PerformanceMark>, @@ -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<Arc<Document>> { - 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>(); 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 e05e182cb..d0df5e6e8 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -8607,6 +8607,69 @@ fn lsp_completions_node_specifier() { } #[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() .use_http_server() |