[ty] Stabilize auto-import

While still under development, it's far enough along now that we think
it's worth enabling it by default. This should also help give us
feedback for how it behaves.

This PR adds a "completion settings" grouping similar to inlay hints. We
only have an auto-import setting there now, but I expect we'll add more
options to configure completion behavior in the future.

Closes astral-sh/ty#1765
This commit is contained in:
Andrew Gallant 2025-12-08 12:30:32 -05:00 committed by Andrew Gallant
parent a0b18bc153
commit 426125f5c0
8 changed files with 183 additions and 54 deletions

View File

@ -380,11 +380,22 @@ pub enum CompletionKind {
TypeParameter, TypeParameter,
} }
#[derive(Clone, Debug, Default)] #[derive(Clone, Debug)]
pub struct CompletionSettings { pub struct CompletionSettings {
pub auto_import: bool, 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>( pub fn completion<'db>(
db: &'db dyn Db, db: &'db dyn Db,
settings: &CompletionSettings, settings: &CompletionSettings,
@ -6610,7 +6621,14 @@ collabc<CURSOR>
fn completion_test_builder(&self) -> CompletionTestBuilder { fn completion_test_builder(&self) -> CompletionTestBuilder {
CompletionTestBuilder { CompletionTestBuilder {
cursor_test: self.build(), 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_builtins: false,
skip_keywords: false, skip_keywords: false,
type_signatures: false, type_signatures: false,

View File

@ -8,7 +8,7 @@ use lsp_types::{
}; };
use ruff_source_file::OneIndexed; use ruff_source_file::OneIndexed;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use ty_ide::{CompletionKind, CompletionSettings, completion}; use ty_ide::{CompletionKind, completion};
use ty_project::ProjectDatabase; use ty_project::ProjectDatabase;
use crate::document::{PositionExt, ToRangeExt}; use crate::document::{PositionExt, ToRangeExt};
@ -56,10 +56,8 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler {
) else { ) else {
return Ok(None); return Ok(None);
}; };
let settings = CompletionSettings { let settings = snapshot.workspace_settings().completions();
auto_import: snapshot.global_settings().is_auto_import_enabled(), let completions = completion(db, settings, file, offset);
};
let completions = completion(db, &settings, file, offset);
if completions.is_empty() { if completions.is_empty() {
return Ok(None); return Ok(None);
} }

View File

@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
use serde_json::Value; use serde_json::Value;
use ty_combine::Combine; 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 as TyOptions;
use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::metadata::options::ProjectOptionsOverrides;
use ty_project::metadata::value::{RangedValue, RelativePathBuf}; use ty_project::metadata::value::{RangedValue, RelativePathBuf};
@ -123,8 +123,11 @@ impl ClientOptions {
} }
#[must_use] #[must_use]
pub fn with_experimental_auto_import(mut self, enabled: bool) -> Self { pub fn with_auto_import(mut self, enabled: bool) -> Self {
self.global.experimental.get_or_insert_default().auto_import = Some(enabled); self.workspace
.completions
.get_or_insert_default()
.auto_import = Some(enabled);
self self
} }
@ -155,7 +158,6 @@ impl GlobalOptions {
.experimental .experimental
.map(|experimental| ExperimentalSettings { .map(|experimental| ExperimentalSettings {
rename: experimental.rename.unwrap_or(false), rename: experimental.rename.unwrap_or(false),
auto_import: experimental.auto_import.unwrap_or(false),
}) })
.unwrap_or_default(); .unwrap_or_default();
@ -178,6 +180,9 @@ pub(crate) struct WorkspaceOptions {
/// Options to configure inlay hints. /// Options to configure inlay hints.
inlay_hints: Option<InlayHintOptions>, inlay_hints: Option<InlayHintOptions>,
/// Options to configure completions.
completions: Option<CompletionOptions>,
/// Information about the currently active Python environment in the VS Code Python extension. /// 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. /// This is relevant only for VS Code and is populated by the ty VS Code extension.
@ -235,6 +240,10 @@ impl WorkspaceOptions {
.inlay_hints .inlay_hints
.map(InlayHintOptions::into_settings) .map(InlayHintOptions::into_settings)
.unwrap_or_default(), .unwrap_or_default(),
completions: self
.completions
.map(CompletionOptions::into_settings)
.unwrap_or_default(),
overrides, overrides,
} }
} }
@ -256,6 +265,26 @@ impl InlayHintOptions {
} }
} }
#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)]
#[serde(rename_all = "camelCase")]
struct CompletionOptions {
auto_import: Option<bool>,
}
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. /// Diagnostic mode for the language server.
#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
@ -300,12 +329,6 @@ impl Combine for DiagnosticMode {
pub(crate) struct Experimental { pub(crate) struct Experimental {
/// Whether to enable the experimental symbol rename feature. /// Whether to enable the experimental symbol rename feature.
pub(crate) rename: Option<bool>, pub(crate) rename: Option<bool>,
/// 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<bool>,
} }
#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[derive(Clone, Debug, Serialize, Deserialize, Default)]

View File

@ -1,6 +1,6 @@
use super::options::DiagnosticMode; use super::options::DiagnosticMode;
use ty_ide::InlayHintSettings; use ty_ide::{CompletionSettings, InlayHintSettings};
use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::metadata::options::ProjectOptionsOverrides;
/// Resolved client settings that are shared across all workspaces. /// Resolved client settings that are shared across all workspaces.
@ -14,10 +14,6 @@ impl GlobalSettings {
pub(crate) fn is_rename_enabled(&self) -> bool { pub(crate) fn is_rename_enabled(&self) -> bool {
self.experimental.rename self.experimental.rename
} }
pub(crate) fn is_auto_import_enabled(&self) -> bool {
self.experimental.auto_import
}
} }
impl GlobalSettings { impl GlobalSettings {
@ -29,7 +25,6 @@ impl GlobalSettings {
#[derive(Clone, Default, Debug, PartialEq)] #[derive(Clone, Default, Debug, PartialEq)]
pub(crate) struct ExperimentalSettings { pub(crate) struct ExperimentalSettings {
pub(super) rename: bool, pub(super) rename: bool,
pub(super) auto_import: bool,
} }
/// Resolved client settings for a specific workspace. /// Resolved client settings for a specific workspace.
@ -40,6 +35,7 @@ pub(crate) struct ExperimentalSettings {
pub(crate) struct WorkspaceSettings { pub(crate) struct WorkspaceSettings {
pub(super) disable_language_services: bool, pub(super) disable_language_services: bool,
pub(super) inlay_hints: InlayHintSettings, pub(super) inlay_hints: InlayHintSettings,
pub(super) completions: CompletionSettings,
pub(super) overrides: Option<ProjectOptionsOverrides>, pub(super) overrides: Option<ProjectOptionsOverrides>,
} }
@ -55,4 +51,8 @@ impl WorkspaceSettings {
pub(crate) fn inlay_hints(&self) -> &InlayHintSettings { pub(crate) fn inlay_hints(&self) -> &InlayHintSettings {
&self.inlay_hints &self.inlay_hints
} }
pub(crate) fn completions(&self) -> &CompletionSettings {
&self.completions
}
} }

View File

@ -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::<PublishDiagnostics>();
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::<PublishDiagnostics>();
let hints = server.completion_request(&server.file_uri(foo), Position::new(0, 6));
insta::assert_json_snapshot!(hints, @"[]");
Ok(())
}

View File

@ -29,6 +29,7 @@
mod code_actions; mod code_actions;
mod commands; mod commands;
mod completions;
mod initialize; mod initialize;
mod inlay_hints; mod inlay_hints;
mod notebook; mod notebook;
@ -51,11 +52,12 @@ use lsp_types::notification::{
Initialized, Notification, Initialized, Notification,
}; };
use lsp_types::request::{ use lsp_types::request::{
DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, Request, Shutdown, Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, Request,
WorkspaceConfiguration, WorkspaceDiagnosticRequest, Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest,
}; };
use lsp_types::{ use lsp_types::{
ClientCapabilities, ConfigurationParams, DiagnosticClientCapabilities, ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse,
CompletionTriggerKind, ConfigurationParams, DiagnosticClientCapabilities,
DidChangeTextDocumentParams, DidChangeWatchedFilesClientCapabilities, DidChangeTextDocumentParams, DidChangeWatchedFilesClientCapabilities,
DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams,
DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams, DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams,
@ -872,6 +874,31 @@ impl TestServer {
let id = self.send_request::<InlayHintRequest>(params); let id = self.send_request::<InlayHintRequest>(params);
self.await_response::<InlayHintRequest>(&id) self.await_response::<InlayHintRequest>(&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<CompletionItem> {
let completions_id = self.send_request::<Completion>(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::<lsp_types::request::Completion>(&completions_id) {
Some(CompletionResponse::Array(array)) => array,
Some(CompletionResponse::List(lsp_types::CompletionList { items, .. })) => items,
None => vec![],
}
}
} }
impl fmt::Debug for TestServer { impl fmt::Debug for TestServer {

View File

@ -1,5 +1,5 @@
use insta::assert_json_snapshot; 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 ruff_db::system::SystemPath;
use ty_server::ClientOptions; use ty_server::ClientOptions;
@ -285,7 +285,7 @@ fn auto_import() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()? let mut server = TestServerBuilder::new()?
.with_workspace( .with_workspace(
SystemPath::new("src"), SystemPath::new("src"),
Some(ClientOptions::default().with_experimental_auto_import(true)), Some(ClientOptions::default().with_auto_import(true)),
)? )?
.build() .build()
.wait_until_workspaces_are_initialized(); .wait_until_workspaces_are_initialized();
@ -325,7 +325,7 @@ fn auto_import_same_cell() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()? let mut server = TestServerBuilder::new()?
.with_workspace( .with_workspace(
SystemPath::new("src"), SystemPath::new("src"),
Some(ClientOptions::default().with_experimental_auto_import(true)), Some(ClientOptions::default().with_auto_import(true)),
)? )?
.build() .build()
.wait_until_workspaces_are_initialized(); .wait_until_workspaces_are_initialized();
@ -360,7 +360,7 @@ fn auto_import_from_future() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()? let mut server = TestServerBuilder::new()?
.with_workspace( .with_workspace(
SystemPath::new("src"), SystemPath::new("src"),
Some(ClientOptions::default().with_experimental_auto_import(true)), Some(ClientOptions::default().with_auto_import(true)),
)? )?
.build() .build()
.wait_until_workspaces_are_initialized(); .wait_until_workspaces_are_initialized();
@ -397,7 +397,7 @@ fn auto_import_docstring() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()? let mut server = TestServerBuilder::new()?
.with_workspace( .with_workspace(
SystemPath::new("src"), SystemPath::new("src"),
Some(ClientOptions::default().with_experimental_auto_import(true)), Some(ClientOptions::default().with_auto_import(true)),
)? )?
.build() .build()
.wait_until_workspaces_are_initialized(); .wait_until_workspaces_are_initialized();
@ -521,31 +521,10 @@ fn literal_completions(
cell: &lsp_types::Url, cell: &lsp_types::Url,
position: Position, position: Position,
) -> Vec<lsp_types::CompletionItem> { ) -> Vec<lsp_types::CompletionItem> {
let completions_id = let mut items = server.completion_request(cell, position);
server.send_request::<lsp_types::request::Completion>(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,
}),
});
// There are a ton of imports we don't care about in here... // 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, // 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 // we can't add `Literal` to the `from typing import TYPE_CHECKING` import in cell 1
let completions = server.await_response::<lsp_types::request::Completion>(&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.retain(|item| item.label.starts_with("Litera"));
items items
} }

View File

@ -10,7 +10,6 @@ Global settings: GlobalSettings {
diagnostic_mode: OpenFilesOnly, diagnostic_mode: OpenFilesOnly,
experimental: ExperimentalSettings { experimental: ExperimentalSettings {
rename: false, rename: false,
auto_import: false,
}, },
} }
Open text documents: 0 Open text documents: 0
@ -22,6 +21,9 @@ Settings: WorkspaceSettings {
variable_types: true, variable_types: true,
call_argument_names: true, call_argument_names: true,
}, },
completions: CompletionSettings {
auto_import: true,
},
overrides: None, overrides: None,
} }