diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 3fc0f95235..a5e342b247 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -380,11 +380,22 @@ pub enum CompletionKind { TypeParameter, } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct CompletionSettings { pub auto_import: bool, } +// N.B. It's important for the defaults here to match the defaults +// established by `CompletionOptions::into_settings`. This is +// because `WorkspaceSettings::default()` uses this definition. +// But `WorkspaceOptions::default().into_settings()` will use the +// `CompletionOptions::into_settings` definition. +impl Default for CompletionSettings { + fn default() -> CompletionSettings { + CompletionSettings { auto_import: true } + } +} + pub fn completion<'db>( db: &'db dyn Db, settings: &CompletionSettings, @@ -6610,7 +6621,14 @@ collabc fn completion_test_builder(&self) -> CompletionTestBuilder { CompletionTestBuilder { cursor_test: self.build(), - settings: CompletionSettings::default(), + settings: CompletionSettings { + // The tests were originally written with auto-import + // disabled, since it was disabled by default. But then + // we enabled it by default. However, we kept the tests + // as written with the assumption that auto-import was + // disabled unless opted into. ---AG + auto_import: false, + }, skip_builtins: false, skip_keywords: false, type_signatures: false, diff --git a/crates/ty_server/src/server/api/requests/completion.rs b/crates/ty_server/src/server/api/requests/completion.rs index 1055f85b56..540546cf49 100644 --- a/crates/ty_server/src/server/api/requests/completion.rs +++ b/crates/ty_server/src/server/api/requests/completion.rs @@ -8,7 +8,7 @@ use lsp_types::{ }; use ruff_source_file::OneIndexed; use ruff_text_size::Ranged; -use ty_ide::{CompletionKind, CompletionSettings, completion}; +use ty_ide::{CompletionKind, completion}; use ty_project::ProjectDatabase; use crate::document::{PositionExt, ToRangeExt}; @@ -56,10 +56,8 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler { ) else { return Ok(None); }; - let settings = CompletionSettings { - auto_import: snapshot.global_settings().is_auto_import_enabled(), - }; - let completions = completion(db, &settings, file, offset); + let settings = snapshot.workspace_settings().completions(); + let completions = completion(db, settings, file, offset); if completions.is_empty() { return Ok(None); } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index 982dccd484..b202270aae 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use ty_combine::Combine; -use ty_ide::InlayHintSettings; +use ty_ide::{CompletionSettings, InlayHintSettings}; use ty_project::metadata::Options as TyOptions; use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::metadata::value::{RangedValue, RelativePathBuf}; @@ -123,8 +123,11 @@ impl ClientOptions { } #[must_use] - pub fn with_experimental_auto_import(mut self, enabled: bool) -> Self { - self.global.experimental.get_or_insert_default().auto_import = Some(enabled); + pub fn with_auto_import(mut self, enabled: bool) -> Self { + self.workspace + .completions + .get_or_insert_default() + .auto_import = Some(enabled); self } @@ -155,7 +158,6 @@ impl GlobalOptions { .experimental .map(|experimental| ExperimentalSettings { rename: experimental.rename.unwrap_or(false), - auto_import: experimental.auto_import.unwrap_or(false), }) .unwrap_or_default(); @@ -178,6 +180,9 @@ pub(crate) struct WorkspaceOptions { /// Options to configure inlay hints. inlay_hints: Option, + /// Options to configure completions. + completions: Option, + /// Information about the currently active Python environment in the VS Code Python extension. /// /// This is relevant only for VS Code and is populated by the ty VS Code extension. @@ -235,6 +240,10 @@ impl WorkspaceOptions { .inlay_hints .map(InlayHintOptions::into_settings) .unwrap_or_default(), + completions: self + .completions + .map(CompletionOptions::into_settings) + .unwrap_or_default(), overrides, } } @@ -256,6 +265,26 @@ impl InlayHintOptions { } } +#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +struct CompletionOptions { + auto_import: Option, +} + +impl CompletionOptions { + // N.B. It's important for the defaults here to + // match the defaults for `CompletionSettings`. + // This is because `WorkspaceSettings::default()` + // uses `CompletionSettings::default()`. But + // `WorkspaceOptions::default().into_settings()` will use this + // definition. + fn into_settings(self) -> CompletionSettings { + CompletionSettings { + auto_import: self.auto_import.unwrap_or(true), + } + } +} + /// Diagnostic mode for the language server. #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -300,12 +329,6 @@ impl Combine for DiagnosticMode { pub(crate) struct Experimental { /// Whether to enable the experimental symbol rename feature. pub(crate) rename: Option, - /// Whether to enable the experimental "auto-import" feature. - /// - /// At time of writing (2025-08-29), this feature is still - /// under active development. It may not work right or may be - /// incomplete. - pub(crate) auto_import: Option, } #[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 7b3e95f3ed..5e73df0102 100644 --- a/crates/ty_server/src/session/settings.rs +++ b/crates/ty_server/src/session/settings.rs @@ -1,6 +1,6 @@ use super::options::DiagnosticMode; -use ty_ide::InlayHintSettings; +use ty_ide::{CompletionSettings, InlayHintSettings}; use ty_project::metadata::options::ProjectOptionsOverrides; /// Resolved client settings that are shared across all workspaces. @@ -14,10 +14,6 @@ impl GlobalSettings { pub(crate) fn is_rename_enabled(&self) -> bool { self.experimental.rename } - - pub(crate) fn is_auto_import_enabled(&self) -> bool { - self.experimental.auto_import - } } impl GlobalSettings { @@ -29,7 +25,6 @@ impl GlobalSettings { #[derive(Clone, Default, Debug, PartialEq)] pub(crate) struct ExperimentalSettings { pub(super) rename: bool, - pub(super) auto_import: bool, } /// Resolved client settings for a specific workspace. @@ -40,6 +35,7 @@ pub(crate) struct ExperimentalSettings { pub(crate) struct WorkspaceSettings { pub(super) disable_language_services: bool, pub(super) inlay_hints: InlayHintSettings, + pub(super) completions: CompletionSettings, pub(super) overrides: Option, } @@ -55,4 +51,8 @@ impl WorkspaceSettings { pub(crate) fn inlay_hints(&self) -> &InlayHintSettings { &self.inlay_hints } + + pub(crate) fn completions(&self) -> &CompletionSettings { + &self.completions + } } diff --git a/crates/ty_server/tests/e2e/completions.rs b/crates/ty_server/tests/e2e/completions.rs new file mode 100644 index 0000000000..80576100b6 --- /dev/null +++ b/crates/ty_server/tests/e2e/completions.rs @@ -0,0 +1,82 @@ +use anyhow::Result; +use lsp_types::{Position, notification::PublishDiagnostics}; +use ruff_db::system::SystemPath; +use ty_server::ClientOptions; + +use crate::TestServerBuilder; + +/// Tests that auto-import is enabled by default. +#[test] +fn default_auto_import() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +walktr +"; + + let mut server = TestServerBuilder::new()? + .with_initialization_options(ClientOptions::default()) + .with_workspace(workspace_root, None)? + .with_file(foo, foo_content)? + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + let _ = server.await_notification::(); + + let hints = server.completion_request(&server.file_uri(foo), Position::new(0, 6)); + + insta::assert_json_snapshot!(hints, @r#" + [ + { + "label": "walktree (import inspect)", + "kind": 3, + "sortText": "0", + "insertText": "walktree", + "additionalTextEdits": [ + { + "range": { + "start": { + "line": 0, + "character": 0 + }, + "end": { + "line": 0, + "character": 0 + } + }, + "newText": "from inspect import walktree\n" + } + ] + } + ] + "#); + + Ok(()) +} + +/// Tests that disabling auto-import works. +#[test] +fn disable_auto_import() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +walktr +"; + + let mut server = TestServerBuilder::new()? + .with_initialization_options(ClientOptions::default().with_auto_import(false)) + .with_workspace(workspace_root, None)? + .with_file(foo, foo_content)? + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + let _ = server.await_notification::(); + + let hints = server.completion_request(&server.file_uri(foo), Position::new(0, 6)); + + insta::assert_json_snapshot!(hints, @"[]"); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index 94557172d0..5ef434cef6 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -29,6 +29,7 @@ mod code_actions; mod commands; +mod completions; mod initialize; mod inlay_hints; mod notebook; @@ -51,11 +52,12 @@ use lsp_types::notification::{ Initialized, Notification, }; use lsp_types::request::{ - DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, Request, Shutdown, - WorkspaceConfiguration, WorkspaceDiagnosticRequest, + Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, Request, + Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest, }; use lsp_types::{ - ClientCapabilities, ConfigurationParams, DiagnosticClientCapabilities, + ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse, + CompletionTriggerKind, ConfigurationParams, DiagnosticClientCapabilities, DidChangeTextDocumentParams, DidChangeWatchedFilesClientCapabilities, DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams, @@ -872,6 +874,31 @@ impl TestServer { let id = self.send_request::(params); self.await_response::(&id) } + + /// Sends a `textDocument/completion` request for the document at the given URL and position. + pub(crate) fn completion_request( + &mut self, + uri: &Url, + position: Position, + ) -> Vec { + let completions_id = self.send_request::(CompletionParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: uri.clone() }, + position, + }, + work_done_progress_params: lsp_types::WorkDoneProgressParams::default(), + partial_result_params: lsp_types::PartialResultParams::default(), + context: Some(lsp_types::CompletionContext { + trigger_kind: CompletionTriggerKind::TRIGGER_FOR_INCOMPLETE_COMPLETIONS, + trigger_character: None, + }), + }); + match self.await_response::(&completions_id) { + Some(CompletionResponse::Array(array)) => array, + Some(CompletionResponse::List(lsp_types::CompletionList { items, .. })) => items, + None => vec![], + } + } } impl fmt::Debug for TestServer { diff --git a/crates/ty_server/tests/e2e/notebook.rs b/crates/ty_server/tests/e2e/notebook.rs index 7c7847dfcb..c8d5c738db 100644 --- a/crates/ty_server/tests/e2e/notebook.rs +++ b/crates/ty_server/tests/e2e/notebook.rs @@ -1,5 +1,5 @@ use insta::assert_json_snapshot; -use lsp_types::{CompletionResponse, CompletionTriggerKind, NotebookCellKind, Position, Range}; +use lsp_types::{NotebookCellKind, Position, Range}; use ruff_db::system::SystemPath; use ty_server::ClientOptions; @@ -285,7 +285,7 @@ fn auto_import() -> anyhow::Result<()> { let mut server = TestServerBuilder::new()? .with_workspace( SystemPath::new("src"), - Some(ClientOptions::default().with_experimental_auto_import(true)), + Some(ClientOptions::default().with_auto_import(true)), )? .build() .wait_until_workspaces_are_initialized(); @@ -325,7 +325,7 @@ fn auto_import_same_cell() -> anyhow::Result<()> { let mut server = TestServerBuilder::new()? .with_workspace( SystemPath::new("src"), - Some(ClientOptions::default().with_experimental_auto_import(true)), + Some(ClientOptions::default().with_auto_import(true)), )? .build() .wait_until_workspaces_are_initialized(); @@ -360,7 +360,7 @@ fn auto_import_from_future() -> anyhow::Result<()> { let mut server = TestServerBuilder::new()? .with_workspace( SystemPath::new("src"), - Some(ClientOptions::default().with_experimental_auto_import(true)), + Some(ClientOptions::default().with_auto_import(true)), )? .build() .wait_until_workspaces_are_initialized(); @@ -397,7 +397,7 @@ fn auto_import_docstring() -> anyhow::Result<()> { let mut server = TestServerBuilder::new()? .with_workspace( SystemPath::new("src"), - Some(ClientOptions::default().with_experimental_auto_import(true)), + Some(ClientOptions::default().with_auto_import(true)), )? .build() .wait_until_workspaces_are_initialized(); @@ -521,31 +521,10 @@ fn literal_completions( cell: &lsp_types::Url, position: Position, ) -> Vec { - let completions_id = - server.send_request::(lsp_types::CompletionParams { - text_document_position: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri: cell.clone() }, - position, - }, - work_done_progress_params: lsp_types::WorkDoneProgressParams::default(), - partial_result_params: lsp_types::PartialResultParams::default(), - context: Some(lsp_types::CompletionContext { - trigger_kind: CompletionTriggerKind::TRIGGER_FOR_INCOMPLETE_COMPLETIONS, - trigger_character: None, - }), - }); - + let mut items = server.completion_request(cell, position); // There are a ton of imports we don't care about in here... // The import bit is that an edit is always restricted to the current cell. That means, // we can't add `Literal` to the `from typing import TYPE_CHECKING` import in cell 1 - let completions = server.await_response::(&completions_id); - let mut items = match completions { - Some(CompletionResponse::Array(array)) => array, - Some(CompletionResponse::List(lsp_types::CompletionList { items, .. })) => items, - None => return vec![], - }; - items.retain(|item| item.label.starts_with("Litera")); - items } 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 0daa6c768a..762b91f71c 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 @@ -10,7 +10,6 @@ Global settings: GlobalSettings { diagnostic_mode: OpenFilesOnly, experimental: ExperimentalSettings { rename: false, - auto_import: false, }, } Open text documents: 0 @@ -22,6 +21,9 @@ Settings: WorkspaceSettings { variable_types: true, call_argument_names: true, }, + completions: CompletionSettings { + auto_import: true, + }, overrides: None, }