From a5bea505d26c1f9441e94bd9660d8c0d1aba6ada Mon Sep 17 00:00:00 2001 From: psteinroe Date: Tue, 21 Jan 2025 15:30:08 -0100 Subject: [PATCH 1/7] initial commit --- crates/pg_cli/src/execute/traverse.rs | 5 +++++ crates/pg_configuration/src/files.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/crates/pg_cli/src/execute/traverse.rs b/crates/pg_cli/src/execute/traverse.rs index 98a831c83..041a7ed4c 100644 --- a/crates/pg_cli/src/execute/traverse.rs +++ b/crates/pg_cli/src/execute/traverse.rs @@ -480,6 +480,11 @@ impl TraversalContext for TraversalOptions<'_, '_> { return false; } + // only allow .sql and .pg files for now + if path.extension().is_some_and(|ext| ext != "sql" && ext != "pg") { + return false; + } + match self.execution.traversal_mode() { TraversalMode::Dummy { .. } => true, TraversalMode::Check { .. } => true, diff --git a/crates/pg_configuration/src/files.rs b/crates/pg_configuration/src/files.rs index 5ae70dab1..503ac2c46 100644 --- a/crates/pg_configuration/src/files.rs +++ b/crates/pg_configuration/src/files.rs @@ -29,6 +29,10 @@ pub struct FilesConfiguration { /// match these patterns. #[partial(bpaf(hide))] pub include: StringSet, + + /// The directory where the migration files are stored + #[partial(bpaf(hide))] + pub migration_dir: String, } impl Default for FilesConfiguration { @@ -37,6 +41,7 @@ impl Default for FilesConfiguration { max_size: DEFAULT_FILE_SIZE_LIMIT, ignore: Default::default(), include: Default::default(), + migration_dir: "migrations".to_string(), } } } From 38733189a4578f39e29c910e06df56a266c4cb6e Mon Sep 17 00:00:00 2001 From: psteinroe Date: Wed, 22 Jan 2025 07:27:55 -0100 Subject: [PATCH 2/7] save --- crates/pg_cli/src/commands/check.rs | 1 + crates/pg_cli/src/execute/traverse.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/pg_cli/src/commands/check.rs b/crates/pg_cli/src/commands/check.rs index 72f4d0076..7834cb125 100644 --- a/crates/pg_cli/src/commands/check.rs +++ b/crates/pg_cli/src/commands/check.rs @@ -39,6 +39,7 @@ impl CommandRunner for CheckCommandPayload { fs: &DynRef<'_, dyn FileSystem>, configuration: &PartialConfiguration, ) -> Result, CliDiagnostic> { + // update this to find migration files let paths = get_files_to_process_with_cli_options( self.since.as_deref(), self.changed, diff --git a/crates/pg_cli/src/execute/traverse.rs b/crates/pg_cli/src/execute/traverse.rs index 041a7ed4c..68327bfd9 100644 --- a/crates/pg_cli/src/execute/traverse.rs +++ b/crates/pg_cli/src/execute/traverse.rs @@ -481,7 +481,7 @@ impl TraversalContext for TraversalOptions<'_, '_> { } // only allow .sql and .pg files for now - if path.extension().is_some_and(|ext| ext != "sql" && ext != "pg") { + if path.extension().is_none_or(|ext| ext != "sql" && ext != "pg") { return false; } From 825a3adb8183351f184f16db970ba941a35cd326 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Sun, 26 Jan 2025 20:27:20 -0100 Subject: [PATCH 3/7] progress --- Cargo.lock | 2 + crates/pg_cli/Cargo.toml | 52 +++---- crates/pg_cli/src/commands/check.rs | 23 +-- crates/pg_cli/src/commands/mod.rs | 170 +--------------------- crates/pg_cli/src/execute/traverse.rs | 5 +- crates/pg_cli/src/lib.rs | 8 +- crates/pg_configuration/src/database.rs | 4 +- crates/pg_configuration/src/files.rs | 9 +- crates/pg_configuration/src/lib.rs | 16 +- crates/pg_configuration/src/migrations.rs | 39 +++++ crates/pg_workspace/src/settings.rs | 77 +++++++++- 11 files changed, 182 insertions(+), 223 deletions(-) create mode 100644 crates/pg_configuration/src/migrations.rs diff --git a/Cargo.lock b/Cargo.lock index f9c464468..d96c57908 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2232,6 +2232,8 @@ name = "pg_cli" version = "0.0.0" dependencies = [ "anyhow", + "biome_deserialize", + "biome_deserialize_macros", "bpaf", "crossbeam", "dashmap 5.5.3", diff --git a/crates/pg_cli/Cargo.toml b/crates/pg_cli/Cargo.toml index d49036985..d04f88c5d 100644 --- a/crates/pg_cli/Cargo.toml +++ b/crates/pg_cli/Cargo.toml @@ -12,31 +12,33 @@ version = "0.0.0" [dependencies] -anyhow = { workspace = true } -bpaf = { workspace = true, features = ["bright-color"] } -crossbeam = { workspace = true } -dashmap = "5.5.3" -hdrhistogram = { version = "7.5.4", default-features = false } -path-absolutize = { version = "3.1.1", optional = false, features = ["use_unix_paths_on_wasm"] } -pg_analyse = { workspace = true } -pg_configuration = { workspace = true } -pg_console = { workspace = true } -pg_diagnostics = { workspace = true } -pg_flags = { workspace = true } -pg_fs = { workspace = true } -pg_lsp = { workspace = true } -pg_text_edit = { workspace = true } -pg_workspace = { workspace = true } -quick-junit = "0.5.0" -rayon = { workspace = true } -rustc-hash = { workspace = true } -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true } -tokio = { workspace = true, features = ["io-std", "io-util", "net", "time", "rt", "sync", "rt-multi-thread", "macros"] } -tracing = { workspace = true } -tracing-appender = "0.2.3" -tracing-subscriber = { workspace = true, features = ["env-filter", "json"] } -tracing-tree = "0.4.0" +anyhow = { workspace = true } +biome_deserialize = { workspace = true } +biome_deserialize_macros = { workspace = true } +bpaf = { workspace = true, features = ["bright-color"] } +crossbeam = { workspace = true } +dashmap = "5.5.3" +hdrhistogram = { version = "7.5.4", default-features = false } +path-absolutize = { version = "3.1.1", optional = false, features = ["use_unix_paths_on_wasm"] } +pg_analyse = { workspace = true } +pg_configuration = { workspace = true } +pg_console = { workspace = true } +pg_diagnostics = { workspace = true } +pg_flags = { workspace = true } +pg_fs = { workspace = true } +pg_lsp = { workspace = true } +pg_text_edit = { workspace = true } +pg_workspace = { workspace = true } +quick-junit = "0.5.0" +rayon = { workspace = true } +rustc-hash = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["io-std", "io-util", "net", "time", "rt", "sync", "rt-multi-thread", "macros"] } +tracing = { workspace = true } +tracing-appender = "0.2.3" +tracing-subscriber = { workspace = true, features = ["env-filter", "json"] } +tracing-tree = "0.4.0" [target.'cfg(unix)'.dependencies] libc = "0.2.161" diff --git a/crates/pg_cli/src/commands/check.rs b/crates/pg_cli/src/commands/check.rs index 7834cb125..ea6084af9 100644 --- a/crates/pg_cli/src/commands/check.rs +++ b/crates/pg_cli/src/commands/check.rs @@ -1,5 +1,6 @@ use crate::cli_options::CliOptions; use crate::{CliDiagnostic, Execution, TraversalMode}; +use biome_deserialize::Merge; use pg_configuration::PartialConfiguration; use pg_console::Console; use pg_fs::FileSystem; @@ -9,9 +10,6 @@ use std::ffi::OsString; use super::{get_files_to_process_with_cli_options, CommandRunner}; pub(crate) struct CheckCommandPayload { - pub(crate) write: bool, - pub(crate) fix: bool, - pub(crate) unsafe_: bool, pub(crate) configuration: Option, pub(crate) paths: Vec, pub(crate) stdin_file_path: Option, @@ -26,12 +24,20 @@ impl CommandRunner for CheckCommandPayload { fn merge_configuration( &mut self, loaded_configuration: LoadedConfiguration, - fs: &DynRef<'_, dyn FileSystem>, - console: &mut dyn Console, + _fs: &DynRef<'_, dyn FileSystem>, + _console: &mut dyn Console, ) -> Result { - let LoadedConfiguration { configuration, .. } = loaded_configuration; + let LoadedConfiguration { + configuration: mut fs_configuration, + .. + } = loaded_configuration; + + if let Some(configuration) = self.configuration.clone() { + // overwrite fs config with cli args + fs_configuration.merge_with(configuration); + } - Ok(configuration) + Ok(fs_configuration) } fn get_files_to_process( @@ -39,7 +45,6 @@ impl CommandRunner for CheckCommandPayload { fs: &DynRef<'_, dyn FileSystem>, configuration: &PartialConfiguration, ) -> Result, CliDiagnostic> { - // update this to find migration files let paths = get_files_to_process_with_cli_options( self.since.as_deref(), self.changed, @@ -57,7 +62,7 @@ impl CommandRunner for CheckCommandPayload { } fn should_write(&self) -> bool { - self.write || self.fix + false } fn get_execution( diff --git a/crates/pg_cli/src/commands/mod.rs b/crates/pg_cli/src/commands/mod.rs index c01cd6a45..6b47ecc65 100644 --- a/crates/pg_cli/src/commands/mod.rs +++ b/crates/pg_cli/src/commands/mod.rs @@ -11,7 +11,7 @@ use pg_console::Console; use pg_fs::FileSystem; use pg_workspace::configuration::{load_configuration, LoadedConfiguration}; use pg_workspace::settings::PartialConfigurationExt; -use pg_workspace::workspace::{FixFileMode, UpdateSettingsParams}; +use pg_workspace::workspace::UpdateSettingsParams; use pg_workspace::{DynRef, Workspace, WorkspaceError}; use std::ffi::OsString; use std::path::PathBuf; @@ -33,18 +33,6 @@ pub enum PgLspCommand { /// Runs everything to the requested files. #[bpaf(command)] Check { - /// Writes safe fixes, formatting and import sorting - #[bpaf(long("write"), switch)] - write: bool, - - /// Allow to do unsafe fixes, should be used with `--write` or `--fix` - #[bpaf(long("unsafe"), switch)] - unsafe_: bool, - - /// Alias for `--write`, writes safe fixes, formatting and import sorting - #[bpaf(long("fix"), switch, hide_usage)] - fix: bool, - #[bpaf(external(partial_configuration), hide_usage, optional)] configuration: Option, #[bpaf(external, hide_usage)] @@ -401,165 +389,9 @@ fn get_files_to_process_with_cli_options( } } -/// Holds the options to determine the fix file mode. -pub(crate) struct FixFileModeOptions { - write: bool, - suppress: bool, - suppression_reason: Option, - fix: bool, - unsafe_: bool, -} - -/// - [Result]: if the given options are incompatible -/// - [Option]: if no fixes are requested -/// - [FixFileMode]: if safe or unsafe fixes are requested -pub(crate) fn determine_fix_file_mode( - options: FixFileModeOptions, - console: &mut dyn Console, -) -> Result, CliDiagnostic> { - let FixFileModeOptions { - write, - fix, - suppress, - suppression_reason: _, - unsafe_, - } = options; - - check_fix_incompatible_arguments(options)?; - - let safe_fixes = write || fix; - let unsafe_fixes = (write || safe_fixes) && unsafe_; - - if unsafe_fixes { - Ok(Some(FixFileMode::SafeAndUnsafeFixes)) - } else if safe_fixes { - Ok(Some(FixFileMode::SafeFixes)) - } else if suppress { - Ok(Some(FixFileMode::ApplySuppressions)) - } else { - Ok(None) - } -} - -/// Checks if the fix file options are incompatible. -fn check_fix_incompatible_arguments(options: FixFileModeOptions) -> Result<(), CliDiagnostic> { - let FixFileModeOptions { - write, - suppress, - suppression_reason, - fix, - .. - } = options; - if write && fix { - return Err(CliDiagnostic::incompatible_arguments("--write", "--fix")); - } else if suppress && write { - return Err(CliDiagnostic::incompatible_arguments( - "--suppress", - "--write", - )); - } else if suppress && fix { - return Err(CliDiagnostic::incompatible_arguments("--suppress", "--fix")); - } else if !suppress && suppression_reason.is_some() { - return Err(CliDiagnostic::unexpected_argument( - "--reason", - "`--reason` is only valid when `--suppress` is used.", - )); - }; - Ok(()) -} - #[cfg(test)] mod tests { use super::*; - use pg_console::BufferConsole; - - #[test] - fn incompatible_arguments() { - { - let (write, suppress, suppression_reason, fix, unsafe_) = - (true, false, None, true, false); - assert!(check_fix_incompatible_arguments(FixFileModeOptions { - write, - suppress, - suppression_reason, - fix, - unsafe_ - }) - .is_err()); - } - } - - #[test] - fn safe_fixes() { - let mut console = BufferConsole::default(); - - for (write, suppress, suppression_reason, fix, unsafe_) in [ - (true, false, None, false, false), // --write - (false, false, None, true, false), // --fix - ] { - assert_eq!( - determine_fix_file_mode( - FixFileModeOptions { - write, - suppress, - suppression_reason, - fix, - unsafe_ - }, - &mut console - ) - .unwrap(), - Some(FixFileMode::SafeFixes) - ); - } - } - - #[test] - fn safe_and_unsafe_fixes() { - let mut console = BufferConsole::default(); - - for (write, suppress, suppression_reason, fix, unsafe_) in [ - (true, false, None, false, true), // --write --unsafe - (false, false, None, true, true), // --fix --unsafe - ] { - assert_eq!( - determine_fix_file_mode( - FixFileModeOptions { - write, - suppress, - suppression_reason, - fix, - unsafe_ - }, - &mut console - ) - .unwrap(), - Some(FixFileMode::SafeAndUnsafeFixes) - ); - } - } - - #[test] - fn no_fix() { - let mut console = BufferConsole::default(); - - let (write, suppress, suppression_reason, fix, unsafe_) = - (false, false, None, false, false); - assert_eq!( - determine_fix_file_mode( - FixFileModeOptions { - write, - suppress, - suppression_reason, - fix, - unsafe_ - }, - &mut console - ) - .unwrap(), - None - ); - } /// Tests that all CLI options adhere to the invariants expected by `bpaf`. #[test] diff --git a/crates/pg_cli/src/execute/traverse.rs b/crates/pg_cli/src/execute/traverse.rs index 68327bfd9..1531a3f8b 100644 --- a/crates/pg_cli/src/execute/traverse.rs +++ b/crates/pg_cli/src/execute/traverse.rs @@ -481,7 +481,10 @@ impl TraversalContext for TraversalOptions<'_, '_> { } // only allow .sql and .pg files for now - if path.extension().is_none_or(|ext| ext != "sql" && ext != "pg") { + if path + .extension() + .is_none_or(|ext| ext != "sql" && ext != "pg") + { return false; } diff --git a/crates/pg_cli/src/lib.rs b/crates/pg_cli/src/lib.rs index 8bed85095..17d892041 100644 --- a/crates/pg_cli/src/lib.rs +++ b/crates/pg_cli/src/lib.rs @@ -67,9 +67,6 @@ impl<'app> CliSession<'app> { let result = match command { PgLspCommand::Version(_) => commands::version::full_version(self), PgLspCommand::Check { - write, - fix, - unsafe_, cli_options, configuration, paths, @@ -77,19 +74,18 @@ impl<'app> CliSession<'app> { staged, changed, since, + after, } => run_command( self, &cli_options, CheckCommandPayload { - write, - fix, - unsafe_, configuration, paths, stdin_file_path, staged, changed, since, + after, }, ), PgLspCommand::Clean => commands::clean::clean(self), diff --git a/crates/pg_configuration/src/database.rs b/crates/pg_configuration/src/database.rs index 3302e5846..9b35ae61b 100644 --- a/crates/pg_configuration/src/database.rs +++ b/crates/pg_configuration/src/database.rs @@ -1,10 +1,10 @@ -use biome_deserialize_macros::Partial; +use biome_deserialize_macros::{Merge, Partial}; use bpaf::Bpaf; use serde::{Deserialize, Serialize}; /// The configuration of the database connection. #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)] -#[partial(derive(Bpaf, Clone, Eq, PartialEq))] +#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] #[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))] pub struct DatabaseConfiguration { /// The host of the database. diff --git a/crates/pg_configuration/src/files.rs b/crates/pg_configuration/src/files.rs index 503ac2c46..039d785be 100644 --- a/crates/pg_configuration/src/files.rs +++ b/crates/pg_configuration/src/files.rs @@ -1,7 +1,7 @@ use std::num::NonZeroU64; use biome_deserialize::StringSet; -use biome_deserialize_macros::Partial; +use biome_deserialize_macros::{Merge, Partial}; use bpaf::Bpaf; use serde::{Deserialize, Serialize}; @@ -12,7 +12,7 @@ pub const DEFAULT_FILE_SIZE_LIMIT: NonZeroU64 = /// The configuration of the filesystem #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)] -#[partial(derive(Bpaf, Clone, Eq, PartialEq))] +#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] #[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))] pub struct FilesConfiguration { /// The maximum allowed size for source code files in bytes. Files above @@ -29,10 +29,6 @@ pub struct FilesConfiguration { /// match these patterns. #[partial(bpaf(hide))] pub include: StringSet, - - /// The directory where the migration files are stored - #[partial(bpaf(hide))] - pub migration_dir: String, } impl Default for FilesConfiguration { @@ -41,7 +37,6 @@ impl Default for FilesConfiguration { max_size: DEFAULT_FILE_SIZE_LIMIT, ignore: Default::default(), include: Default::default(), - migration_dir: "migrations".to_string(), } } } diff --git a/crates/pg_configuration/src/lib.rs b/crates/pg_configuration/src/lib.rs index 6d2e5f600..9dd64f892 100644 --- a/crates/pg_configuration/src/lib.rs +++ b/crates/pg_configuration/src/lib.rs @@ -8,6 +8,7 @@ pub mod database; pub mod diagnostics; pub mod files; pub mod generated; +pub mod migrations; pub mod vcs; pub use crate::diagnostics::ConfigurationDiagnostic; @@ -21,12 +22,15 @@ pub use analyser::{ RuleConfiguration, RuleFixConfiguration, RulePlainConfiguration, RuleSelector, RuleWithFixOptions, RuleWithOptions, Rules, }; -use biome_deserialize_macros::Partial; +use biome_deserialize_macros::{Merge, Partial}; use bpaf::Bpaf; use database::{ partial_database_configuration, DatabaseConfiguration, PartialDatabaseConfiguration, }; use files::{partial_files_configuration, FilesConfiguration, PartialFilesConfiguration}; +use migrations::{ + partial_migrations_configuration, MigrationsConfiguration, PartialMigrationsConfiguration, +}; use serde::{Deserialize, Serialize}; use vcs::VcsClientKind; @@ -37,7 +41,7 @@ pub const VERSION: &str = match option_env!("PGLSP_VERSION") { /// The configuration that is contained inside the configuration file. #[derive(Clone, Debug, Default, Deserialize, Eq, Partial, PartialEq, Serialize)] -#[partial(derive(Bpaf, Clone, Eq, PartialEq))] +#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] #[partial(cfg_attr(feature = "schema", derive(schemars::JsonSchema)))] #[partial(serde(deny_unknown_fields, rename_all = "snake_case"))] pub struct Configuration { @@ -52,6 +56,13 @@ pub struct Configuration { )] pub files: FilesConfiguration, + /// Configure migrations + #[partial( + type, + bpaf(external(partial_migrations_configuration), optional, hide_usage) + )] + pub migrations: MigrationsConfiguration, + /// The configuration for the linter #[partial(type, bpaf(external(partial_linter_configuration), optional))] pub linter: LinterConfiguration, @@ -72,6 +83,7 @@ impl PartialConfiguration { ignore: Some(Default::default()), ..Default::default() }), + migrations: None, vcs: Some(PartialVcsConfiguration { enabled: Some(false), client_kind: Some(VcsClientKind::Git), diff --git a/crates/pg_configuration/src/migrations.rs b/crates/pg_configuration/src/migrations.rs new file mode 100644 index 000000000..f11390098 --- /dev/null +++ b/crates/pg_configuration/src/migrations.rs @@ -0,0 +1,39 @@ +use std::str::FromStr; + +use biome_deserialize_macros::{Merge, Partial}; +use bpaf::Bpaf; +use serde::{Deserialize, Serialize}; + +/// The configuration of the filesystem +#[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize, Default)] +#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] +#[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))] +pub struct MigrationsConfiguration { + /// The directory where the migration files are stored + #[partial(bpaf(hide))] + pub migration_dir: Option, + + /// The pattern used to store migration files + #[partial(bpaf(hide))] + pub migration_pattern: Option, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Merge)] +pub enum MigrationPattern { + /// The migration files are stored in the root of the migration directory + Root, + /// The migration files are stored in a subdirectory of the migration directory + Subdirectory, +} + +impl FromStr for MigrationPattern { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "root" => Ok(Self::Root), + "subdirectory" => Ok(Self::Subdirectory), + _ => Err(format!("Invalid migration pattern: {}", s)), + } + } +} diff --git a/crates/pg_workspace/src/settings.rs b/crates/pg_workspace/src/settings.rs index 376a4aa68..9ecfea675 100644 --- a/crates/pg_workspace/src/settings.rs +++ b/crates/pg_workspace/src/settings.rs @@ -9,8 +9,11 @@ use std::{ use ignore::gitignore::{Gitignore, GitignoreBuilder}; use pg_configuration::{ - database::PartialDatabaseConfiguration, diagnostics::InvalidIgnorePattern, - files::FilesConfiguration, ConfigurationDiagnostic, LinterConfiguration, PartialConfiguration, + database::PartialDatabaseConfiguration, + diagnostics::InvalidIgnorePattern, + files::{FilesConfiguration, MigrationPattern}, + migrations::{MigrationsConfiguration, PartialMigrationsConfiguration}, + ConfigurationDiagnostic, LinterConfiguration, PartialConfiguration, }; use pg_fs::FileSystem; @@ -27,6 +30,9 @@ pub struct Settings { /// Linter settings applied to all files in the workspace pub linter: LinterSettings, + + /// Migrations settings + pub migrations: Option, } #[derive(Debug)] @@ -99,6 +105,8 @@ impl Settings { to_linter_settings(working_directory.clone(), LinterConfiguration::from(linter))?; } + // TODO migrations + Ok(()) } @@ -304,6 +312,71 @@ pub struct FilesSettings { pub git_ignore: Option, } +/// Migration settings +#[derive(Debug)] +pub(crate) struct Migrations { + path: PathBuf, + pattern: MigrationPattern, +} + +pub(crate) struct Migration { + timestamp: u64, + name: String, +} + +impl Migrations { + fn get_migration(&self, path: &Path) -> Option { + // check if path is a child of the migration directory + match path.canonicalize() { + Ok(canonical_child) => match self.path.canonicalize() { + Ok(canonical_dir) => canonical_child.starts_with(&canonical_dir), + Err(_) => return None, + }, + Err(_) => return None, + }; + + match self.pattern { + // supabase style migrations/0001_create_table.sql + MigrationPattern::Root => { + let file_name = path.file_name()?.to_str()?; + let timestamp = file_name.split('_').next()?; + let name = file_name + .split('_') + .skip(1) + .collect::>() + .join("_"); + let timestamp = timestamp.parse().ok()?; + Some(Migration { timestamp, name }) + } + // drizzle / prisma style migrations/0001_create_table/migration.sql + MigrationPattern::Subdirectory => { + let relative_path = path.strip_prefix(&self.path).ok()?; + let components: Vec<_> = relative_path.components().collect(); + if components.len() != 2 { + return None; + } + let dir_name = components[0].as_os_str().to_str()?; + let parts: Vec<&str> = dir_name.splitn(2, '_').collect(); + if parts.len() != 2 { + return None; + } + let timestamp = parts[0].parse().ok()?; + let name = parts[1].to_string(); + Some(Migration { timestamp, name }) + } + } + } +} + +impl From for Migrations { + fn from(value: MigrationsConfiguration) -> Self { + Self { + path: value.migration_dir, + pattern: value.migration_pattern, + } + } +} + /// Limit the size of files to 1.0 MiB by default pub(crate) const DEFAULT_FILE_SIZE_LIMIT: NonZeroU64 = // SAFETY: This constant is initialized with a non-zero value From 8a215d3faf247b10d85caf2190e1af859d561156 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 27 Jan 2025 20:29:35 -0100 Subject: [PATCH 4/7] finish --- Cargo.lock | 6 +- crates/pg_cli/src/execute/traverse.rs | 18 ++- crates/pg_cli/src/lib.rs | 2 - crates/pg_configuration/src/migrations.rs | 32 +---- crates/pg_workspace/Cargo.toml | 1 + crates/pg_workspace/src/settings.rs | 93 +++++--------- crates/pg_workspace/src/workspace/server.rs | 29 ++++- .../src/workspace/server/migration.rs | 116 ++++++++++++++++++ pglsp.toml | 4 + 9 files changed, 198 insertions(+), 103 deletions(-) create mode 100644 crates/pg_workspace/src/workspace/server/migration.rs diff --git a/Cargo.lock b/Cargo.lock index d96c57908..e52723863 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2660,6 +2660,7 @@ dependencies = [ "serde", "serde_json", "sqlx", + "tempfile", "text-size", "tokio", "toml", @@ -3790,12 +3791,13 @@ checksum = "42a4d50cdb458045afc8131fd91b64904da29548bcb63c7236e0844936c13078" [[package]] name = "tempfile" -version = "3.14.0" +version = "3.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28cce251fcbc87fac86a866eeb0d6c2d536fc16d06f184bb61aeae11aa4cee0c" +checksum = "9a8a559c81686f576e8cd0290cd2a24a2a9ad80c98b3478856500fcbd7acd704" dependencies = [ "cfg-if", "fastrand 2.3.0", + "getrandom", "once_cell", "rustix 0.38.42", "windows-sys 0.59.0", diff --git a/crates/pg_cli/src/execute/traverse.rs b/crates/pg_cli/src/execute/traverse.rs index 1531a3f8b..873b89053 100644 --- a/crates/pg_cli/src/execute/traverse.rs +++ b/crates/pg_cli/src/execute/traverse.rs @@ -456,7 +456,13 @@ impl TraversalContext for TraversalOptions<'_, '_> { fn can_handle(&self, pglsp_path: &PgLspPath) -> bool { let path = pglsp_path.as_path(); - if self.fs.path_is_dir(path) || self.fs.path_is_symlink(path) { + + let is_valid_file = self.fs.path_is_file(path) + && path + .extension() + .is_some_and(|ext| ext == "sql" || ext == "pg"); + + if self.fs.path_is_dir(path) || self.fs.path_is_symlink(path) || is_valid_file { // handle: // - directories // - symlinks @@ -476,15 +482,7 @@ impl TraversalContext for TraversalOptions<'_, '_> { } // bail on fifo and socket files - if !self.fs.path_is_file(path) { - return false; - } - - // only allow .sql and .pg files for now - if path - .extension() - .is_none_or(|ext| ext != "sql" && ext != "pg") - { + if !is_valid_file { return false; } diff --git a/crates/pg_cli/src/lib.rs b/crates/pg_cli/src/lib.rs index 17d892041..8dabff3ad 100644 --- a/crates/pg_cli/src/lib.rs +++ b/crates/pg_cli/src/lib.rs @@ -74,7 +74,6 @@ impl<'app> CliSession<'app> { staged, changed, since, - after, } => run_command( self, &cli_options, @@ -85,7 +84,6 @@ impl<'app> CliSession<'app> { staged, changed, since, - after, }, ), PgLspCommand::Clean => commands::clean::clean(self), diff --git a/crates/pg_configuration/src/migrations.rs b/crates/pg_configuration/src/migrations.rs index f11390098..2c7842203 100644 --- a/crates/pg_configuration/src/migrations.rs +++ b/crates/pg_configuration/src/migrations.rs @@ -1,5 +1,3 @@ -use std::str::FromStr; - use biome_deserialize_macros::{Merge, Partial}; use bpaf::Bpaf; use serde::{Deserialize, Serialize}; @@ -10,30 +8,10 @@ use serde::{Deserialize, Serialize}; #[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))] pub struct MigrationsConfiguration { /// The directory where the migration files are stored - #[partial(bpaf(hide))] - pub migration_dir: Option, - - /// The pattern used to store migration files - #[partial(bpaf(hide))] - pub migration_pattern: Option, -} - -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Merge)] -pub enum MigrationPattern { - /// The migration files are stored in the root of the migration directory - Root, - /// The migration files are stored in a subdirectory of the migration directory - Subdirectory, -} - -impl FromStr for MigrationPattern { - type Err = String; + #[partial(bpaf(long("migrations-dir")))] + pub migrations_dir: String, - fn from_str(s: &str) -> Result { - match s { - "root" => Ok(Self::Root), - "subdirectory" => Ok(Self::Subdirectory), - _ => Err(format!("Invalid migration pattern: {}", s)), - } - } + /// Ignore any migrations before this timestamp + #[partial(bpaf(long("after")))] + pub after: u64, } diff --git a/crates/pg_workspace/Cargo.toml b/crates/pg_workspace/Cargo.toml index 69ccc89ca..159ac509c 100644 --- a/crates/pg_workspace/Cargo.toml +++ b/crates/pg_workspace/Cargo.toml @@ -39,6 +39,7 @@ tree-sitter.workspace = true tree_sitter_sql.workspace = true [dev-dependencies] +tempfile = "3.15.0" [lib] doctest = false diff --git a/crates/pg_workspace/src/settings.rs b/crates/pg_workspace/src/settings.rs index 9ecfea675..d4142e9d0 100644 --- a/crates/pg_workspace/src/settings.rs +++ b/crates/pg_workspace/src/settings.rs @@ -4,6 +4,7 @@ use std::{ borrow::Cow, num::NonZeroU64, path::{Path, PathBuf}, + str::FromStr, sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}, }; @@ -11,8 +12,8 @@ use ignore::gitignore::{Gitignore, GitignoreBuilder}; use pg_configuration::{ database::PartialDatabaseConfiguration, diagnostics::InvalidIgnorePattern, - files::{FilesConfiguration, MigrationPattern}, - migrations::{MigrationsConfiguration, PartialMigrationsConfiguration}, + files::FilesConfiguration, + migrations::{self, MigrationsConfiguration, PartialMigrationsConfiguration}, ConfigurationDiagnostic, LinterConfiguration, PartialConfiguration, }; use pg_fs::FileSystem; @@ -32,7 +33,7 @@ pub struct Settings { pub linter: LinterSettings, /// Migrations settings - pub migrations: Option, + pub migrations: Option, } #[derive(Debug)] @@ -105,7 +106,13 @@ impl Settings { to_linter_settings(working_directory.clone(), LinterConfiguration::from(linter))?; } - // TODO migrations + // Migrations settings + if let Some(migrations) = configuration.migrations { + self.migrations = to_migration_settings( + working_directory.clone(), + MigrationsConfiguration::from(migrations), + ); + } Ok(()) } @@ -313,68 +320,34 @@ pub struct FilesSettings { } /// Migration settings -#[derive(Debug)] -pub(crate) struct Migrations { - path: PathBuf, - pattern: MigrationPattern, -} - -pub(crate) struct Migration { - timestamp: u64, - name: String, +#[derive(Debug, Default)] +pub struct MigrationSettings { + pub path: Option, + pub after: Option, } -impl Migrations { - fn get_migration(&self, path: &Path) -> Option { - // check if path is a child of the migration directory - match path.canonicalize() { - Ok(canonical_child) => match self.path.canonicalize() { - Ok(canonical_dir) => canonical_child.starts_with(&canonical_dir), - Err(_) => return None, - }, - Err(_) => return None, - }; - - match self.pattern { - // supabase style migrations/0001_create_table.sql - MigrationPattern::Root => { - let file_name = path.file_name()?.to_str()?; - let timestamp = file_name.split('_').next()?; - let name = file_name - .split('_') - .skip(1) - .collect::>() - .join("_"); - let timestamp = timestamp.parse().ok()?; - Some(Migration { timestamp, name }) - } - // drizzle / prisma style migrations/0001_create_table/migration.sql - MigrationPattern::Subdirectory => { - let relative_path = path.strip_prefix(&self.path).ok()?; - let components: Vec<_> = relative_path.components().collect(); - if components.len() != 2 { - return None; - } - let dir_name = components[0].as_os_str().to_str()?; - let parts: Vec<&str> = dir_name.splitn(2, '_').collect(); - if parts.len() != 2 { - return None; - } - let timestamp = parts[0].parse().ok()?; - let name = parts[1].to_string(); - Some(Migration { timestamp, name }) - } +impl From for MigrationSettings { + fn from(value: PartialMigrationsConfiguration) -> Self { + Self { + path: value.migrations_dir.map(PathBuf::from), + after: value.after, } } } -impl From for Migrations { - fn from(value: MigrationsConfiguration) -> Self { - Self { - path: value.migration_dir, - pattern: value.migration_pattern, - } - } +fn to_migration_settings( + working_directory: Option, + conf: MigrationsConfiguration, +) -> Option { + tracing::debug!( + "Migrations configuration: {:?}, dir: {:?}", + conf, + working_directory + ); + working_directory.map(|working_directory| MigrationSettings { + path: Some(working_directory.join(conf.migrations_dir)), + after: Some(conf.after), + }) } /// Limit the size of files to 1.0 MiB by default diff --git a/crates/pg_workspace/src/workspace/server.rs b/crates/pg_workspace/src/workspace/server.rs index d1173c5fa..a52ee055a 100644 --- a/crates/pg_workspace/src/workspace/server.rs +++ b/crates/pg_workspace/src/workspace/server.rs @@ -1,4 +1,10 @@ -use std::{fs, future::Future, panic::RefUnwindSafe, path::Path, sync::RwLock}; +use std::{ + fs, + future::Future, + panic::RefUnwindSafe, + path::{Path, PathBuf}, + sync::RwLock, +}; use analyser::AnalyserVisitorBuilder; use change::StatementChange; @@ -33,6 +39,7 @@ use super::{ mod analyser; mod change; mod document; +mod migration; mod pg_query; mod tree_sitter; @@ -150,13 +157,31 @@ impl WorkspaceServer { Ok(()) } + fn is_ignored_by_migration_config(&self, path: &Path) -> bool { + let set = self.settings(); + set.as_ref() + .migrations + .as_ref() + .and_then(|migration_settings| { + tracing::info!("Checking migration settings: {:?}", migration_settings); + let ignore_before = migration_settings.after.as_ref()?; + tracing::info!("Checking migration settings: {:?}", ignore_before); + let migrations_dir = migration_settings.path.as_ref()?; + tracing::info!("Checking migration settings: {:?}", migrations_dir); + let migration = migration::get_migration(path, migrations_dir)?; + + Some(&migration.timestamp <= ignore_before) + }) + .unwrap_or(false) + } + /// Check whether a file is ignored in the top-level config `files.ignore`/`files.include` fn is_ignored(&self, path: &Path) -> bool { let file_name = path.file_name().and_then(|s| s.to_str()); // Never ignore PGLSP's config file regardless `include`/`ignore` (file_name != Some(ConfigName::pglsp_toml())) && // Apply top-level `include`/`ignore - (self.is_ignored_by_top_level_config(path)) + (self.is_ignored_by_top_level_config(path) || self.is_ignored_by_migration_config(path)) } /// Check whether a file is ignored in the top-level config `files.ignore`/`files.include` diff --git a/crates/pg_workspace/src/workspace/server/migration.rs b/crates/pg_workspace/src/workspace/server/migration.rs new file mode 100644 index 000000000..994da2853 --- /dev/null +++ b/crates/pg_workspace/src/workspace/server/migration.rs @@ -0,0 +1,116 @@ +use std::path::Path; + +#[derive(Debug)] +pub(crate) struct Migration { + pub(crate) timestamp: u64, + pub(crate) name: String, +} + +/// Get the migration associated with a path, if it is a migration file +pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { + // Check if path is a child of the migration directory + let is_child = path + .canonicalize() + .ok() + .and_then(|canonical_child| { + migrations_dir + .canonicalize() + .ok() + .map(|canonical_dir| canonical_child.starts_with(&canonical_dir)) + }) + .unwrap_or(false); + + if !is_child { + return None; + } + + // Try Root pattern + let root_migration = path + .file_name() + .and_then(|os_str| os_str.to_str()) + .and_then(|file_name| { + let mut parts = file_name.splitn(2, '_'); + let timestamp = parts.next()?.parse().ok()?; + let name = parts.next()?.to_string(); + Some(Migration { timestamp, name }) + }); + + if root_migration.is_some() { + return root_migration; + } + + // Try Subdirectory pattern + path.parent() + .and_then(|parent| parent.file_name()) + .and_then(|os_str| os_str.to_str()) + .and_then(|dir_name| { + let mut parts = dir_name.splitn(2, '_'); + let timestamp = parts.next()?.parse().ok()?; + let name = parts.next()?.to_string(); + Some(Migration { timestamp, name }) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::path::PathBuf; + use tempfile::TempDir; + + fn setup() -> TempDir { + TempDir::new().expect("Failed to create temp dir") + } + + #[test] + fn test_get_migration_root_pattern() { + let temp_dir = setup(); + let migrations_dir = temp_dir.path().to_path_buf(); + let path = migrations_dir.join("1234567890_create_users.sql"); + fs::write(&path, "").unwrap(); + + let migration = get_migration(&path, &migrations_dir); + + assert!(migration.is_some()); + let migration = migration.unwrap(); + assert_eq!(migration.timestamp, 1234567890); + assert_eq!(migration.name, "create_users.sql"); + } + + #[test] + fn test_get_migration_subdirectory_pattern() { + let temp_dir = setup(); + let migrations_dir = temp_dir.path().to_path_buf(); + let subdir = migrations_dir.join("1234567890_create_users"); + fs::create_dir(&subdir).unwrap(); + let path = subdir.join("up.sql"); + fs::write(&path, "").unwrap(); + + let migration = get_migration(&path, &migrations_dir); + + assert!(migration.is_some()); + let migration = migration.unwrap(); + assert_eq!(migration.timestamp, 1234567890); + assert_eq!(migration.name, "create_users"); + } + + #[test] + fn test_get_migration_non_migration_file() { + let migrations_dir = PathBuf::from("/tmp/migrations"); + let path = migrations_dir.join("not_a_migration.sql"); + + let migration = get_migration(&path, &migrations_dir); + + assert!(migration.is_none()); + } + + #[test] + fn test_get_migration_outside_migrations_dir() { + let migrations_dir = PathBuf::from("/tmp/migrations"); + let path = PathBuf::from("/tmp/other/1234567890_create_users.sql"); + + let migration = get_migration(&path, &migrations_dir); + + assert!(migration.is_none()); + } +} diff --git a/pglsp.toml b/pglsp.toml index b2273c4c9..42960a55e 100644 --- a/pglsp.toml +++ b/pglsp.toml @@ -13,6 +13,10 @@ username = "postgres" password = "postgres" database = "postgres" +[migrations] +migrations_dir = "migrations" +after = 20230912094327 + [linter] enabled = true From 61a4478447961aaa9bbfc0689cca647fb4278720 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 27 Jan 2025 20:34:23 -0100 Subject: [PATCH 5/7] cleanup --- crates/pg_workspace/src/settings.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/pg_workspace/src/settings.rs b/crates/pg_workspace/src/settings.rs index d4142e9d0..e016e4bb9 100644 --- a/crates/pg_workspace/src/settings.rs +++ b/crates/pg_workspace/src/settings.rs @@ -339,11 +339,6 @@ fn to_migration_settings( working_directory: Option, conf: MigrationsConfiguration, ) -> Option { - tracing::debug!( - "Migrations configuration: {:?}, dir: {:?}", - conf, - working_directory - ); working_directory.map(|working_directory| MigrationSettings { path: Some(working_directory.join(conf.migrations_dir)), after: Some(conf.after), From 661efe93d5262429db4ffd1e8ebe3707efb0c284 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 27 Jan 2025 20:34:41 -0100 Subject: [PATCH 6/7] cleanup --- crates/pg_workspace/src/workspace/server.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/pg_workspace/src/workspace/server.rs b/crates/pg_workspace/src/workspace/server.rs index a52ee055a..2f7a8fbae 100644 --- a/crates/pg_workspace/src/workspace/server.rs +++ b/crates/pg_workspace/src/workspace/server.rs @@ -163,11 +163,8 @@ impl WorkspaceServer { .migrations .as_ref() .and_then(|migration_settings| { - tracing::info!("Checking migration settings: {:?}", migration_settings); let ignore_before = migration_settings.after.as_ref()?; - tracing::info!("Checking migration settings: {:?}", ignore_before); let migrations_dir = migration_settings.path.as_ref()?; - tracing::info!("Checking migration settings: {:?}", migrations_dir); let migration = migration::get_migration(path, migrations_dir)?; Some(&migration.timestamp <= ignore_before) From 0942a35dbbcf4129e18a5fef7b150b70231c96f9 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Wed, 29 Jan 2025 08:51:58 -0100 Subject: [PATCH 7/7] pr review --- crates/pg_workspace/src/workspace/server/migration.rs | 11 ++++++++--- pglsp.toml | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/pg_workspace/src/workspace/server/migration.rs b/crates/pg_workspace/src/workspace/server/migration.rs index 994da2853..baa736dd8 100644 --- a/crates/pg_workspace/src/workspace/server/migration.rs +++ b/crates/pg_workspace/src/workspace/server/migration.rs @@ -24,7 +24,11 @@ pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option_.sql. + // this is used by supabase let root_migration = path .file_name() .and_then(|os_str| os_str.to_str()) @@ -39,7 +43,8 @@ pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option_ + // this is used by prisma and drizzle path.parent() .and_then(|parent| parent.file_name()) .and_then(|os_str| os_str.to_str()) @@ -95,7 +100,7 @@ mod tests { } #[test] - fn test_get_migration_non_migration_file() { + fn test_get_migration_not_timestamp_in_filename() { let migrations_dir = PathBuf::from("/tmp/migrations"); let path = migrations_dir.join("not_a_migration.sql"); diff --git a/pglsp.toml b/pglsp.toml index 42960a55e..ea3943611 100644 --- a/pglsp.toml +++ b/pglsp.toml @@ -13,9 +13,9 @@ username = "postgres" password = "postgres" database = "postgres" -[migrations] -migrations_dir = "migrations" -after = 20230912094327 +# [migrations] +# migrations_dir = "migrations" +# after = 20230912094327 [linter] enabled = true