diff --git a/crates/pglt_completions/src/providers/columns.rs b/crates/pglt_completions/src/providers/columns.rs index 3749c67b..21aff8b6 100644 --- a/crates/pglt_completions/src/providers/columns.rs +++ b/crates/pglt_completions/src/providers/columns.rs @@ -72,13 +72,13 @@ mod tests { message: "correctly handles nested queries", query: format!( r#" - select + select * from ( select id, na{} from private.audio_books ) as subquery - join public.users u + join public.users u on u.id = subquery.id; "#, CURSOR_POS diff --git a/crates/pglt_completions/src/providers/functions.rs b/crates/pglt_completions/src/providers/functions.rs index 4ac571a1..2fbcaacf 100644 --- a/crates/pglt_completions/src/providers/functions.rs +++ b/crates/pglt_completions/src/providers/functions.rs @@ -29,7 +29,7 @@ mod tests { #[tokio::test] async fn completes_fn() { let setup = r#" - create or replace function cool() + create or replace function cool() returns trigger language plpgsql security invoker @@ -62,7 +62,7 @@ mod tests { name text ); - create or replace function cool() + create or replace function cool() returns trigger language plpgsql security invoker @@ -96,7 +96,7 @@ mod tests { name text ); - create or replace function cool() + create or replace function cool() returns trigger language plpgsql security invoker @@ -130,7 +130,7 @@ mod tests { name text ); - create or replace function cool() + create or replace function cool() returns trigger language plpgsql security invoker diff --git a/crates/pglt_completions/src/providers/tables.rs b/crates/pglt_completions/src/providers/tables.rs index d7b97e57..c2fa753d 100644 --- a/crates/pglt_completions/src/providers/tables.rs +++ b/crates/pglt_completions/src/providers/tables.rs @@ -148,7 +148,7 @@ mod tests { name text ); - create or replace function cool() + create or replace function cool() returns trigger language plpgsql security invoker diff --git a/crates/pglt_lsp/tests/server.rs b/crates/pglt_lsp/tests/server.rs index a3632678..6a4a7654 100644 --- a/crates/pglt_lsp/tests/server.rs +++ b/crates/pglt_lsp/tests/server.rs @@ -28,6 +28,13 @@ use tower_lsp::LspService; use tower_lsp::jsonrpc; use tower_lsp::jsonrpc::Response; use tower_lsp::lsp_types as lsp; +use tower_lsp::lsp_types::CompletionParams; +use tower_lsp::lsp_types::CompletionResponse; +use tower_lsp::lsp_types::PartialResultParams; +use tower_lsp::lsp_types::Position; +use tower_lsp::lsp_types::Range; +use tower_lsp::lsp_types::TextDocumentPositionParams; +use tower_lsp::lsp_types::WorkDoneProgressParams; use tower_lsp::lsp_types::{ ClientCapabilities, DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, InitializeResult, InitializedParams, @@ -248,6 +255,18 @@ impl Server { .await } + async fn get_completion( + &mut self, + params: tower_lsp::lsp_types::CompletionParams, + ) -> Result> { + self.request::( + "textDocument/completion", + "_get_completion", + params, + ) + .await + } + /// Basic implementation of the `pglt/shutdown` request for tests async fn pglt_shutdown(&mut self) -> Result<()> { self.request::<_, ()>("pglt/shutdown", "_pglt_shutdown", ()) @@ -431,3 +450,104 @@ async fn server_shutdown() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn test_completions() -> Result<()> { + let factory = ServerFactory::default(); + let mut fs = MemoryFileSystem::default(); + let test_db = get_new_test_db().await; + + let setup = r#" + create table public.users ( + id serial primary key, + name varchar(255) not null + ); + "#; + + test_db + .execute(setup) + .await + .expect("Failed to setup test database"); + + let mut conf = PartialConfiguration::init(); + conf.merge_with(PartialConfiguration { + db: Some(PartialDatabaseConfiguration { + database: Some( + test_db + .connect_options() + .get_database() + .unwrap() + .to_string(), + ), + ..Default::default() + }), + ..Default::default() + }); + fs.insert( + url!("pglt.toml").to_file_path().unwrap(), + toml::to_string(&conf).unwrap(), + ); + + let (service, client) = factory + .create_with_fs(None, DynRef::Owned(Box::new(fs))) + .into_inner(); + + let (stream, sink) = client.split(); + let mut server = Server::new(service); + + let (sender, _) = channel(CHANNEL_BUFFER_SIZE); + let reader = tokio::spawn(client_handler(stream, sink, sender)); + + server.initialize().await?; + server.initialized().await?; + + server.load_configuration().await?; + + server + .open_document("alter table appointment alter column end_time drop not null;\n") + .await?; + + server + .change_document( + 3, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 0, + character: 24, + }, + end: Position { + line: 0, + character: 24, + }, + }), + range_length: Some(0), + text: " ".to_string(), + }], + ) + .await?; + + let res = server + .get_completion(CompletionParams { + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + context: None, + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: url!("document.sql"), + }, + position: Position { + line: 0, + character: 25, + }, + }, + }) + .await?; + + assert!(res.is_some()); + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} diff --git a/crates/pglt_workspace/src/workspace/server.rs b/crates/pglt_workspace/src/workspace/server.rs index 4d3d7810..4b4b9a6e 100644 --- a/crates/pglt_workspace/src/workspace/server.rs +++ b/crates/pglt_workspace/src/workspace/server.rs @@ -52,9 +52,6 @@ pub(super) struct WorkspaceServer { tree_sitter: TreeSitterStore, pg_query: PgQueryStore, - /// Stores the statements that have changed since the last analysis - changed_stmts: DashSet, - connection: RwLock, } @@ -78,7 +75,6 @@ impl WorkspaceServer { documents: DashMap::default(), tree_sitter: TreeSitterStore::new(), pg_query: PgQueryStore::new(), - changed_stmts: DashSet::default(), schema_cache: SchemaCacheManager::default(), connection: RwLock::default(), } @@ -224,23 +220,16 @@ impl Workspace for WorkspaceServer { tracing::debug!("Adding statement: {:?}", added); self.tree_sitter.add_statement(&added.stmt, &added.text); self.pg_query.add_statement(&added.stmt, &added.text); - - self.changed_stmts.insert(added.stmt.clone()); } StatementChange::Deleted(s) => { tracing::debug!("Deleting statement: {:?}", s); self.tree_sitter.remove_statement(s); self.pg_query.remove_statement(s); - - self.changed_stmts.remove(s); } StatementChange::Modified(s) => { tracing::debug!("Modifying statement: {:?}", s); self.tree_sitter.modify_statement(s); self.pg_query.modify_statement(s); - - self.changed_stmts.remove(&s.old_stmt); - self.changed_stmts.insert(s.new_stmt.clone()); } } } diff --git a/crates/pglt_workspace/src/workspace/server/change.rs b/crates/pglt_workspace/src/workspace/server/change.rs index c4a1e9ad..174c75b9 100644 --- a/crates/pglt_workspace/src/workspace/server/change.rs +++ b/crates/pglt_workspace/src/workspace/server/change.rs @@ -278,28 +278,25 @@ impl Document { let new_id = self.id_generator.next(); self.positions[affected_idx] = (new_id, new_range); - if !change.is_whitespace() { - // whitespace-only changes should not invalidate the statement - changed.push(StatementChange::Modified(ModifiedStatement { - old_stmt: Statement { - id: old_id, - path: self.path.clone(), - }, - old_stmt_text: self.content[old_range].to_string(), - - new_stmt: Statement { - id: new_id, - path: self.path.clone(), - }, - new_stmt_text: changed_content[new_ranges[0]].to_string(), - // change must be relative to the statement - change_text: change.text.clone(), - // make sure we always have a valid range >= 0 - change_range: change_range - .checked_sub(old_range.start()) - .unwrap_or(change_range.sub(change_range.start())), - })); - } + changed.push(StatementChange::Modified(ModifiedStatement { + old_stmt: Statement { + id: old_id, + path: self.path.clone(), + }, + old_stmt_text: self.content[old_range].to_string(), + + new_stmt: Statement { + id: new_id, + path: self.path.clone(), + }, + new_stmt_text: changed_content[new_ranges[0]].to_string(), + // change must be relative to the statement + change_text: change.text.clone(), + // make sure we always have a valid range >= 0 + change_range: change_range + .checked_sub(old_range.start()) + .unwrap_or(change_range.sub(change_range.start())), + })); self.content = new_content; @@ -375,10 +372,6 @@ impl Document { } impl ChangeParams { - pub fn is_whitespace(&self) -> bool { - self.text.chars().count() > 0 && self.text.chars().all(char::is_whitespace) - } - pub fn diff_size(&self) -> TextSize { match self.range { Some(range) => { @@ -664,7 +657,7 @@ mod tests { }; let changed1 = d.apply_file_change(&change1); - assert_eq!(changed1.len(), 0, "should not emit change"); + assert_eq!(changed1.len(), 1); assert_eq!( d.content, "alter table deal alter column value drop not null;\n" @@ -681,7 +674,7 @@ mod tests { }; let changed2 = d.apply_file_change(&change2); - assert_eq!(changed2.len(), 0); + assert_eq!(changed2.len(), 1); assert_eq!( d.content, "alter table deal alter column value drop not null;\n" @@ -698,7 +691,7 @@ mod tests { }; let changed3 = d.apply_file_change(&change3); - assert_eq!(changed3.len(), 0); + assert_eq!(changed3.len(), 1); assert_eq!( d.content, "alter table deal alter column value drop not null;\n" @@ -715,7 +708,7 @@ mod tests { }; let changed4 = d.apply_file_change(&change4); - assert_eq!(changed4.len(), 0); + assert_eq!(changed4.len(), 1); assert_eq!( d.content, "alter table deal alter column value drop not null;\n" @@ -741,11 +734,7 @@ mod tests { }; let changed1 = d.apply_file_change(&change1); - assert_eq!( - changed1.len(), - 0, - "should not emit change if its only whitespace" - ); + assert_eq!(changed1.len(), 1); assert_eq!( d.content, "select\n *\nfrom\n test;\n\nselect \n\nalter table test\n\ndrop column id;" @@ -867,7 +856,7 @@ mod tests { let changed = d.apply_file_change(&change); - assert_eq!(changed.len(), 0); + assert_eq!(changed.len(), 1); assert_document_integrity(&d); }