fix: emit whitespace changes after all (#232)

* fix: emit whitespace changes after all

* fix: adapt tests

Author: psteinroe

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

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

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

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<Option<CompletionResponse>> {
+        self.request::<tower_lsp::lsp_types::CompletionParams, CompletionResponse>(
+            "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(())
+}

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<Statement>,
-
     connection: RwLock<DbConnection>,
 }
 
@@ -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());
                 }
             }
         }

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);
     }