diff --git a/crates/ty_ide/src/rename.rs b/crates/ty_ide/src/rename.rs index ecea0ffc36..5d7951846a 100644 --- a/crates/ty_ide/src/rename.rs +++ b/crates/ty_ide/src/rename.rs @@ -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)] diff --git a/crates/ty_server/src/capabilities.rs b/crates/ty_server/src/capabilities.rs index 60b5298280..e08ca6ef00 100644 --- a/crates/ty_server/src/capabilities.rs +++ b/crates/ty_server/src/capabilities.rs @@ -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 { diff --git a/crates/ty_server/src/server/api/requests/prepare_rename.rs b/crates/ty_server/src/server/api/requests/prepare_rename.rs index 8601aa2995..2fd8228201 100644 --- a/crates/ty_server/src/server/api/requests/prepare_rename.rs +++ b/crates/ty_server/src/server/api/requests/prepare_rename.rs @@ -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); } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index eca0f4da42..18b211a8cc 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -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 } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index b202270aae..cc02037d0c 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -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, +pub(crate) struct Experimental; + +impl Experimental { + #[expect(clippy::unused_self)] + fn into_settings(self) -> ExperimentalSettings { + ExperimentalSettings {} + } } #[derive(Clone, Debug, Serialize, Deserialize, Default)] diff --git a/crates/ty_server/src/session/settings.rs b/crates/ty_server/src/session/settings.rs index 5e73df0102..3b88c2d1f0 100644 --- a/crates/ty_server/src/session/settings.rs +++ b/crates/ty_server/src/session/settings.rs @@ -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. /// diff --git a/crates/ty_server/tests/e2e/initialize.rs b/crates/ty_server/tests/e2e/initialize.rs index 8611afdccd..fb715bf929 100644 --- a/crates/ty_server/tests/e2e/initialize.rs +++ b/crates/ty_server/tests/e2e/initialize.rs @@ -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::(); - 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::(); 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 - } } ] "#); diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index 579e730129..d0ac098527 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -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(&mut self, params: R::Params) -> R::Result + where + R: Request, + { + let id = self.send_request::(params); + self.try_await_response::(&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::(params); } + pub(crate) fn rename( + &mut self, + document: &Url, + position: lsp_types::Position, + new_name: &str, + ) -> Result, ()> { + if self + .send_request_await::(lsp_types::TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: document.clone(), + }, + position, + }) + .is_none() + { + return Err(()); + } + + Ok( + self.send_request_await::(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 diff --git a/crates/ty_server/tests/e2e/rename.rs b/crates/ty_server/tests/e2e/rename.rs new file mode 100644 index 0000000000..56737686a2 --- /dev/null +++ b/crates/ty_server/tests/e2e/rename.rs @@ -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(()) +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap index 762b91f71c..21c4b05014 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap @@ -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 diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__rename__notebook.snap b/crates/ty_server/tests/e2e/snapshots/e2e__rename__notebook.snap new file mode 100644 index 0000000000..997c0bf686 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__rename__notebook.snap @@ -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" + } + ] + } +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__rename__text_document.snap b/crates/ty_server/tests/e2e/snapshots/e2e__rename__text_document.snap new file mode 100644 index 0000000000..fbd47699f5 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__rename__text_document.snap @@ -0,0 +1,36 @@ +--- +source: crates/ty_server/tests/e2e/rename.rs +expression: edits +--- +{ + "changes": { + "file:///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" + } + ] + } +}