From 315bf80eed4fead4b73e889eb864a292f6b9480f Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 12 Dec 2025 12:30:56 +0100 Subject: [PATCH 1/5] [ty] Fix outdated version in publish diagnostics after `didChange` (#21943) --- .../server/api/notifications/did_change.rs | 2 +- .../api/notifications/did_change_notebook.rs | 2 +- crates/ty_server/src/session.rs | 18 ++++++++-- .../tests/e2e/publish_diagnostics.rs | 36 +++++++++++++++++++ ...e__publish_diagnostics__on_did_change.snap | 21 +++++++++++ 5 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change.snap diff --git a/crates/ty_server/src/server/api/notifications/did_change.rs b/crates/ty_server/src/server/api/notifications/did_change.rs index 6c01fa2214..ef0631f772 100644 --- a/crates/ty_server/src/server/api/notifications/did_change.rs +++ b/crates/ty_server/src/server/api/notifications/did_change.rs @@ -26,7 +26,7 @@ impl SyncNotificationHandler for DidChangeTextDocumentHandler { content_changes, } = params; - let document = session + let mut document = session .document_handle(&uri) .with_failure_code(ErrorCode::InternalError)?; diff --git a/crates/ty_server/src/server/api/notifications/did_change_notebook.rs b/crates/ty_server/src/server/api/notifications/did_change_notebook.rs index 736a351e40..36490412b3 100644 --- a/crates/ty_server/src/server/api/notifications/did_change_notebook.rs +++ b/crates/ty_server/src/server/api/notifications/did_change_notebook.rs @@ -24,7 +24,7 @@ impl SyncNotificationHandler for DidChangeNotebookHandler { change: types::NotebookDocumentChangeEvent { cells, metadata }, }: types::DidChangeNotebookDocumentParams, ) -> Result<()> { - let document = session + let mut document = session .document_handle(&uri) .with_failure_code(ErrorCode::InternalError)?; diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 4ebfe7ead0..eca0f4da42 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -1451,7 +1451,7 @@ impl DocumentHandle { } pub(crate) fn update_text_document( - &self, + &mut self, session: &mut Session, content_changes: Vec, new_version: DocumentVersion, @@ -1471,6 +1471,8 @@ impl DocumentHandle { } else { document.apply_changes(content_changes, new_version, position_encoding); } + + self.set_version(document.version()); } self.update_in_db(session); @@ -1479,7 +1481,7 @@ impl DocumentHandle { } pub(crate) fn update_notebook_document( - &self, + &mut self, session: &mut Session, cells: Option, metadata: Option, @@ -1496,6 +1498,8 @@ impl DocumentHandle { new_version, position_encoding, )?; + + self.set_version(new_version); } self.update_in_db(session); @@ -1516,6 +1520,16 @@ impl DocumentHandle { session.apply_changes(path, changes); } + fn set_version(&mut self, version: DocumentVersion) { + let self_version = match self { + DocumentHandle::Text { version, .. } + | DocumentHandle::Notebook { version, .. } + | DocumentHandle::Cell { version, .. } => version, + }; + + *self_version = version; + } + /// De-registers a document, specified by its key. /// Calling this multiple times for the same document is a logic error. /// diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index bbe7094325..aea31db40c 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -33,6 +33,42 @@ def foo() -> str: Ok(()) } +#[test] +fn on_did_change() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(workspace_root, None)? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(false) + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + let _ = server.await_notification::(); + + let changes = vec![lsp_types::TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: "def foo() -> int: return 42".to_string(), + }]; + + server.change_text_document(foo, changes, 2); + + let diagnostics = server.await_notification::(); + + assert_eq!(diagnostics.version, Some(2)); + + insta::assert_debug_snapshot!(diagnostics); + + Ok(()) +} + #[test] fn message_without_related_information_support() -> Result<()> { let workspace_root = SystemPath::new("src"); diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change.snap new file mode 100644 index 0000000000..02ea49ca92 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change.snap @@ -0,0 +1,21 @@ +--- +source: crates/ty_server/tests/e2e/publish_diagnostics.rs +expression: diagnostics +--- +PublishDiagnosticsParams { + uri: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/src/foo.py", + query: None, + fragment: None, + }, + diagnostics: [], + version: Some( + 2, + ), +} From 8cc7c993de0ae64a292201fb078e1b93aa30b114 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 12 Dec 2025 12:31:13 +0100 Subject: [PATCH 2/5] [ty] Attach db to background request handler task (#21941) --- crates/ty_server/src/server/api.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ty_server/src/server/api.rs b/crates/ty_server/src/server/api.rs index a5c54e4518..e369717824 100644 --- a/crates/ty_server/src/server/api.rs +++ b/crates/ty_server/src/server/api.rs @@ -299,7 +299,9 @@ where } if let Err(error) = ruff_db::panic::catch_unwind(|| { - R::handle_request(&id, &db, document, client, params); + salsa::attach(&db, || { + R::handle_request(&id, &db, document, client, params); + }); }) { panic_response::(&id, client, &error, retry); } From 0181568fb5f3eceea90ad5a64b29a23ce4965321 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Thu, 11 Dec 2025 14:40:29 -0500 Subject: [PATCH 3/5] [ty] Ignore `__all__` for document and workspace symbol requests We also ignore names introduced by import statements, which seems to match pylance behavior. Fixes astral-sh/ty#1856 --- crates/ty_ide/src/symbols.rs | 43 +++++++++++++++++--- crates/ty_ide/src/workspace_symbols.rs | 56 ++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/crates/ty_ide/src/symbols.rs b/crates/ty_ide/src/symbols.rs index ba33af9ef2..89c41cb359 100644 --- a/crates/ty_ide/src/symbols.rs +++ b/crates/ty_ide/src/symbols.rs @@ -431,7 +431,11 @@ struct SymbolVisitor<'db> { /// This is true even when we're inside a function definition /// that is inside a class. in_class: bool, - global_only: bool, + /// When enabled, the visitor should only try to extract + /// symbols from a module that we believed form the "exported" + /// interface for that module. i.e., `__all__` is only respected + /// when this is enabled. It's otherwise ignored. + exports_only: bool, /// The origin of an `__all__` variable, if found. all_origin: Option, /// A set of names extracted from `__all__`. @@ -451,7 +455,7 @@ impl<'db> SymbolVisitor<'db> { symbol_stack: vec![], in_function: false, in_class: false, - global_only: false, + exports_only: false, all_origin: None, all_names: FxHashSet::default(), all_invalid: false, @@ -460,7 +464,7 @@ impl<'db> SymbolVisitor<'db> { fn globals(db: &'db dyn Db, file: File) -> Self { Self { - global_only: true, + exports_only: true, ..Self::tree(db, file) } } @@ -585,6 +589,11 @@ impl<'db> SymbolVisitor<'db> { /// /// If the assignment isn't for `__all__`, then this is a no-op. fn add_all_assignment(&mut self, targets: &[ast::Expr], value: Option<&ast::Expr>) { + // We don't care about `__all__` unless we're + // specifically looking for exported symbols. + if !self.exports_only { + return; + } if self.in_function || self.in_class { return; } @@ -865,7 +874,7 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { import_kind: None, }; - if self.global_only { + if self.exports_only { self.add_symbol(symbol); // If global_only, don't walk function bodies return; @@ -894,7 +903,7 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { import_kind: None, }; - if self.global_only { + if self.exports_only { self.add_symbol(symbol); // If global_only, don't walk class bodies return; @@ -943,6 +952,12 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { ast::Stmt::AugAssign(ast::StmtAugAssign { target, op, value, .. }) => { + // We don't care about `__all__` unless we're + // specifically looking for exported symbols. + if !self.exports_only { + return; + } + if self.all_origin.is_none() { // We can't update `__all__` if it doesn't already // exist. @@ -961,6 +976,12 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { } } ast::Stmt::Expr(expr) => { + // We don't care about `__all__` unless we're + // specifically looking for exported symbols. + if !self.exports_only { + return; + } + if self.all_origin.is_none() { // We can't update `__all__` if it doesn't already exist. return; @@ -990,6 +1011,12 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { source_order::walk_stmt(self, stmt); } ast::Stmt::Import(import) => { + // We ignore any names introduced by imports + // unless we're specifically looking for the + // set of exported symbols. + if !self.exports_only { + return; + } // We only consider imports in global scope. if self.in_function { return; @@ -999,6 +1026,12 @@ impl SourceOrderVisitor<'_> for SymbolVisitor<'_> { } } ast::Stmt::ImportFrom(import_from) => { + // We ignore any names introduced by imports + // unless we're specifically looking for the + // set of exported symbols. + if !self.exports_only { + return; + } // We only consider imports in global scope. if self.in_function { return; diff --git a/crates/ty_ide/src/workspace_symbols.rs b/crates/ty_ide/src/workspace_symbols.rs index 3224c50baf..d2b01e0c94 100644 --- a/crates/ty_ide/src/workspace_symbols.rs +++ b/crates/ty_ide/src/workspace_symbols.rs @@ -150,6 +150,62 @@ class Test: "); } + #[test] + fn ignore_all() { + let test = CursorTest::builder() + .source( + "utils.py", + " +__all__ = [] +class Test: + def from_path(): ... +", + ) + .build(); + + assert_snapshot!(test.workspace_symbols("from"), @r" + info[workspace-symbols]: WorkspaceSymbolInfo + --> utils.py:4:9 + | + 2 | __all__ = [] + 3 | class Test: + 4 | def from_path(): ... + | ^^^^^^^^^ + | + info: Method from_path + "); + } + + #[test] + fn ignore_imports() { + let test = CursorTest::builder() + .source( + "utils.py", + " +import re +import json as json +from collections import defaultdict +foo = 1 +", + ) + .build(); + + assert_snapshot!(test.workspace_symbols("foo"), @r" + info[workspace-symbols]: WorkspaceSymbolInfo + --> utils.py:5:1 + | + 3 | import json as json + 4 | from collections import defaultdict + 5 | foo = 1 + | ^^^ + | + info: Variable foo + "); + assert_snapshot!(test.workspace_symbols("re"), @"No symbols found"); + assert_snapshot!(test.workspace_symbols("json"), @"No symbols found"); + assert_snapshot!(test.workspace_symbols("default"), @"No symbols found"); + } + impl CursorTest { fn workspace_symbols(&self, query: &str) -> String { let symbols = workspace_symbols(&self.db, query); From 4249736d74ee524733ad1bd61eb483725d15d389 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 12 Dec 2025 13:52:47 +0100 Subject: [PATCH 4/5] [ty] Stabilize rename (#21940) --- crates/ty_ide/src/rename.rs | 2 +- crates/ty_server/src/capabilities.rs | 29 +----- .../src/server/api/requests/prepare_rename.rs | 1 - crates/ty_server/src/session.rs | 33 +------ crates/ty_server/src/session/options.rs | 20 ++-- crates/ty_server/src/session/settings.rs | 10 +- crates/ty_server/tests/e2e/initialize.rs | 92 +------------------ crates/ty_server/tests/e2e/main.rs | 61 +++++++++--- crates/ty_server/tests/e2e/rename.rs | 84 +++++++++++++++++ .../e2e__commands__debug_command.snap | 4 +- .../e2e/snapshots/e2e__rename__notebook.snap | 75 +++++++++++++++ .../snapshots/e2e__rename__text_document.snap | 36 ++++++++ 12 files changed, 262 insertions(+), 185 deletions(-) create mode 100644 crates/ty_server/tests/e2e/rename.rs create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__rename__notebook.snap create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__rename__text_document.snap 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" + } + ] + } +} From bc8efa2fd86088bf136f8d5fa54ba1ea694fcfa4 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 12 Dec 2025 13:54:37 +0100 Subject: [PATCH 5/5] [ty] Classify `cls` as class parameter (#21944) --- crates/ty_ide/src/semantic_tokens.rs | 50 ++++++++++++------- .../src/types/subclass_of.rs | 2 +- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/crates/ty_ide/src/semantic_tokens.rs b/crates/ty_ide/src/semantic_tokens.rs index 79b37265aa..7cb1ea0580 100644 --- a/crates/ty_ide/src/semantic_tokens.rs +++ b/crates/ty_ide/src/semantic_tokens.rs @@ -302,17 +302,25 @@ impl<'db> SemanticTokenVisitor<'db> { let parsed = parsed_module(db, definition.file(db)); let ty = parameter.node(&parsed.load(db)).inferred_type(&model); - if let Some(ty) = ty - && let Type::TypeVar(type_var) = ty - { - match type_var.typevar(db).kind(db) { - TypeVarKind::TypingSelf => { - return Some((SemanticTokenType::SelfParameter, modifiers)); + if let Some(ty) = ty { + let type_var = match ty { + Type::TypeVar(type_var) => Some((type_var, false)), + Type::SubclassOf(subclass_of) => { + subclass_of.into_type_var().map(|var| (var, true)) } - TypeVarKind::Legacy - | TypeVarKind::ParamSpec - | TypeVarKind::Pep695ParamSpec - | TypeVarKind::Pep695 => {} + _ => None, + }; + + if let Some((type_var, is_cls)) = type_var + && matches!(type_var.typevar(db).kind(db), TypeVarKind::TypingSelf) + { + let kind = if is_cls { + SemanticTokenType::ClsParameter + } else { + SemanticTokenType::SelfParameter + }; + + return Some((kind, modifiers)); } } @@ -1203,7 +1211,7 @@ class MyClass: " class MyClass: @classmethod - def method(cls, x): pass + def method(cls, x): print(cls) ", ); @@ -1215,6 +1223,8 @@ class MyClass: "method" @ 41..47: Method [definition] "cls" @ 48..51: ClsParameter [definition] "x" @ 53..54: Parameter [definition] + "print" @ 57..62: Function + "cls" @ 63..66: ClsParameter "#); } @@ -1246,7 +1256,7 @@ class MyClass: class MyClass: def method(instance, x): pass @classmethod - def other(klass, y): pass + def other(klass, y): print(klass) def complex_method(instance, posonly, /, regular, *args, kwonly, **kwargs): pass ", ); @@ -1262,13 +1272,15 @@ class MyClass: "other" @ 75..80: Method [definition] "klass" @ 81..86: ClsParameter [definition] "y" @ 88..89: Parameter [definition] - "complex_method" @ 105..119: Method [definition] - "instance" @ 120..128: SelfParameter [definition] - "posonly" @ 130..137: Parameter [definition] - "regular" @ 142..149: Parameter [definition] - "args" @ 152..156: Parameter [definition] - "kwonly" @ 158..164: Parameter [definition] - "kwargs" @ 168..174: Parameter [definition] + "print" @ 92..97: Function + "klass" @ 98..103: ClsParameter + "complex_method" @ 113..127: Method [definition] + "instance" @ 128..136: SelfParameter [definition] + "posonly" @ 138..145: Parameter [definition] + "regular" @ 150..157: Parameter [definition] + "args" @ 160..164: Parameter [definition] + "kwonly" @ 166..172: Parameter [definition] + "kwargs" @ 176..182: Parameter [definition] "#); } diff --git a/crates/ty_python_semantic/src/types/subclass_of.rs b/crates/ty_python_semantic/src/types/subclass_of.rs index c6bb9d0378..eb016199a5 100644 --- a/crates/ty_python_semantic/src/types/subclass_of.rs +++ b/crates/ty_python_semantic/src/types/subclass_of.rs @@ -119,7 +119,7 @@ impl<'db> SubclassOfType<'db> { subclass_of.is_type_var() } - pub(crate) const fn into_type_var(self) -> Option> { + pub const fn into_type_var(self) -> Option> { self.subclass_of.into_type_var() }