Skip to content

Commit fc72ff4

Browse files
authored
[ty] Send a single request for registrations/unregistrations (#19822)
## Summary This is a small refactor to update the server to send a single request to perform registrations and unregistrations of dynamic capabilities. ## Test Plan Existing E2E test cases pass, add a new test case to verify multiple registrations.
1 parent 827456f commit fc72ff4

File tree

2 files changed

+154
-102
lines changed

2 files changed

+154
-102
lines changed

crates/ty_server/src/session.rs

Lines changed: 104 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -570,8 +570,7 @@ impl Session {
570570
self.global_settings = Arc::new(global_settings);
571571
}
572572

573-
self.register_diagnostic_capability(client);
574-
self.register_rename_capability(client);
573+
self.register_capabilities(client);
575574

576575
assert!(
577576
self.workspaces.all_initialized(),
@@ -587,134 +586,137 @@ impl Session {
587586
}
588587
}
589588

590-
// TODO: Merge the following two methods as `register_capability` and `unregister_capability`
591-
592-
/// Sends a registration notification to the client to enable / disable workspace diagnostics
593-
/// as per the `ty.diagnosticMode` global setting.
589+
/// Registers the dynamic capabilities with the client as per the resolved global settings.
590+
///
591+
/// ## Diagnostic capability
592+
///
593+
/// This capability is used to enable / disable workspace diagnostics as per the
594+
/// `ty.diagnosticMode` global setting.
595+
///
596+
/// ## Rename capability
594597
///
595-
/// This method is a no-op if the client doesn't support dynamic registration of diagnostic
596-
/// capability.
597-
fn register_diagnostic_capability(&mut self, client: &Client) {
598+
/// This capability is used to enable / disable rename functionality as per the
599+
/// `ty.experimental.rename` global setting.
600+
fn register_capabilities(&mut self, client: &Client) {
598601
static DIAGNOSTIC_REGISTRATION_ID: &str = "ty/textDocument/diagnostic";
602+
static RENAME_REGISTRATION_ID: &str = "ty/textDocument/rename";
603+
604+
let mut registrations = vec![];
605+
let mut unregistrations = vec![];
599606

600-
if !self
607+
if self
601608
.resolved_client_capabilities
602609
.supports_diagnostic_dynamic_registration()
603610
{
604-
return;
605-
}
606-
607-
let registered = self
608-
.registrations
609-
.contains(DocumentDiagnosticRequest::METHOD);
610-
611-
if registered {
612-
client.send_request::<UnregisterCapability>(
613-
self,
614-
UnregistrationParams {
615-
unregisterations: vec![Unregistration {
616-
id: DIAGNOSTIC_REGISTRATION_ID.into(),
617-
method: DocumentDiagnosticRequest::METHOD.into(),
618-
}],
619-
},
620-
|_: &Client, ()| {
621-
tracing::debug!("Unregistered diagnostic capability");
622-
},
623-
);
624-
}
625-
626-
let diagnostic_mode = self.global_settings.diagnostic_mode;
627-
628-
let registration = Registration {
629-
id: DIAGNOSTIC_REGISTRATION_ID.into(),
630-
method: DocumentDiagnosticRequest::METHOD.into(),
631-
register_options: Some(
632-
serde_json::to_value(DiagnosticServerCapabilities::RegistrationOptions(
633-
DiagnosticRegistrationOptions {
634-
diagnostic_options: server_diagnostic_options(
635-
diagnostic_mode.is_workspace(),
636-
),
637-
..Default::default()
638-
},
639-
))
640-
.unwrap(),
641-
),
642-
};
611+
if self
612+
.registrations
613+
.contains(DocumentDiagnosticRequest::METHOD)
614+
{
615+
unregistrations.push(Unregistration {
616+
id: DIAGNOSTIC_REGISTRATION_ID.into(),
617+
method: DocumentDiagnosticRequest::METHOD.into(),
618+
});
619+
}
643620

644-
client.send_request::<RegisterCapability>(
645-
self,
646-
RegistrationParams {
647-
registrations: vec![registration],
648-
},
649-
move |_: &Client, ()| {
650-
tracing::debug!(
651-
"Registered diagnostic capability in {diagnostic_mode:?} diagnostic mode"
652-
);
653-
},
654-
);
621+
let diagnostic_mode = self.global_settings.diagnostic_mode;
655622

656-
if !registered {
657-
self.registrations
658-
.insert(DocumentDiagnosticRequest::METHOD.to_string());
623+
tracing::debug!(
624+
"Registering diagnostic capability with {diagnostic_mode:?} diagnostic mode"
625+
);
626+
registrations.push(Registration {
627+
id: DIAGNOSTIC_REGISTRATION_ID.into(),
628+
method: DocumentDiagnosticRequest::METHOD.into(),
629+
register_options: Some(
630+
serde_json::to_value(DiagnosticServerCapabilities::RegistrationOptions(
631+
DiagnosticRegistrationOptions {
632+
diagnostic_options: server_diagnostic_options(
633+
diagnostic_mode.is_workspace(),
634+
),
635+
..Default::default()
636+
},
637+
))
638+
.unwrap(),
639+
),
640+
});
659641
}
660-
}
661-
662-
/// Sends a registration notification to the client to enable / disable rename capability as
663-
/// per the `ty.experimental.rename` global setting.
664-
///
665-
/// This method is a no-op if the client doesn't support dynamic registration of rename
666-
/// capability.
667-
fn register_rename_capability(&mut self, client: &Client) {
668-
static RENAME_REGISTRATION_ID: &str = "ty/textDocument/rename";
669642

670-
if !self
643+
if self
671644
.resolved_client_capabilities
672645
.supports_rename_dynamic_registration()
673646
{
674-
return;
675-
}
647+
let is_rename_enabled = self.global_settings.is_rename_enabled();
676648

677-
let registered = self.registrations.contains(Rename::METHOD);
678-
679-
if registered {
680-
client.send_request::<UnregisterCapability>(
681-
self,
682-
UnregistrationParams {
683-
unregisterations: vec![Unregistration {
649+
if !is_rename_enabled {
650+
tracing::debug!("Rename capability is disabled in the resolved global settings");
651+
if self.registrations.contains(Rename::METHOD) {
652+
unregistrations.push(Unregistration {
684653
id: RENAME_REGISTRATION_ID.into(),
685654
method: Rename::METHOD.into(),
686-
}],
687-
},
688-
move |_: &Client, ()| {
689-
tracing::debug!("Unregistered rename capability");
690-
},
691-
);
655+
});
656+
}
657+
}
658+
659+
if is_rename_enabled {
660+
registrations.push(Registration {
661+
id: RENAME_REGISTRATION_ID.into(),
662+
method: Rename::METHOD.into(),
663+
register_options: Some(serde_json::to_value(server_rename_options()).unwrap()),
664+
});
665+
}
692666
}
693667

694-
if !self.global_settings.experimental.rename {
695-
tracing::debug!("Rename capability is disabled in the client settings");
668+
// First, unregister any existing capabilities and then register or re-register them.
669+
self.unregister_dynamic_capability(client, unregistrations);
670+
self.register_dynamic_capability(client, registrations);
671+
}
672+
673+
/// Registers a list of dynamic capabilities with the client.
674+
fn register_dynamic_capability(&mut self, client: &Client, registrations: Vec<Registration>) {
675+
if registrations.is_empty() {
696676
return;
697677
}
698678

699-
let registration = Registration {
700-
id: RENAME_REGISTRATION_ID.into(),
701-
method: Rename::METHOD.into(),
702-
register_options: Some(serde_json::to_value(server_rename_options()).unwrap()),
703-
};
679+
for registration in &registrations {
680+
self.registrations.insert(registration.method.clone());
681+
}
704682

705683
client.send_request::<RegisterCapability>(
706684
self,
707-
RegistrationParams {
708-
registrations: vec![registration],
709-
},
710-
move |_: &Client, ()| {
711-
tracing::debug!("Registered rename capability");
685+
RegistrationParams { registrations },
686+
|_: &Client, ()| {
687+
tracing::debug!("Registered dynamic capabilities");
712688
},
713689
);
690+
}
714691

715-
if !registered {
716-
self.registrations.insert(Rename::METHOD.to_string());
692+
/// Unregisters a list of dynamic capabilities with the client.
693+
fn unregister_dynamic_capability(
694+
&mut self,
695+
client: &Client,
696+
unregistrations: Vec<Unregistration>,
697+
) {
698+
if unregistrations.is_empty() {
699+
return;
717700
}
701+
702+
for unregistration in &unregistrations {
703+
if !self.registrations.remove(&unregistration.method) {
704+
tracing::debug!(
705+
"Unregistration for `{}` was requested, but it was not registered",
706+
unregistration.method
707+
);
708+
}
709+
}
710+
711+
client.send_request::<UnregisterCapability>(
712+
self,
713+
UnregistrationParams {
714+
unregisterations: unregistrations,
715+
},
716+
|_: &Client, ()| {
717+
tracing::debug!("Unregistered dynamic capabilities");
718+
},
719+
);
718720
}
719721

720722
/// Creates a document snapshot with the URL referencing the document to snapshot.

crates/ty_server/tests/e2e/initialize.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,3 +508,53 @@ fn not_register_rename_capability_when_disabled() -> Result<()> {
508508

509509
Ok(())
510510
}
511+
512+
/// Tests that the server can register multiple capabilities at once.
513+
///
514+
/// This test would need to be updated when the server supports additional capabilities in the
515+
/// future.
516+
#[test]
517+
fn register_multiple_capabilities() -> Result<()> {
518+
let workspace_root = SystemPath::new("foo");
519+
let mut server = TestServerBuilder::new()?
520+
.with_workspace(workspace_root, None)?
521+
.with_initialization_options(
522+
ClientOptions::default()
523+
.with_experimental_rename(true)
524+
.with_diagnostic_mode(DiagnosticMode::Workspace),
525+
)
526+
.enable_rename_dynamic_registration(true)
527+
.enable_diagnostic_dynamic_registration(true)
528+
.build()?
529+
.wait_until_workspaces_are_initialized()?;
530+
531+
let (_, params) = server.await_request::<RegisterCapability>()?;
532+
let registrations = params.registrations;
533+
534+
assert_eq!(registrations.len(), 2);
535+
536+
insta::assert_json_snapshot!(registrations, @r#"
537+
[
538+
{
539+
"id": "ty/textDocument/diagnostic",
540+
"method": "textDocument/diagnostic",
541+
"registerOptions": {
542+
"documentSelector": null,
543+
"identifier": "ty",
544+
"interFileDependencies": true,
545+
"workDoneProgress": true,
546+
"workspaceDiagnostics": true
547+
}
548+
},
549+
{
550+
"id": "ty/textDocument/rename",
551+
"method": "textDocument/rename",
552+
"registerOptions": {
553+
"prepareProvider": true
554+
}
555+
}
556+
]
557+
"#);
558+
559+
Ok(())
560+
}

0 commit comments

Comments
 (0)