[ty] Stabilize rename (#21940)

This commit is contained in:
Micha Reiser 2025-12-12 13:52:47 +01:00 committed by GitHub
parent 0181568fb5
commit 4249736d74
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 262 additions and 185 deletions

View File

@ -84,7 +84,7 @@ pub fn rename(
/// Helper function to check if a file is included in the project.
fn is_file_in_project(db: &dyn Db, file: File) -> bool {
db.project().files(db).contains(&file)
file.path(db).is_system_virtual_path() || db.project().files(db).contains(&file)
}
#[cfg(test)]

View File

@ -34,9 +34,8 @@ bitflags::bitflags! {
const RELATIVE_FILE_WATCHER_SUPPORT = 1 << 13;
const DIAGNOSTIC_DYNAMIC_REGISTRATION = 1 << 14;
const WORKSPACE_CONFIGURATION = 1 << 15;
const RENAME_DYNAMIC_REGISTRATION = 1 << 16;
const COMPLETION_ITEM_LABEL_DETAILS_SUPPORT = 1 << 17;
const DIAGNOSTIC_RELATED_INFORMATION = 1 << 18;
const COMPLETION_ITEM_LABEL_DETAILS_SUPPORT = 1 << 16;
const DIAGNOSTIC_RELATED_INFORMATION = 1 << 17;
}
}
@ -169,11 +168,6 @@ impl ResolvedClientCapabilities {
self.contains(Self::DIAGNOSTIC_RELATED_INFORMATION)
}
/// Returns `true` if the client supports dynamic registration for rename capabilities.
pub(crate) const fn supports_rename_dynamic_registration(self) -> bool {
self.contains(Self::RENAME_DYNAMIC_REGISTRATION)
}
/// Returns `true` if the client supports "label details" in completion items.
pub(crate) const fn supports_completion_item_label_details(self) -> bool {
self.contains(Self::COMPLETION_ITEM_LABEL_DETAILS_SUPPORT)
@ -326,13 +320,6 @@ impl ResolvedClientCapabilities {
flags |= Self::HIERARCHICAL_DOCUMENT_SYMBOL_SUPPORT;
}
if text_document
.and_then(|text_document| text_document.rename.as_ref()?.dynamic_registration)
.unwrap_or_default()
{
flags |= Self::RENAME_DYNAMIC_REGISTRATION;
}
if client_capabilities
.window
.as_ref()
@ -373,16 +360,6 @@ pub(crate) fn server_capabilities(
))
};
let rename_provider = if resolved_client_capabilities.supports_rename_dynamic_registration() {
// If the client supports dynamic registration, we will register the rename capabilities
// dynamically based on the `ty.experimental.rename` setting.
None
} else {
// Otherwise, we always register the rename provider and bail out in `prepareRename` if
// the feature is disabled.
Some(OneOf::Right(server_rename_options()))
};
ServerCapabilities {
position_encoding: Some(position_encoding.into()),
code_action_provider: Some(types::CodeActionProviderCapability::Options(
@ -413,7 +390,7 @@ pub(crate) fn server_capabilities(
definition_provider: Some(OneOf::Left(true)),
declaration_provider: Some(DeclarationCapability::Simple(true)),
references_provider: Some(OneOf::Left(true)),
rename_provider,
rename_provider: Some(OneOf::Right(server_rename_options())),
document_highlight_provider: Some(OneOf::Left(true)),
hover_provider: Some(HoverProviderCapability::Simple(true)),
signature_help_provider: Some(SignatureHelpOptions {

View File

@ -32,7 +32,6 @@ impl BackgroundDocumentRequestHandler for PrepareRenameRequestHandler {
if snapshot
.workspace_settings()
.is_language_services_disabled()
|| !snapshot.global_settings().is_rename_enabled()
{
return Ok(None);
}

View File

@ -9,7 +9,7 @@ use anyhow::{Context, anyhow};
use lsp_server::{Message, RequestId};
use lsp_types::notification::{DidChangeWatchedFiles, Exit, Notification};
use lsp_types::request::{
DocumentDiagnosticRequest, RegisterCapability, Rename, Request, Shutdown, UnregisterCapability,
DocumentDiagnosticRequest, RegisterCapability, Request, Shutdown, UnregisterCapability,
WorkspaceDiagnosticRequest,
};
use lsp_types::{
@ -32,9 +32,7 @@ use options::GlobalOptions;
pub(crate) use self::options::InitializationOptions;
pub use self::options::{ClientOptions, DiagnosticMode};
pub(crate) use self::settings::{GlobalSettings, WorkspaceSettings};
use crate::capabilities::{
ResolvedClientCapabilities, server_diagnostic_options, server_rename_options,
};
use crate::capabilities::{ResolvedClientCapabilities, server_diagnostic_options};
use crate::document::{DocumentKey, DocumentVersion, NotebookDocument};
use crate::server::{Action, publish_settings_diagnostics};
use crate::session::client::Client;
@ -583,7 +581,6 @@ impl Session {
/// `ty.experimental.rename` global setting.
fn register_capabilities(&mut self, client: &Client) {
static DIAGNOSTIC_REGISTRATION_ID: &str = "ty/textDocument/diagnostic";
static RENAME_REGISTRATION_ID: &str = "ty/textDocument/rename";
static FILE_WATCHER_REGISTRATION_ID: &str = "ty/workspace/didChangeWatchedFiles";
let mut registrations = vec![];
@ -625,31 +622,6 @@ impl Session {
});
}
if self
.resolved_client_capabilities
.supports_rename_dynamic_registration()
{
let is_rename_enabled = self.global_settings.is_rename_enabled();
if !is_rename_enabled {
tracing::debug!("Rename capability is disabled in the resolved global settings");
if self.registrations.contains(Rename::METHOD) {
unregistrations.push(Unregistration {
id: RENAME_REGISTRATION_ID.into(),
method: Rename::METHOD.into(),
});
}
}
if is_rename_enabled {
registrations.push(Registration {
id: RENAME_REGISTRATION_ID.into(),
method: Rename::METHOD.into(),
register_options: Some(serde_json::to_value(server_rename_options()).unwrap()),
});
}
}
if let Some(register_options) = self.file_watcher_registration_options() {
if self.registrations.contains(DidChangeWatchedFiles::METHOD) {
unregistrations.push(Unregistration {
@ -1020,6 +992,7 @@ impl DocumentSnapshot {
}
/// Returns the client settings for all workspaces.
#[expect(unused)]
pub(crate) fn global_settings(&self) -> &GlobalSettings {
&self.global_settings
}

View File

@ -116,12 +116,6 @@ impl ClientOptions {
self
}
#[must_use]
pub fn with_experimental_rename(mut self, enabled: bool) -> Self {
self.global.experimental.get_or_insert_default().rename = Some(enabled);
self
}
#[must_use]
pub fn with_auto_import(mut self, enabled: bool) -> Self {
self.workspace
@ -156,9 +150,7 @@ impl GlobalOptions {
pub(crate) fn into_settings(self) -> GlobalSettings {
let experimental = self
.experimental
.map(|experimental| ExperimentalSettings {
rename: experimental.rename.unwrap_or(false),
})
.map(Experimental::into_settings)
.unwrap_or_default();
GlobalSettings {
@ -326,9 +318,13 @@ impl Combine for DiagnosticMode {
#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)]
#[serde(rename_all = "camelCase")]
pub(crate) struct Experimental {
/// Whether to enable the experimental symbol rename feature.
pub(crate) rename: Option<bool>,
pub(crate) struct Experimental;
impl Experimental {
#[expect(clippy::unused_self)]
fn into_settings(self) -> ExperimentalSettings {
ExperimentalSettings {}
}
}
#[derive(Clone, Debug, Serialize, Deserialize, Default)]

View File

@ -10,12 +10,6 @@ pub(crate) struct GlobalSettings {
pub(super) experimental: ExperimentalSettings,
}
impl GlobalSettings {
pub(crate) fn is_rename_enabled(&self) -> bool {
self.experimental.rename
}
}
impl GlobalSettings {
pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode {
self.diagnostic_mode
@ -23,9 +17,7 @@ impl GlobalSettings {
}
#[derive(Clone, Default, Debug, PartialEq)]
pub(crate) struct ExperimentalSettings {
pub(super) rename: bool,
}
pub(crate) struct ExperimentalSettings;
/// Resolved client settings for a specific workspace.
///

View File

@ -434,96 +434,21 @@ fn unknown_options_in_workspace_configuration() -> Result<()> {
Ok(())
}
/// Tests that the server sends a registration request for the rename capability if the client
/// setting is set to true and dynamic registration is enabled.
#[test]
fn register_rename_capability_when_enabled() -> Result<()> {
let workspace_root = SystemPath::new("foo");
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_initialization_options(ClientOptions::default().with_experimental_rename(true))
.enable_rename_dynamic_registration(true)
.build()
.wait_until_workspaces_are_initialized();
let (_, params) = server.await_request::<RegisterCapability>();
let [registration] = params.registrations.as_slice() else {
panic!(
"Expected a single registration, got: {:#?}",
params.registrations
);
};
insta::assert_json_snapshot!(registration, @r#"
{
"id": "ty/textDocument/rename",
"method": "textDocument/rename",
"registerOptions": {
"prepareProvider": true
}
}
"#);
Ok(())
}
/// Tests that rename capability is statically registered during initialization if the client
/// doesn't support dynamic registration, but the server is configured to support it.
#[test]
fn rename_available_without_dynamic_registration() -> Result<()> {
let workspace_root = SystemPath::new("foo");
let server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_initialization_options(ClientOptions::default().with_experimental_rename(true))
.enable_rename_dynamic_registration(false)
.build()
.wait_until_workspaces_are_initialized();
let initialization_result = server.initialization_result().unwrap();
insta::assert_json_snapshot!(initialization_result.capabilities.rename_provider, @r#"
{
"prepareProvider": true
}
"#);
Ok(())
}
/// Tests that the server does not send a registration request for the rename capability if the
/// client setting is set to false and dynamic registration is enabled.
#[test]
fn not_register_rename_capability_when_disabled() -> Result<()> {
let workspace_root = SystemPath::new("foo");
TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_initialization_options(ClientOptions::default().with_experimental_rename(false))
.enable_rename_dynamic_registration(true)
.build()
.wait_until_workspaces_are_initialized();
// The `Drop` implementation will make sure that the client did not receive any registration
// request.
Ok(())
}
/// Tests that the server can register multiple capabilities at once.
///
/// This test would need to be updated when the server supports additional capabilities in the
/// future.
///
/// TODO: This test currently only verifies a single capability. It should be
/// updated with more dynamic capabilities when the server supports it.
#[test]
fn register_multiple_capabilities() -> Result<()> {
let workspace_root = SystemPath::new("foo");
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_initialization_options(
ClientOptions::default()
.with_experimental_rename(true)
.with_diagnostic_mode(DiagnosticMode::Workspace),
ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace),
)
.enable_rename_dynamic_registration(true)
.enable_diagnostic_dynamic_registration(true)
.build()
.wait_until_workspaces_are_initialized();
@ -531,8 +456,6 @@ fn register_multiple_capabilities() -> Result<()> {
let (_, params) = server.await_request::<RegisterCapability>();
let registrations = params.registrations;
assert_eq!(registrations.len(), 2);
insta::assert_json_snapshot!(registrations, @r#"
[
{
@ -545,13 +468,6 @@ fn register_multiple_capabilities() -> Result<()> {
"workDoneProgress": true,
"workspaceDiagnostics": true
}
},
{
"id": "ty/textDocument/rename",
"method": "textDocument/rename",
"registerOptions": {
"prepareProvider": true
}
}
]
"#);

View File

@ -35,6 +35,7 @@ mod inlay_hints;
mod notebook;
mod publish_diagnostics;
mod pull_diagnostics;
mod rename;
use std::collections::{BTreeMap, HashMap, VecDeque};
use std::num::NonZeroUsize;
@ -52,8 +53,8 @@ use lsp_types::notification::{
Initialized, Notification,
};
use lsp_types::request::{
Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, Request,
Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest,
Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest,
PrepareRenameRequest, Request, Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest,
};
use lsp_types::{
ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse,
@ -67,12 +68,11 @@ use lsp_types::{
TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentItem,
TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkDoneProgressParams,
WorkspaceClientCapabilities, WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult,
WorkspaceFolder,
WorkspaceEdit, WorkspaceFolder,
};
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf, TestSystem};
use rustc_hash::FxHashMap;
use tempfile::TempDir;
use ty_server::{ClientOptions, LogLevel, Server, init_logging};
/// Number of times to retry receiving a message before giving up
@ -420,6 +420,16 @@ impl TestServer {
.unwrap_or_else(|err| panic!("Failed to receive response for request {id}: {err}"))
}
#[track_caller]
pub(crate) fn send_request_await<R>(&mut self, params: R::Params) -> R::Result
where
R: Request,
{
let id = self.send_request::<R>(params);
self.try_await_response::<R>(&id, None)
.unwrap_or_else(|err| panic!("Failed to receive response for request {id}: {err}"))
}
/// Wait for a server response corresponding to the given request ID.
///
/// This should only be called if a request was already sent to the server via [`send_request`]
@ -802,6 +812,38 @@ impl TestServer {
self.send_notification::<DidChangeWatchedFiles>(params);
}
pub(crate) fn rename(
&mut self,
document: &Url,
position: lsp_types::Position,
new_name: &str,
) -> Result<Option<WorkspaceEdit>, ()> {
if self
.send_request_await::<PrepareRenameRequest>(lsp_types::TextDocumentPositionParams {
text_document: TextDocumentIdentifier {
uri: document.clone(),
},
position,
})
.is_none()
{
return Err(());
}
Ok(
self.send_request_await::<lsp_types::request::Rename>(lsp_types::RenameParams {
text_document_position: TextDocumentPositionParams {
text_document: TextDocumentIdentifier {
uri: document.clone(),
},
position,
},
new_name: new_name.to_string(),
work_done_progress_params: WorkDoneProgressParams::default(),
}),
)
}
/// Send a `textDocument/diagnostic` request for the document at the given path.
pub(crate) fn document_diagnostic_request(
&mut self,
@ -1082,17 +1124,6 @@ impl TestServerBuilder {
self
}
/// Enable or disable dynamic registration of rename capability
pub(crate) fn enable_rename_dynamic_registration(mut self, enabled: bool) -> Self {
self.client_capabilities
.text_document
.get_or_insert_default()
.rename
.get_or_insert_default()
.dynamic_registration = Some(enabled);
self
}
/// Enable or disable workspace configuration capability
pub(crate) fn enable_workspace_configuration(mut self, enabled: bool) -> Self {
self.client_capabilities

View File

@ -0,0 +1,84 @@
use crate::TestServerBuilder;
use crate::notebook::NotebookBuilder;
use insta::assert_json_snapshot;
#[test]
fn text_document() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()?
.with_file("foo.py", "")?
.enable_pull_diagnostics(true)
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(
"foo.py",
r#"def test(): ...
test()
"#,
1,
);
let edits = server
.rename(
&server.file_uri("foo.py"),
lsp_types::Position {
line: 0,
character: 5,
},
"new_name",
)
.expect("Can rename `test` function");
assert_json_snapshot!(edits);
Ok(())
}
#[test]
fn notebook() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()?
.with_file("test.ipynb", "")?
.enable_pull_diagnostics(true)
.build()
.wait_until_workspaces_are_initialized();
let mut builder = NotebookBuilder::virtual_file("test.ipynb");
builder.add_python_cell(
r#"from typing import Literal
type Style = Literal["italic", "bold", "underline"]"#,
);
let cell2 = builder.add_python_cell(
r#"def with_style(line: str, word, style: Style) -> str:
if style == "italic":
return line.replace(word, f"*{word}*")
elif style == "bold":
return line.replace(word, f"__{word}__")
position = line.find(word)
output = line + "\n"
output += " " * position
output += "-" * len(word)
"#,
);
builder.open(&mut server);
let edits = server
.rename(
&cell2,
lsp_types::Position {
line: 0,
character: 16,
},
"text",
)
.expect("Can rename `line` parameter");
assert_json_snapshot!(edits);
server.collect_publish_diagnostic_notifications(2);
Ok(())
}

View File

@ -8,9 +8,7 @@ Client capabilities: ResolvedClientCapabilities(
Position encoding: UTF16
Global settings: GlobalSettings {
diagnostic_mode: OpenFilesOnly,
experimental: ExperimentalSettings {
rename: false,
},
experimental: ExperimentalSettings,
}
Open text documents: 0

View File

@ -0,0 +1,75 @@
---
source: crates/ty_server/tests/e2e/rename.rs
expression: edits
---
{
"changes": {
"vscode-notebook-cell://test.ipynb#1": [
{
"range": {
"start": {
"line": 0,
"character": 15
},
"end": {
"line": 0,
"character": 19
}
},
"newText": "text"
},
{
"range": {
"start": {
"line": 2,
"character": 15
},
"end": {
"line": 2,
"character": 19
}
},
"newText": "text"
},
{
"range": {
"start": {
"line": 4,
"character": 15
},
"end": {
"line": 4,
"character": 19
}
},
"newText": "text"
},
{
"range": {
"start": {
"line": 6,
"character": 15
},
"end": {
"line": 6,
"character": 19
}
},
"newText": "text"
},
{
"range": {
"start": {
"line": 7,
"character": 13
},
"end": {
"line": 7,
"character": 17
}
},
"newText": "text"
}
]
}
}

View File

@ -0,0 +1,36 @@
---
source: crates/ty_server/tests/e2e/rename.rs
expression: edits
---
{
"changes": {
"file://<temp_dir>/foo.py": [
{
"range": {
"start": {
"line": 0,
"character": 4
},
"end": {
"line": 0,
"character": 8
}
},
"newText": "new_name"
},
{
"range": {
"start": {
"line": 2,
"character": 0
},
"end": {
"line": 2,
"character": 4
}
},
"newText": "new_name"
}
]
}
}