diff --git a/Cargo.lock b/Cargo.lock index 40e1549b..78f56b56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1164,6 +1164,27 @@ dependencies = [ "syn 2.0.90", ] +[[package]] +name = "env_filter" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "186e05a59d4c50738528153b83b0b0194d3a29507dfec16eccd4b342903397d0" +dependencies = [ + "log", +] + +[[package]] +name = "env_logger" +version = "0.11.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3716d7a920fb4fac5d84e9d4bce8ceb321e9414b4409da61b07b75c1e3d0697" +dependencies = [ + "anstream", + "anstyle", + "env_filter", + "log", +] + [[package]] name = "equivalent" version = "1.0.1" @@ -2353,7 +2374,6 @@ dependencies = [ "pgt_console", "pgt_diagnostics", "pgt_query_ext", - "pgt_schema_cache", "pgt_text_size", "rustc-hash 2.1.0", "schemars", @@ -2568,6 +2588,7 @@ dependencies = [ "serde_json", "sqlx", "strum", + "test-log", "tokio", "tower", "tower-lsp", @@ -3909,6 +3930,28 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" +[[package]] +name = "test-log" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7f46083d221181166e5b6f6b1e5f1d499f3a76888826e6cb1d057554157cd0f" +dependencies = [ + "env_logger", + "test-log-macros", + "tracing-subscriber", +] + +[[package]] +name = "test-log-macros" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "888d0c3c6db53c0fdab160d2ed5e12ba745383d3e85813f2ea0f2b1475ab553f" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.90", +] + [[package]] name = "thiserror" version = "1.0.69" diff --git a/Cargo.toml b/Cargo.toml index 3fcbf7f6..aae2eddb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ strum = { version = "0.27.1", features = ["derive"] } sqlx = { version = "0.8.2", features = ["runtime-tokio", "runtime-async-std", "postgres", "json"] } syn = "1.0.109" termcolor = "1.4.1" +test-log = "0.2.17" tokio = { version = "1.40.0", features = ["full"] } tower-lsp = "0.20.0" tracing = { version = "0.1.40", default-features = false, features = ["std"] } diff --git a/crates/pgt_analyse/Cargo.toml b/crates/pgt_analyse/Cargo.toml index 4d30784c..75eb0211 100644 --- a/crates/pgt_analyse/Cargo.toml +++ b/crates/pgt_analyse/Cargo.toml @@ -13,11 +13,10 @@ version = "0.0.0" [dependencies] -pgt_console.workspace = true -pgt_diagnostics.workspace = true -pgt_query_ext.workspace = true -pgt_schema_cache.workspace = true -rustc-hash = { workspace = true } +pgt_console.workspace = true +pgt_diagnostics.workspace = true +pgt_query_ext.workspace = true +rustc-hash = { workspace = true } biome_deserialize = { workspace = true, optional = true } biome_deserialize_macros = { workspace = true, optional = true } diff --git a/crates/pgt_lsp/Cargo.toml b/crates/pgt_lsp/Cargo.toml index ff3d00dc..f2aca70a 100644 --- a/crates/pgt_lsp/Cargo.toml +++ b/crates/pgt_lsp/Cargo.toml @@ -35,6 +35,7 @@ tracing = { workspace = true, features = ["attributes"] } [dev-dependencies] pgt_test_utils = { workspace = true } sqlx = { workspace = true } +test-log = { workspace = true } tokio = { workspace = true, features = ["macros"] } tower = { version = "0.4.13", features = ["timeout"] } diff --git a/crates/pgt_lsp/tests/server.rs b/crates/pgt_lsp/tests/server.rs index fd5e15d0..7e5cc210 100644 --- a/crates/pgt_lsp/tests/server.rs +++ b/crates/pgt_lsp/tests/server.rs @@ -23,6 +23,7 @@ use sqlx::Executor; use std::any::type_name; use std::fmt::Display; use std::time::Duration; +use test_log::test; use tower::timeout::Timeout; use tower::{Service, ServiceExt}; use tower_lsp::LspService; @@ -558,6 +559,208 @@ async fn test_completions() -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_issue_271() -> 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("CREATE COLLATION ignore_accent_case (provider = icu, deterministic = false, locale = 'und-u-ks-level1');\n\n-- CREATE OR REPLACE FUNCTION\n-- add_one(integer)\n-- RETURNS\n-- integer\n-- AS\n-- 'add_one.so', 'add_one'\n-- LANGUAGE\n-- C \n-- STRICT;\n\n\nSELECT pwhash, FROM users;") + .await?; + + server + .change_document( + 3, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 13, + }, + end: Position { + line: 13, + character: 14, + }, + }), + range_length: Some(0), + text: "".to_string(), + }], + ) + .await?; + + server + .change_document( + 1, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 13, + }, + end: Position { + line: 13, + character: 13, + }, + }), + range_length: Some(0), + text: ",".to_string(), + }], + ) + .await?; + + server + .change_document( + 2, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 14, + }, + end: Position { + line: 13, + character: 14, + }, + }), + range_length: Some(0), + text: " ".to_string(), + }], + ) + .await?; + + server + .change_document( + 3, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 15, + }, + end: Position { + line: 13, + character: 15, + }, + }), + range_length: Some(0), + text: "county_name".to_string(), + }], + ) + .await?; + + server + .change_document( + 4, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 13, + }, + end: Position { + line: 13, + character: 26, + }, + }), + range_length: Some(13), + text: "".to_string(), + }], + ) + .await?; + + server + .change_document( + 5, + vec![TextDocumentContentChangeEvent { + range: Some(Range { + start: Position { + line: 13, + character: 13, + }, + end: Position { + line: 13, + character: 13, + }, + }), + range_length: Some(0), + text: ",".to_string(), + }], + ) + .await?; + + // crashes with range end index 37 out of range for slice of length 26 + 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: 13, + character: 14, + }, + }, + }) + .await?; + + assert!(res.is_some()); + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} + #[tokio::test] async fn test_execute_statement() -> Result<()> { let factory = ServerFactory::default(); diff --git a/crates/pgt_treesitter_queries/src/queries/relations.rs b/crates/pgt_treesitter_queries/src/queries/relations.rs index 7765c054..f9061ce8 100644 --- a/crates/pgt_treesitter_queries/src/queries/relations.rs +++ b/crates/pgt_treesitter_queries/src/queries/relations.rs @@ -7,7 +7,7 @@ use super::QueryTryFrom; static TS_QUERY: LazyLock = LazyLock::new(|| { static QUERY_STR: &str = r#" (relation - (object_reference + (object_reference . (identifier) @schema_or_table "."? @@ -38,7 +38,7 @@ impl RelationMatch<'_> { pub fn get_table(&self, sql: &str) -> String { self.table .utf8_text(sql.as_bytes()) - .expect("Failed to get schema from RelationMatch") + .expect("Failed to get table from RelationMatch") .to_string() } } diff --git a/crates/pgt_workspace/src/workspace/server/tree_sitter.rs b/crates/pgt_workspace/src/workspace/server/tree_sitter.rs index ad4eb0d6..09cff74c 100644 --- a/crates/pgt_workspace/src/workspace/server/tree_sitter.rs +++ b/crates/pgt_workspace/src/workspace/server/tree_sitter.rs @@ -71,7 +71,7 @@ impl TreeSitterStore { } } -// i wont pretend to know whats going on here but it seems to work +// Converts character positions and replacement text into a tree-sitter InputEdit fn edit_from_change( text: &str, start_char: usize, @@ -87,6 +87,7 @@ fn edit_from_change( let mut column_start = 0; let mut column_end = 0; + // Find the byte positions corresponding to the character positions for (idx, c) in text.char_indices() { if chars_counted == start_char { start_byte = idx; @@ -94,16 +95,7 @@ fn edit_from_change( } if chars_counted == end_char { end_byte = idx; - // Calculate column_end based on replacement_text - let replacement_lines: Vec<&str> = replacement_text.split('\n').collect(); - if replacement_lines.len() > 1 { - // If replacement text spans multiple lines, adjust line and column_end accordingly - line += replacement_lines.len() - 1; - column_end = replacement_lines.last().unwrap().chars().count(); - } else { - // Single line replacement, adjust column_end based on replacement text length - column_end = column_start + replacement_text.chars().count(); - } + column_end = chars_counted - current_line_char_start; break; // Found both start and end } if c == '\n' { @@ -113,29 +105,30 @@ fn edit_from_change( chars_counted += 1; } - // Adjust end_byte based on the byte length of the replacement text - if start_byte != end_byte { - // Ensure there's a range to replace - end_byte = start_byte + replacement_text.len(); - } else if chars_counted < text.chars().count() && end_char == chars_counted { - // For insertions at the end of text - end_byte += replacement_text.len(); + // Handle case where end_char is at the end of the text + if end_char == chars_counted && end_byte == 0 { + end_byte = text.len(); + column_end = chars_counted - current_line_char_start; } let start_point = tree_sitter::Point::new(line, column_start); - let end_point = tree_sitter::Point::new(line, column_end); + let old_end_point = tree_sitter::Point::new(line, column_end); - // Calculate the new end byte after the insertion + // Calculate the new end byte after the edit let new_end_byte = start_byte + replacement_text.len(); // Calculate the new end position - let new_lines = replacement_text.matches('\n').count(); // Count how many new lines are in the inserted text - let last_line_length = replacement_text - .lines() - .last() - .unwrap_or("") - .chars() - .count(); // Length of the last line in the insertion + let new_lines = replacement_text.matches('\n').count(); + let last_line_length = if new_lines > 0 { + replacement_text + .split('\n') + .next_back() + .unwrap_or("") + .chars() + .count() + } else { + replacement_text.chars().count() + }; let new_end_position = if new_lines > 0 { // If there are new lines, the row is offset by the number of new lines, and the column is the length of the last line @@ -150,7 +143,7 @@ fn edit_from_change( old_end_byte: end_byte, new_end_byte, start_position: start_point, - old_end_position: end_point, + old_end_position: old_end_point, new_end_position, } }