From 0c8f379d3304196ddc1579fa654fd3e102a20974 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 5 May 2025 20:34:18 +0200 Subject: [PATCH 1/5] fix: multiple changes at once --- crates/pgt_workspace/src/workspace.rs | 11 +++- .../src/workspace/server/change.rs | 53 +++++++++++++++++-- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/crates/pgt_workspace/src/workspace.rs b/crates/pgt_workspace/src/workspace.rs index 873dd83e..bad28c39 100644 --- a/crates/pgt_workspace/src/workspace.rs +++ b/crates/pgt_workspace/src/workspace.rs @@ -4,7 +4,7 @@ pub use self::client::{TransportRequest, WorkspaceClient, WorkspaceTransport}; use pgt_analyse::RuleCategories; use pgt_configuration::{PartialConfiguration, RuleSelector}; use pgt_fs::PgTPath; -use pgt_text_size::TextRange; +use pgt_text_size::{TextRange, TextSize}; use serde::{Deserialize, Serialize}; use crate::{ @@ -58,6 +58,15 @@ impl ChangeParams { pub fn overwrite(text: String) -> Self { Self { range: None, text } } + + pub fn push_back(&self, by: TextSize) -> Self { + Self { + range: self + .range + .map(|r| TextRange::new(r.start() + by, r.end() + by)), + text: self.text.clone(), + } + } } #[derive(Debug, serde::Serialize, serde::Deserialize)] diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index 6e86abcf..580ee9ae 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -58,16 +58,27 @@ struct Affected { impl Document { /// Applies a file change to the document and returns the affected statements pub fn apply_file_change(&mut self, change: &ChangeFileParams) -> Vec { + tracing::debug!("apply_file_change: {:?}", change); // cleanup all diagnostics with every change because we cannot guarantee that they are still valid // this is because we know their ranges only by finding slices within the content which is // very much not guaranteed to result in correct ranges self.diagnostics.clear(); - let changes = change - .changes - .iter() - .flat_map(|c| self.apply_change(c)) - .collect(); + let mut changes = Vec::new(); + + let mut push_back: TextSize = 0.into(); + + for change in &change.changes { + let change = if push_back > 0.into() { + &change.push_back(push_back) + } else { + change + }; + + changes.extend(self.apply_change(change)); + + push_back += change.diff_size(); + } self.version = change.version; @@ -1522,6 +1533,38 @@ mod tests { assert_document_integrity(&doc); } + #[test] + fn multiple_changes_at_once() { + let path = PgTPath::new("test.sql"); + + let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN +KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0); + + let change = ChangeFileParams { + path: path.clone(), + version: 1, + changes: vec![ + ChangeParams { + range: Some(TextRange::new(31.into(), 38.into())), + text: "omni_channel_message".to_string(), + }, + ChangeParams { + range: Some(TextRange::new(60.into(), 67.into())), + text: "omni_channel_message".to_string(), + }, + ], + }; + + let changed = doc.apply_file_change(&change); + + assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"omni_channel_message\"\n ADD CONSTRAINT \"omni_channel_message_organisation_id_fkey\" FOREIGN +KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n"); + + assert_eq!(changed.len(), 2); + + assert_document_integrity(&doc); + } + #[test] fn remove_inbetween_whitespace() { let path = PgTPath::new("test.sql"); From f84ed29a6b59103eff6c4bf8cf633c3da12c1bce Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 6 May 2025 08:07:35 +0200 Subject: [PATCH 2/5] fix: also handle deletion case --- crates/pgt_workspace/src/workspace.rs | 11 +-- .../src/workspace/server/change.rs | 75 +++++++++++++++++-- 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/crates/pgt_workspace/src/workspace.rs b/crates/pgt_workspace/src/workspace.rs index bad28c39..c7e7c9e4 100644 --- a/crates/pgt_workspace/src/workspace.rs +++ b/crates/pgt_workspace/src/workspace.rs @@ -46,7 +46,7 @@ pub struct ChangeFileParams { pub changes: Vec, } -#[derive(Debug, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct ChangeParams { /// The range of the file that changed. If `None`, the whole file changed. @@ -58,15 +58,6 @@ impl ChangeParams { pub fn overwrite(text: String) -> Self { Self { range: None, text } } - - pub fn push_back(&self, by: TextSize) -> Self { - Self { - range: self - .range - .map(|r| TextRange::new(r.start() + by, r.end() + by)), - text: self.text.clone(), - } - } } #[derive(Debug, serde::Serialize, serde::Deserialize)] diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index 580ee9ae..46c9a853 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -1,5 +1,9 @@ +use biome_deserialize::Text; use pgt_text_size::{TextLen, TextRange, TextSize}; -use std::ops::{Add, Sub}; +use std::{ + i64, + ops::{Add, Sub}, +}; use crate::workspace::{ChangeFileParams, ChangeParams}; @@ -58,26 +62,37 @@ struct Affected { impl Document { /// Applies a file change to the document and returns the affected statements pub fn apply_file_change(&mut self, change: &ChangeFileParams) -> Vec { - tracing::debug!("apply_file_change: {:?}", change); // cleanup all diagnostics with every change because we cannot guarantee that they are still valid // this is because we know their ranges only by finding slices within the content which is // very much not guaranteed to result in correct ranges self.diagnostics.clear(); + // when we recieive more than one change, we need to push back the changes based on the + // total range of the previous ones. This is because the ranges are always related to the original state. let mut changes = Vec::new(); - let mut push_back: TextSize = 0.into(); + let mut offset: i64 = 0; for change in &change.changes { - let change = if push_back > 0.into() { - &change.push_back(push_back) + let adjusted_change = if offset != 0 && change.range.is_some() { + &ChangeParams { + text: change.text.clone(), + range: change.range.map(|range| { + let start = u32::from(range.start()); + let end = u32::from(range.end()); + TextRange::new( + TextSize::from((start as i64 + offset).try_into().unwrap_or(0)), + TextSize::from((end as i64 + offset).try_into().unwrap_or(0)), + ) + }), + } } else { change }; - changes.extend(self.apply_change(change)); + changes.extend(self.apply_change(adjusted_change)); - push_back += change.diff_size(); + offset += change.change_size(); } self.version = change.version; @@ -367,6 +382,18 @@ impl Document { } impl ChangeParams { + /// For lack of a better name, this returns the change in size of the text compared to the range + pub fn change_size(&self) -> i64 { + match self.range { + Some(range) => { + let range_length: usize = range.len().into(); + let text_length = self.text.chars().count(); + text_length as i64 - range_length as i64 + } + None => i64::try_from(self.text.chars().count()).unwrap(), + } + } + pub fn diff_size(&self) -> TextSize { match self.range { Some(range) => { @@ -1534,7 +1561,39 @@ mod tests { } #[test] - fn multiple_changes_at_once() { + fn multiple_deletions_at_once() { + let path = PgTPath::new("test.sql"); + + let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN +KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0); + + let change = ChangeFileParams { + path: path.clone(), + version: 1, + changes: vec![ + ChangeParams { + range: Some(TextRange::new(31.into(), 38.into())), + text: "te".to_string(), + }, + ChangeParams { + range: Some(TextRange::new(60.into(), 67.into())), + text: "te".to_string(), + }, + ], + }; + + let changed = doc.apply_file_change(&change); + + assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"te\"\n ADD CONSTRAINT \"te_organisation_id_fkey\" FOREIGN +KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n"); + + assert_eq!(changed.len(), 2); + + assert_document_integrity(&doc); + } + + #[test] + fn multiple_additions_at_once() { let path = PgTPath::new("test.sql"); let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN From 76211cb5d02ddbcf33cf5f864708b53e0fa3379f Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 6 May 2025 08:08:28 +0200 Subject: [PATCH 3/5] cleanup --- crates/pgt_workspace/src/workspace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pgt_workspace/src/workspace.rs b/crates/pgt_workspace/src/workspace.rs index c7e7c9e4..7d41bce1 100644 --- a/crates/pgt_workspace/src/workspace.rs +++ b/crates/pgt_workspace/src/workspace.rs @@ -46,7 +46,7 @@ pub struct ChangeFileParams { pub changes: Vec, } -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +#[derive(Debug, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct ChangeParams { /// The range of the file that changed. If `None`, the whole file changed. From fc81d748ab029ec9a19aca981cd1aba6e6fc008c Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 6 May 2025 08:08:48 +0200 Subject: [PATCH 4/5] cleanup --- crates/pgt_workspace/src/workspace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pgt_workspace/src/workspace.rs b/crates/pgt_workspace/src/workspace.rs index 7d41bce1..873dd83e 100644 --- a/crates/pgt_workspace/src/workspace.rs +++ b/crates/pgt_workspace/src/workspace.rs @@ -4,7 +4,7 @@ pub use self::client::{TransportRequest, WorkspaceClient, WorkspaceTransport}; use pgt_analyse::RuleCategories; use pgt_configuration::{PartialConfiguration, RuleSelector}; use pgt_fs::PgTPath; -use pgt_text_size::{TextRange, TextSize}; +use pgt_text_size::TextRange; use serde::{Deserialize, Serialize}; use crate::{ From 4023d1ede458888cbeb6eb0feef73123ccb9faff Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 6 May 2025 08:09:15 +0200 Subject: [PATCH 5/5] cleanup --- crates/pgt_workspace/src/workspace/server/change.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index 46c9a853..c8799922 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -1,9 +1,5 @@ -use biome_deserialize::Text; use pgt_text_size::{TextLen, TextRange, TextSize}; -use std::{ - i64, - ops::{Add, Sub}, -}; +use std::ops::{Add, Sub}; use crate::workspace::{ChangeFileParams, ChangeParams};