From 57373a7e4d0f8427d52900cabf781d17eee4e42e Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 25 Jul 2025 09:33:03 +0530 Subject: [PATCH] [ty] Derive `Serialize` unconditionally on client options (#19549) --- Cargo.lock | 1 - crates/ty_server/Cargo.toml | 5 ----- crates/ty_server/src/session/options.rs | 29 +++++++++---------------- crates/ty_server/tests/e2e/main.rs | 16 +++++++------- 4 files changed, 18 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 074fd197a6..e409e30866 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4345,7 +4345,6 @@ dependencies = [ "ty_ide", "ty_project", "ty_python_semantic", - "ty_server", "ty_vendored", ] diff --git a/crates/ty_server/Cargo.toml b/crates/ty_server/Cargo.toml index 0602f78eba..f0efbfe1c5 100644 --- a/crates/ty_server/Cargo.toml +++ b/crates/ty_server/Cargo.toml @@ -38,16 +38,11 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["chrono"] } [dev-dependencies] -ty_server = { workspace = true, features = ["testing"] } - dunce = { workspace = true } insta = { workspace = true, features = ["filters", "json"] } regex = { workspace = true } tempfile = { workspace = true } -[features] -testing = [] - [target.'cfg(target_vendor = "apple")'.dependencies] libc = { workspace = true } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index f9eb42af4c..0301720835 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -2,7 +2,7 @@ use lsp_types::Url; use ruff_db::system::SystemPathBuf; use ruff_python_ast::PythonVersion; use rustc_hash::FxHashMap; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use ty_project::CheckMode; use ty_project::metadata::Options; use ty_project::metadata::options::ProjectOptionsOverrides; @@ -48,9 +48,8 @@ struct WorkspaceOptions { } /// This is a direct representation of the settings schema sent by the client. -#[derive(Clone, Debug, Deserialize, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub struct ClientOptions { /// Settings under the `python.*` namespace in VS Code that are useful for the ty language @@ -63,9 +62,8 @@ pub struct ClientOptions { } /// Diagnostic mode for the language server. -#[derive(Clone, Copy, Debug, Default, Deserialize)] +#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub(crate) enum DiagnosticMode { /// Check only currently open files. @@ -148,25 +146,22 @@ impl ClientOptions { // would be useful to instead use `workspace/configuration` instead. This would be then used to get // all settings and not just the ones in "python.*". -#[derive(Clone, Debug, Deserialize, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] struct Python { ty: Option, } -#[derive(Clone, Debug, Deserialize, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] struct PythonExtension { active_environment: Option, } -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub(crate) struct ActiveEnvironment { pub(crate) executable: PythonExecutable, @@ -174,9 +169,8 @@ pub(crate) struct ActiveEnvironment { pub(crate) version: Option, } -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub(crate) struct EnvironmentVersion { pub(crate) major: i64, @@ -187,9 +181,8 @@ pub(crate) struct EnvironmentVersion { pub(crate) sys_version: String, } -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub(crate) struct PythonEnvironment { pub(crate) folder_uri: Url, @@ -200,9 +193,8 @@ pub(crate) struct PythonEnvironment { pub(crate) name: Option, } -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] pub(crate) struct PythonExecutable { #[allow(dead_code)] @@ -210,9 +202,8 @@ pub(crate) struct PythonExecutable { pub(crate) sys_prefix: SystemPathBuf, } -#[derive(Clone, Debug, Deserialize, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] -#[cfg_attr(feature = "testing", derive(serde::Serialize))] #[serde(rename_all = "camelCase")] struct Ty { disable_language_services: Option, diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index 6c8cf3606a..6082ad7295 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -154,7 +154,7 @@ impl TestServer { /// Create a new test server with the given workspace configurations fn new( workspaces: Vec<(WorkspaceFolder, ClientOptions)>, - test_dir: TestContext, + test_context: TestContext, capabilities: ClientCapabilities, ) -> Result { setup_tracing(); @@ -162,7 +162,7 @@ impl TestServer { let (server_connection, client_connection) = Connection::memory(); // Create OS system with the test directory as cwd - let os_system = OsSystem::new(test_dir.root()); + let os_system = OsSystem::new(test_context.root()); // Start the server in a separate thread let server_thread = std::thread::spawn(move || { @@ -195,7 +195,7 @@ impl TestServer { Self { server_thread: Some(server_thread), client_connection: Some(client_connection), - test_context: test_dir, + test_context, request_counter: 0, responses: FxHashMap::default(), notifications: VecDeque::new(), @@ -707,7 +707,7 @@ impl Drop for TestServer { /// Builder for creating test servers with specific configurations pub(crate) struct TestServerBuilder { - test_dir: TestContext, + test_context: TestContext, workspaces: Vec<(WorkspaceFolder, ClientOptions)>, client_capabilities: ClientCapabilities, } @@ -734,7 +734,7 @@ impl TestServerBuilder { Ok(Self { workspaces: Vec::new(), - test_dir: TestContext::new()?, + test_context: TestContext::new()?, client_capabilities, }) } @@ -753,7 +753,7 @@ impl TestServerBuilder { anyhow::bail!("Test server doesn't support multiple workspaces yet"); } - let workspace_path = self.test_dir.root().join(workspace_root); + let workspace_path = self.test_context.root().join(workspace_root); fs::create_dir_all(workspace_path.as_std_path())?; self.workspaces.push(( @@ -812,7 +812,7 @@ impl TestServerBuilder { path: impl AsRef, content: impl AsRef, ) -> Result { - let file_path = self.test_dir.root().join(path.as_ref()); + let file_path = self.test_context.root().join(path.as_ref()); // Ensure parent directories exists if let Some(parent) = file_path.parent() { fs::create_dir_all(parent.as_std_path())?; @@ -837,7 +837,7 @@ impl TestServerBuilder { /// Build the test server pub(crate) fn build(self) -> Result { - TestServer::new(self.workspaces, self.test_dir, self.client_capabilities) + TestServer::new(self.workspaces, self.test_context, self.client_capabilities) } }