From 2f735947bfdb80ab1cf75653aa72a51bb5668145 Mon Sep 17 00:00:00 2001 From: psteinroe <philipp@steinroetter.com> Date: Mon, 31 Mar 2025 10:13:14 +0200 Subject: [PATCH 1/3] try to add repro --- crates/pgt_lsp/tests/server.rs | 93 +++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 8 deletions(-) diff --git a/crates/pgt_lsp/tests/server.rs b/crates/pgt_lsp/tests/server.rs index fd5e15d0..c2726fa6 100644 --- a/crates/pgt_lsp/tests/server.rs +++ b/crates/pgt_lsp/tests/server.rs @@ -30,7 +30,6 @@ use tower_lsp::jsonrpc; use tower_lsp::jsonrpc::Response; use tower_lsp::lsp_types as lsp; use tower_lsp::lsp_types::CodeActionContext; -use tower_lsp::lsp_types::CodeActionOrCommand; use tower_lsp::lsp_types::CodeActionParams; use tower_lsp::lsp_types::CodeActionResponse; use tower_lsp::lsp_types::CompletionParams; @@ -616,16 +615,15 @@ async fn test_execute_statement() -> Result<()> { result.unwrap().exists.unwrap() }; - assert_eq!( - users_tbl_exists().await, - false, + assert!( + !(users_tbl_exists().await), "The user table shouldn't exist at this point." ); let doc_content = r#" create table users ( - id serial primary key, - name text, + id serial primary key, + name text, email text ); "#; @@ -686,9 +684,8 @@ async fn test_execute_statement() -> Result<()> { ) .await?; - assert_eq!( + assert!( users_tbl_exists().await, - true, "Users table did not exists even though it should've been created by the workspace/executeStatement command." ); @@ -697,3 +694,83 @@ async fn test_execute_statement() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn test_issue_281() -> 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!("postgrestools.jsonc").to_file_path().unwrap(), + serde_json::to_string_pretty(&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("select a").await?; + + server + .change_document( + 3, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 0, + character: 7, + }, + end: Position { + line: 0, + character: 7, + }, + }), + range_length: Some(0), + text: "ы".to_string(), + }], + ) + .await?; + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} From ffb05090e13650a4680809d221e33760e48c9976 Mon Sep 17 00:00:00 2001 From: psteinroe <philipp@steinroetter.com> Date: Tue, 1 Apr 2025 13:31:28 +0200 Subject: [PATCH 2/3] fix: convert to byte indices --- crates/pgt_lsp/tests/server.rs | 46 ++++++++++--------- .../src/workspace/server/change.rs | 34 ++++++++++---- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/crates/pgt_lsp/tests/server.rs b/crates/pgt_lsp/tests/server.rs index c2726fa6..afcbdd5b 100644 --- a/crates/pgt_lsp/tests/server.rs +++ b/crates/pgt_lsp/tests/server.rs @@ -747,27 +747,31 @@ async fn test_issue_281() -> Result<()> { server.load_configuration().await?; - server.open_document("select a").await?; - - server - .change_document( - 3, - vec![TextDocumentContentChangeEvent { - range: Some(Range { - start: Position { - line: 0, - character: 7, - }, - end: Position { - line: 0, - character: 7, - }, - }), - range_length: Some(0), - text: "ы".to_string(), - }], - ) - .await?; + server.open_document("\n------------- Meta -------------\n\n-- name: GetValueFromMetaKVStore :one\nSELECT value FROM meta_kv\nWHERE key = $1;\n\n-- name: SetValueToMetaKVStore :exec\nINSERT INTO meta_kv (key, value)\nVALUES ($1, $2)\nON CONFLICT (key) DO UPDATE\nSET value = excluded.value;\n\n\nasdsadsad\n\nыывфыв khgk\nasdыdsf\ndsdsjdfnfmdsвтьвыаыdsfsmndf,m\nы\n").await?; + + let chars = ["s", "n", ",", "d", "f", "j", "s", "d", "f", "в"]; + + for (i, c) in chars.iter().enumerate() { + server + .change_document( + i as i32 + 4, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 20, + character: i as u32, + }, + end: Position { + line: 20, + character: i as u32, + }, + }), + range_length: Some(0), + text: c.to_string(), + }], + ) + .await?; + } server.shutdown().await?; reader.abort(); diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index 92422a8a..f020d9cd 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -249,10 +249,19 @@ impl Document { // if within a statement, we can modify it if the change results in also a single statement if affected_indices.len() == 1 { - let changed_content = new_content - .as_str() - .get(usize::from(affected_range.start())..usize::from(affected_range.end())) - .unwrap(); + let start_byte = new_content + .char_indices() + .nth(usize::from(affected_range.start())) + .map(|(i, _)| i) + .unwrap_or(new_content.len()); + + let end_byte = new_content + .char_indices() + .nth(usize::from(affected_range.end())) + .map(|(i, _)| i) + .unwrap_or(new_content.len()); + + let changed_content = &new_content[start_byte..end_byte]; let (new_ranges, diags) = document::split_with_diagnostics(changed_content, Some(affected_range.start())); @@ -305,10 +314,19 @@ impl Document { } // in any other case, parse the full affected range - let changed_content = new_content - .as_str() - .get(usize::from(full_affected_range.start())..usize::from(full_affected_range.end())) - .unwrap(); + let start_byte = new_content + .char_indices() + .nth(usize::from(full_affected_range.start())) + .map(|(i, _)| i) + .unwrap_or(new_content.len()); + + let end_byte = new_content + .char_indices() + .nth(usize::from(full_affected_range.end())) + .map(|(i, _)| i) + .unwrap_or(new_content.len()); + + let changed_content = &new_content[start_byte..end_byte]; let (new_ranges, diags) = document::split_with_diagnostics(changed_content, Some(full_affected_range.start())); From 12dc7daf655ee502298a9b569de72707427a74e8 Mon Sep 17 00:00:00 2001 From: psteinroe <philipp@steinroetter.com> Date: Tue, 1 Apr 2025 19:33:44 +0200 Subject: [PATCH 3/3] refactor: add helper --- .../src/workspace/server/change.rs | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index f020d9cd..226a0ffd 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -249,19 +249,7 @@ impl Document { // if within a statement, we can modify it if the change results in also a single statement if affected_indices.len() == 1 { - let start_byte = new_content - .char_indices() - .nth(usize::from(affected_range.start())) - .map(|(i, _)| i) - .unwrap_or(new_content.len()); - - let end_byte = new_content - .char_indices() - .nth(usize::from(affected_range.end())) - .map(|(i, _)| i) - .unwrap_or(new_content.len()); - - let changed_content = &new_content[start_byte..end_byte]; + let changed_content = get_affected(&new_content, affected_range); let (new_ranges, diags) = document::split_with_diagnostics(changed_content, Some(affected_range.start())); @@ -314,19 +302,7 @@ impl Document { } // in any other case, parse the full affected range - let start_byte = new_content - .char_indices() - .nth(usize::from(full_affected_range.start())) - .map(|(i, _)| i) - .unwrap_or(new_content.len()); - - let end_byte = new_content - .char_indices() - .nth(usize::from(full_affected_range.end())) - .map(|(i, _)| i) - .unwrap_or(new_content.len()); - - let changed_content = &new_content[start_byte..end_byte]; + let changed_content = get_affected(&new_content, full_affected_range); let (new_ranges, diags) = document::split_with_diagnostics(changed_content, Some(full_affected_range.start())); @@ -430,6 +406,22 @@ impl ChangeParams { } } +fn get_affected(content: &str, range: TextRange) -> &str { + let start_byte = content + .char_indices() + .nth(usize::from(range.start())) + .map(|(i, _)| i) + .unwrap_or(content.len()); + + let end_byte = content + .char_indices() + .nth(usize::from(range.end())) + .map(|(i, _)| i) + .unwrap_or(content.len()); + + &content[start_byte..end_byte] +} + #[cfg(test)] mod tests { use super::*;