Skip to content

Commit 440ccc7

Browse files
authored
fix: emit whitespace changes after all (#232)
* fix: emit whitespace changes after all * fix: adapt tests
1 parent 126fa27 commit 440ccc7

File tree

6 files changed

+152
-54
lines changed

6 files changed

+152
-54
lines changed

crates/pglt_completions/src/providers/columns.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ mod tests {
7272
message: "correctly handles nested queries",
7373
query: format!(
7474
r#"
75-
select
75+
select
7676
*
7777
from (
7878
select id, na{}
7979
from private.audio_books
8080
) as subquery
81-
join public.users u
81+
join public.users u
8282
on u.id = subquery.id;
8383
"#,
8484
CURSOR_POS

crates/pglt_completions/src/providers/functions.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ mod tests {
2929
#[tokio::test]
3030
async fn completes_fn() {
3131
let setup = r#"
32-
create or replace function cool()
32+
create or replace function cool()
3333
returns trigger
3434
language plpgsql
3535
security invoker
@@ -62,7 +62,7 @@ mod tests {
6262
name text
6363
);
6464
65-
create or replace function cool()
65+
create or replace function cool()
6666
returns trigger
6767
language plpgsql
6868
security invoker
@@ -96,7 +96,7 @@ mod tests {
9696
name text
9797
);
9898
99-
create or replace function cool()
99+
create or replace function cool()
100100
returns trigger
101101
language plpgsql
102102
security invoker
@@ -130,7 +130,7 @@ mod tests {
130130
name text
131131
);
132132
133-
create or replace function cool()
133+
create or replace function cool()
134134
returns trigger
135135
language plpgsql
136136
security invoker

crates/pglt_completions/src/providers/tables.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ mod tests {
148148
name text
149149
);
150150
151-
create or replace function cool()
151+
create or replace function cool()
152152
returns trigger
153153
language plpgsql
154154
security invoker

crates/pglt_lsp/tests/server.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ use tower_lsp::LspService;
2828
use tower_lsp::jsonrpc;
2929
use tower_lsp::jsonrpc::Response;
3030
use tower_lsp::lsp_types as lsp;
31+
use tower_lsp::lsp_types::CompletionParams;
32+
use tower_lsp::lsp_types::CompletionResponse;
33+
use tower_lsp::lsp_types::PartialResultParams;
34+
use tower_lsp::lsp_types::Position;
35+
use tower_lsp::lsp_types::Range;
36+
use tower_lsp::lsp_types::TextDocumentPositionParams;
37+
use tower_lsp::lsp_types::WorkDoneProgressParams;
3138
use tower_lsp::lsp_types::{
3239
ClientCapabilities, DidChangeConfigurationParams, DidChangeTextDocumentParams,
3340
DidCloseTextDocumentParams, DidOpenTextDocumentParams, InitializeResult, InitializedParams,
@@ -248,6 +255,18 @@ impl Server {
248255
.await
249256
}
250257

258+
async fn get_completion(
259+
&mut self,
260+
params: tower_lsp::lsp_types::CompletionParams,
261+
) -> Result<Option<CompletionResponse>> {
262+
self.request::<tower_lsp::lsp_types::CompletionParams, CompletionResponse>(
263+
"textDocument/completion",
264+
"_get_completion",
265+
params,
266+
)
267+
.await
268+
}
269+
251270
/// Basic implementation of the `pglt/shutdown` request for tests
252271
async fn pglt_shutdown(&mut self) -> Result<()> {
253272
self.request::<_, ()>("pglt/shutdown", "_pglt_shutdown", ())
@@ -431,3 +450,104 @@ async fn server_shutdown() -> Result<()> {
431450

432451
Ok(())
433452
}
453+
454+
#[tokio::test]
455+
async fn test_completions() -> Result<()> {
456+
let factory = ServerFactory::default();
457+
let mut fs = MemoryFileSystem::default();
458+
let test_db = get_new_test_db().await;
459+
460+
let setup = r#"
461+
create table public.users (
462+
id serial primary key,
463+
name varchar(255) not null
464+
);
465+
"#;
466+
467+
test_db
468+
.execute(setup)
469+
.await
470+
.expect("Failed to setup test database");
471+
472+
let mut conf = PartialConfiguration::init();
473+
conf.merge_with(PartialConfiguration {
474+
db: Some(PartialDatabaseConfiguration {
475+
database: Some(
476+
test_db
477+
.connect_options()
478+
.get_database()
479+
.unwrap()
480+
.to_string(),
481+
),
482+
..Default::default()
483+
}),
484+
..Default::default()
485+
});
486+
fs.insert(
487+
url!("pglt.toml").to_file_path().unwrap(),
488+
toml::to_string(&conf).unwrap(),
489+
);
490+
491+
let (service, client) = factory
492+
.create_with_fs(None, DynRef::Owned(Box::new(fs)))
493+
.into_inner();
494+
495+
let (stream, sink) = client.split();
496+
let mut server = Server::new(service);
497+
498+
let (sender, _) = channel(CHANNEL_BUFFER_SIZE);
499+
let reader = tokio::spawn(client_handler(stream, sink, sender));
500+
501+
server.initialize().await?;
502+
server.initialized().await?;
503+
504+
server.load_configuration().await?;
505+
506+
server
507+
.open_document("alter table appointment alter column end_time drop not null;\n")
508+
.await?;
509+
510+
server
511+
.change_document(
512+
3,
513+
vec![TextDocumentContentChangeEvent {
514+
range: Some(Range {
515+
start: Position {
516+
line: 0,
517+
character: 24,
518+
},
519+
end: Position {
520+
line: 0,
521+
character: 24,
522+
},
523+
}),
524+
range_length: Some(0),
525+
text: " ".to_string(),
526+
}],
527+
)
528+
.await?;
529+
530+
let res = server
531+
.get_completion(CompletionParams {
532+
work_done_progress_params: WorkDoneProgressParams::default(),
533+
partial_result_params: PartialResultParams::default(),
534+
context: None,
535+
text_document_position: TextDocumentPositionParams {
536+
text_document: TextDocumentIdentifier {
537+
uri: url!("document.sql"),
538+
},
539+
position: Position {
540+
line: 0,
541+
character: 25,
542+
},
543+
},
544+
})
545+
.await?;
546+
547+
assert!(res.is_some());
548+
549+
server.shutdown().await?;
550+
reader.abort();
551+
552+
Ok(())
553+
}

crates/pglt_workspace/src/workspace/server.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ pub(super) struct WorkspaceServer {
5252
tree_sitter: TreeSitterStore,
5353
pg_query: PgQueryStore,
5454

55-
/// Stores the statements that have changed since the last analysis
56-
changed_stmts: DashSet<Statement>,
57-
5855
connection: RwLock<DbConnection>,
5956
}
6057

@@ -78,7 +75,6 @@ impl WorkspaceServer {
7875
documents: DashMap::default(),
7976
tree_sitter: TreeSitterStore::new(),
8077
pg_query: PgQueryStore::new(),
81-
changed_stmts: DashSet::default(),
8278
schema_cache: SchemaCacheManager::default(),
8379
connection: RwLock::default(),
8480
}
@@ -224,23 +220,16 @@ impl Workspace for WorkspaceServer {
224220
tracing::debug!("Adding statement: {:?}", added);
225221
self.tree_sitter.add_statement(&added.stmt, &added.text);
226222
self.pg_query.add_statement(&added.stmt, &added.text);
227-
228-
self.changed_stmts.insert(added.stmt.clone());
229223
}
230224
StatementChange::Deleted(s) => {
231225
tracing::debug!("Deleting statement: {:?}", s);
232226
self.tree_sitter.remove_statement(s);
233227
self.pg_query.remove_statement(s);
234-
235-
self.changed_stmts.remove(s);
236228
}
237229
StatementChange::Modified(s) => {
238230
tracing::debug!("Modifying statement: {:?}", s);
239231
self.tree_sitter.modify_statement(s);
240232
self.pg_query.modify_statement(s);
241-
242-
self.changed_stmts.remove(&s.old_stmt);
243-
self.changed_stmts.insert(s.new_stmt.clone());
244233
}
245234
}
246235
}

crates/pglt_workspace/src/workspace/server/change.rs

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -278,28 +278,25 @@ impl Document {
278278
let new_id = self.id_generator.next();
279279
self.positions[affected_idx] = (new_id, new_range);
280280

281-
if !change.is_whitespace() {
282-
// whitespace-only changes should not invalidate the statement
283-
changed.push(StatementChange::Modified(ModifiedStatement {
284-
old_stmt: Statement {
285-
id: old_id,
286-
path: self.path.clone(),
287-
},
288-
old_stmt_text: self.content[old_range].to_string(),
289-
290-
new_stmt: Statement {
291-
id: new_id,
292-
path: self.path.clone(),
293-
},
294-
new_stmt_text: changed_content[new_ranges[0]].to_string(),
295-
// change must be relative to the statement
296-
change_text: change.text.clone(),
297-
// make sure we always have a valid range >= 0
298-
change_range: change_range
299-
.checked_sub(old_range.start())
300-
.unwrap_or(change_range.sub(change_range.start())),
301-
}));
302-
}
281+
changed.push(StatementChange::Modified(ModifiedStatement {
282+
old_stmt: Statement {
283+
id: old_id,
284+
path: self.path.clone(),
285+
},
286+
old_stmt_text: self.content[old_range].to_string(),
287+
288+
new_stmt: Statement {
289+
id: new_id,
290+
path: self.path.clone(),
291+
},
292+
new_stmt_text: changed_content[new_ranges[0]].to_string(),
293+
// change must be relative to the statement
294+
change_text: change.text.clone(),
295+
// make sure we always have a valid range >= 0
296+
change_range: change_range
297+
.checked_sub(old_range.start())
298+
.unwrap_or(change_range.sub(change_range.start())),
299+
}));
303300

304301
self.content = new_content;
305302

@@ -375,10 +372,6 @@ impl Document {
375372
}
376373

377374
impl ChangeParams {
378-
pub fn is_whitespace(&self) -> bool {
379-
self.text.chars().count() > 0 && self.text.chars().all(char::is_whitespace)
380-
}
381-
382375
pub fn diff_size(&self) -> TextSize {
383376
match self.range {
384377
Some(range) => {
@@ -664,7 +657,7 @@ mod tests {
664657
};
665658

666659
let changed1 = d.apply_file_change(&change1);
667-
assert_eq!(changed1.len(), 0, "should not emit change");
660+
assert_eq!(changed1.len(), 1);
668661
assert_eq!(
669662
d.content,
670663
"alter table deal alter column value drop not null;\n"
@@ -681,7 +674,7 @@ mod tests {
681674
};
682675

683676
let changed2 = d.apply_file_change(&change2);
684-
assert_eq!(changed2.len(), 0);
677+
assert_eq!(changed2.len(), 1);
685678
assert_eq!(
686679
d.content,
687680
"alter table deal alter column value drop not null;\n"
@@ -698,7 +691,7 @@ mod tests {
698691
};
699692

700693
let changed3 = d.apply_file_change(&change3);
701-
assert_eq!(changed3.len(), 0);
694+
assert_eq!(changed3.len(), 1);
702695
assert_eq!(
703696
d.content,
704697
"alter table deal alter column value drop not null;\n"
@@ -715,7 +708,7 @@ mod tests {
715708
};
716709

717710
let changed4 = d.apply_file_change(&change4);
718-
assert_eq!(changed4.len(), 0);
711+
assert_eq!(changed4.len(), 1);
719712
assert_eq!(
720713
d.content,
721714
"alter table deal alter column value drop not null;\n"
@@ -741,11 +734,7 @@ mod tests {
741734
};
742735

743736
let changed1 = d.apply_file_change(&change1);
744-
assert_eq!(
745-
changed1.len(),
746-
0,
747-
"should not emit change if its only whitespace"
748-
);
737+
assert_eq!(changed1.len(), 1);
749738
assert_eq!(
750739
d.content,
751740
"select\n *\nfrom\n test;\n\nselect \n\nalter table test\n\ndrop column id;"
@@ -867,7 +856,7 @@ mod tests {
867856

868857
let changed = d.apply_file_change(&change);
869858

870-
assert_eq!(changed.len(), 0);
859+
assert_eq!(changed.len(), 1);
871860

872861
assert_document_integrity(&d);
873862
}

0 commit comments

Comments
 (0)