From ab64d6ee87df02b265b94c243483fcd3f642af13 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 26 Oct 2026 14:35:02 +0000 Subject: [PATCH 1/5] refactor(workspace): one adapter per migration tool The dbmate and goose markers, the migration file names and the Supabase project detection of `init` move into a `migrations` module of pgls_workspace, with one adapter per tool behind a `MigrationTool` trait. A document gets the adapter from the settings of its file when it is opened, and again when the settings change, and keeps the sections of its content instead of computing them on every request. --- Cargo.lock | 2 +- crates/pgls_cli/Cargo.toml | 1 - crates/pgls_cli/src/commands/init.rs | 186 +------ crates/pgls_workspace/Cargo.toml | 1 + crates/pgls_workspace/src/lib.rs | 1 + crates/pgls_workspace/src/migrations.rs | 87 +++ .../pgls_workspace/src/migrations/dbmate.rs | 176 ++++++ crates/pgls_workspace/src/migrations/files.rs | 149 +++++ crates/pgls_workspace/src/migrations/goose.rs | 170 ++++++ .../pgls_workspace/src/migrations/markers.rs | 128 +++++ .../pgls_workspace/src/migrations/supabase.rs | 196 +++++++ crates/pgls_workspace/src/workspace/server.rs | 116 ++-- .../src/workspace/server.tests.rs | 59 ++ .../src/workspace/server/document.rs | 60 +- .../src/workspace/server/migration.rs | 523 ------------------ 15 files changed, 1114 insertions(+), 741 deletions(-) create mode 100644 crates/pgls_workspace/src/migrations.rs create mode 100644 crates/pgls_workspace/src/migrations/dbmate.rs create mode 100644 crates/pgls_workspace/src/migrations/files.rs create mode 100644 crates/pgls_workspace/src/migrations/goose.rs create mode 100644 crates/pgls_workspace/src/migrations/markers.rs create mode 100644 crates/pgls_workspace/src/migrations/supabase.rs delete mode 100644 crates/pgls_workspace/src/workspace/server/migration.rs diff --git a/Cargo.lock b/Cargo.lock index c99237b663..b39dc5dbd4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2442,7 +2442,6 @@ dependencies = [ "tempfile", "tikv-jemallocator", "tokio", - "toml", "tracing", "tracing-appender", "tracing-bunyan-formatter", @@ -3023,6 +3022,7 @@ dependencies = [ "strum", "tempfile", "tokio", + "toml", "tracing", "tree-sitter", "url", diff --git a/crates/pgls_cli/Cargo.toml b/crates/pgls_cli/Cargo.toml index 8accd18738..7f652c9d18 100644 --- a/crates/pgls_cli/Cargo.toml +++ b/crates/pgls_cli/Cargo.toml @@ -38,7 +38,6 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sqlx = { workspace = true } tokio = { workspace = true, features = ["io-std", "io-util", "net", "time", "rt", "sync", "rt-multi-thread", "macros"] } -toml = { workspace = true } tracing = { workspace = true } tracing-appender = "0.2.3" tracing-bunyan-formatter = { workspace = true } diff --git a/crates/pgls_cli/src/commands/init.rs b/crates/pgls_cli/src/commands/init.rs index 9be414947a..b8d985ec37 100644 --- a/crates/pgls_cli/src/commands/init.rs +++ b/crates/pgls_cli/src/commands/init.rs @@ -1,32 +1,14 @@ use crate::{CliDiagnostic, CliSession}; use pgls_configuration::PartialConfiguration; -use pgls_configuration::migrations::PartialMigrationsConfiguration; use pgls_console::{ConsoleExt, markup}; -use pgls_fs::{ConfigName, FileSystem, OpenOptions}; +use pgls_fs::ConfigName; use pgls_workspace::configuration::create_config; -use std::ffi::OsStr; +use pgls_workspace::migrations::supabase::{ + SUPABASE_CONFIG_FILE, SUPABASE_DEFAULT_DB_PORT, detect_supabase_project, + find_supabase_directories, is_inside_supabase_directory, +}; use std::path::Path; -/// The folder of a project managed by the Supabase CLI, and its configuration file in it. -const SUPABASE_DIRECTORY: &str = "supabase"; -const SUPABASE_CONFIG_FILE: &str = "config.toml"; - -/// The port of the local Supabase database if `[db] port` is not set. -const SUPABASE_DEFAULT_DB_PORT: u16 = 54322; - -/// Folders that never hold the Supabase project of a monorepo. -const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", SUPABASE_DIRECTORY]; - -/// A Supabase project and its local database, as reported by `supabase status`. -struct SupabaseProject { - /// The `supabase` folder, relative to the folder of the configuration file, with `/` as the - /// separator. - directory: String, - port: u16, - /// Whether the port could not be read from the Supabase configuration. - port_unreadable: bool, -} - pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { let fs = &mut session.app.fs; let config = &mut PartialConfiguration::init(); @@ -47,17 +29,7 @@ pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { directories => (None, directories.to_vec()), }; if let Some(supabase) = &supabase { - let db = config.db.get_or_insert_with(Default::default); - db.host = Some("127.0.0.1".to_string()); - db.port = Some(supabase.port); - db.username = Some("postgres".to_string()); - db.password = Some("postgres".to_string()); - db.database = Some("postgres".to_string()); - // Statement execution stays off: enabling it is the user's call. - config.migrations = Some(PartialMigrationsConfiguration { - migrations_dir: Some(format!("{}/migrations", supabase.directory)), - ..Default::default() - }); + supabase.configure(config); } create_config(fs, project_directory, config)?; let file_created = project_directory @@ -82,7 +54,7 @@ Welcome to the Postgres Language Server! Let's get you started... }); } let port = supabase.port; - let migrations = format!("{}/migrations", supabase.directory); + let migrations = supabase.migrations_dir(); session.app.console.log(markup! { "Supabase project detected"" in "{supabase.directory}": the database connection is set to the local Supabase database at 127.0.0.1:"{port}", and the migrations to "{migrations}". " @@ -95,147 +67,3 @@ Welcome to the Postgres Language Server! Let's get you started... } Ok(()) } - -/// Whether the working directory is the `supabase` folder of a project: it is named like it and -/// has a `config.toml` with a `[db]` section. -fn is_inside_supabase_directory(fs: &dyn FileSystem) -> bool { - let is_named_supabase = fs - .working_directory() - .is_some_and(|directory| directory.file_name() == Some(OsStr::new(SUPABASE_DIRECTORY))); - let mut contents = String::new(); - is_named_supabase - && fs - .open_with_options( - Path::new(SUPABASE_CONFIG_FILE), - OpenOptions::default().read(true), - ) - .and_then(|mut file| file.read_to_string(&mut contents)) - .is_ok() - && contents - .parse::() - .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) -} - -/// The `supabase` folders with a `config.toml`, relative to `project_directory`: the one in it, -/// or else, if it is the working directory, the ones in the folders one or two levels below it, -/// as in a monorepo (`supabase/`, `db/supabase/`, `apps/web/supabase/`). -fn find_supabase_directories(fs: &dyn FileSystem, project_directory: &Path) -> Vec { - let is_project = |directory: &Path| { - fs.path_is_file( - &directory - .join(SUPABASE_DIRECTORY) - .join(SUPABASE_CONFIG_FILE), - ) - }; - if is_project(project_directory) { - return vec![SUPABASE_DIRECTORY.to_string()]; - } - if !project_directory.as_os_str().is_empty() { - return Vec::new(); - } - let Some(working_directory) = fs.working_directory() else { - return Vec::new(); - }; - let mut found = Vec::new(); - for child in subdirectories(&working_directory) { - if is_project(&working_directory.join(&child)) { - found.push(format!("{child}/{SUPABASE_DIRECTORY}")); - continue; - } - for grandchild in subdirectories(&working_directory.join(&child)) { - if is_project(&working_directory.join(&child).join(&grandchild)) { - found.push(format!("{child}/{grandchild}/{SUPABASE_DIRECTORY}")); - } - } - } - found -} - -/// The names of the folders in `directory`, sorted, without hidden and skipped ones. -fn subdirectories(directory: &Path) -> Vec { - let Ok(entries) = std::fs::read_dir(directory) else { - return Vec::new(); - }; - let mut names: Vec = entries - .filter_map(Result::ok) - .filter(|entry| entry.file_type().is_ok_and(|file_type| file_type.is_dir())) - .filter_map(|entry| entry.file_name().into_string().ok()) - .filter(|name| !name.starts_with('.') && !SKIPPED_DIRECTORIES.contains(&name.as_str())) - .collect(); - names.sort(); - names -} - -/// Reads the Supabase project in `directory`, relative to `project_directory`, which is relative -/// to the working directory. -fn detect_supabase_project( - fs: &dyn FileSystem, - project_directory: &Path, - directory: &str, -) -> Option { - let path = project_directory.join(directory).join(SUPABASE_CONFIG_FILE); - if !fs.path_is_file(&path) { - return None; - } - let mut contents = String::new(); - let port = fs - .open_with_options(&path, OpenOptions::default().read(true)) - .and_then(|mut file| file.read_to_string(&mut contents)) - .ok() - .and_then(|_| supabase_db_port(&contents)); - Some(SupabaseProject { - directory: directory.to_string(), - port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), - port_unreadable: port.is_none(), - }) -} - -/// Returns `[db] port` of a Supabase configuration, or the default port if it is not set. -/// Returns `None` if the configuration is not valid TOML or the port is not a valid port. -fn supabase_db_port(contents: &str) -> Option { - let config = contents.parse::().ok()?; - let Some(db) = config.get("db") else { - return Some(SUPABASE_DEFAULT_DB_PORT); - }; - match db.as_table()?.get("port") { - Some(port) => port - .as_integer() - .and_then(|port| u16::try_from(port).ok()) - .filter(|&port| port != 0), - None => Some(SUPABASE_DEFAULT_DB_PORT), - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn reads_the_port_from_the_db_section() { - assert_eq!(supabase_db_port("[db]\nport = 6543\n"), Some(6543)); - } - - #[test] - fn defaults_the_port_without_db_section() { - assert_eq!( - supabase_db_port("project_id = \"test\"\n"), - Some(SUPABASE_DEFAULT_DB_PORT) - ); - } - - #[test] - fn defaults_the_port_without_port_in_db_section() { - assert_eq!( - supabase_db_port("[db]\nmajor_version = 17\n"), - Some(SUPABASE_DEFAULT_DB_PORT) - ); - } - - #[test] - fn rejects_invalid_config() { - assert_eq!(supabase_db_port("[db\nport = 6543\n"), None); - assert_eq!(supabase_db_port("[db]\nport = \"6543\"\n"), None); - assert_eq!(supabase_db_port("[db]\nport = 70000\n"), None); - assert_eq!(supabase_db_port("[db]\nport = 0\n"), None); - } -} diff --git a/crates/pgls_workspace/Cargo.toml b/crates/pgls_workspace/Cargo.toml index 32f31e0cf3..52a49f62ea 100644 --- a/crates/pgls_workspace/Cargo.toml +++ b/crates/pgls_workspace/Cargo.toml @@ -47,6 +47,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["raw_value"] } slotmap = { workspace = true, features = ["serde"] } strum = { workspace = true } +toml = { workspace = true } tracing = { workspace = true, features = ["attributes", "log"] } tree-sitter.workspace = true diff --git a/crates/pgls_workspace/src/lib.rs b/crates/pgls_workspace/src/lib.rs index fafec400e9..dd617453e2 100644 --- a/crates/pgls_workspace/src/lib.rs +++ b/crates/pgls_workspace/src/lib.rs @@ -7,6 +7,7 @@ pub mod configuration; pub mod diagnostics; pub mod dome; pub mod features; +pub mod migrations; pub mod settings; pub mod workspace; diff --git a/crates/pgls_workspace/src/migrations.rs b/crates/pgls_workspace/src/migrations.rs new file mode 100644 index 0000000000..50929a442d --- /dev/null +++ b/crates/pgls_workspace/src/migrations.rs @@ -0,0 +1,87 @@ +//! Migration tools: how they name migration files, how they split a file into sections, and how +//! to find a project of theirs. +//! +//! - `files`: which files are migrations, and their sequence numbers, for every tool. +//! - `MigrationTool`: one adapter per tool with section markers, selected with +//! `migrations.sectionMarkers`. A document gets the adapter from the settings of its file, and +//! keeps the sections of its current content. +//! - [`supabase`]: finds the projects of the Supabase CLI for `init`. + +mod dbmate; +mod files; +mod goose; +mod markers; +pub mod supabase; + +use pgls_analyser::MigrationSection; +use pgls_configuration::migrations::SectionMarkers; +use pgls_text_size::TextRange; + +pub(crate) use files::{get_migration, is_in_migrations_dir}; +pub(crate) use markers::SectionMarkerDiagnostic; + +use dbmate::Dbmate; +use goose::Goose; + +/// A migration tool that puts the up and the down migration in the same file, separated by +/// marker comments, and runs each of them on its own. +pub(crate) trait MigrationTool: Sync { + /// The sections of a migration file, in file order. A file without markers has none, and is + /// one migration. + /// + /// Markers are found line by line, like the migration tools do, without looking at the SQL. + fn sections(&self, text: &str) -> Vec; + + /// The problems with the markers of a file with the given sections and statements: + /// statements before the first marker, which the tools reject, and lines that look like + /// markers but that the tools read as plain comments. + fn marker_diagnostics( + &self, + text: &str, + sections: &[MigrationSection], + statements: &[TextRange], + ) -> Vec; +} + +/// The migration tool of `migrations.sectionMarkers`, or `None` if every file is one migration. +pub(crate) fn migration_tool(markers: SectionMarkers) -> Option<&'static dyn MigrationTool> { + match markers { + SectionMarkers::None => None, + SectionMarkers::Dbmate => Some(&Dbmate), + SectionMarkers::Goose => Some(&Goose), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pgls_analyser::Direction; + + pub(super) fn sections(text: &str, markers: SectionMarkers) -> Vec<(&str, Direction, bool)> { + migration_tool(markers) + .map_or_else(Vec::new, |tool| tool.sections(text)) + .into_iter() + .map(|section| (&text[section.range], section.direction, section.transaction)) + .collect() + } + + pub(super) fn marker_problems(text: &str, markers: SectionMarkers) -> Vec<(&str, String)> { + let Some(tool) = migration_tool(markers) else { + return Vec::new(); + }; + let sections = tool.sections(text); + let statements: Vec = pgls_statement_splitter::split(text).ranges; + tool.marker_diagnostics(text, §ions, &statements) + .into_iter() + .map(|diagnostic| (&text[diagnostic.span], diagnostic.message.to_string())) + .collect() + } + + #[test] + fn no_markers() { + let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; + assert!(sections(text, SectionMarkers::None).is_empty()); + assert!(marker_problems(text, SectionMarkers::None).is_empty()); + assert!(sections("select 1;", SectionMarkers::Dbmate).is_empty()); + } +} diff --git a/crates/pgls_workspace/src/migrations/dbmate.rs b/crates/pgls_workspace/src/migrations/dbmate.rs new file mode 100644 index 0000000000..e27fa141bd --- /dev/null +++ b/crates/pgls_workspace/src/migrations/dbmate.rs @@ -0,0 +1,176 @@ +//! dbmate: `-- migrate:up` and `-- migrate:down`, see +//! https://github.com/amacneil/dbmate/blob/v2.37.0/pkg/dbmate/migration.go + +use pgls_analyser::{Direction, MigrationSection}; +use pgls_text_size::TextRange; + +use super::{ + MigrationTool, SectionMarkerDiagnostic, + markers::{self, MarkerSyntax, SectionsBuilder, lines}, +}; + +pub(super) struct Dbmate; + +const SYNTAX: MarkerSyntax = MarkerSyntax { + tool: "dbmate", + first_marker: "-- migrate:up", + is_marker: |line| marker(line).is_some(), + look_alike_message: |_| "dbmate only reads `-- migrate:up` and `-- migrate:down` in lowercase and at the start of the line, so this line is a plain comment.", +}; + +impl MigrationTool for Dbmate { + fn sections(&self, text: &str) -> Vec { + let mut sections = SectionsBuilder::new(text); + for (range, line) in lines(text) { + if let Some((direction, transaction)) = marker(line) { + sections.start(range.start(), direction, transaction); + } + } + let mut sections = sections.finish(); + // dbmate sends each section to Postgres as one query. + for section in &mut sections { + section.queries = vec![section.range]; + } + sections + } + + fn marker_diagnostics( + &self, + text: &str, + sections: &[MigrationSection], + statements: &[TextRange], + ) -> Vec { + markers::marker_diagnostics(&SYNTAX, text, sections, statements) + } +} + +/// `-- migrate:up` or `-- migrate:down` at the start of the line, with options like +/// `transaction:false`. See +/// https://github.com/amacneil/dbmate/blob/v2.37.0/pkg/dbmate/migration.go#L62 +fn marker(line: &str) -> Option<(Direction, bool)> { + let directive = line + .strip_prefix("--")? + .trim_start() + .strip_prefix("migrate:")?; + let (direction, options) = if let Some(options) = directive.strip_prefix("up") { + (Direction::Up, options) + } else { + (Direction::Down, directive.strip_prefix("down")?) + }; + if !options.is_empty() && !options.starts_with(char::is_whitespace) { + return None; + } + let transaction = options + .split_whitespace() + .filter_map(|option| option.split_once(':')) + .filter(|(_, value)| !value.contains(':')) + .all(|(key, value)| key != "transaction" || value != "false"); + Some((direction, transaction)) +} + +#[cfg(test)] +mod tests { + use pgls_analyser::Direction; + use pgls_configuration::migrations::SectionMarkers; + + use crate::migrations::{ + migration_tool, + tests::{marker_problems, sections}, + }; + + #[test] + fn dbmate_sections() { + let text = "-- migrate:up\ncreate table t (id int);\n\n-- migrate:down transaction:false\ndrop table t;\n"; + assert_eq!( + sections(text, SectionMarkers::Dbmate), + vec![ + ( + "-- migrate:up\ncreate table t (id int);\n\n", + Direction::Up, + true + ), + ( + "-- migrate:down transaction:false\ndrop table t;\n", + Direction::Down, + false + ), + ] + ); + } + + #[test] + fn dbmate_options_and_spacing() { + let directions = |text| { + sections(text, SectionMarkers::Dbmate) + .into_iter() + .map(|(_, direction, transaction)| (direction, transaction)) + .collect::>() + }; + assert_eq!( + directions("--migrate:up foo:bar transaction:false\r\nselect 1;"), + vec![(Direction::Up, false)] + ); + assert_eq!( + directions("-- migrate:up transaction:true"), + vec![(Direction::Up, true)] + ); + // Not markers: indented, a longer word, or a different tool. + assert!(directions(" -- migrate:up\n-- migrate:upgrade\n-- +goose Up\n").is_empty()); + } + + #[test] + fn queries_sent_at_once() { + let text = + "-- migrate:up transaction:false\nselect 1;\nselect 2;\n-- migrate:down\nselect 3;\n"; + let queries = migration_tool(SectionMarkers::Dbmate) + .map_or_else(Vec::new, |tool| tool.sections(text)) + .into_iter() + .map(|section| { + section + .queries + .iter() + .map(|query| &text[*query]) + .collect::>() + }) + .collect::>(); + // dbmate sends each section as one query. + assert_eq!( + queries, + vec![ + vec!["-- migrate:up transaction:false\nselect 1;\nselect 2;\n"], + vec!["-- migrate:down\nselect 3;\n"], + ] + ); + } + + #[test] + fn statements_before_the_first_marker() { + let text = + "create table t (id int);\n-- migrate:up\nselect 1;\n-- migrate:down\nselect 2;\n"; + assert_eq!( + marker_problems(text, SectionMarkers::Dbmate), + vec![( + "create table t (id int);", + "dbmate rejects a migration with statements before the first `-- migrate:up`." + .to_string() + )] + ); + // Without markers, the file is one migration. + assert!(marker_problems("select 1;\n", SectionMarkers::Dbmate).is_empty()); + } + + #[test] + fn lines_that_look_like_markers() { + let text = "-- migrate:up\nselect 1;\n-- Migrate:Down\n -- migrate:down\nselect 2;\n-- migrate:upgrade\n"; + let problems = marker_problems(text, SectionMarkers::Dbmate); + assert_eq!( + problems.iter().map(|(line, _)| *line).collect::>(), + ["-- Migrate:Down", " -- migrate:down"] + ); + assert!( + problems[0] + .1 + .starts_with("dbmate only reads `-- migrate:up`") + ); + } +} diff --git a/crates/pgls_workspace/src/migrations/files.rs b/crates/pgls_workspace/src/migrations/files.rs new file mode 100644 index 0000000000..d254473b75 --- /dev/null +++ b/crates/pgls_workspace/src/migrations/files.rs @@ -0,0 +1,149 @@ +//! Which files are migrations, and their sequence numbers. The tools name the files the same way, +//! so this is shared by all of them. + +use std::path::Path; + +#[derive(Debug)] +pub(crate) struct Migration { + pub(crate) sequence_number: u64, + #[allow(unused)] + pub(crate) name: String, +} + +/// Whether the path is inside the migrations directory. +pub(crate) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { + if path.starts_with(migrations_dir) { + return true; + } + path.canonicalize() + .ok() + .and_then(|canonical_child| { + migrations_dir + .canonicalize() + .ok() + .map(|canonical_dir| canonical_child.starts_with(&canonical_dir)) + }) + .unwrap_or(false) +} + +/// Get the migration associated with a path, if it is a migration file +pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { + if !is_in_migrations_dir(path, migrations_dir) { + return None; + } + + // we are trying to match patterns used by popular migration tools + + // in the "root" pattern, all files are directly within the migrations directory + // and their names follow _.sql. + // this is used by supabase + let root_migration = path + .file_name() + .and_then(|os_str| os_str.to_str()) + .and_then(parse_migration_name); + + if root_migration.is_some() { + return root_migration; + } + + // in the "subdirectory" pattern, each migration is in a subdirectory named _ + // this is used by prisma and drizzle + path.parent() + .and_then(|parent| parent.file_name()) + .and_then(|os_str| os_str.to_str()) + .and_then(parse_migration_name) +} + +fn parse_migration_name(name: &str) -> Option { + let mut parts = name.splitn(2, '_'); + // remove leading zeros to support numeric + let sequence_number: u64 = parts.next()?.trim_start_matches('0').parse().ok()?; + let full_name = parts.next()?; + let name = full_name + .strip_suffix(".sql") + .unwrap_or(full_name) + .to_string(); + Some(Migration { + sequence_number, + 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.sequence_number, 1234567890); + assert_eq!(migration.name, "create_users"); + } + + #[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.sequence_number, 1234567890); + assert_eq!(migration.name, "create_users"); + } + + #[test] + fn test_get_migration_prefix_number() { + let temp_dir = setup(); + let migrations_dir = temp_dir.path().to_path_buf(); + let path = migrations_dir.join("000201_a_migration.sql"); + fs::write(&path, "").unwrap(); + + let migration = get_migration(&path, &migrations_dir); + + assert!(migration.is_some()); + let migration = migration.unwrap(); + assert_eq!(migration.sequence_number, 201); + assert_eq!(migration.name, "a_migration"); + } + + #[test] + fn test_get_migration_not_timestamp_in_filename() { + 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/crates/pgls_workspace/src/migrations/goose.rs b/crates/pgls_workspace/src/migrations/goose.rs new file mode 100644 index 0000000000..f5fde5209c --- /dev/null +++ b/crates/pgls_workspace/src/migrations/goose.rs @@ -0,0 +1,170 @@ +//! goose: `-- +goose Up`, `-- +goose Down` and the other `-- +goose` annotations, see +//! https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go + +use pgls_analyser::{Direction, MigrationSection}; +use pgls_text_size::TextRange; + +use super::{ + MigrationTool, SectionMarkerDiagnostic, + markers::{self, MarkerSyntax, SectionsBuilder, lines}, +}; + +pub(super) struct Goose; + +const SYNTAX: MarkerSyntax = MarkerSyntax { + tool: "goose", + first_marker: "-- +goose Up", + is_marker: |line| annotation(line).is_some(), + look_alike_message: |line| { + // goose fails on an annotation with leading whitespace instead of ignoring it, see + // https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L321 + if line.contains("+goose") { + "goose rejects a migration with an annotation that doesn't start at the beginning of the line." + } else { + "goose only reads `-- +goose` annotations with `+goose` in lowercase, so this line is a plain comment." + } + }, +}; + +impl MigrationTool for Goose { + fn sections(&self, text: &str) -> Vec { + let mut sections = SectionsBuilder::new(text); + let mut no_transaction = false; + // The start of the open `StatementBegin` block. + let mut statement_begin = None; + for (range, line) in lines(text) { + match annotation(line) { + Some(annotation) if annotation.eq_ignore_ascii_case("up") => { + sections.start(range.start(), Direction::Up, true); + } + Some(annotation) if annotation.eq_ignore_ascii_case("down") => { + sections.start(range.start(), Direction::Down, true); + } + Some(annotation) if annotation.eq_ignore_ascii_case("no transaction") => { + no_transaction = true; + } + Some(annotation) if annotation.eq_ignore_ascii_case("statementbegin") => { + statement_begin = Some(range.start()); + } + Some(annotation) if annotation.eq_ignore_ascii_case("statementend") => { + // goose sends the statements between `StatementBegin` and `StatementEnd` + // to Postgres as one query. + if let (Some(begin), Some(section)) = + (statement_begin.take(), sections.current()) + { + section.queries.push(TextRange::new(begin, range.end())); + } + } + _ => {} + } + } + + let mut sections = sections.finish(); + // goose runs both directions of the file without a transaction. + if no_transaction { + for section in &mut sections { + section.transaction = false; + } + } + sections + } + + fn marker_diagnostics( + &self, + text: &str, + sections: &[MigrationSection], + statements: &[TextRange], + ) -> Vec { + markers::marker_diagnostics(&SYNTAX, text, sections, statements) + } +} + +/// The annotation of a `-- +goose ` line. See +/// https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L319 +fn annotation(line: &str) -> Option<&str> { + let annotation = line + .strip_prefix("--")? + .trim_start() + .strip_prefix("+goose")?; + Some(annotation.trim()) +} + +#[cfg(test)] +mod tests { + use pgls_analyser::Direction; + use pgls_configuration::migrations::SectionMarkers; + + use crate::migrations::{ + migration_tool, + tests::{marker_problems, sections}, + }; + + #[test] + fn goose_sections() { + let text = "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n-- +goose down\nselect 2;"; + assert_eq!( + sections(text, SectionMarkers::Goose), + vec![ + ( + "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n", + Direction::Up, + true + ), + ("-- +goose down\nselect 2;", Direction::Down, true), + ] + ); + } + + #[test] + fn queries_sent_at_once() { + let text = "-- +goose NO TRANSACTION\n-- +goose Up\nselect 1;\n-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n-- +goose Down\nselect 4;\n"; + let queries = migration_tool(SectionMarkers::Goose) + .map_or_else(Vec::new, |tool| tool.sections(text)) + .into_iter() + .map(|section| { + section + .queries + .iter() + .map(|query| &text[*query]) + .collect::>() + }) + .collect::>(); + // goose sends a `StatementBegin` block as one query, and the other statements one by one. + assert_eq!( + queries, + vec![ + vec!["-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n"], + vec![], + ] + ); + } + + #[test] + fn statements_before_the_first_marker() { + let text = "select 1;\n-- +goose Up\nselect 2;\n"; + assert_eq!(marker_problems(text, SectionMarkers::Goose).len(), 1); + } + + #[test] + fn lines_that_look_like_markers() { + let text = + "-- +goose Up\nselect 1;\n-- +Goose Down\n -- +goose StatementBegin\nselect 2;\n"; + let problems = marker_problems(text, SectionMarkers::Goose); + assert_eq!( + problems.iter().map(|(line, _)| *line).collect::>(), + ["-- +Goose Down", " -- +goose StatementBegin"] + ); + assert!(problems[0].1.ends_with("so this line is a plain comment.")); + assert!(problems[1].1.starts_with("goose rejects a migration")); + } + + #[test] + fn goose_no_transaction_applies_to_the_whole_file() { + let text = "-- +goose Up\nselect 1;\n-- +goose Down\nselect 2;\n-- +goose NO TRANSACTION\n"; + assert!( + sections(text, SectionMarkers::Goose) + .iter() + .all(|(_, _, transaction)| !transaction) + ); + } +} diff --git a/crates/pgls_workspace/src/migrations/markers.rs b/crates/pgls_workspace/src/migrations/markers.rs new file mode 100644 index 0000000000..7ee5346ce2 --- /dev/null +++ b/crates/pgls_workspace/src/migrations/markers.rs @@ -0,0 +1,128 @@ +//! What the adapters of the migration tools share: reading a file line by line, building its +//! sections from the markers, and the diagnostics for misplaced markers. + +use pgls_analyser::{Direction, MigrationSection}; +use pgls_diagnostics::{Diagnostic, MessageAndDescription}; +use pgls_text_size::{TextRange, TextSize}; + +/// The lines of `text` without their line endings, with the range of the whole line including +/// its line ending. +pub(super) fn lines(text: &str) -> impl Iterator { + let mut offset = TextSize::from(0); + text.split_inclusive('\n').map(move |line| { + let range = TextRange::at(offset, TextSize::of(line)); + offset = range.end(); + (range, line.trim_end_matches(['\n', '\r'])) + }) +} + +/// The sections of a file, built from its markers in file order. +pub(super) struct SectionsBuilder { + sections: Vec, + end: TextSize, +} + +impl SectionsBuilder { + pub(super) fn new(text: &str) -> Self { + Self { + sections: Vec::new(), + end: TextSize::of(text), + } + } + + /// Starts a section at the marker at `start`. It ends where the next one starts, or at the + /// end of the file. + pub(super) fn start(&mut self, start: TextSize, direction: Direction, transaction: bool) { + if let Some(previous) = self.sections.last_mut() { + previous.range = TextRange::new(previous.range.start(), start); + } + self.sections.push(MigrationSection { + range: TextRange::new(start, self.end), + direction, + transaction, + queries: Vec::new(), + }); + } + + /// The section that the line being read is in. + pub(super) fn current(&mut self) -> Option<&mut MigrationSection> { + self.sections.last_mut() + } + + pub(super) fn finish(self) -> Vec { + self.sections + } +} + +/// A problem with the section markers of a migration file that makes the migration tool reject +/// the file, or ignore a marker. +#[derive(Debug, Diagnostic)] +#[diagnostic(category = "migrations/sectionMarkers", severity = Warning)] +pub(crate) struct SectionMarkerDiagnostic { + #[location(span)] + pub(super) span: TextRange, + #[message] + #[description] + pub(super) message: MessageAndDescription, +} + +impl SectionMarkerDiagnostic { + fn new(span: TextRange, message: &str) -> Self { + Self { + span, + message: MessageAndDescription::from(message.to_string()), + } + } +} + +/// The markers of a migration tool, as far as the diagnostics need them. +pub(super) struct MarkerSyntax { + /// The name of the tool. + pub(super) tool: &'static str, + /// The marker that starts the first section, like `-- migrate:up`. + pub(super) first_marker: &'static str, + /// Whether the tool reads the line as a marker. + pub(super) is_marker: fn(&str) -> bool, + /// What the tool does with a line that looks like a marker but isn't one. + pub(super) look_alike_message: fn(&str) -> &'static str, +} + +/// See [`super::MigrationTool::marker_diagnostics`]. +pub(super) fn marker_diagnostics( + syntax: &MarkerSyntax, + text: &str, + sections: &[MigrationSection], + statements: &[TextRange], +) -> Vec { + let is_marker = syntax.is_marker; + + let mut diagnostics = Vec::new(); + if let Some(first) = sections.first() { + let message = format!( + "{} rejects a migration with statements before the first `{}`.", + syntax.tool, syntax.first_marker + ); + diagnostics.extend( + statements + .iter() + .take_while(|statement| statement.start() < first.range.start()) + .map(|statement| SectionMarkerDiagnostic::new(*statement, &message)), + ); + } + + for (range, line) in lines(text) { + let looks_like_marker = || { + let trimmed = line.trim_start(); + is_marker(trimmed) || is_marker(&trimmed.to_ascii_lowercase()) + }; + if !is_marker(line) && looks_like_marker() { + let span = TextRange::at(range.start(), TextSize::of(line)); + diagnostics.push(SectionMarkerDiagnostic::new( + span, + (syntax.look_alike_message)(line), + )); + } + } + diagnostics.sort_by_key(|diagnostic| diagnostic.span.start()); + diagnostics +} diff --git a/crates/pgls_workspace/src/migrations/supabase.rs b/crates/pgls_workspace/src/migrations/supabase.rs new file mode 100644 index 0000000000..6b2390c57e --- /dev/null +++ b/crates/pgls_workspace/src/migrations/supabase.rs @@ -0,0 +1,196 @@ +//! Supabase: finds the projects of the Supabase CLI and their local database, so that `init` can +//! point the configuration at them. + +use std::ffi::OsStr; +use std::path::Path; + +use pgls_configuration::PartialConfiguration; +use pgls_configuration::migrations::PartialMigrationsConfiguration; +use pgls_fs::{FileSystem, OpenOptions}; + +/// The folder of a project managed by the Supabase CLI, and its configuration file in it. +const SUPABASE_DIRECTORY: &str = "supabase"; +pub const SUPABASE_CONFIG_FILE: &str = "config.toml"; + +/// The port of the local Supabase database if `[db] port` is not set. +pub const SUPABASE_DEFAULT_DB_PORT: u16 = 54322; + +/// Folders that never hold the Supabase project of a monorepo. +const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", SUPABASE_DIRECTORY]; + +/// A Supabase project and its local database, as reported by `supabase status`. +pub struct SupabaseProject { + /// The `supabase` folder, relative to the folder of the configuration file, with `/` as the + /// separator. + pub directory: String, + pub port: u16, + /// Whether the port could not be read from the Supabase configuration. + pub port_unreadable: bool, +} + +impl SupabaseProject { + /// The folder of the migrations, relative to the folder of the configuration file. + pub fn migrations_dir(&self) -> String { + format!("{}/migrations", self.directory) + } + + /// Points the database connection at the local Supabase database, and the migrations at the + /// migrations of the project. + pub fn configure(&self, config: &mut PartialConfiguration) { + let db = config.db.get_or_insert_with(Default::default); + db.host = Some("127.0.0.1".to_string()); + db.port = Some(self.port); + db.username = Some("postgres".to_string()); + db.password = Some("postgres".to_string()); + db.database = Some("postgres".to_string()); + // Statement execution stays off: enabling it is the user's call. + config.migrations = Some(PartialMigrationsConfiguration { + migrations_dir: Some(self.migrations_dir()), + ..Default::default() + }); + } +} + +/// Whether the working directory is the `supabase` folder of a project: it is named like it and +/// has a `config.toml` with a `[db]` section. +pub fn is_inside_supabase_directory(fs: &dyn FileSystem) -> bool { + let is_named_supabase = fs + .working_directory() + .is_some_and(|directory| directory.file_name() == Some(OsStr::new(SUPABASE_DIRECTORY))); + let mut contents = String::new(); + is_named_supabase + && fs + .open_with_options( + Path::new(SUPABASE_CONFIG_FILE), + OpenOptions::default().read(true), + ) + .and_then(|mut file| file.read_to_string(&mut contents)) + .is_ok() + && contents + .parse::() + .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) +} + +/// The `supabase` folders with a `config.toml`, relative to `project_directory`: the one in it, +/// or else, if it is the working directory, the ones in the folders one or two levels below it, +/// as in a monorepo (`supabase/`, `db/supabase/`, `apps/web/supabase/`). +pub fn find_supabase_directories(fs: &dyn FileSystem, project_directory: &Path) -> Vec { + let is_project = |directory: &Path| { + fs.path_is_file( + &directory + .join(SUPABASE_DIRECTORY) + .join(SUPABASE_CONFIG_FILE), + ) + }; + if is_project(project_directory) { + return vec![SUPABASE_DIRECTORY.to_string()]; + } + if !project_directory.as_os_str().is_empty() { + return Vec::new(); + } + let Some(working_directory) = fs.working_directory() else { + return Vec::new(); + }; + let mut found = Vec::new(); + for child in subdirectories(&working_directory) { + if is_project(&working_directory.join(&child)) { + found.push(format!("{child}/{SUPABASE_DIRECTORY}")); + continue; + } + for grandchild in subdirectories(&working_directory.join(&child)) { + if is_project(&working_directory.join(&child).join(&grandchild)) { + found.push(format!("{child}/{grandchild}/{SUPABASE_DIRECTORY}")); + } + } + } + found +} + +/// The names of the folders in `directory`, sorted, without hidden and skipped ones. +fn subdirectories(directory: &Path) -> Vec { + let Ok(entries) = std::fs::read_dir(directory) else { + return Vec::new(); + }; + let mut names: Vec = entries + .filter_map(Result::ok) + .filter(|entry| entry.file_type().is_ok_and(|file_type| file_type.is_dir())) + .filter_map(|entry| entry.file_name().into_string().ok()) + .filter(|name| !name.starts_with('.') && !SKIPPED_DIRECTORIES.contains(&name.as_str())) + .collect(); + names.sort(); + names +} + +/// Reads the Supabase project in `directory`, relative to `project_directory`, which is relative +/// to the working directory. +pub fn detect_supabase_project( + fs: &dyn FileSystem, + project_directory: &Path, + directory: &str, +) -> Option { + let path = project_directory.join(directory).join(SUPABASE_CONFIG_FILE); + if !fs.path_is_file(&path) { + return None; + } + let mut contents = String::new(); + let port = fs + .open_with_options(&path, OpenOptions::default().read(true)) + .and_then(|mut file| file.read_to_string(&mut contents)) + .ok() + .and_then(|_| supabase_db_port(&contents)); + Some(SupabaseProject { + directory: directory.to_string(), + port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), + port_unreadable: port.is_none(), + }) +} + +/// Returns `[db] port` of a Supabase configuration, or the default port if it is not set. +/// Returns `None` if the configuration is not valid TOML or the port is not a valid port. +fn supabase_db_port(contents: &str) -> Option { + let config = contents.parse::().ok()?; + let Some(db) = config.get("db") else { + return Some(SUPABASE_DEFAULT_DB_PORT); + }; + match db.as_table()?.get("port") { + Some(port) => port + .as_integer() + .and_then(|port| u16::try_from(port).ok()) + .filter(|&port| port != 0), + None => Some(SUPABASE_DEFAULT_DB_PORT), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reads_the_port_from_the_db_section() { + assert_eq!(supabase_db_port("[db]\nport = 6543\n"), Some(6543)); + } + + #[test] + fn defaults_the_port_without_db_section() { + assert_eq!( + supabase_db_port("project_id = \"test\"\n"), + Some(SUPABASE_DEFAULT_DB_PORT) + ); + } + + #[test] + fn defaults_the_port_without_port_in_db_section() { + assert_eq!( + supabase_db_port("[db]\nmajor_version = 17\n"), + Some(SUPABASE_DEFAULT_DB_PORT) + ); + } + + #[test] + fn rejects_invalid_config() { + assert_eq!(supabase_db_port("[db\nport = 6543\n"), None); + assert_eq!(supabase_db_port("[db]\nport = \"6543\"\n"), None); + assert_eq!(supabase_db_port("[db]\nport = 70000\n"), None); + assert_eq!(supabase_db_port("[db]\nport = 0\n"), None); + } +} diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 2222a477a3..a1392f36b1 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -66,6 +66,7 @@ use crate::{ on_hover::{OnHoverParams, OnHoverResult}, semantic_tokens::{PullSemanticTokensParams, PullSemanticTokensResult, SemanticToken}, }, + migrations::{self, MigrationTool}, settings::{Settings, WorkspaceSettings, WorkspaceSettingsHandle, WorkspaceSettingsHandleMut}, workspace::{AnalyserDiagnosticsMapper, WithCSTandASTMapper}, }; @@ -88,7 +89,6 @@ mod connection_key; mod connection_manager; pub(crate) mod document; mod expand_star; -mod migration; mod pg_query; #[cfg(feature = "db")] mod pgpass; @@ -221,6 +221,45 @@ impl WorkspaceServer { settings.files.is_ignored(path) || is_ignored_by_migration_settings(settings, path) } + /// The migration tool of the settings that apply to the file. + fn migration_tool( + &self, + project_key: ProjectKey, + path: &Path, + ) -> Option<&'static dyn MigrationTool> { + self.workspaces() + .settings_for(project_key, path) + .and_then(|settings| settings.migrations.as_ref()) + .and_then(|migrations| migrations::migration_tool(migrations.section_markers)) + } + + /// Applies the changed settings of a project to its open documents. + fn apply_settings_to_documents(&self, project_key: ProjectKey) { + // Neither the documents nor the settings are locked while taking the other lock, since + // other methods take them in either order. + let paths: Vec = self + .documents + .read() + .unwrap() + .iter() + .filter(|(_, doc)| doc.project_key() == project_key) + .map(|(path, _)| path.clone()) + .collect(); + let tools: Vec<_> = paths + .into_iter() + .map(|path| { + let tool = self.migration_tool(project_key, &path); + (path, tool) + }) + .collect(); + let mut documents = self.documents.write().unwrap(); + for (path, tool) in tools { + if let Some(doc) = documents.get_mut(&path) { + doc.set_migration_tool(tool); + } + } + } + /// Whether the file is a migration. Migration-only rules don't run on other files. fn file_kind(&self, settings: &Settings, path: &Path) -> FileKind { match settings @@ -228,7 +267,7 @@ impl WorkspaceServer { .as_ref() .and_then(|migrations| migrations.path.as_ref()) { - Some(migrations_dir) if migration::is_in_migrations_dir(path, migrations_dir) => { + Some(migrations_dir) if migrations::is_in_migrations_dir(path, migrations_dir) => { FileKind::Migration } Some(_) => FileKind::Other, @@ -299,7 +338,7 @@ impl WorkspaceServer { * Each section is analysed as a migration of its own, and the down sections are only * checked with `lintDown`. Syntax errors are reported everywhere. */ - let sections = document_sections(settings, doc); + let sections = doc.migration_sections(); let skipped_sections: Vec = sections .iter() .filter(|section| { @@ -332,7 +371,7 @@ impl WorkspaceServer { catalog_base, search_path, file_kind: self.file_kind(settings, path.as_path()), - sections, + sections: sections.to_vec(), typecheck, }); @@ -472,14 +511,10 @@ impl WorkspaceServer { end: TextSize, mut visit: impl FnMut(&pgls_analyser::AnalysableStatement, &Catalog, &Session), ) -> Catalog { - let (search_path, sections) = settings - .map(|settings| { - ( - settings.typecheck.search_path.clone(), - document_sections(settings, doc), - ) - }) + let search_path = settings + .map(|settings| settings.typecheck.search_path.clone()) .unwrap_or_default(); + let sections = doc.migration_sections(); let search_path = snapshot.expand_search_path(&search_path); let new_session = |section: Option<&MigrationSection>| { Session::new(search_path.clone()).with_transaction_mode( @@ -897,7 +932,7 @@ fn is_ignored_by_migration_settings(settings: &Settings, path: &Path) -> bool { .and_then(|migration_settings| { let ignore_before = migration_settings.after.as_ref()?; let migrations_dir = migration_settings.path.as_ref()?; - let migration = migration::get_migration(path, migrations_dir)?; + let migration = migrations::get_migration(path, migrations_dir)?; Some(&migration.sequence_number <= ignore_before) }) @@ -944,6 +979,8 @@ impl Workspace for WorkspaceServer { params.vcs_base_path, params.gitignore_matches.as_slice(), )?; + drop(workspace); + self.apply_settings_to_documents(params.project_key); // The search path and the migration sections change what a statement sees. *self.snapshot_before.lock().unwrap() = None; @@ -1014,6 +1051,7 @@ impl Workspace for WorkspaceServer { self.workspaces_mut() .as_mut() .set_nested_settings(params.project_key, nested); + self.apply_settings_to_documents(params.project_key); // The search path and the migration sections change what a statement sees. *self.snapshot_before.lock().unwrap() = None; Ok(result) @@ -1023,10 +1061,16 @@ impl Workspace for WorkspaceServer { #[ignored_path(project=params.project_key, path=¶ms.path)] #[tracing::instrument(level = "info", skip_all, fields(path = params.path.as_path().as_os_str().to_str()), err)] fn open_file(&self, params: OpenFileParams) -> Result<(), WorkspaceError> { + let migration_tool = self.migration_tool(params.project_key, ¶ms.path); let mut documents = self.documents.write().unwrap(); - documents - .entry(params.path.clone()) - .or_insert_with(|| Document::new(params.content, params.version)); + documents.entry(params.path.clone()).or_insert_with(|| { + Document::open( + params.project_key, + migration_tool, + params.content, + params.version, + ) + }); Ok(()) } @@ -1049,6 +1093,9 @@ impl Workspace for WorkspaceServer { ), err)] #[ignored_path(project=params.project_key, path=¶ms.path)] fn change_file(&self, params: super::ChangeFileParams) -> Result<(), WorkspaceError> { + // For a new document. An open document keeps its tool until the settings change, see + // `apply_settings_to_documents`. Read before locking the documents, like there. + let migration_tool = self.migration_tool(params.project_key, ¶ms.path); let mut documents = self.documents.write().unwrap(); match documents.entry(params.path.clone()) { @@ -1058,7 +1105,12 @@ impl Workspace for WorkspaceServer { .update_content(params.content, params.version); } std::collections::hash_map::Entry::Vacant(entry) => { - entry.insert(Document::new(params.content, params.version)); + entry.insert(Document::open( + params.project_key, + migration_tool, + params.content, + params.version, + )); } } @@ -1183,7 +1235,7 @@ impl Workspace for WorkspaceServer { let mut skipped_suggested_fixes = 0; let mut converged = false; for _ in 0..MAX_PASSES { - let doc = Document::new(code.clone(), 0); + let doc = Document::open(doc.project_key(), doc.migration_tool(), code.clone(), 0); let diagnostics = self.lint_diagnostics(settings, ¶ms.path, &doc, ¶ms.only, ¶ms.skip); @@ -1366,16 +1418,18 @@ impl Workspace for WorkspaceServer { let mut diagnostics: Vec = doc.document_diagnostics().to_vec(); let path = params.path.as_path().display().to_string(); - if let Some(migrations) = &settings.migrations { + if let Some(migration_tool) = doc.migration_tool() { diagnostics.extend( - migration::section_marker_diagnostics( - doc.get_document_content(), - migrations.section_markers, - &document_sections(settings, doc), - doc.statement_ranges(), - ) - .into_iter() - .map(|diagnostic| SDiagnostic::new(Error::from(diagnostic).with_file_path(&path))), + migration_tool + .marker_diagnostics( + doc.get_document_content(), + doc.migration_sections(), + doc.statement_ranges(), + ) + .into_iter() + .map(|diagnostic| { + SDiagnostic::new(Error::from(diagnostic).with_file_path(&path)) + }), ); } @@ -2149,16 +2203,6 @@ fn in_database(references: &[Reference], others: &[Reference]) -> bool { }) } -/// The migration sections of `doc`, like the up and down migrations of dbmate and goose. -fn document_sections(settings: &Settings, doc: &Document) -> Vec { - settings - .migrations - .as_ref() - .map_or_else(Vec::new, |migrations| { - migration::migration_sections(doc.get_document_content(), migrations.section_markers) - }) -} - /// The edit that adds `-- pgls-ignore ` on its own line above the statement at `offset`, /// with the statement's indentation and the file's line ending. Above the statement, the comment /// suppresses the rule for all of the statement, including the bodies of its functions. diff --git a/crates/pgls_workspace/src/workspace/server.tests.rs b/crates/pgls_workspace/src/workspace/server.tests.rs index d635618a75..73dfe9b50f 100644 --- a/crates/pgls_workspace/src/workspace/server.tests.rs +++ b/crates/pgls_workspace/src/workspace/server.tests.rs @@ -292,6 +292,65 @@ DROP INDEX CONCURRENTLY IF EXISTS users_name_idx; ); } +#[test] +fn section_markers_follow_settings_changes() { + let (workspace, project_key) = + get_test_workspace(None).expect("Unable to create test workspace"); + let path = PgLSPath::new("20250301120000_migration.sql"); + workspace + .open_file(OpenFileParams { + project_key, + path: path.clone(), + content: "select 1;\n-- migrate:up\nselect 2;\n".into(), + version: 1, + }) + .expect("Unable to open test file"); + + let set_section_markers = |section_markers| { + workspace + .update_settings(UpdateSettingsParams { + project_key, + configuration: PartialConfiguration { + migrations: Some(PartialMigrationsConfiguration { + section_markers: Some(section_markers), + ..Default::default() + }), + ..Default::default() + }, + gitignore_matches: vec![], + vcs_base_path: None, + workspace_directory: None, + }) + .expect("Unable to update the settings"); + }; + // The statement before the first marker. + let marker_diagnostics = || { + workspace + .pull_file_diagnostics(crate::workspace::PullFileDiagnosticsParams { + project_key, + path: path.clone(), + categories: RuleCategories::all(), + max_diagnostics: 100, + only: vec![], + skip: vec![], + }) + .expect("Unable to pull diagnostics") + .diagnostics + .iter() + .filter(|d| { + d.category() + .is_some_and(|c| c.name() == "migrations/sectionMarkers") + }) + .count() + }; + + assert_eq!(marker_diagnostics(), 0); + set_section_markers(SectionMarkers::Dbmate); + assert_eq!(marker_diagnostics(), 1); + set_section_markers(SectionMarkers::None); + assert_eq!(marker_diagnostics(), 0); +} + #[test] fn test_legacy_suppression_still_suppresses_and_warns() { let (workspace, project_key) = diff --git a/crates/pgls_workspace/src/workspace/server/document.rs b/crates/pgls_workspace/src/workspace/server/document.rs index 6482113c48..86ec8d4f90 100644 --- a/crates/pgls_workspace/src/workspace/server/document.rs +++ b/crates/pgls_workspace/src/workspace/server/document.rs @@ -1,12 +1,14 @@ use std::sync::Arc; -use pgls_analyser::{AnalysableStatement, UnparsableStatement}; +use pgls_analyser::{AnalysableStatement, MigrationSection, UnparsableStatement}; use pgls_diagnostics::{Diagnostic, DiagnosticExt, serde::Diagnostic as SDiagnostic}; use pgls_query_ext::{diagnostics::SyntaxDiagnostic, utils::sql_function_body}; use pgls_suppressions::Suppressions; use pgls_text_size::{TextRange, TextSize}; use rustc_hash::FxHashSet; +use crate::{migrations::MigrationTool, workspace::ProjectKey}; + use super::{ annotation::AnnotationStore, pg_query::{PgQueryStore, convert_to_positional_params_with_metadata}, @@ -24,15 +26,35 @@ pub struct Document { #[allow(dead_code)] annotation_db: AnnotationStore, suppressions: Suppressions, + /// The project whose settings apply to the document. + project_key: ProjectKey, + /// The migration tool that runs the file, from its settings. See [`Document::open`]. + migration_tool: Option<&'static dyn MigrationTool>, + /// The sections of the content for `migration_tool`. + migration_sections: Vec, } impl Document { + /// A document without a project, and so without a migration tool. + #[cfg(test)] pub fn new(content: String, version: i32) -> Document { + Self::open(ProjectKey::default(), None, content, version) + } + + /// A document of the project `project_key`, whose settings make `migration_tool` run the + /// file. When the settings change, [`Document::set_migration_tool`] applies them. + pub(crate) fn open( + project_key: ProjectKey, + migration_tool: Option<&'static dyn MigrationTool>, + content: String, + version: i32, + ) -> Document { let cst_db = TreeSitterStore::new(); let ast_db = PgQueryStore::new(); let annotation_db = AnnotationStore::new(); let (ranges, diagnostics) = split_with_diagnostics(&content, None); let suppressions = Suppressions::from(content.as_str()).with_statements(ranges.clone()); + let migration_sections = sections_for(migration_tool, &content); Document { ranges, @@ -43,12 +65,16 @@ impl Document { cst_db, annotation_db, suppressions, + project_key, + migration_tool, + migration_sections, } } pub fn update_content(&mut self, content: String, version: i32) { self.content = content; self.version = version; + self.migration_sections = sections_for(self.migration_tool, &self.content); let (ranges, diagnostics) = split_with_diagnostics(&self.content, None); @@ -70,6 +96,30 @@ impl Document { self.annotation_db.retain(is_present); } + /// Applies the migration tool of changed settings. + pub(crate) fn set_migration_tool( + &mut self, + migration_tool: Option<&'static dyn MigrationTool>, + ) { + self.migration_tool = migration_tool; + self.migration_sections = sections_for(migration_tool, &self.content); + } + + pub(crate) fn project_key(&self) -> ProjectKey { + self.project_key + } + + /// The migration tool that runs the file, if its settings name one. + pub(crate) fn migration_tool(&self) -> Option<&'static dyn MigrationTool> { + self.migration_tool + } + + /// The migration sections of the document, like the up and down migrations of dbmate and + /// goose. Without a migration tool, there are none. + pub fn migration_sections(&self) -> &[MigrationSection] { + &self.migration_sections + } + pub fn suppressions(&self) -> &Suppressions { &self.suppressions } @@ -433,6 +483,14 @@ impl StatementFilter<'_> for IdFilter { } } +/// The migration sections of `content`, see [`MigrationTool::sections`]. +fn sections_for( + migration_tool: Option<&'static dyn MigrationTool>, + content: &str, +) -> Vec { + migration_tool.map_or_else(Vec::new, |tool| tool.sections(content)) +} + /// Helper function that wraps the statement splitter and returns the ranges with unified /// diagnostics pub(crate) fn split_with_diagnostics( diff --git a/crates/pgls_workspace/src/workspace/server/migration.rs b/crates/pgls_workspace/src/workspace/server/migration.rs deleted file mode 100644 index 2b1328f7f5..0000000000 --- a/crates/pgls_workspace/src/workspace/server/migration.rs +++ /dev/null @@ -1,523 +0,0 @@ -use std::path::Path; - -use pgls_analyser::{Direction, MigrationSection}; -use pgls_configuration::migrations::SectionMarkers; -use pgls_diagnostics::{Diagnostic, MessageAndDescription}; -use pgls_text_size::{TextRange, TextSize}; - -#[derive(Debug)] -pub(crate) struct Migration { - pub(crate) sequence_number: u64, - #[allow(unused)] - pub(crate) name: String, -} - -/// Whether the path is inside the migrations directory. -pub(crate) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { - if path.starts_with(migrations_dir) { - return true; - } - path.canonicalize() - .ok() - .and_then(|canonical_child| { - migrations_dir - .canonicalize() - .ok() - .map(|canonical_dir| canonical_child.starts_with(&canonical_dir)) - }) - .unwrap_or(false) -} - -/// Get the migration associated with a path, if it is a migration file -pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { - if !is_in_migrations_dir(path, migrations_dir) { - return None; - } - - // we are trying to match patterns used by popular migration tools - - // in the "root" pattern, all files are directly within the migrations directory - // and their names follow _.sql. - // this is used by supabase - let root_migration = path - .file_name() - .and_then(|os_str| os_str.to_str()) - .and_then(parse_migration_name); - - if root_migration.is_some() { - return root_migration; - } - - // in the "subdirectory" pattern, each migration is in a subdirectory named _ - // this is used by prisma and drizzle - path.parent() - .and_then(|parent| parent.file_name()) - .and_then(|os_str| os_str.to_str()) - .and_then(parse_migration_name) -} - -fn parse_migration_name(name: &str) -> Option { - let mut parts = name.splitn(2, '_'); - // remove leading zeros to support numeric - let sequence_number: u64 = parts.next()?.trim_start_matches('0').parse().ok()?; - let full_name = parts.next()?; - let name = full_name - .strip_suffix(".sql") - .unwrap_or(full_name) - .to_string(); - Some(Migration { - sequence_number, - name, - }) -} - -/// The sections of a migration file, in file order. Without markers, there are none. -/// -/// Markers are found line by line, like the migration tools do, without looking at the SQL. -pub(crate) fn migration_sections(text: &str, markers: SectionMarkers) -> Vec { - let mut sections: Vec = Vec::new(); - let mut no_transaction = false; - // The start of the open goose `StatementBegin` block. - let mut statement_begin = None; - let mut offset = 0; - for line in text.split_inclusive('\n') { - let start = TextSize::from(offset as u32); - offset += line.len(); - let line = line.trim_end_matches(['\n', '\r']); - - let marker = match markers { - SectionMarkers::None => None, - SectionMarkers::Dbmate => dbmate_marker(line), - SectionMarkers::Goose => match goose_annotation(line) { - Some(annotation) if annotation.eq_ignore_ascii_case("up") => { - Some((Direction::Up, true)) - } - Some(annotation) if annotation.eq_ignore_ascii_case("down") => { - Some((Direction::Down, true)) - } - Some(annotation) if annotation.eq_ignore_ascii_case("no transaction") => { - no_transaction = true; - None - } - Some(annotation) if annotation.eq_ignore_ascii_case("statementbegin") => { - statement_begin = Some(start); - None - } - Some(annotation) if annotation.eq_ignore_ascii_case("statementend") => { - // goose sends the statements between `StatementBegin` and `StatementEnd` - // to Postgres as one query. - if let (Some(begin), Some(section)) = - (statement_begin.take(), sections.last_mut()) - { - section - .queries - .push(TextRange::new(begin, TextSize::from(offset as u32))); - } - None - } - _ => None, - }, - }; - - if let Some((direction, transaction)) = marker { - if let Some(previous) = sections.last_mut() { - previous.range = TextRange::new(previous.range.start(), start); - } - let range = TextRange::new(start, TextSize::from(text.len() as u32)); - sections.push(MigrationSection { - range, - direction, - transaction, - queries: Vec::new(), - }); - } - } - - match markers { - // dbmate sends each section to Postgres as one query. - SectionMarkers::Dbmate => { - for section in &mut sections { - section.queries = vec![section.range]; - } - } - // goose runs both directions of the file without a transaction. - SectionMarkers::Goose if no_transaction => { - for section in &mut sections { - section.transaction = false; - } - } - _ => {} - } - - sections -} - -/// A problem with the section markers of a migration file that makes the migration tool reject -/// the file, or ignore a marker. -#[derive(Debug, Diagnostic)] -#[diagnostic(category = "migrations/sectionMarkers", severity = Warning)] -pub(crate) struct SectionMarkerDiagnostic { - #[location(span)] - span: TextRange, - #[message] - #[description] - message: MessageAndDescription, -} - -impl SectionMarkerDiagnostic { - fn new(span: TextRange, message: &str) -> Self { - Self { - span, - message: MessageAndDescription::from(message.to_string()), - } - } -} - -/// The problems with the markers of a file with the given sections and statements: -/// statements before the first marker, which the tools reject, and lines that look like markers -/// but that the tools read as plain comments. -pub(crate) fn section_marker_diagnostics( - text: &str, - markers: SectionMarkers, - sections: &[MigrationSection], - statements: &[TextRange], -) -> Vec { - let (tool, first_marker, ignored_marker) = match markers { - SectionMarkers::None => return Vec::new(), - SectionMarkers::Dbmate => ( - "dbmate", - "-- migrate:up", - "dbmate only reads `-- migrate:up` and `-- migrate:down` in lowercase and at the start of the line, so this line is a plain comment.", - ), - SectionMarkers::Goose => ( - "goose", - "-- +goose Up", - "goose only reads `-- +goose` annotations with `+goose` in lowercase, so this line is a plain comment.", - ), - }; - let is_marker = |line: &str| match markers { - SectionMarkers::None => false, - SectionMarkers::Dbmate => dbmate_marker(line).is_some(), - SectionMarkers::Goose => goose_annotation(line).is_some(), - }; - - let mut diagnostics = Vec::new(); - if let Some(first) = sections.first() { - let message = format!( - "{tool} rejects a migration with statements before the first `{first_marker}`." - ); - diagnostics.extend( - statements - .iter() - .take_while(|statement| statement.start() < first.range.start()) - .map(|statement| SectionMarkerDiagnostic::new(*statement, &message)), - ); - } - - let mut offset = 0; - for line in text.split_inclusive('\n') { - let start = TextSize::from(offset as u32); - offset += line.len(); - let line = line.trim_end_matches(['\n', '\r']); - let looks_like_marker = || { - let trimmed = line.trim_start(); - is_marker(trimmed) || is_marker(&trimmed.to_ascii_lowercase()) - }; - if !is_marker(line) && looks_like_marker() { - let span = TextRange::at(start, TextSize::of(line)); - // goose fails on an annotation with leading whitespace instead of ignoring it, see - // https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L321 - let message = if markers == SectionMarkers::Goose && line.contains("+goose") { - "goose rejects a migration with an annotation that doesn't start at the beginning of the line." - } else { - ignored_marker - }; - diagnostics.push(SectionMarkerDiagnostic::new(span, message)); - } - } - diagnostics.sort_by_key(|diagnostic| diagnostic.span.start()); - diagnostics -} - -/// `-- migrate:up` or `-- migrate:down` at the start of the line, with options like -/// `transaction:false`. See -/// https://github.com/amacneil/dbmate/blob/v2.37.0/pkg/dbmate/migration.go#L62 -fn dbmate_marker(line: &str) -> Option<(Direction, bool)> { - let directive = line - .strip_prefix("--")? - .trim_start() - .strip_prefix("migrate:")?; - let (direction, options) = if let Some(options) = directive.strip_prefix("up") { - (Direction::Up, options) - } else { - (Direction::Down, directive.strip_prefix("down")?) - }; - if !options.is_empty() && !options.starts_with(char::is_whitespace) { - return None; - } - let transaction = options - .split_whitespace() - .filter_map(|option| option.split_once(':')) - .filter(|(_, value)| !value.contains(':')) - .all(|(key, value)| key != "transaction" || value != "false"); - Some((direction, transaction)) -} - -/// The annotation of a `-- +goose ` line. See -/// https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L319 -fn goose_annotation(line: &str) -> Option<&str> { - let annotation = line - .strip_prefix("--")? - .trim_start() - .strip_prefix("+goose")?; - Some(annotation.trim()) -} - -#[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.sequence_number, 1234567890); - assert_eq!(migration.name, "create_users"); - } - - #[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.sequence_number, 1234567890); - assert_eq!(migration.name, "create_users"); - } - - #[test] - fn test_get_migration_prefix_number() { - let temp_dir = setup(); - let migrations_dir = temp_dir.path().to_path_buf(); - let path = migrations_dir.join("000201_a_migration.sql"); - fs::write(&path, "").unwrap(); - - let migration = get_migration(&path, &migrations_dir); - - assert!(migration.is_some()); - let migration = migration.unwrap(); - assert_eq!(migration.sequence_number, 201); - assert_eq!(migration.name, "a_migration"); - } - - #[test] - fn test_get_migration_not_timestamp_in_filename() { - 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()); - } - - fn sections(text: &str, markers: SectionMarkers) -> Vec<(&str, Direction, bool)> { - migration_sections(text, markers) - .into_iter() - .map(|section| (&text[section.range], section.direction, section.transaction)) - .collect() - } - - #[test] - fn dbmate_sections() { - let text = "-- migrate:up\ncreate table t (id int);\n\n-- migrate:down transaction:false\ndrop table t;\n"; - assert_eq!( - sections(text, SectionMarkers::Dbmate), - vec![ - ( - "-- migrate:up\ncreate table t (id int);\n\n", - Direction::Up, - true - ), - ( - "-- migrate:down transaction:false\ndrop table t;\n", - Direction::Down, - false - ), - ] - ); - } - - #[test] - fn dbmate_options_and_spacing() { - let directions = |text| { - sections(text, SectionMarkers::Dbmate) - .into_iter() - .map(|(_, direction, transaction)| (direction, transaction)) - .collect::>() - }; - assert_eq!( - directions("--migrate:up foo:bar transaction:false\r\nselect 1;"), - vec![(Direction::Up, false)] - ); - assert_eq!( - directions("-- migrate:up transaction:true"), - vec![(Direction::Up, true)] - ); - // Not markers: indented, a longer word, or a different tool. - assert!(directions(" -- migrate:up\n-- migrate:upgrade\n-- +goose Up\n").is_empty()); - } - - #[test] - fn goose_sections() { - let text = "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n-- +goose down\nselect 2;"; - assert_eq!( - sections(text, SectionMarkers::Goose), - vec![ - ( - "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n", - Direction::Up, - true - ), - ("-- +goose down\nselect 2;", Direction::Down, true), - ] - ); - } - - #[test] - fn queries_sent_at_once() { - let queries = |text, markers| { - migration_sections(text, markers) - .into_iter() - .map(|section| { - section - .queries - .iter() - .map(|query| &text[*query]) - .collect::>() - }) - .collect::>() - }; - // dbmate sends each section as one query. - assert_eq!( - queries( - "-- migrate:up transaction:false\nselect 1;\nselect 2;\n-- migrate:down\nselect 3;\n", - SectionMarkers::Dbmate - ), - vec![ - vec!["-- migrate:up transaction:false\nselect 1;\nselect 2;\n"], - vec!["-- migrate:down\nselect 3;\n"], - ] - ); - // goose sends a `StatementBegin` block as one query, and the other statements one by one. - assert_eq!( - queries( - "-- +goose NO TRANSACTION\n-- +goose Up\nselect 1;\n-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n-- +goose Down\nselect 4;\n", - SectionMarkers::Goose - ), - vec![ - vec!["-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n"], - vec![], - ] - ); - } - - fn marker_problems(text: &str, markers: SectionMarkers) -> Vec<(&str, String)> { - let sections = migration_sections(text, markers); - let statements: Vec = pgls_statement_splitter::split(text).ranges; - section_marker_diagnostics(text, markers, §ions, &statements) - .into_iter() - .map(|diagnostic| (&text[diagnostic.span], diagnostic.message.to_string())) - .collect() - } - - #[test] - fn statements_before_the_first_marker() { - let text = - "create table t (id int);\n-- migrate:up\nselect 1;\n-- migrate:down\nselect 2;\n"; - assert_eq!( - marker_problems(text, SectionMarkers::Dbmate), - vec![( - "create table t (id int);", - "dbmate rejects a migration with statements before the first `-- migrate:up`." - .to_string() - )] - ); - let text = "select 1;\n-- +goose Up\nselect 2;\n"; - assert_eq!(marker_problems(text, SectionMarkers::Goose).len(), 1); - // Without markers, the file is one migration. - assert!(marker_problems("select 1;\n", SectionMarkers::Dbmate).is_empty()); - } - - #[test] - fn lines_that_look_like_markers() { - let text = "-- migrate:up\nselect 1;\n-- Migrate:Down\n -- migrate:down\nselect 2;\n-- migrate:upgrade\n"; - let problems = marker_problems(text, SectionMarkers::Dbmate); - assert_eq!( - problems.iter().map(|(line, _)| *line).collect::>(), - ["-- Migrate:Down", " -- migrate:down"] - ); - assert!( - problems[0] - .1 - .starts_with("dbmate only reads `-- migrate:up`") - ); - - let text = - "-- +goose Up\nselect 1;\n-- +Goose Down\n -- +goose StatementBegin\nselect 2;\n"; - let problems = marker_problems(text, SectionMarkers::Goose); - assert_eq!( - problems.iter().map(|(line, _)| *line).collect::>(), - ["-- +Goose Down", " -- +goose StatementBegin"] - ); - assert!(problems[0].1.ends_with("so this line is a plain comment.")); - assert!(problems[1].1.starts_with("goose rejects a migration")); - } - - #[test] - fn goose_no_transaction_applies_to_the_whole_file() { - let text = "-- +goose Up\nselect 1;\n-- +goose Down\nselect 2;\n-- +goose NO TRANSACTION\n"; - assert!( - sections(text, SectionMarkers::Goose) - .iter() - .all(|(_, _, transaction)| !transaction) - ); - } - - #[test] - fn no_markers() { - let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; - assert!(sections(text, SectionMarkers::None).is_empty()); - assert!(sections("select 1;", SectionMarkers::Dbmate).is_empty()); - } -} From 7ae5fbd71bcf0d681b7ec75b1d75c0e626a9f643 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Thu, 5 Nov 2026 10:57:09 +0000 Subject: [PATCH 2/5] refactor(workspace): one model for every migration tool, Supabase included `MigrationTool` now describes everything the language server knows about a tool: how `init` finds its projects and what it sets for them, its default migrations folder, which files are migrations and their sequence numbers, and how a file splits into sections with which marker diagnostics. Plain files and Supabase run every file as one migration, dbmate and goose split it at their markers. `init` goes through `find_projects`, which asks every tool, instead of the Supabase helpers. It also detects dbmate projects (a first migration in `db/migrations` with `-- migrate:up`). goose has no default folder or configuration file, so it isn't detected. --- crates/pgls_cli/src/commands/init.rs | 93 +++-- crates/pgls_cli/tests/assert_init.rs | 45 +++ crates/pgls_workspace/src/migrations.rs | 147 ++++++-- .../pgls_workspace/src/migrations/dbmate.rs | 65 +++- crates/pgls_workspace/src/migrations/files.rs | 9 +- crates/pgls_workspace/src/migrations/goose.rs | 15 +- .../pgls_workspace/src/migrations/project.rs | 318 ++++++++++++++++++ .../pgls_workspace/src/migrations/supabase.rs | 183 ++++------ crates/pgls_workspace/src/workspace/server.rs | 60 ++-- .../src/workspace/server/document.rs | 35 +- docs/guides/checking_migrations.md | 2 + docs/guides/configure_database.md | 2 +- 12 files changed, 734 insertions(+), 240 deletions(-) create mode 100644 crates/pgls_workspace/src/migrations/project.rs diff --git a/crates/pgls_cli/src/commands/init.rs b/crates/pgls_cli/src/commands/init.rs index b8d985ec37..e62d403115 100644 --- a/crates/pgls_cli/src/commands/init.rs +++ b/crates/pgls_cli/src/commands/init.rs @@ -3,33 +3,20 @@ use pgls_configuration::PartialConfiguration; use pgls_console::{ConsoleExt, markup}; use pgls_fs::ConfigName; use pgls_workspace::configuration::create_config; -use pgls_workspace::migrations::supabase::{ - SUPABASE_CONFIG_FILE, SUPABASE_DEFAULT_DB_PORT, detect_supabase_project, - find_supabase_directories, is_inside_supabase_directory, -}; -use std::path::Path; +use pgls_workspace::migrations::project::{Project, configuration_directory, find_projects}; pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { let fs = &mut session.app.fs; let config = &mut PartialConfiguration::init(); - // Inside the `supabase` folder, the configuration goes into the project folder above it, so - // that it applies to the whole project like the Supabase configuration does, and the editor - // finds it when it opens the project. - let project_directory = if is_inside_supabase_directory(&**fs) { - Path::new("..") + let project_directory = configuration_directory(&**fs); + let mut projects = find_projects(&**fs, project_directory); + let (project, ambiguous) = if projects.len() == 1 { + (projects.pop(), Vec::new()) } else { - Path::new("") + (None, projects) }; - let (supabase, ambiguous) = match find_supabase_directories(&**fs, project_directory).as_slice() - { - [directory] => ( - detect_supabase_project(&**fs, project_directory, directory), - Vec::new(), - ), - directories => (None, directories.to_vec()), - }; - if let Some(supabase) = &supabase { - supabase.configure(config); + if let Some(project) = &project { + project.configure(config); } create_config(fs, project_directory, config)?; let file_created = project_directory @@ -46,24 +33,70 @@ Welcome to the Postgres Language Server! Let's get you started... Your project configuration. " }); - if let Some(supabase) = supabase { - let config_path = format!("{}/{SUPABASE_CONFIG_FILE}", supabase.directory); - if supabase.port_unreadable { + if let Some(project) = project { + if let Some(warning) = &project.warning { session.app.console.log(markup! { - "Could not read the database port from "{config_path}", using the default port "{SUPABASE_DEFAULT_DB_PORT}"." + {warning} }); } - let port = supabase.port; - let migrations = supabase.migrations_dir(); + let tool = project.tool; + let directory = &project.directory; + let settings = configured_settings(&project); session.app.console.log(markup! { - "Supabase project detected"" in "{supabase.directory}": the database connection is set to the local Supabase database at 127.0.0.1:"{port}", and the migrations to "{migrations}". + {tool}" project detected"" in "{directory}": "{settings}". " }); } else if !ambiguous.is_empty() { - let directories = ambiguous.join(", "); + let first = ambiguous[0].tool; + let projects = if ambiguous.iter().all(|project| project.tool == first) { + let directories: Vec<&str> = ambiguous + .iter() + .map(|project| project.directory.as_str()) + .collect(); + format!("several {first} projects ({})", directories.join(", ")) + } else { + let directories: Vec = ambiguous + .iter() + .map(|project| format!("{}: {}", project.directory, project.tool)) + .collect(); + format!("several migration projects ({})", directories.join(", ")) + }; session.app.console.log(markup! { - "Found several Supabase projects ("{directories}"), so the database connection is not set. Run ""init"" in the folder of one of them, or set ""db"" and ""migrations.migrationsDir"" yourself." + "Found "{projects}", so the database connection is not set. Run ""init"" in the folder of one of them, or set ""db"" and ""migrations.migrationsDir"" yourself." }); } Ok(()) } + +/// The settings that `init` took from the project, like "the database connection is set to the +/// local Supabase database at 127.0.0.1:54322, and the migrations to supabase/migrations". +fn configured_settings(project: &Project) -> String { + let mut settings = Vec::new(); + if let Some(database) = &project.database { + settings.push(( + "the database connection", + "is", + format!( + "the local {} database at {}:{}", + project.tool, database.host, database.port + ), + )); + } + if let Some(migrations_dir) = &project.migrations_dir { + settings.push(("the migrations", "are", migrations_dir.clone())); + } + if project.section_markers.is_some() { + settings.push(("the section markers", "are", project.tool.to_string())); + } + let mut text = String::new(); + for (index, (setting, verb, value)) in settings.iter().enumerate() { + if index == 0 { + text.push_str(&format!("{setting} {verb} set to {value}")); + } else if index + 1 == settings.len() { + text.push_str(&format!(", and {setting} to {value}")); + } else { + text.push_str(&format!(", {setting} to {value}")); + } + } + text +} diff --git a/crates/pgls_cli/tests/assert_init.rs b/crates/pgls_cli/tests/assert_init.rs index 13fc6a99b4..2633413613 100644 --- a/crates/pgls_cli/tests/assert_init.rs +++ b/crates/pgls_cli/tests/assert_init.rs @@ -205,3 +205,48 @@ fn db_config(port: u16) -> Value { "database": "postgres", }) } + +#[test] +fn init_with_dbmate_project_sets_the_migrations_and_section_markers() { + let dir = tempfile::tempdir().unwrap(); + fs::create_dir_all(dir.path().join("db/migrations")).unwrap(); + fs::write( + dir.path() + .join("db/migrations/20240101000000_create_users.sql"), + "-- migrate:up\ncreate table users (id int);\n\n-- migrate:down\ndrop table users;\n", + ) + .unwrap(); + + let stdout = run_init(dir.path()); + + assert!( + stdout.contains("dbmate project detected in db: the migrations are set to db/migrations, and the section markers to dbmate."), + "{stdout}" + ); + let config = read_config(dir.path()); + assert_eq!(config["migrations"]["migrationsDir"], "db/migrations"); + assert_eq!(config["migrations"]["sectionMarkers"], "dbmate"); + // dbmate reads the database from `DATABASE_URL`, so the connection stays the default one. + assert_eq!(read_db_config(dir.path()), db_config(5432)); +} + +#[test] +fn init_with_supabase_and_dbmate_projects_leaves_the_configuration_alone() { + let dir = tempfile::tempdir().unwrap(); + write_supabase_config(dir.path(), "[db]\nport = 6543\n"); + fs::create_dir_all(dir.path().join("db/migrations")).unwrap(); + fs::write( + dir.path().join("db/migrations/1_init.sql"), + "-- migrate:up\n", + ) + .unwrap(); + + let stdout = run_init(dir.path()); + + assert!( + stdout.contains("Found several migration projects (supabase: Supabase, db: dbmate)"), + "{stdout}" + ); + assert_eq!(read_db_config(dir.path()), db_config(5432)); + assert!(read_config(dir.path()).get("migrations").is_none()); +} diff --git a/crates/pgls_workspace/src/migrations.rs b/crates/pgls_workspace/src/migrations.rs index 50929a442d..b506fb3bee 100644 --- a/crates/pgls_workspace/src/migrations.rs +++ b/crates/pgls_workspace/src/migrations.rs @@ -1,54 +1,123 @@ -//! Migration tools: how they name migration files, how they split a file into sections, and how -//! to find a project of theirs. +//! Migration tools: everything the language server knows about a tool is one [`MigrationTool`]. //! -//! - `files`: which files are migrations, and their sequence numbers, for every tool. -//! - `MigrationTool`: one adapter per tool with section markers, selected with -//! `migrations.sectionMarkers`. A document gets the adapter from the settings of its file, and -//! keeps the sections of its current content. -//! - [`supabase`]: finds the projects of the Supabase CLI for `init`. +//! - How to find a project of the tool, and what `init` sets for it ([`project`]). +//! - Which files are migrations, and their sequence numbers, for `migrations.after`. +//! - How a file splits into up and down sections, and the diagnostics for its markers. +//! +//! The workspace runs a file with the tool of `migrations.sectionMarkers`, see +//! [`migration_tool`]. A document gets the tool from the settings of its file, and keeps the +//! sections of its current content. +//! +//! The tools: +//! - [`Plain`]: no tool, every file is one migration. +//! - [`Supabase`]: the Supabase CLI. Every file is one migration. +//! - [`Dbmate`] and [`Goose`]: up and down sections in one file, separated by marker comments. mod dbmate; mod files; mod goose; mod markers; -pub mod supabase; +pub mod project; +mod supabase; + +use std::path::Path; use pgls_analyser::MigrationSection; use pgls_configuration::migrations::SectionMarkers; +use pgls_fs::FileSystem; use pgls_text_size::TextRange; -pub(crate) use files::{get_migration, is_in_migrations_dir}; +pub(crate) use files::Migration; pub(crate) use markers::SectionMarkerDiagnostic; use dbmate::Dbmate; use goose::Goose; +use project::{Project, Root}; +use supabase::Supabase; -/// A migration tool that puts the up and the down migration in the same file, separated by -/// marker comments, and runs each of them on its own. +/// What the language server knows about a migration tool. Every method but the name and the +/// setting has a default that fits a tool that runs every file as one migration. pub(crate) trait MigrationTool: Sync { + /// The name of the tool, as its users know it. + fn name(&self) -> &'static str; + + /// The value of `migrations.sectionMarkers` that makes the workspace run files with the tool. + fn section_markers(&self) -> SectionMarkers; + + /// The folder of the migrations in a project of the tool, relative to the project, if the + /// tool has one by default. + fn default_migrations_dir(&self) -> Option<&'static str> { + None + } + + /// The project of the tool in `root`, for `init`. Only tools with a reliable sign of their + /// projects find them. + fn detect(&self, _fs: &dyn FileSystem, _root: &Root) -> Option { + None + } + + /// Whether `directory` is the folder of the tool inside a project, like `supabase`, so that + /// `init` configures the project above it. + fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { + false + } + + /// Whether the file is a migration, given `migrations.migrationsDir`. Migration-only rules + /// don't run on other files. + fn is_migration(&self, path: &Path, migrations_dir: &Path) -> bool { + files::is_in_migrations_dir(path, migrations_dir) + } + + /// The migration of a file in the migrations folder, with its sequence number, if its name + /// is one of a migration. + fn migration(&self, path: &Path, migrations_dir: &Path) -> Option { + files::get_migration(path, migrations_dir) + } + /// The sections of a migration file, in file order. A file without markers has none, and is /// one migration. /// /// Markers are found line by line, like the migration tools do, without looking at the SQL. - fn sections(&self, text: &str) -> Vec; + fn sections(&self, _text: &str) -> Vec { + Vec::new() + } /// The problems with the markers of a file with the given sections and statements: /// statements before the first marker, which the tools reject, and lines that look like /// markers but that the tools read as plain comments. fn marker_diagnostics( &self, - text: &str, - sections: &[MigrationSection], - statements: &[TextRange], - ) -> Vec; + _text: &str, + _sections: &[MigrationSection], + _statements: &[TextRange], + ) -> Vec { + Vec::new() + } +} + +/// Migration files without a tool: every file is one migration. +pub(crate) struct Plain; + +impl MigrationTool for Plain { + fn name(&self) -> &'static str { + "plain SQL" + } + + fn section_markers(&self) -> SectionMarkers { + SectionMarkers::None + } } -/// The migration tool of `migrations.sectionMarkers`, or `None` if every file is one migration. -pub(crate) fn migration_tool(markers: SectionMarkers) -> Option<&'static dyn MigrationTool> { +/// The tools, in the order `init` reports their projects. +const TOOLS: &[&dyn MigrationTool] = &[&Supabase, &Dbmate, &Goose, &Plain]; + +/// The tool that the workspace runs files with for `migrations.sectionMarkers`. Supabase runs +/// every file as one migration, so its projects use [`Plain`]. +pub(crate) fn migration_tool(markers: SectionMarkers) -> &'static dyn MigrationTool { match markers { - SectionMarkers::None => None, - SectionMarkers::Dbmate => Some(&Dbmate), - SectionMarkers::Goose => Some(&Goose), + SectionMarkers::None => &Plain, + SectionMarkers::Dbmate => &Dbmate, + SectionMarkers::Goose => &Goose, } } @@ -59,16 +128,14 @@ mod tests { pub(super) fn sections(text: &str, markers: SectionMarkers) -> Vec<(&str, Direction, bool)> { migration_tool(markers) - .map_or_else(Vec::new, |tool| tool.sections(text)) + .sections(text) .into_iter() .map(|section| (&text[section.range], section.direction, section.transaction)) .collect() } pub(super) fn marker_problems(text: &str, markers: SectionMarkers) -> Vec<(&str, String)> { - let Some(tool) = migration_tool(markers) else { - return Vec::new(); - }; + let tool = migration_tool(markers); let sections = tool.sections(text); let statements: Vec = pgls_statement_splitter::split(text).ranges; tool.marker_diagnostics(text, §ions, &statements) @@ -84,4 +151,34 @@ mod tests { assert!(marker_problems(text, SectionMarkers::None).is_empty()); assert!(sections("select 1;", SectionMarkers::Dbmate).is_empty()); } + + #[test] + fn supabase_files_are_one_migration() { + let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; + assert!(Supabase.sections(text).is_empty()); + assert!( + Supabase + .marker_diagnostics(text, &[], &[TextRange::default()]) + .is_empty() + ); + } + + #[test] + fn selects_the_tool_of_the_section_markers() { + for markers in [ + SectionMarkers::None, + SectionMarkers::Dbmate, + SectionMarkers::Goose, + ] { + assert_eq!(migration_tool(markers).section_markers(), markers); + } + assert_eq!(migration_tool(SectionMarkers::None).name(), "plain SQL"); + assert_eq!(migration_tool(SectionMarkers::Dbmate).name(), "dbmate"); + assert_eq!(migration_tool(SectionMarkers::Goose).name(), "goose"); + // Supabase has no markers of its own, so its files run like plain ones. + assert_eq!( + migration_tool(Supabase.section_markers()).name(), + "plain SQL" + ); + } } diff --git a/crates/pgls_workspace/src/migrations/dbmate.rs b/crates/pgls_workspace/src/migrations/dbmate.rs index e27fa141bd..dab757e66f 100644 --- a/crates/pgls_workspace/src/migrations/dbmate.rs +++ b/crates/pgls_workspace/src/migrations/dbmate.rs @@ -1,14 +1,25 @@ //! dbmate: `-- migrate:up` and `-- migrate:down`, see //! https://github.com/amacneil/dbmate/blob/v2.37.0/pkg/dbmate/migration.go +//! +//! Its migrations are `_.sql` files in `db/migrations`, unless +//! `--migrations-dir` says otherwise, see +//! https://github.com/amacneil/dbmate/blob/v2.37.0/README.md#creating-migrations use pgls_analyser::{Direction, MigrationSection}; +use pgls_configuration::migrations::SectionMarkers; +use pgls_fs::{FileSystem, OpenOptions}; use pgls_text_size::TextRange; use super::{ MigrationTool, SectionMarkerDiagnostic, markers::{self, MarkerSyntax, SectionsBuilder, lines}, + project::{self, Project, Root}, }; +/// The folder of dbmate in a project, and its migrations in it. +const DBMATE_DIRECTORY: &str = "db"; +const DBMATE_MIGRATIONS_DIR: &str = "db/migrations"; + pub(super) struct Dbmate; const SYNTAX: MarkerSyntax = MarkerSyntax { @@ -19,6 +30,38 @@ const SYNTAX: MarkerSyntax = MarkerSyntax { }; impl MigrationTool for Dbmate { + fn name(&self) -> &'static str { + "dbmate" + } + + fn section_markers(&self) -> SectionMarkers { + SectionMarkers::Dbmate + } + + fn default_migrations_dir(&self) -> Option<&'static str> { + Some(DBMATE_MIGRATIONS_DIR) + } + + /// A first migration in `db/migrations` with `-- migrate:up`, which dbmate requires in every + /// migration. dbmate has no configuration file, and reads the database from `DATABASE_URL`, + /// so the connection is left alone. + fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { + let migrations_dir = root.path(DBMATE_MIGRATIONS_DIR); + let first = project::files(&migrations_dir) + .into_iter() + .find(|name| is_migration_file_name(name))?; + let mut contents = String::new(); + fs.open_with_options( + &migrations_dir.join(first), + OpenOptions::default().read(true), + ) + .and_then(|mut file| file.read_to_string(&mut contents)) + .ok()?; + lines(&contents) + .any(|(_, line)| marker(line).is_some_and(|(direction, _)| direction == Direction::Up)) + .then(|| Project::new(self, root, DBMATE_DIRECTORY)) + } + fn sections(&self, text: &str) -> Vec { let mut sections = SectionsBuilder::new(text); for (range, line) in lines(text) { @@ -44,6 +87,12 @@ impl MigrationTool for Dbmate { } } +/// `_.sql`, where the version is all leading digits. +fn is_migration_file_name(name: &str) -> bool { + let description = name.trim_start_matches(|c: char| c.is_ascii_digit()); + description.len() < name.len() && description.starts_with('_') && name.ends_with(".sql") +} + /// `-- migrate:up` or `-- migrate:down` at the start of the line, with options like /// `transaction:false`. See /// https://github.com/amacneil/dbmate/blob/v2.37.0/pkg/dbmate/migration.go#L62 @@ -78,6 +127,20 @@ mod tests { tests::{marker_problems, sections}, }; + #[test] + fn migration_file_names() { + assert!(super::is_migration_file_name( + "20151127184807_create_users.sql" + )); + assert!(super::is_migration_file_name("1_init.sql")); + assert!(!super::is_migration_file_name("_init.sql")); + assert!(!super::is_migration_file_name("20151127184807.sql")); + assert!(!super::is_migration_file_name( + "20151127184807_create_users.sql.bak" + )); + assert!(!super::is_migration_file_name("schema.sql")); + } + #[test] fn dbmate_sections() { let text = "-- migrate:up\ncreate table t (id int);\n\n-- migrate:down transaction:false\ndrop table t;\n"; @@ -123,7 +186,7 @@ mod tests { let text = "-- migrate:up transaction:false\nselect 1;\nselect 2;\n-- migrate:down\nselect 3;\n"; let queries = migration_tool(SectionMarkers::Dbmate) - .map_or_else(Vec::new, |tool| tool.sections(text)) + .sections(text) .into_iter() .map(|section| { section diff --git a/crates/pgls_workspace/src/migrations/files.rs b/crates/pgls_workspace/src/migrations/files.rs index d254473b75..03ae50e49a 100644 --- a/crates/pgls_workspace/src/migrations/files.rs +++ b/crates/pgls_workspace/src/migrations/files.rs @@ -1,5 +1,6 @@ -//! Which files are migrations, and their sequence numbers. The tools name the files the same way, -//! so this is shared by all of them. +//! Which files are migrations, and their sequence numbers: the defaults of +//! [`super::MigrationTool::is_migration`] and [`super::MigrationTool::migration`]. Supabase, +//! dbmate and goose name the files the same way, `_.sql`. use std::path::Path; @@ -11,7 +12,7 @@ pub(crate) struct Migration { } /// Whether the path is inside the migrations directory. -pub(crate) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { +pub(super) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { if path.starts_with(migrations_dir) { return true; } @@ -27,7 +28,7 @@ pub(crate) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { } /// Get the migration associated with a path, if it is a migration file -pub(crate) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { +pub(super) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { if !is_in_migrations_dir(path, migrations_dir) { return None; } diff --git a/crates/pgls_workspace/src/migrations/goose.rs b/crates/pgls_workspace/src/migrations/goose.rs index f5fde5209c..0e21a38e45 100644 --- a/crates/pgls_workspace/src/migrations/goose.rs +++ b/crates/pgls_workspace/src/migrations/goose.rs @@ -1,7 +1,12 @@ //! goose: `-- +goose Up`, `-- +goose Down` and the other `-- +goose` annotations, see //! https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go +//! +//! goose has no configuration file and no migrations folder of its own: it runs the migrations +//! in the folder of `-dir` or `GOOSE_MIGRATION_DIR`, `.` by default. So its projects aren't +//! detected. use pgls_analyser::{Direction, MigrationSection}; +use pgls_configuration::migrations::SectionMarkers; use pgls_text_size::TextRange; use super::{ @@ -27,6 +32,14 @@ const SYNTAX: MarkerSyntax = MarkerSyntax { }; impl MigrationTool for Goose { + fn name(&self) -> &'static str { + "goose" + } + + fn section_markers(&self) -> SectionMarkers { + SectionMarkers::Goose + } + fn sections(&self, text: &str) -> Vec { let mut sections = SectionsBuilder::new(text); let mut no_transaction = false; @@ -119,7 +132,7 @@ mod tests { fn queries_sent_at_once() { let text = "-- +goose NO TRANSACTION\n-- +goose Up\nselect 1;\n-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n-- +goose Down\nselect 4;\n"; let queries = migration_tool(SectionMarkers::Goose) - .map_or_else(Vec::new, |tool| tool.sections(text)) + .sections(text) .into_iter() .map(|section| { section diff --git a/crates/pgls_workspace/src/migrations/project.rs b/crates/pgls_workspace/src/migrations/project.rs new file mode 100644 index 0000000000..f2f218e2f3 --- /dev/null +++ b/crates/pgls_workspace/src/migrations/project.rs @@ -0,0 +1,318 @@ +//! The projects of the migration tools: `init` finds them and points the configuration at them. + +use std::path::{Path, PathBuf}; + +use pgls_configuration::PartialConfiguration; +use pgls_configuration::migrations::{PartialMigrationsConfiguration, SectionMarkers}; +use pgls_fs::FileSystem; + +use super::{MigrationTool, TOOLS}; + +/// Folders that never hold the project of a migration tool in a monorepo. +const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", "supabase"]; + +/// A project of a migration tool, and the settings it tells. +#[derive(Debug)] +pub struct Project { + /// The name of the tool. + pub tool: &'static str, + /// The folder of the tool in the project, like `supabase` or `apps/web/supabase`, relative to + /// the folder of the configuration file, with `/` as the separator. + pub directory: String, + /// The folder of the migrations, relative to the folder of the configuration file. + pub migrations_dir: Option, + /// The section markers of the tool's migration files, if it has any. + pub section_markers: Option, + /// The local database of the project, if the tool runs one. + pub database: Option, + /// A problem with the project that doesn't stop `init`, like a port it couldn't read. + pub warning: Option, +} + +/// The local database of a project, like the one of `supabase start`. +#[derive(Debug)] +pub struct LocalDatabase { + pub host: &'static str, + pub port: u16, + pub username: &'static str, + pub password: &'static str, + pub database: &'static str, +} + +impl Project { + /// A project of `tool` with the tool's folder `directory` in `root`, and the tool's default + /// migrations folder. + pub(super) fn new(tool: &dyn MigrationTool, root: &Root, directory: &str) -> Self { + Self { + tool: tool.name(), + directory: root.relative(directory), + migrations_dir: tool + .default_migrations_dir() + .map(|migrations_dir| root.relative(migrations_dir)), + section_markers: Some(tool.section_markers()) + .filter(|markers| *markers != SectionMarkers::None), + database: None, + warning: None, + } + } + + /// Points the database connection at the local database of the project, and the migrations + /// at the migrations of the project. + pub fn configure(&self, config: &mut PartialConfiguration) { + if let Some(database) = &self.database { + let db = config.db.get_or_insert_with(Default::default); + db.host = Some(database.host.to_string()); + db.port = Some(database.port); + db.username = Some(database.username.to_string()); + db.password = Some(database.password.to_string()); + db.database = Some(database.database.to_string()); + // Statement execution stays off: enabling it is the user's call. + } + if self.migrations_dir.is_some() || self.section_markers.is_some() { + config.migrations = Some(PartialMigrationsConfiguration { + migrations_dir: self.migrations_dir.clone(), + section_markers: self.section_markers, + ..Default::default() + }); + } + } +} + +/// A folder that may hold the project of a migration tool. +pub(crate) struct Root { + /// The folder, for the file system. + path: PathBuf, + /// The folder relative to the folder of the configuration file, with `/` as the separator and + /// at the end, or empty for that folder. + prefix: String, +} + +impl Root { + /// A path in the folder, for the file system. + pub(super) fn path(&self, relative: &str) -> PathBuf { + self.path.join(relative) + } + + /// A path in the folder, relative to the folder of the configuration file. + pub(super) fn relative(&self, relative: &str) -> String { + format!("{}{relative}", self.prefix) + } +} + +/// The folder of the configuration, relative to the working directory: the folder above it if +/// the working directory is the folder of a tool inside a project, like `supabase`, so that the +/// configuration applies to the whole project like the tool's does, and the editor finds it +/// when it opens the project. +pub fn configuration_directory(fs: &dyn FileSystem) -> &'static Path { + let working_directory = fs.working_directory().unwrap_or_default(); + if TOOLS + .iter() + .any(|tool| tool.is_tool_directory(fs, &working_directory)) + { + Path::new("..") + } else { + Path::new("") + } +} + +/// The projects of the migration tools for the configuration in `configuration_directory`, +/// relative to the working directory: the ones in it, or else, if it is the working directory, +/// the ones in the folders one or two levels below it, as in a monorepo (`supabase/`, +/// `db/supabase/`, `apps/web/supabase/`). +pub fn find_projects(fs: &dyn FileSystem, configuration_directory: &Path) -> Vec { + let working_directory = fs.working_directory().unwrap_or_default(); + let detect = |relative: &str| { + let root = Root { + path: working_directory + .join(configuration_directory) + .join(relative), + prefix: if relative.is_empty() { + String::new() + } else { + format!("{relative}/") + }, + }; + TOOLS + .iter() + .filter_map(|tool| tool.detect(fs, &root)) + .collect::>() + }; + let found = detect(""); + if !found.is_empty() || !configuration_directory.as_os_str().is_empty() { + return found; + } + let mut found = Vec::new(); + for child in subdirectories(&working_directory) { + let projects = detect(&child); + if !projects.is_empty() { + found.extend(projects); + continue; + } + for grandchild in subdirectories(&working_directory.join(&child)) { + found.extend(detect(&format!("{child}/{grandchild}"))); + } + } + found +} + +/// The names of the folders in `directory`, sorted, without hidden and skipped ones. +fn subdirectories(directory: &Path) -> Vec { + entries(directory, true) + .into_iter() + .filter(|name| !name.starts_with('.') && !SKIPPED_DIRECTORIES.contains(&name.as_str())) + .collect() +} + +/// The names of the files in `directory`, sorted. +pub(super) fn files(directory: &Path) -> Vec { + entries(directory, false) +} + +fn entries(directory: &Path, directories: bool) -> Vec { + let Ok(entries) = std::fs::read_dir(directory) else { + return Vec::new(); + }; + let mut names: Vec = entries + .filter_map(Result::ok) + .filter(|entry| { + entry + .file_type() + .is_ok_and(|file_type| file_type.is_dir() == directories) + }) + .filter_map(|entry| entry.file_name().into_string().ok()) + .collect(); + names.sort(); + names +} + +#[cfg(test)] +mod tests { + use std::fs; + + use pgls_fs::OsFileSystem; + use tempfile::TempDir; + + use super::*; + + fn find(dir: &TempDir) -> Vec<(String, &'static str, Option)> { + let fs = OsFileSystem::new(dir.path().to_path_buf()); + find_projects(&fs, Path::new("")) + .into_iter() + .map(|project| (project.directory, project.tool, project.migrations_dir)) + .collect() + } + + fn write(dir: &TempDir, path: &str, contents: &str) { + let path = dir.path().join(path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); + } + + #[test] + fn finds_a_supabase_project() { + let dir = TempDir::new().unwrap(); + write(&dir, "supabase/config.toml", "[db]\nport = 6543\n"); + let fs = OsFileSystem::new(dir.path().to_path_buf()); + let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); + assert_eq!(project.tool, "Supabase"); + assert_eq!(project.directory, "supabase"); + assert_eq!( + project.migrations_dir.as_deref(), + Some("supabase/migrations") + ); + assert_eq!(project.section_markers, None); + assert_eq!(project.database.map(|database| database.port), Some(6543)); + assert!(project.warning.is_none()); + } + + #[test] + fn finds_a_dbmate_project() { + let dir = TempDir::new().unwrap(); + write( + &dir, + "db/migrations/20240101000000_create_users.sql", + "-- migrate:up\ncreate table users (id int);\n\n-- migrate:down\ndrop table users;\n", + ); + let fs = OsFileSystem::new(dir.path().to_path_buf()); + let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); + assert_eq!(project.tool, "dbmate"); + assert_eq!(project.directory, "db"); + assert_eq!(project.migrations_dir.as_deref(), Some("db/migrations")); + assert_eq!(project.section_markers, Some(SectionMarkers::Dbmate)); + assert!(project.database.is_none()); + + let mut config = PartialConfiguration::default(); + project.configure(&mut config); + let migrations = config.migrations.unwrap(); + assert_eq!(migrations.migrations_dir.as_deref(), Some("db/migrations")); + assert_eq!(migrations.section_markers, Some(SectionMarkers::Dbmate)); + assert!(config.db.is_none()); + } + + #[test] + fn db_migrations_without_dbmate_markers_are_no_project() { + let dir = TempDir::new().unwrap(); + write( + &dir, + "db/migrations/20240101000000_create_users.sql", + "create table users (id int);\n", + ); + write(&dir, "db/migrations/notes.sql", "-- migrate:up\n"); + assert!(find(&dir).is_empty()); + } + + #[test] + fn finds_the_projects_below_the_working_directory() { + let dir = TempDir::new().unwrap(); + write(&dir, "apps/web/supabase/config.toml", ""); + write( + &dir, + "services/api/db/migrations/1_init.sql", + "-- migrate:up\n", + ); + write(&dir, "node_modules/pkg/supabase/config.toml", ""); + assert_eq!( + find(&dir), + vec![ + ( + "apps/web/supabase".to_string(), + "Supabase", + Some("apps/web/supabase/migrations".to_string()) + ), + ( + "services/api/db".to_string(), + "dbmate", + Some("services/api/db/migrations".to_string()) + ), + ] + ); + } + + #[test] + fn the_projects_in_the_working_directory_come_first() { + let dir = TempDir::new().unwrap(); + write(&dir, "supabase/config.toml", ""); + write(&dir, "db/migrations/1_init.sql", "-- migrate:up\n"); + write(&dir, "apps/web/supabase/config.toml", ""); + assert_eq!( + find(&dir) + .into_iter() + .map(|(directory, tool, _)| (directory, tool)) + .collect::>(), + vec![ + ("supabase".to_string(), "Supabase"), + ("db".to_string(), "dbmate") + ] + ); + } + + #[test] + fn configures_the_project_above_the_supabase_folder() { + let dir = TempDir::new().unwrap(); + write(&dir, "supabase/config.toml", "[db]\nport = 6543\n"); + let fs = OsFileSystem::new(dir.path().join("supabase")); + assert_eq!(configuration_directory(&fs), Path::new("..")); + let fs = OsFileSystem::new(dir.path().to_path_buf()); + assert_eq!(configuration_directory(&fs), Path::new("")); + } +} diff --git a/crates/pgls_workspace/src/migrations/supabase.rs b/crates/pgls_workspace/src/migrations/supabase.rs index 6b2390c57e..c838da22d3 100644 --- a/crates/pgls_workspace/src/migrations/supabase.rs +++ b/crates/pgls_workspace/src/migrations/supabase.rs @@ -1,148 +1,83 @@ -//! Supabase: finds the projects of the Supabase CLI and their local database, so that `init` can -//! point the configuration at them. +//! Supabase: the projects of the Supabase CLI and their local database. The Supabase CLI runs +//! every migration file as one migration. use std::ffi::OsStr; use std::path::Path; -use pgls_configuration::PartialConfiguration; -use pgls_configuration::migrations::PartialMigrationsConfiguration; +use pgls_configuration::migrations::SectionMarkers; use pgls_fs::{FileSystem, OpenOptions}; +use super::{ + MigrationTool, + project::{LocalDatabase, Project, Root}, +}; + /// The folder of a project managed by the Supabase CLI, and its configuration file in it. const SUPABASE_DIRECTORY: &str = "supabase"; -pub const SUPABASE_CONFIG_FILE: &str = "config.toml"; +const SUPABASE_CONFIG_FILE: &str = "config.toml"; /// The port of the local Supabase database if `[db] port` is not set. -pub const SUPABASE_DEFAULT_DB_PORT: u16 = 54322; - -/// Folders that never hold the Supabase project of a monorepo. -const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", SUPABASE_DIRECTORY]; - -/// A Supabase project and its local database, as reported by `supabase status`. -pub struct SupabaseProject { - /// The `supabase` folder, relative to the folder of the configuration file, with `/` as the - /// separator. - pub directory: String, - pub port: u16, - /// Whether the port could not be read from the Supabase configuration. - pub port_unreadable: bool, -} +const SUPABASE_DEFAULT_DB_PORT: u16 = 54322; -impl SupabaseProject { - /// The folder of the migrations, relative to the folder of the configuration file. - pub fn migrations_dir(&self) -> String { - format!("{}/migrations", self.directory) - } +pub(super) struct Supabase; - /// Points the database connection at the local Supabase database, and the migrations at the - /// migrations of the project. - pub fn configure(&self, config: &mut PartialConfiguration) { - let db = config.db.get_or_insert_with(Default::default); - db.host = Some("127.0.0.1".to_string()); - db.port = Some(self.port); - db.username = Some("postgres".to_string()); - db.password = Some("postgres".to_string()); - db.database = Some("postgres".to_string()); - // Statement execution stays off: enabling it is the user's call. - config.migrations = Some(PartialMigrationsConfiguration { - migrations_dir: Some(self.migrations_dir()), - ..Default::default() - }); +impl MigrationTool for Supabase { + fn name(&self) -> &'static str { + "Supabase" } -} -/// Whether the working directory is the `supabase` folder of a project: it is named like it and -/// has a `config.toml` with a `[db]` section. -pub fn is_inside_supabase_directory(fs: &dyn FileSystem) -> bool { - let is_named_supabase = fs - .working_directory() - .is_some_and(|directory| directory.file_name() == Some(OsStr::new(SUPABASE_DIRECTORY))); - let mut contents = String::new(); - is_named_supabase - && fs - .open_with_options( - Path::new(SUPABASE_CONFIG_FILE), - OpenOptions::default().read(true), - ) - .and_then(|mut file| file.read_to_string(&mut contents)) - .is_ok() - && contents - .parse::() - .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) -} - -/// The `supabase` folders with a `config.toml`, relative to `project_directory`: the one in it, -/// or else, if it is the working directory, the ones in the folders one or two levels below it, -/// as in a monorepo (`supabase/`, `db/supabase/`, `apps/web/supabase/`). -pub fn find_supabase_directories(fs: &dyn FileSystem, project_directory: &Path) -> Vec { - let is_project = |directory: &Path| { - fs.path_is_file( - &directory - .join(SUPABASE_DIRECTORY) - .join(SUPABASE_CONFIG_FILE), - ) - }; - if is_project(project_directory) { - return vec![SUPABASE_DIRECTORY.to_string()]; + fn section_markers(&self) -> SectionMarkers { + SectionMarkers::None } - if !project_directory.as_os_str().is_empty() { - return Vec::new(); + + fn default_migrations_dir(&self) -> Option<&'static str> { + Some("supabase/migrations") } - let Some(working_directory) = fs.working_directory() else { - return Vec::new(); - }; - let mut found = Vec::new(); - for child in subdirectories(&working_directory) { - if is_project(&working_directory.join(&child)) { - found.push(format!("{child}/{SUPABASE_DIRECTORY}")); - continue; + + /// A `supabase` folder with a `config.toml`, and the local database of `supabase status`. + fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { + let path = root.path(SUPABASE_DIRECTORY).join(SUPABASE_CONFIG_FILE); + if !fs.path_is_file(&path) { + return None; } - for grandchild in subdirectories(&working_directory.join(&child)) { - if is_project(&working_directory.join(&child).join(&grandchild)) { - found.push(format!("{child}/{grandchild}/{SUPABASE_DIRECTORY}")); - } + let mut contents = String::new(); + let port = fs + .open_with_options(&path, OpenOptions::default().read(true)) + .and_then(|mut file| file.read_to_string(&mut contents)) + .ok() + .and_then(|_| supabase_db_port(&contents)); + let mut project = Project::new(self, root, SUPABASE_DIRECTORY); + if port.is_none() { + project.warning = Some(format!( + "Could not read the database port from {}/{SUPABASE_CONFIG_FILE}, using the default port {SUPABASE_DEFAULT_DB_PORT}.", + project.directory + )); } + project.database = Some(LocalDatabase { + host: "127.0.0.1", + port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), + username: "postgres", + password: "postgres", + database: "postgres", + }); + Some(project) } - found -} - -/// The names of the folders in `directory`, sorted, without hidden and skipped ones. -fn subdirectories(directory: &Path) -> Vec { - let Ok(entries) = std::fs::read_dir(directory) else { - return Vec::new(); - }; - let mut names: Vec = entries - .filter_map(Result::ok) - .filter(|entry| entry.file_type().is_ok_and(|file_type| file_type.is_dir())) - .filter_map(|entry| entry.file_name().into_string().ok()) - .filter(|name| !name.starts_with('.') && !SKIPPED_DIRECTORIES.contains(&name.as_str())) - .collect(); - names.sort(); - names -} -/// Reads the Supabase project in `directory`, relative to `project_directory`, which is relative -/// to the working directory. -pub fn detect_supabase_project( - fs: &dyn FileSystem, - project_directory: &Path, - directory: &str, -) -> Option { - let path = project_directory.join(directory).join(SUPABASE_CONFIG_FILE); - if !fs.path_is_file(&path) { - return None; + /// The folder is named `supabase` and has a `config.toml` with a `[db]` section. + fn is_tool_directory(&self, fs: &dyn FileSystem, directory: &Path) -> bool { + let mut contents = String::new(); + directory.file_name() == Some(OsStr::new(SUPABASE_DIRECTORY)) + && fs + .open_with_options( + &directory.join(SUPABASE_CONFIG_FILE), + OpenOptions::default().read(true), + ) + .and_then(|mut file| file.read_to_string(&mut contents)) + .is_ok() + && contents + .parse::() + .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) } - let mut contents = String::new(); - let port = fs - .open_with_options(&path, OpenOptions::default().read(true)) - .and_then(|mut file| file.read_to_string(&mut contents)) - .ok() - .and_then(|_| supabase_db_port(&contents)); - Some(SupabaseProject { - directory: directory.to_string(), - port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), - port_unreadable: port.is_none(), - }) } /// Returns `[db] port` of a Supabase configuration, or the default port if it is not set. diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index a1392f36b1..0acacfa8e9 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -222,15 +222,14 @@ impl WorkspaceServer { } /// The migration tool of the settings that apply to the file. - fn migration_tool( - &self, - project_key: ProjectKey, - path: &Path, - ) -> Option<&'static dyn MigrationTool> { - self.workspaces() + fn migration_tool(&self, project_key: ProjectKey, path: &Path) -> &'static dyn MigrationTool { + let section_markers = self + .workspaces() .settings_for(project_key, path) .and_then(|settings| settings.migrations.as_ref()) - .and_then(|migrations| migrations::migration_tool(migrations.section_markers)) + .map(|migrations| migrations.section_markers) + .unwrap_or_default(); + migrations::migration_tool(section_markers) } /// Applies the changed settings of a project to its open documents. @@ -262,16 +261,16 @@ impl WorkspaceServer { /// Whether the file is a migration. Migration-only rules don't run on other files. fn file_kind(&self, settings: &Settings, path: &Path) -> FileKind { - match settings - .migrations - .as_ref() - .and_then(|migrations| migrations.path.as_ref()) - { - Some(migrations_dir) if migrations::is_in_migrations_dir(path, migrations_dir) => { - FileKind::Migration - } - Some(_) => FileKind::Other, - None => FileKind::Unknown, + let Some((tool, migrations_dir)) = settings.migrations.as_ref().and_then(|migrations| { + let tool = migrations::migration_tool(migrations.section_markers); + Some((tool, migrations.path.as_ref()?)) + }) else { + return FileKind::Unknown; + }; + if tool.is_migration(path, migrations_dir) { + FileKind::Migration + } else { + FileKind::Other } } @@ -932,7 +931,8 @@ fn is_ignored_by_migration_settings(settings: &Settings, path: &Path) -> bool { .and_then(|migration_settings| { let ignore_before = migration_settings.after.as_ref()?; let migrations_dir = migration_settings.path.as_ref()?; - let migration = migrations::get_migration(path, migrations_dir)?; + let migration = migrations::migration_tool(migration_settings.section_markers) + .migration(path, migrations_dir)?; Some(&migration.sequence_number <= ignore_before) }) @@ -1418,20 +1418,16 @@ impl Workspace for WorkspaceServer { let mut diagnostics: Vec = doc.document_diagnostics().to_vec(); let path = params.path.as_path().display().to_string(); - if let Some(migration_tool) = doc.migration_tool() { - diagnostics.extend( - migration_tool - .marker_diagnostics( - doc.get_document_content(), - doc.migration_sections(), - doc.statement_ranges(), - ) - .into_iter() - .map(|diagnostic| { - SDiagnostic::new(Error::from(diagnostic).with_file_path(&path)) - }), - ); - } + diagnostics.extend( + doc.migration_tool() + .marker_diagnostics( + doc.get_document_content(), + doc.migration_sections(), + doc.statement_ranges(), + ) + .into_iter() + .map(|diagnostic| SDiagnostic::new(Error::from(diagnostic).with_file_path(&path))), + ); let (analysis, syntax_diagnostics, disabled_rules, skipped_sections) = self .analyse_document( diff --git a/crates/pgls_workspace/src/workspace/server/document.rs b/crates/pgls_workspace/src/workspace/server/document.rs index 86ec8d4f90..2452c12f3a 100644 --- a/crates/pgls_workspace/src/workspace/server/document.rs +++ b/crates/pgls_workspace/src/workspace/server/document.rs @@ -7,6 +7,8 @@ use pgls_suppressions::Suppressions; use pgls_text_size::{TextRange, TextSize}; use rustc_hash::FxHashSet; +#[cfg(test)] +use crate::migrations::Plain; use crate::{migrations::MigrationTool, workspace::ProjectKey}; use super::{ @@ -29,23 +31,23 @@ pub struct Document { /// The project whose settings apply to the document. project_key: ProjectKey, /// The migration tool that runs the file, from its settings. See [`Document::open`]. - migration_tool: Option<&'static dyn MigrationTool>, + migration_tool: &'static dyn MigrationTool, /// The sections of the content for `migration_tool`. migration_sections: Vec, } impl Document { - /// A document without a project, and so without a migration tool. + /// A document without a project, and so without a migration tool: it is one migration. #[cfg(test)] pub fn new(content: String, version: i32) -> Document { - Self::open(ProjectKey::default(), None, content, version) + Self::open(ProjectKey::default(), &Plain, content, version) } /// A document of the project `project_key`, whose settings make `migration_tool` run the /// file. When the settings change, [`Document::set_migration_tool`] applies them. pub(crate) fn open( project_key: ProjectKey, - migration_tool: Option<&'static dyn MigrationTool>, + migration_tool: &'static dyn MigrationTool, content: String, version: i32, ) -> Document { @@ -54,7 +56,7 @@ impl Document { let annotation_db = AnnotationStore::new(); let (ranges, diagnostics) = split_with_diagnostics(&content, None); let suppressions = Suppressions::from(content.as_str()).with_statements(ranges.clone()); - let migration_sections = sections_for(migration_tool, &content); + let migration_sections = migration_tool.sections(&content); Document { ranges, @@ -74,7 +76,7 @@ impl Document { pub fn update_content(&mut self, content: String, version: i32) { self.content = content; self.version = version; - self.migration_sections = sections_for(self.migration_tool, &self.content); + self.migration_sections = self.migration_tool.sections(&self.content); let (ranges, diagnostics) = split_with_diagnostics(&self.content, None); @@ -97,25 +99,22 @@ impl Document { } /// Applies the migration tool of changed settings. - pub(crate) fn set_migration_tool( - &mut self, - migration_tool: Option<&'static dyn MigrationTool>, - ) { + pub(crate) fn set_migration_tool(&mut self, migration_tool: &'static dyn MigrationTool) { self.migration_tool = migration_tool; - self.migration_sections = sections_for(migration_tool, &self.content); + self.migration_sections = migration_tool.sections(&self.content); } pub(crate) fn project_key(&self) -> ProjectKey { self.project_key } - /// The migration tool that runs the file, if its settings name one. - pub(crate) fn migration_tool(&self) -> Option<&'static dyn MigrationTool> { + /// The migration tool that runs the file, from its settings. + pub(crate) fn migration_tool(&self) -> &'static dyn MigrationTool { self.migration_tool } /// The migration sections of the document, like the up and down migrations of dbmate and - /// goose. Without a migration tool, there are none. + /// goose. A tool without section markers has none. pub fn migration_sections(&self) -> &[MigrationSection] { &self.migration_sections } @@ -483,14 +482,6 @@ impl StatementFilter<'_> for IdFilter { } } -/// The migration sections of `content`, see [`MigrationTool::sections`]. -fn sections_for( - migration_tool: Option<&'static dyn MigrationTool>, - content: &str, -) -> Vec { - migration_tool.map_or_else(Vec::new, |tool| tool.sections(content)) -} - /// Helper function that wraps the statement splitter and returns the ranges with unified /// diagnostics pub(crate) fn split_with_diagnostics( diff --git a/docs/guides/checking_migrations.md b/docs/guides/checking_migrations.md index 7763fa2849..39ea30d7f2 100644 --- a/docs/guides/checking_migrations.md +++ b/docs/guides/checking_migrations.md @@ -60,6 +60,8 @@ Some migration tools put the up and the down migration in the same file, separat | `"goose"` | `-- +goose Up`, `-- +goose Down` | `-- +goose NO TRANSACTION` (whole file) | | `"none"` (default) | | | +In a dbmate project, `postgres-language-server init` sets both: it detects dbmate by a first migration in `db/migrations`, dbmate's default folder, that starts with `-- migrate:up`. goose has no default folder or configuration file, so `init` doesn't detect it. + With section markers: - The transaction, lock, and session state starts fresh in each section. A lock taken or a `lock_timeout` set in the up section doesn't carry over to the down section. diff --git a/docs/guides/configure_database.md b/docs/guides/configure_database.md index e46693f001..020ebc6498 100644 --- a/docs/guides/configure_database.md +++ b/docs/guides/configure_database.md @@ -43,7 +43,7 @@ Configure database connection details in your `postgres-language-server.jsonc` f When you run `postgres-language-server init` in a directory that contains a `supabase/config.toml`, the generated configuration connects to the local Supabase database started by `supabase start`: host `127.0.0.1`, the port from `[db] port` in `supabase/config.toml` (default: `54322`), and `postgres` as username, password, and database. If the port cannot be read from `supabase/config.toml`, `init` uses `54322` and prints a warning. It also sets `migrations.migrationsDir` to `supabase/migrations`, so that the [migration rules](checking_migrations.md) apply to your migrations. Executing statements stays disabled; allow it with `db.allowStatementExecutionsAgainst` if you want to. -In a monorepo, run `init` at the root: if the root has no `supabase/config.toml`, `init` looks for one in the folders one or two levels below it, such as `apps/web/supabase/config.toml`, and uses it if there is exactly one. With several, it leaves the connection alone and lists them, so run `init` in the folder of the project you want instead. +In a monorepo, run `init` at the root: if the root has no `supabase/config.toml`, `init` looks for one in the folders one or two levels below it, such as `apps/web/supabase/config.toml`, and uses it if there is exactly one. With several, or with a [dbmate project](checking_migrations.md#up-and-down-migrations-in-one-file) next to it, it leaves the connection alone and lists them, so run `init` in the folder of the project you want instead. Run inside the `supabase/` folder itself (a folder named `supabase` with a `config.toml` that has a `[db]` section), `init` writes the configuration into the project folder above it, next to `supabase/`, so that it applies to the whole project and the editor finds it when it opens the project. From 2351e8c8b196e5e412abddb4e705b3b14fabad18 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 9 Nov 2026 07:55:24 +0000 Subject: [PATCH 3/5] refactor(migrations): one MigrationTool trait without defaults, in its own crate Move the migration tools out of pgls_workspace into the new leaf crate pgls_migrations. Every tool (plain SQL, Supabase, dbmate, goose) implements every method of the trait: id, name, detect, is_tool_directory, default_dir, file, version and read. What tools share are helpers they call: naming, directives, units and detect. read() returns a Script with the units of the file (replacing pgls_analyser::MigrationSection) and the marker problems; the document keeps it per version. file() returns the role and version of a path, used for the migration-only rules and migrations.after. init uses detect::find_projects and writes the configuration itself. No behaviour change. --- AGENTS.md | 1 + ARCHITECTURE.md | 4 + Cargo.lock | 16 +- Cargo.toml | 1 + crates/pgls_analyser/Cargo.toml | 1 + crates/pgls_analyser/src/lib.rs | 20 +- crates/pgls_analyser/src/migration.rs | 36 ---- crates/pgls_analyser/tests/rules_tests.rs | 11 +- crates/pgls_cli/Cargo.toml | 1 + crates/pgls_cli/src/commands/init.rs | 58 +++++- crates/pgls_migrations/Cargo.toml | 28 +++ .../src/detect.rs} | 74 +++---- crates/pgls_migrations/src/directives.rs | 59 ++++++ crates/pgls_migrations/src/lib.rs | 183 +++++++++++++++++ crates/pgls_migrations/src/naming.rs | 138 +++++++++++++ .../src/tools}/dbmate.rs | 109 ++++++----- .../src/tools}/goose.rs | 117 ++++++----- crates/pgls_migrations/src/tools/mod.rs | 11 ++ crates/pgls_migrations/src/tools/plain.rs | 61 ++++++ .../src/tools}/supabase.rs | 43 ++-- crates/pgls_migrations/src/units.rs | 63 ++++++ crates/pgls_workspace/Cargo.toml | 2 +- crates/pgls_workspace/src/lib.rs | 1 - crates/pgls_workspace/src/migrations.rs | 184 ------------------ crates/pgls_workspace/src/migrations/files.rs | 150 -------------- .../pgls_workspace/src/migrations/markers.rs | 128 ------------ crates/pgls_workspace/src/workspace/server.rs | 53 ++--- .../src/workspace/server.tests.rs | 1 + .../src/workspace/server/document.rs | 29 +-- 29 files changed, 865 insertions(+), 718 deletions(-) delete mode 100644 crates/pgls_analyser/src/migration.rs create mode 100644 crates/pgls_migrations/Cargo.toml rename crates/{pgls_workspace/src/migrations/project.rs => pgls_migrations/src/detect.rs} (78%) create mode 100644 crates/pgls_migrations/src/directives.rs create mode 100644 crates/pgls_migrations/src/lib.rs create mode 100644 crates/pgls_migrations/src/naming.rs rename crates/{pgls_workspace/src/migrations => pgls_migrations/src/tools}/dbmate.rs (75%) rename crates/{pgls_workspace/src/migrations => pgls_migrations/src/tools}/goose.rs (66%) create mode 100644 crates/pgls_migrations/src/tools/mod.rs create mode 100644 crates/pgls_migrations/src/tools/plain.rs rename crates/{pgls_workspace/src/migrations => pgls_migrations/src/tools}/supabase.rs (80%) create mode 100644 crates/pgls_migrations/src/units.rs delete mode 100644 crates/pgls_workspace/src/migrations.rs delete mode 100644 crates/pgls_workspace/src/migrations/files.rs delete mode 100644 crates/pgls_workspace/src/migrations/markers.rs diff --git a/AGENTS.md b/AGENTS.md index e52dfecced..eeccb031a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,6 +127,7 @@ The project uses a modular Rust workspace with crates prefixed with `pgls_`: - `pgls_text_edit` - Text manipulation utilities - `pgls_line_index` - Converts offsets to line/column positions in UTF-8, UTF-16 or UTF-32 and back, and the character positions that Postgres reports to offsets. Convert positions with it, not by hand - `pgls_suppressions` - Rule suppression handling +- `pgls_migrations` - Migration tools (Supabase, dbmate, goose): migration files and versions, up/down sections, project detection for `init` ### TypeScript Packages Located in `packages/`: diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ee6082f0d1..60c9e9f2f6 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -101,6 +101,10 @@ The fallback type check against the database (via `EXPLAIN`), for statements tha Handles `-- pgls-ignore` style rule suppression comments. +##### `crates/pgls_migrations` + +What the language server knows about migration tools (Supabase, dbmate, goose, and plain SQL files). Every tool implements every method of one `MigrationTool` trait, and calls shared helpers for what it does like other tools: which files are migrations and their versions, how a file splits into up and down sections with their transaction mode, the diagnostics for misplaced markers, and how `init` finds the tool's projects. The workspace keeps what the tool of a file's settings says about its text, and the analyser runs each section on its own. + #### Feature crates ##### `crates/pgls_completions`, `crates/pgls_hover` diff --git a/Cargo.lock b/Cargo.lock index 03c02083e1..4312080ca8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2293,6 +2293,7 @@ dependencies = [ "pgls_console", "pgls_diagnostics", "pgls_lexer", + "pgls_migrations", "pgls_postgres_regress", "pgls_query", "pgls_query_ext", @@ -2352,6 +2353,7 @@ dependencies = [ "pgls_fs", "pgls_lexer", "pgls_lsp", + "pgls_migrations", "pgls_query", "pgls_test_utils", "pgls_text_edit", @@ -2620,6 +2622,18 @@ dependencies = [ "rustc-hash", ] +[[package]] +name = "pgls_migrations" +version = "0.0.0" +dependencies = [ + "pgls_diagnostics", + "pgls_fs", + "pgls_statement_splitter", + "pgls_text_size", + "tempfile", + "toml", +] + [[package]] name = "pgls_pglinter" version = "0.0.0" @@ -2930,6 +2944,7 @@ dependencies = [ "pgls_lexer", "pgls_line_index", "pgls_matcher", + "pgls_migrations", "pgls_pglinter", "pgls_plpgsql_check", "pgls_pretty_print", @@ -2956,7 +2971,6 @@ dependencies = [ "strum", "tempfile", "tokio", - "toml", "tracing", "tree-sitter", "url", diff --git a/Cargo.toml b/Cargo.toml index 89cc7632d9..9951c95311 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,7 @@ pgls_line_index = { path = "./crates/pgls_line_index", version = "0. pgls_lsp = { path = "./crates/pgls_lsp", version = "0.0.0" } pgls_markup = { path = "./crates/pgls_markup", version = "0.0.0" } pgls_matcher = { path = "./crates/pgls_matcher", version = "0.0.0" } +pgls_migrations = { path = "./crates/pgls_migrations", version = "0.0.0" } pgls_pglinter = { path = "./crates/pgls_pglinter", version = "0.0.0" } pgls_plpgsql_check = { path = "./crates/pgls_plpgsql_check", version = "0.0.0" } pgls_postgres_regress = { path = "./crates/pgls_postgres_regress", version = "0.0.0" } diff --git a/crates/pgls_analyser/Cargo.toml b/crates/pgls_analyser/Cargo.toml index eb9825737d..9a4425c4e4 100644 --- a/crates/pgls_analyser/Cargo.toml +++ b/crates/pgls_analyser/Cargo.toml @@ -17,6 +17,7 @@ pgls_catalog = { workspace = true } pgls_console = { workspace = true } pgls_diagnostics = { workspace = true } pgls_lexer = { workspace = true } +pgls_migrations = { workspace = true } pgls_query = { workspace = true } pgls_query_ext = { workspace = true } pgls_text_size = { workspace = true } diff --git a/crates/pgls_analyser/src/lib.rs b/crates/pgls_analyser/src/lib.rs index a2bb8b82b7..0e433f967a 100644 --- a/crates/pgls_analyser/src/lib.rs +++ b/crates/pgls_analyser/src/lib.rs @@ -6,6 +6,7 @@ use std::{ use pgls_analyse::{AnalysisFilter, AppliesTo, MetadataRegistry}; use pgls_catalog::{Catalog, CatalogBase, Session, TransactionMode, resolve::FunctionContext}; use pgls_diagnostics::DiagnosticTags; +use pgls_migrations::{Direction, Unit}; use pgls_query_ext::function_body::{EmbeddedStatement, OffsetMap}; use pgls_text_size::{TextRange, TextSize}; pub use registry::visit_registry; @@ -16,7 +17,6 @@ mod linter_context; mod linter_options; mod linter_registry; mod linter_rule; -mod migration; pub mod options; mod registry; @@ -27,7 +27,6 @@ pub use linter_registry::{ LinterRegistryRuleParams, LinterRuleRegistry, LinterRuleRegistryBuilder, }; pub use linter_rule::{FixEdit, LinterDiagnostic, LinterRule, RuleFix}; -pub use migration::{Direction, MigrationSection}; // For convenience in macros and rule files - keep these shorter names pub use LinterDiagnostic as RuleDiagnostic; @@ -166,7 +165,7 @@ pub struct AnalyserParams { pub file_kind: FileKind, /// The sections of a migration file, like dbmate's up and down migrations. Each section is /// analysed as a migration of its own. Without sections, the whole file is one migration. - pub sections: Vec, + pub sections: Vec, /// Whether the rules of the typecheck group run. pub typecheck: bool, /// The text of the file. Fixes that also edit after their statement, like the `COMMIT` that @@ -269,7 +268,7 @@ impl<'a> Analyser<'a> { // Each section runs in a session and transaction of its own. let session = Session::new(params.search_path.clone()).with_transaction_mode( unit.section - .map_or(TransactionMode::Unknown, MigrationSection::transaction_mode), + .map_or(TransactionMode::Unknown, transaction_mode), ); let stmts = ¶ms.stmts[unit.stmts.clone()]; // The last statement of the unit, including those that don't parse. The statements @@ -520,9 +519,18 @@ fn debug_assert_in_statement(diagnostic: &LinterDiagnostic, stmt: &AnalysableSta } } +/// The transaction mode that a migration section starts in. +pub fn transaction_mode(section: &Unit) -> TransactionMode { + if section.transaction { + TransactionMode::Transaction + } else { + TransactionMode::Autocommit + } +} + /// Statements that run together: those before the first section marker, then each section. struct MigrationUnit<'a> { - section: Option<&'a MigrationSection>, + section: Option<&'a Unit>, /// The indices of the statements. stmts: Range, range: TextRange, @@ -530,7 +538,7 @@ struct MigrationUnit<'a> { fn migration_units<'a>( stmts: &[AnalysableStatement], - sections: &'a [MigrationSection], + sections: &'a [Unit], ) -> Vec> { let before_sections = sections .first() diff --git a/crates/pgls_analyser/src/migration.rs b/crates/pgls_analyser/src/migration.rs deleted file mode 100644 index 22a1ae543c..0000000000 --- a/crates/pgls_analyser/src/migration.rs +++ /dev/null @@ -1,36 +0,0 @@ -//! Migration sections. Tools like dbmate and goose put the up and the down migration in the same -//! file and run each of them on its own. - -use pgls_catalog::TransactionMode; -use pgls_text_size::TextRange; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Direction { - Up, - Down, -} - -/// A part of a migration file that the migration tool runs on its own. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct MigrationSection { - /// From the start of the section to the start of the next one, or to the end of the file. - pub range: TextRange, - pub direction: Direction, - /// Whether the migration tool runs the section in a transaction. - pub transaction: bool, - /// The parts of the section that the migration tool sends to Postgres as one query, like a - /// whole dbmate section or a goose `StatementBegin` block. Postgres runs a query with more - /// than one statement in a transaction block, also in a section without a transaction. The - /// other statements of the section are sent one by one. - pub queries: Vec, -} - -impl MigrationSection { - pub fn transaction_mode(&self) -> TransactionMode { - if self.transaction { - TransactionMode::Transaction - } else { - TransactionMode::Autocommit - } - } -} diff --git a/crates/pgls_analyser/tests/rules_tests.rs b/crates/pgls_analyser/tests/rules_tests.rs index 1167e5945c..fb84c23f3d 100644 --- a/crates/pgls_analyser/tests/rules_tests.rs +++ b/crates/pgls_analyser/tests/rules_tests.rs @@ -9,11 +9,12 @@ use std::{ use pgls_analyse::{AnalysisFilter, Applicability, RuleFilter}; use pgls_analyser::{ - AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams, Direction, LinterDiagnostic, - LinterOptions, MigrationSection, RuleFix, UnparsableStatement, + AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams, LinterDiagnostic, LinterOptions, + RuleFix, UnparsableStatement, }; use pgls_console::StdDisplay; use pgls_diagnostics::PrintDiagnostic; +use pgls_migrations::{Direction, Unit}; use pgls_text_size::{TextRange, TextSize}; static TYPECHECK_CATALOG: OnceLock> = OnceLock::new(); @@ -229,8 +230,8 @@ fn sql_function_body( /// The migration sections of the file, each starting at a `-- section: up` or /// `-- section: down transaction:false` line. Like dbmate, each section is sent to Postgres as /// one query, unless `queries:separate` follows, like goose without `StatementBegin`. -fn migration_sections(query: &str) -> Vec { - let mut sections: Vec = Vec::new(); +fn migration_sections(query: &str) -> Vec { + let mut sections: Vec = Vec::new(); let mut one_query = Vec::new(); let mut offset = 0; for line in query.split_inclusive('\n') { @@ -257,7 +258,7 @@ fn migration_sections(query: &str) -> Vec { if let Some(previous) = sections.last_mut() { previous.range = TextRange::new(previous.range.start(), start); } - sections.push(MigrationSection { + sections.push(Unit { range: TextRange::new(start, TextSize::from(query.len() as u32)), direction, transaction, diff --git a/crates/pgls_cli/Cargo.toml b/crates/pgls_cli/Cargo.toml index 7f652c9d18..81343a0d66 100644 --- a/crates/pgls_cli/Cargo.toml +++ b/crates/pgls_cli/Cargo.toml @@ -27,6 +27,7 @@ pgls_env = { workspace = true } pgls_fs = { workspace = true } pgls_lexer = { workspace = true } pgls_lsp = { workspace = true } +pgls_migrations = { workspace = true } pgls_query = { workspace = true } pgls_text_edit = { workspace = true } pgls_workspace = { workspace = true } diff --git a/crates/pgls_cli/src/commands/init.rs b/crates/pgls_cli/src/commands/init.rs index e62d403115..5f257992d1 100644 --- a/crates/pgls_cli/src/commands/init.rs +++ b/crates/pgls_cli/src/commands/init.rs @@ -1,9 +1,13 @@ use crate::{CliDiagnostic, CliSession}; use pgls_configuration::PartialConfiguration; +use pgls_configuration::migrations::{PartialMigrationsConfiguration, SectionMarkers}; use pgls_console::{ConsoleExt, markup}; use pgls_fs::ConfigName; +use pgls_migrations::{ + Detected, ToolId, + detect::{configuration_directory, find_projects}, +}; use pgls_workspace::configuration::create_config; -use pgls_workspace::migrations::project::{Project, configuration_directory, find_projects}; pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { let fs = &mut session.app.fs; @@ -16,7 +20,7 @@ pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { (None, projects) }; if let Some(project) = &project { - project.configure(config); + configure(project, config); } create_config(fs, project_directory, config)?; let file_created = project_directory @@ -39,7 +43,7 @@ Welcome to the Postgres Language Server! Let's get you started... {warning} }); } - let tool = project.tool; + let tool = project.tool.tool().name(); let directory = &project.directory; let settings = configured_settings(&project); session.app.console.log(markup! { @@ -49,6 +53,7 @@ Welcome to the Postgres Language Server! Let's get you started... } else if !ambiguous.is_empty() { let first = ambiguous[0].tool; let projects = if ambiguous.iter().all(|project| project.tool == first) { + let first = first.tool().name(); let directories: Vec<&str> = ambiguous .iter() .map(|project| project.directory.as_str()) @@ -57,7 +62,7 @@ Welcome to the Postgres Language Server! Let's get you started... } else { let directories: Vec = ambiguous .iter() - .map(|project| format!("{}: {}", project.directory, project.tool)) + .map(|project| format!("{}: {}", project.directory, project.tool.tool().name())) .collect(); format!("several migration projects ({})", directories.join(", ")) }; @@ -68,9 +73,40 @@ Welcome to the Postgres Language Server! Let's get you started... Ok(()) } +/// The section markers of the tool's migration files, if it has any. +fn section_markers(tool: ToolId) -> Option { + match tool { + ToolId::Plain | ToolId::Supabase => None, + ToolId::Dbmate => Some(SectionMarkers::Dbmate), + ToolId::Goose => Some(SectionMarkers::Goose), + } +} + +/// Points the database connection at the local database of the project, and the migrations at +/// the migrations of the project. +fn configure(project: &Detected, config: &mut PartialConfiguration) { + if let Some(database) = &project.database { + let db = config.db.get_or_insert_with(Default::default); + db.host = Some(database.host.to_string()); + db.port = Some(database.port); + db.username = Some(database.username.to_string()); + db.password = Some(database.password.to_string()); + db.database = Some(database.database.to_string()); + // Statement execution stays off: enabling it is the user's call. + } + let section_markers = section_markers(project.tool); + if project.migrations_dir.is_some() || section_markers.is_some() { + config.migrations = Some(PartialMigrationsConfiguration { + migrations_dir: project.migrations_dir.clone(), + section_markers, + ..Default::default() + }); + } +} + /// The settings that `init` took from the project, like "the database connection is set to the /// local Supabase database at 127.0.0.1:54322, and the migrations to supabase/migrations". -fn configured_settings(project: &Project) -> String { +fn configured_settings(project: &Detected) -> String { let mut settings = Vec::new(); if let Some(database) = &project.database { settings.push(( @@ -78,15 +114,21 @@ fn configured_settings(project: &Project) -> String { "is", format!( "the local {} database at {}:{}", - project.tool, database.host, database.port + project.tool.tool().name(), + database.host, + database.port ), )); } if let Some(migrations_dir) = &project.migrations_dir { settings.push(("the migrations", "are", migrations_dir.clone())); } - if project.section_markers.is_some() { - settings.push(("the section markers", "are", project.tool.to_string())); + if section_markers(project.tool).is_some() { + settings.push(( + "the section markers", + "are", + project.tool.tool().name().to_string(), + )); } let mut text = String::new(); for (index, (setting, verb, value)) in settings.iter().enumerate() { diff --git a/crates/pgls_migrations/Cargo.toml b/crates/pgls_migrations/Cargo.toml new file mode 100644 index 0000000000..8c50647514 --- /dev/null +++ b/crates/pgls_migrations/Cargo.toml @@ -0,0 +1,28 @@ +[package] +authors.workspace = true +categories.workspace = true +description = "What the language server knows about migration tools: their projects, files and markers" +edition.workspace = true +homepage.workspace = true +keywords.workspace = true +license.workspace = true +name = "pgls_migrations" +repository.workspace = true +version = "0.0.0" + + +[dependencies] +pgls_diagnostics = { workspace = true } +pgls_fs = { workspace = true, features = ["serde"] } +pgls_text_size = { workspace = true } +toml = { workspace = true } + +[dev-dependencies] +pgls_statement_splitter = { workspace = true } +tempfile = "3.15.0" + +[lib] +doctest = false + +[lints] +workspace = true diff --git a/crates/pgls_workspace/src/migrations/project.rs b/crates/pgls_migrations/src/detect.rs similarity index 78% rename from crates/pgls_workspace/src/migrations/project.rs rename to crates/pgls_migrations/src/detect.rs index f2f218e2f3..4f2403474b 100644 --- a/crates/pgls_workspace/src/migrations/project.rs +++ b/crates/pgls_migrations/src/detect.rs @@ -2,27 +2,23 @@ use std::path::{Path, PathBuf}; -use pgls_configuration::PartialConfiguration; -use pgls_configuration::migrations::{PartialMigrationsConfiguration, SectionMarkers}; use pgls_fs::FileSystem; -use super::{MigrationTool, TOOLS}; +use crate::{MigrationTool, TOOLS, ToolId}; /// Folders that never hold the project of a migration tool in a monorepo. const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", "supabase"]; /// A project of a migration tool, and the settings it tells. #[derive(Debug)] -pub struct Project { - /// The name of the tool. - pub tool: &'static str, +pub struct Detected { + /// The tool. + pub tool: ToolId, /// The folder of the tool in the project, like `supabase` or `apps/web/supabase`, relative to /// the folder of the configuration file, with `/` as the separator. pub directory: String, /// The folder of the migrations, relative to the folder of the configuration file. pub migrations_dir: Option, - /// The section markers of the tool's migration files, if it has any. - pub section_markers: Option, /// The local database of the project, if the tool runs one. pub database: Option, /// A problem with the project that doesn't stop `init`, like a port it couldn't read. @@ -39,47 +35,24 @@ pub struct LocalDatabase { pub database: &'static str, } -impl Project { +impl Detected { /// A project of `tool` with the tool's folder `directory` in `root`, and the tool's default /// migrations folder. - pub(super) fn new(tool: &dyn MigrationTool, root: &Root, directory: &str) -> Self { + pub(crate) fn new(tool: &dyn MigrationTool, root: &Root, directory: &str) -> Self { Self { - tool: tool.name(), + tool: tool.id(), directory: root.relative(directory), migrations_dir: tool - .default_migrations_dir() + .default_dir() .map(|migrations_dir| root.relative(migrations_dir)), - section_markers: Some(tool.section_markers()) - .filter(|markers| *markers != SectionMarkers::None), database: None, warning: None, } } - - /// Points the database connection at the local database of the project, and the migrations - /// at the migrations of the project. - pub fn configure(&self, config: &mut PartialConfiguration) { - if let Some(database) = &self.database { - let db = config.db.get_or_insert_with(Default::default); - db.host = Some(database.host.to_string()); - db.port = Some(database.port); - db.username = Some(database.username.to_string()); - db.password = Some(database.password.to_string()); - db.database = Some(database.database.to_string()); - // Statement execution stays off: enabling it is the user's call. - } - if self.migrations_dir.is_some() || self.section_markers.is_some() { - config.migrations = Some(PartialMigrationsConfiguration { - migrations_dir: self.migrations_dir.clone(), - section_markers: self.section_markers, - ..Default::default() - }); - } - } } /// A folder that may hold the project of a migration tool. -pub(crate) struct Root { +pub struct Root { /// The folder, for the file system. path: PathBuf, /// The folder relative to the folder of the configuration file, with `/` as the separator and @@ -89,12 +62,12 @@ pub(crate) struct Root { impl Root { /// A path in the folder, for the file system. - pub(super) fn path(&self, relative: &str) -> PathBuf { + pub(crate) fn path(&self, relative: &str) -> PathBuf { self.path.join(relative) } /// A path in the folder, relative to the folder of the configuration file. - pub(super) fn relative(&self, relative: &str) -> String { + pub(crate) fn relative(&self, relative: &str) -> String { format!("{}{relative}", self.prefix) } } @@ -119,7 +92,7 @@ pub fn configuration_directory(fs: &dyn FileSystem) -> &'static Path { /// relative to the working directory: the ones in it, or else, if it is the working directory, /// the ones in the folders one or two levels below it, as in a monorepo (`supabase/`, /// `db/supabase/`, `apps/web/supabase/`). -pub fn find_projects(fs: &dyn FileSystem, configuration_directory: &Path) -> Vec { +pub fn find_projects(fs: &dyn FileSystem, configuration_directory: &Path) -> Vec { let working_directory = fs.working_directory().unwrap_or_default(); let detect = |relative: &str| { let root = Root { @@ -164,7 +137,7 @@ fn subdirectories(directory: &Path) -> Vec { } /// The names of the files in `directory`, sorted. -pub(super) fn files(directory: &Path) -> Vec { +pub(crate) fn files(directory: &Path) -> Vec { entries(directory, false) } @@ -198,7 +171,13 @@ mod tests { let fs = OsFileSystem::new(dir.path().to_path_buf()); find_projects(&fs, Path::new("")) .into_iter() - .map(|project| (project.directory, project.tool, project.migrations_dir)) + .map(|project| { + ( + project.directory, + project.tool.tool().name(), + project.migrations_dir, + ) + }) .collect() } @@ -214,13 +193,12 @@ mod tests { write(&dir, "supabase/config.toml", "[db]\nport = 6543\n"); let fs = OsFileSystem::new(dir.path().to_path_buf()); let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); - assert_eq!(project.tool, "Supabase"); + assert_eq!(project.tool, ToolId::Supabase); assert_eq!(project.directory, "supabase"); assert_eq!( project.migrations_dir.as_deref(), Some("supabase/migrations") ); - assert_eq!(project.section_markers, None); assert_eq!(project.database.map(|database| database.port), Some(6543)); assert!(project.warning.is_none()); } @@ -235,18 +213,10 @@ mod tests { ); let fs = OsFileSystem::new(dir.path().to_path_buf()); let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); - assert_eq!(project.tool, "dbmate"); + assert_eq!(project.tool, ToolId::Dbmate); assert_eq!(project.directory, "db"); assert_eq!(project.migrations_dir.as_deref(), Some("db/migrations")); - assert_eq!(project.section_markers, Some(SectionMarkers::Dbmate)); assert!(project.database.is_none()); - - let mut config = PartialConfiguration::default(); - project.configure(&mut config); - let migrations = config.migrations.unwrap(); - assert_eq!(migrations.migrations_dir.as_deref(), Some("db/migrations")); - assert_eq!(migrations.section_markers, Some(SectionMarkers::Dbmate)); - assert!(config.db.is_none()); } #[test] diff --git a/crates/pgls_migrations/src/directives.rs b/crates/pgls_migrations/src/directives.rs new file mode 100644 index 0000000000..faa0f8a8f9 --- /dev/null +++ b/crates/pgls_migrations/src/directives.rs @@ -0,0 +1,59 @@ +//! The marker comments of the migration tools, like `-- migrate:up` or `-- +goose Up`. The tools +//! find them line by line, without looking at the SQL, and so does the language server. + +use pgls_diagnostics::{Diagnostic, MessageAndDescription}; +use pgls_text_size::{TextRange, TextSize}; + +/// The lines of `text` without their line endings, with the range of the whole line including +/// its line ending. +pub(crate) fn lines(text: &str) -> impl Iterator { + let mut offset = TextSize::from(0); + text.split_inclusive('\n').map(move |line| { + let range = TextRange::at(offset, TextSize::of(line)); + offset = range.end(); + (range, line.trim_end_matches(['\n', '\r'])) + }) +} + +/// The lines that look like markers but that the tool doesn't read as one: they only are with +/// the leading whitespace removed or in lowercase. `message` says what the tool does with the +/// line instead. +pub(crate) fn look_alikes( + text: &str, + is_marker: impl Fn(&str) -> bool, + message: impl Fn(&str) -> &'static str, +) -> Vec<(TextRange, &'static str)> { + lines(text) + .filter(|(_, line)| { + let trimmed = line.trim_start(); + !is_marker(line) && (is_marker(trimmed) || is_marker(&trimmed.to_ascii_lowercase())) + }) + .map(|(range, line)| { + ( + TextRange::at(range.start(), TextSize::of(line)), + message(line), + ) + }) + .collect() +} + +/// A problem with the section markers of a migration file that makes the migration tool reject +/// the file, or ignore a marker. +#[derive(Debug, Diagnostic)] +#[diagnostic(category = "migrations/sectionMarkers", severity = Warning)] +pub struct SectionMarkerDiagnostic { + #[location(span)] + pub span: TextRange, + #[message] + #[description] + pub message: MessageAndDescription, +} + +impl SectionMarkerDiagnostic { + pub(crate) fn new(span: TextRange, message: &str) -> Self { + Self { + span, + message: MessageAndDescription::from(message.to_string()), + } + } +} diff --git a/crates/pgls_migrations/src/lib.rs b/crates/pgls_migrations/src/lib.rs new file mode 100644 index 0000000000..89066ad76c --- /dev/null +++ b/crates/pgls_migrations/src/lib.rs @@ -0,0 +1,183 @@ +//! Migration tools: everything the language server knows about a tool is one [`MigrationTool`]. +//! +//! Every tool answers every question of the trait, so its file says all it does. What tools +//! share are helpers that they call: [`naming`] for the file names, [`directives`] for the +//! marker comments, [`units`] for the parts of a file that run on their own, and [`detect`] for +//! finding projects. +//! +//! The workspace asks the tool of a file's settings two things, and keeps the answers: +//! - [`MigrationTool::file`]: what the path says, like the version of the migration. +//! - [`MigrationTool::read`]: what the text says, like its up and down sections. +//! +//! `init` asks every tool for its projects with [`detect::find_projects`]. +//! +//! The tools: +//! - [`Plain`]: no tool, every file is one migration. +//! - [`Supabase`]: the Supabase CLI. Every file is one migration. +//! - [`Dbmate`] and [`Goose`]: up and down sections in one file, separated by marker comments. + +pub mod detect; +mod directives; +mod naming; +mod tools; +mod units; + +use std::path::Path; + +use pgls_fs::FileSystem; +use pgls_text_size::TextRange; + +pub use detect::{Detected, LocalDatabase, Root}; +pub use directives::SectionMarkerDiagnostic; +pub use tools::{Dbmate, Goose, Plain, Supabase}; +pub use units::{Direction, Unit}; + +/// A migration tool. Implementations have no defaults: each one says what the tool does, and +/// calls the shared helpers for what it does like other tools. +pub trait MigrationTool: Sync { + /// The tool, as the settings name it. + fn id(&self) -> ToolId; + + /// The name of the tool, as its users know it. + fn name(&self) -> &'static str; + + /// The project of the tool in `root`, for `init`, if the tool has a reliable sign of its + /// projects. + fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option; + + /// Whether `directory` is the folder of the tool inside a project, like `supabase`, so that + /// `init` configures the project above it. + fn is_tool_directory(&self, fs: &dyn FileSystem, directory: &Path) -> bool; + + /// The folder of the migrations in a project of the tool, relative to the project, if the + /// tool has one by default. + fn default_dir(&self) -> Option<&'static str>; + + /// What the path of a file says, given `migrations.migrationsDir`: whether it is a + /// migration, and its version. + fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo; + + /// The version of a migration in the tool's format, like the number that starts the name of + /// its file. + fn version(&self, text: &str) -> Option; + + /// How the tool runs the text of a migration file: its units, and the problems with its + /// markers. + /// + /// Markers are found line by line, like the migration tools do, without looking at the SQL. + fn read(&self, text: &str) -> Script; +} + +/// The migration tools, as the settings name them. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ToolId { + Plain, + Supabase, + Dbmate, + Goose, +} + +impl ToolId { + pub fn tool(self) -> &'static dyn MigrationTool { + match self { + ToolId::Plain => &Plain, + ToolId::Supabase => &Supabase, + ToolId::Dbmate => &Dbmate, + ToolId::Goose => &Goose, + } + } +} + +/// The tools, in the order `init` reports their projects. +pub const TOOLS: &[&dyn MigrationTool] = &[&Supabase, &Dbmate, &Goose, &Plain]; + +/// What the path of a file says. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileInfo { + pub role: Role, + /// The version of a migration whose name has one. `migrations.after` compares it. + pub version: Option, +} + +/// What a file is to the tool. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Role { + /// A migration, in the migrations folder. Migration-only rules run on it. + Migration, + /// Any other file. + Other, +} + +/// The version of a migration. Versions of one tool are ordered. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub enum Version { + /// A number, like the timestamp of `20240101120000_init.sql` or the `1` of `0001_init.sql`. + Number(u64), +} + +/// How a tool runs the text of a migration file. +#[derive(Debug, Default)] +pub struct Script { + /// The parts of the file that the tool runs on their own, in file order. A file without + /// markers has none, and is one migration. + pub units: Vec, + /// Lines that look like markers, but that the tool reads differently, with what the tool + /// does with them. + look_alikes: Vec<(TextRange, &'static str)>, + /// What the tool says about statements before the first unit, if it rejects them. + before_first_unit: Option, +} + +impl Script { + /// The problems with the markers of the file, given its statements: statements before the + /// first marker, which the tools reject, and lines that look like markers but that the tools + /// read as plain comments. + pub fn marker_diagnostics(&self, statements: &[TextRange]) -> Vec { + let mut diagnostics: Vec = self + .look_alikes + .iter() + .map(|(span, message)| SectionMarkerDiagnostic::new(*span, message)) + .collect(); + if let (Some(first), Some(message)) = (self.units.first(), &self.before_first_unit) { + diagnostics.extend( + statements + .iter() + .take_while(|statement| statement.start() < first.range.start()) + .map(|statement| SectionMarkerDiagnostic::new(*statement, message)), + ); + } + diagnostics.sort_by_key(|diagnostic| diagnostic.span.start()); + diagnostics + } +} + +#[cfg(test)] +mod tests { + use super::*; + + pub(crate) fn sections(text: &str, tool: ToolId) -> Vec<(&str, Direction, bool)> { + tool.tool() + .read(text) + .units + .into_iter() + .map(|unit| (&text[unit.range], unit.direction, unit.transaction)) + .collect() + } + + pub(crate) fn marker_problems(text: &str, tool: ToolId) -> Vec<(&str, String)> { + let statements: Vec = pgls_statement_splitter::split(text).ranges; + tool.tool() + .read(text) + .marker_diagnostics(&statements) + .into_iter() + .map(|diagnostic| (&text[diagnostic.span], diagnostic.message.to_string())) + .collect() + } + + #[test] + fn every_tool_has_its_id() { + for tool in TOOLS { + assert_eq!(tool.id().tool().name(), tool.name()); + } + } +} diff --git a/crates/pgls_migrations/src/naming.rs b/crates/pgls_migrations/src/naming.rs new file mode 100644 index 0000000000..747f459701 --- /dev/null +++ b/crates/pgls_migrations/src/naming.rs @@ -0,0 +1,138 @@ +//! The names of migration files. Supabase, dbmate and goose name them the same way, +//! `_.sql`. + +use std::path::Path; + +use crate::{FileInfo, Role, Version}; + +/// A file in the migrations folder is a migration. Its version is the number that starts its +/// name, `_.sql`, or the name of its folder, `_/up.sql`. +pub(crate) fn numbered( + path: &Path, + migrations_dir: &Path, + version: impl Fn(&str) -> Option, +) -> FileInfo { + if !is_in_migrations_dir(path, migrations_dir) { + return FileInfo { + role: Role::Other, + version: None, + }; + } + + // In the "root" pattern, all files are directly within the migrations directory, and their + // names follow _.sql. This is used by Supabase. In the "subdirectory" + // pattern, each migration is in a subdirectory named _. This is used by + // Prisma and Drizzle. + let version_of = |name: Option<&std::ffi::OsStr>| { + let (number, _name) = name?.to_str()?.split_once('_')?; + version(number) + }; + FileInfo { + role: Role::Migration, + version: version_of(path.file_name()) + .or_else(|| version_of(path.parent().and_then(Path::file_name))), + } +} + +/// A number with leading zeros, like `000201` or `20240101120000`. +pub(crate) fn number(text: &str) -> Option { + text.trim_start_matches('0') + .parse() + .ok() + .map(Version::Number) +} + +/// Whether the path is inside the migrations directory. +fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { + if path.starts_with(migrations_dir) { + return true; + } + path.canonicalize() + .ok() + .and_then(|canonical_child| { + migrations_dir + .canonicalize() + .ok() + .map(|canonical_dir| canonical_child.starts_with(&canonical_dir)) + }) + .unwrap_or(false) +} + +#[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") + } + + fn version_of(path: &Path, migrations_dir: &Path) -> Option { + let file = numbered(path, migrations_dir, number); + assert_eq!(file.role == Role::Migration, file.version.is_some()); + file.version + } + + #[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(); + + assert_eq!( + version_of(&path, &migrations_dir), + Some(Version::Number(1234567890)) + ); + } + + #[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(); + + assert_eq!( + version_of(&path, &migrations_dir), + Some(Version::Number(1234567890)) + ); + } + + #[test] + fn test_get_migration_prefix_number() { + let temp_dir = setup(); + let migrations_dir = temp_dir.path().to_path_buf(); + let path = migrations_dir.join("000201_a_migration.sql"); + fs::write(&path, "").unwrap(); + + assert_eq!( + version_of(&path, &migrations_dir), + Some(Version::Number(201)) + ); + } + + #[test] + fn test_get_migration_not_timestamp_in_filename() { + let migrations_dir = PathBuf::from("/tmp/migrations"); + let path = migrations_dir.join("not_a_migration.sql"); + + let file = numbered(&path, &migrations_dir, number); + assert_eq!(file.role, Role::Migration); + assert_eq!(file.version, 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 file = numbered(&path, &migrations_dir, number); + assert_eq!(file.role, Role::Other); + assert_eq!(file.version, None); + } +} diff --git a/crates/pgls_workspace/src/migrations/dbmate.rs b/crates/pgls_migrations/src/tools/dbmate.rs similarity index 75% rename from crates/pgls_workspace/src/migrations/dbmate.rs rename to crates/pgls_migrations/src/tools/dbmate.rs index dab757e66f..0b21fe55d9 100644 --- a/crates/pgls_workspace/src/migrations/dbmate.rs +++ b/crates/pgls_migrations/src/tools/dbmate.rs @@ -5,49 +5,42 @@ //! `--migrations-dir` says otherwise, see //! https://github.com/amacneil/dbmate/blob/v2.37.0/README.md#creating-migrations -use pgls_analyser::{Direction, MigrationSection}; -use pgls_configuration::migrations::SectionMarkers; +use std::path::Path; + use pgls_fs::{FileSystem, OpenOptions}; -use pgls_text_size::TextRange; -use super::{ - MigrationTool, SectionMarkerDiagnostic, - markers::{self, MarkerSyntax, SectionsBuilder, lines}, - project::{self, Project, Root}, +use crate::{ + Detected, Direction, FileInfo, MigrationTool, Root, Script, ToolId, Version, detect, + directives::{self, lines}, + naming, + units::UnitsBuilder, }; /// The folder of dbmate in a project, and its migrations in it. const DBMATE_DIRECTORY: &str = "db"; const DBMATE_MIGRATIONS_DIR: &str = "db/migrations"; -pub(super) struct Dbmate; - -const SYNTAX: MarkerSyntax = MarkerSyntax { - tool: "dbmate", - first_marker: "-- migrate:up", - is_marker: |line| marker(line).is_some(), - look_alike_message: |_| "dbmate only reads `-- migrate:up` and `-- migrate:down` in lowercase and at the start of the line, so this line is a plain comment.", -}; +pub struct Dbmate; impl MigrationTool for Dbmate { - fn name(&self) -> &'static str { - "dbmate" + fn id(&self) -> ToolId { + ToolId::Dbmate } - fn section_markers(&self) -> SectionMarkers { - SectionMarkers::Dbmate + fn name(&self) -> &'static str { + "dbmate" } - fn default_migrations_dir(&self) -> Option<&'static str> { + fn default_dir(&self) -> Option<&'static str> { Some(DBMATE_MIGRATIONS_DIR) } /// A first migration in `db/migrations` with `-- migrate:up`, which dbmate requires in every /// migration. dbmate has no configuration file, and reads the database from `DATABASE_URL`, /// so the connection is left alone. - fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { + fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { let migrations_dir = root.path(DBMATE_MIGRATIONS_DIR); - let first = project::files(&migrations_dir) + let first = detect::files(&migrations_dir) .into_iter() .find(|name| is_migration_file_name(name))?; let mut contents = String::new(); @@ -59,31 +52,46 @@ impl MigrationTool for Dbmate { .ok()?; lines(&contents) .any(|(_, line)| marker(line).is_some_and(|(direction, _)| direction == Direction::Up)) - .then(|| Project::new(self, root, DBMATE_DIRECTORY)) + .then(|| Detected::new(self, root, DBMATE_DIRECTORY)) + } + + /// dbmate has no folder of its own to run `init` in. + fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { + false } - fn sections(&self, text: &str) -> Vec { - let mut sections = SectionsBuilder::new(text); + fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { + naming::numbered(path, migrations_dir, |text| self.version(text)) + } + + fn version(&self, text: &str) -> Option { + naming::number(text) + } + + fn read(&self, text: &str) -> Script { + let mut units = UnitsBuilder::new(text); for (range, line) in lines(text) { if let Some((direction, transaction)) = marker(line) { - sections.start(range.start(), direction, transaction); + units.start(range.start(), direction, transaction); } } - let mut sections = sections.finish(); + let mut units = units.finish(); // dbmate sends each section to Postgres as one query. - for section in &mut sections { - section.queries = vec![section.range]; + for unit in &mut units { + unit.queries = vec![unit.range]; + } + Script { + units, + look_alikes: directives::look_alikes( + text, + |line| marker(line).is_some(), + |_| "dbmate only reads `-- migrate:up` and `-- migrate:down` in lowercase and at the start of the line, so this line is a plain comment.", + ), + before_first_unit: Some( + "dbmate rejects a migration with statements before the first `-- migrate:up`." + .to_string(), + ), } - sections - } - - fn marker_diagnostics( - &self, - text: &str, - sections: &[MigrationSection], - statements: &[TextRange], - ) -> Vec { - markers::marker_diagnostics(&SYNTAX, text, sections, statements) } } @@ -119,11 +127,8 @@ fn marker(line: &str) -> Option<(Direction, bool)> { #[cfg(test)] mod tests { - use pgls_analyser::Direction; - use pgls_configuration::migrations::SectionMarkers; - - use crate::migrations::{ - migration_tool, + use crate::{ + Direction, ToolId, tests::{marker_problems, sections}, }; @@ -145,7 +150,7 @@ mod tests { fn dbmate_sections() { let text = "-- migrate:up\ncreate table t (id int);\n\n-- migrate:down transaction:false\ndrop table t;\n"; assert_eq!( - sections(text, SectionMarkers::Dbmate), + sections(text, ToolId::Dbmate), vec![ ( "-- migrate:up\ncreate table t (id int);\n\n", @@ -164,7 +169,7 @@ mod tests { #[test] fn dbmate_options_and_spacing() { let directions = |text| { - sections(text, SectionMarkers::Dbmate) + sections(text, ToolId::Dbmate) .into_iter() .map(|(_, direction, transaction)| (direction, transaction)) .collect::>() @@ -185,8 +190,10 @@ mod tests { fn queries_sent_at_once() { let text = "-- migrate:up transaction:false\nselect 1;\nselect 2;\n-- migrate:down\nselect 3;\n"; - let queries = migration_tool(SectionMarkers::Dbmate) - .sections(text) + let queries = ToolId::Dbmate + .tool() + .read(text) + .units .into_iter() .map(|section| { section @@ -211,7 +218,7 @@ mod tests { let text = "create table t (id int);\n-- migrate:up\nselect 1;\n-- migrate:down\nselect 2;\n"; assert_eq!( - marker_problems(text, SectionMarkers::Dbmate), + marker_problems(text, ToolId::Dbmate), vec![( "create table t (id int);", "dbmate rejects a migration with statements before the first `-- migrate:up`." @@ -219,13 +226,13 @@ mod tests { )] ); // Without markers, the file is one migration. - assert!(marker_problems("select 1;\n", SectionMarkers::Dbmate).is_empty()); + assert!(marker_problems("select 1;\n", ToolId::Dbmate).is_empty()); } #[test] fn lines_that_look_like_markers() { let text = "-- migrate:up\nselect 1;\n-- Migrate:Down\n -- migrate:down\nselect 2;\n-- migrate:upgrade\n"; - let problems = marker_problems(text, SectionMarkers::Dbmate); + let problems = marker_problems(text, ToolId::Dbmate); assert_eq!( problems.iter().map(|(line, _)| *line).collect::>(), ["-- Migrate:Down", " -- migrate:down"] diff --git a/crates/pgls_workspace/src/migrations/goose.rs b/crates/pgls_migrations/src/tools/goose.rs similarity index 66% rename from crates/pgls_workspace/src/migrations/goose.rs rename to crates/pgls_migrations/src/tools/goose.rs index 0e21a38e45..931b02d5cb 100644 --- a/crates/pgls_workspace/src/migrations/goose.rs +++ b/crates/pgls_migrations/src/tools/goose.rs @@ -5,43 +5,52 @@ //! in the folder of `-dir` or `GOOSE_MIGRATION_DIR`, `.` by default. So its projects aren't //! detected. -use pgls_analyser::{Direction, MigrationSection}; -use pgls_configuration::migrations::SectionMarkers; +use std::path::Path; + +use pgls_fs::FileSystem; use pgls_text_size::TextRange; -use super::{ - MigrationTool, SectionMarkerDiagnostic, - markers::{self, MarkerSyntax, SectionsBuilder, lines}, +use crate::{ + Detected, Direction, FileInfo, MigrationTool, Root, Script, ToolId, Version, + directives::{self, lines}, + naming, + units::UnitsBuilder, }; -pub(super) struct Goose; - -const SYNTAX: MarkerSyntax = MarkerSyntax { - tool: "goose", - first_marker: "-- +goose Up", - is_marker: |line| annotation(line).is_some(), - look_alike_message: |line| { - // goose fails on an annotation with leading whitespace instead of ignoring it, see - // https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L321 - if line.contains("+goose") { - "goose rejects a migration with an annotation that doesn't start at the beginning of the line." - } else { - "goose only reads `-- +goose` annotations with `+goose` in lowercase, so this line is a plain comment." - } - }, -}; +pub struct Goose; impl MigrationTool for Goose { + fn id(&self) -> ToolId { + ToolId::Goose + } + fn name(&self) -> &'static str { "goose" } - fn section_markers(&self) -> SectionMarkers { - SectionMarkers::Goose + /// goose has no sign of its projects, see above. + fn detect(&self, _fs: &dyn FileSystem, _root: &Root) -> Option { + None + } + + fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { + false + } + + fn default_dir(&self) -> Option<&'static str> { + None } - fn sections(&self, text: &str) -> Vec { - let mut sections = SectionsBuilder::new(text); + fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { + naming::numbered(path, migrations_dir, |text| self.version(text)) + } + + fn version(&self, text: &str) -> Option { + naming::number(text) + } + + fn read(&self, text: &str) -> Script { + let mut sections = UnitsBuilder::new(text); let mut no_transaction = false; // The start of the open `StatementBegin` block. let mut statement_begin = None; @@ -72,23 +81,34 @@ impl MigrationTool for Goose { } } - let mut sections = sections.finish(); + let mut units = sections.finish(); // goose runs both directions of the file without a transaction. if no_transaction { - for section in &mut sections { - section.transaction = false; + for unit in &mut units { + unit.transaction = false; } } - sections - } - - fn marker_diagnostics( - &self, - text: &str, - sections: &[MigrationSection], - statements: &[TextRange], - ) -> Vec { - markers::marker_diagnostics(&SYNTAX, text, sections, statements) + Script { + units, + look_alikes: directives::look_alikes( + text, + |line| annotation(line).is_some(), + |line| { + // goose fails on an annotation with leading whitespace instead of ignoring + // it, see + // https://github.com/pressly/goose/blob/v3.29.0/internal/sqlparser/parser.go#L321 + if line.contains("+goose") { + "goose rejects a migration with an annotation that doesn't start at the beginning of the line." + } else { + "goose only reads `-- +goose` annotations with `+goose` in lowercase, so this line is a plain comment." + } + }, + ), + before_first_unit: Some( + "goose rejects a migration with statements before the first `-- +goose Up`." + .to_string(), + ), + } } } @@ -104,11 +124,8 @@ fn annotation(line: &str) -> Option<&str> { #[cfg(test)] mod tests { - use pgls_analyser::Direction; - use pgls_configuration::migrations::SectionMarkers; - - use crate::migrations::{ - migration_tool, + use crate::{ + Direction, ToolId, tests::{marker_problems, sections}, }; @@ -116,7 +133,7 @@ mod tests { fn goose_sections() { let text = "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n-- +goose down\nselect 2;"; assert_eq!( - sections(text, SectionMarkers::Goose), + sections(text, ToolId::Goose), vec![ ( "-- +goose Up\n-- +goose StatementBegin\nselect 1;\n-- +goose StatementEnd\n", @@ -131,8 +148,10 @@ mod tests { #[test] fn queries_sent_at_once() { let text = "-- +goose NO TRANSACTION\n-- +goose Up\nselect 1;\n-- +goose StatementBegin\nselect 2;\nselect 3;\n-- +goose StatementEnd\n-- +goose Down\nselect 4;\n"; - let queries = migration_tool(SectionMarkers::Goose) - .sections(text) + let queries = ToolId::Goose + .tool() + .read(text) + .units .into_iter() .map(|section| { section @@ -155,14 +174,14 @@ mod tests { #[test] fn statements_before_the_first_marker() { let text = "select 1;\n-- +goose Up\nselect 2;\n"; - assert_eq!(marker_problems(text, SectionMarkers::Goose).len(), 1); + assert_eq!(marker_problems(text, ToolId::Goose).len(), 1); } #[test] fn lines_that_look_like_markers() { let text = "-- +goose Up\nselect 1;\n-- +Goose Down\n -- +goose StatementBegin\nselect 2;\n"; - let problems = marker_problems(text, SectionMarkers::Goose); + let problems = marker_problems(text, ToolId::Goose); assert_eq!( problems.iter().map(|(line, _)| *line).collect::>(), ["-- +Goose Down", " -- +goose StatementBegin"] @@ -175,7 +194,7 @@ mod tests { fn goose_no_transaction_applies_to_the_whole_file() { let text = "-- +goose Up\nselect 1;\n-- +goose Down\nselect 2;\n-- +goose NO TRANSACTION\n"; assert!( - sections(text, SectionMarkers::Goose) + sections(text, ToolId::Goose) .iter() .all(|(_, _, transaction)| !transaction) ); diff --git a/crates/pgls_migrations/src/tools/mod.rs b/crates/pgls_migrations/src/tools/mod.rs new file mode 100644 index 0000000000..4825da2edb --- /dev/null +++ b/crates/pgls_migrations/src/tools/mod.rs @@ -0,0 +1,11 @@ +//! The migration tools, one file each. + +mod dbmate; +mod goose; +mod plain; +mod supabase; + +pub use dbmate::Dbmate; +pub use goose::Goose; +pub use plain::Plain; +pub use supabase::Supabase; diff --git a/crates/pgls_migrations/src/tools/plain.rs b/crates/pgls_migrations/src/tools/plain.rs new file mode 100644 index 0000000000..8b995130d2 --- /dev/null +++ b/crates/pgls_migrations/src/tools/plain.rs @@ -0,0 +1,61 @@ +//! Migration files without a tool: every file is one migration. + +use std::path::Path; + +use pgls_fs::FileSystem; + +use crate::{Detected, FileInfo, MigrationTool, Root, Script, ToolId, Version, naming}; + +pub struct Plain; + +impl MigrationTool for Plain { + fn id(&self) -> ToolId { + ToolId::Plain + } + + fn name(&self) -> &'static str { + "plain SQL" + } + + /// Plain SQL files have no sign of a project. + fn detect(&self, _fs: &dyn FileSystem, _root: &Root) -> Option { + None + } + + fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { + false + } + + fn default_dir(&self) -> Option<&'static str> { + None + } + + fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { + naming::numbered(path, migrations_dir, |text| self.version(text)) + } + + fn version(&self, text: &str) -> Option { + naming::number(text) + } + + /// The file is one migration, whatever its comments say. + fn read(&self, _text: &str) -> Script { + Script::default() + } +} + +#[cfg(test)] +mod tests { + use crate::{ + ToolId, + tests::{marker_problems, sections}, + }; + + #[test] + fn no_markers() { + let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; + assert!(sections(text, ToolId::Plain).is_empty()); + assert!(marker_problems(text, ToolId::Plain).is_empty()); + assert!(sections("select 1;", ToolId::Dbmate).is_empty()); + } +} diff --git a/crates/pgls_workspace/src/migrations/supabase.rs b/crates/pgls_migrations/src/tools/supabase.rs similarity index 80% rename from crates/pgls_workspace/src/migrations/supabase.rs rename to crates/pgls_migrations/src/tools/supabase.rs index c838da22d3..b2d856a68a 100644 --- a/crates/pgls_workspace/src/migrations/supabase.rs +++ b/crates/pgls_migrations/src/tools/supabase.rs @@ -4,12 +4,10 @@ use std::ffi::OsStr; use std::path::Path; -use pgls_configuration::migrations::SectionMarkers; use pgls_fs::{FileSystem, OpenOptions}; -use super::{ - MigrationTool, - project::{LocalDatabase, Project, Root}, +use crate::{ + Detected, FileInfo, LocalDatabase, MigrationTool, Root, Script, ToolId, Version, naming, }; /// The folder of a project managed by the Supabase CLI, and its configuration file in it. @@ -19,23 +17,23 @@ const SUPABASE_CONFIG_FILE: &str = "config.toml"; /// The port of the local Supabase database if `[db] port` is not set. const SUPABASE_DEFAULT_DB_PORT: u16 = 54322; -pub(super) struct Supabase; +pub struct Supabase; impl MigrationTool for Supabase { - fn name(&self) -> &'static str { - "Supabase" + fn id(&self) -> ToolId { + ToolId::Supabase } - fn section_markers(&self) -> SectionMarkers { - SectionMarkers::None + fn name(&self) -> &'static str { + "Supabase" } - fn default_migrations_dir(&self) -> Option<&'static str> { + fn default_dir(&self) -> Option<&'static str> { Some("supabase/migrations") } /// A `supabase` folder with a `config.toml`, and the local database of `supabase status`. - fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { + fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { let path = root.path(SUPABASE_DIRECTORY).join(SUPABASE_CONFIG_FILE); if !fs.path_is_file(&path) { return None; @@ -46,7 +44,7 @@ impl MigrationTool for Supabase { .and_then(|mut file| file.read_to_string(&mut contents)) .ok() .and_then(|_| supabase_db_port(&contents)); - let mut project = Project::new(self, root, SUPABASE_DIRECTORY); + let mut project = Detected::new(self, root, SUPABASE_DIRECTORY); if port.is_none() { project.warning = Some(format!( "Could not read the database port from {}/{SUPABASE_CONFIG_FILE}, using the default port {SUPABASE_DEFAULT_DB_PORT}.", @@ -78,6 +76,19 @@ impl MigrationTool for Supabase { .parse::() .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) } + + fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { + naming::numbered(path, migrations_dir, |text| self.version(text)) + } + + fn version(&self, text: &str) -> Option { + naming::number(text) + } + + /// The Supabase CLI runs every migration file as one migration. + fn read(&self, _text: &str) -> Script { + Script::default() + } } /// Returns `[db] port` of a Supabase configuration, or the default port if it is not set. @@ -99,6 +110,14 @@ fn supabase_db_port(contents: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::{ToolId, tests::marker_problems}; + + #[test] + fn supabase_files_are_one_migration() { + let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; + assert!(Supabase.read(text).units.is_empty()); + assert!(marker_problems(text, ToolId::Supabase).is_empty()); + } #[test] fn reads_the_port_from_the_db_section() { diff --git a/crates/pgls_migrations/src/units.rs b/crates/pgls_migrations/src/units.rs new file mode 100644 index 0000000000..147c2f3f0d --- /dev/null +++ b/crates/pgls_migrations/src/units.rs @@ -0,0 +1,63 @@ +//! The units of a migration file: the parts that the tool runs on their own, like dbmate's up and +//! down sections. + +use pgls_text_size::{TextRange, TextSize}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Direction { + Up, + Down, +} + +/// A part of a migration file that the migration tool runs on its own. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Unit { + /// From the start of the unit to the start of the next one, or to the end of the file. + pub range: TextRange, + pub direction: Direction, + /// Whether the migration tool runs the unit in a transaction. + pub transaction: bool, + /// The parts of the unit that the migration tool sends to Postgres as one query, like a + /// whole dbmate section or a goose `StatementBegin` block. Postgres runs a query with more + /// than one statement in a transaction block, also in a unit without a transaction. The + /// other statements of the unit are sent one by one. + pub queries: Vec, +} + +/// The units of a file, built from its markers in file order. +pub(crate) struct UnitsBuilder { + units: Vec, + end: TextSize, +} + +impl UnitsBuilder { + pub(crate) fn new(text: &str) -> Self { + Self { + units: Vec::new(), + end: TextSize::of(text), + } + } + + /// Starts a unit at the marker at `start`. It ends where the next one starts, or at the end + /// of the file. + pub(crate) fn start(&mut self, start: TextSize, direction: Direction, transaction: bool) { + if let Some(previous) = self.units.last_mut() { + previous.range = TextRange::new(previous.range.start(), start); + } + self.units.push(Unit { + range: TextRange::new(start, self.end), + direction, + transaction, + queries: Vec::new(), + }); + } + + /// The unit that the line being read is in. + pub(crate) fn current(&mut self) -> Option<&mut Unit> { + self.units.last_mut() + } + + pub(crate) fn finish(self) -> Vec { + self.units + } +} diff --git a/crates/pgls_workspace/Cargo.toml b/crates/pgls_workspace/Cargo.toml index 5d4f3b6025..4df2bea440 100644 --- a/crates/pgls_workspace/Cargo.toml +++ b/crates/pgls_workspace/Cargo.toml @@ -18,6 +18,7 @@ ignore = { workspace = true } json-strip-comments = { workspace = true } pgls_analyse = { workspace = true, features = ["serde"] } pgls_analyser = { workspace = true } +pgls_migrations = { workspace = true } pgls_catalog = { workspace = true } pgls_completions = { workspace = true } pgls_configuration = { workspace = true } @@ -47,7 +48,6 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["raw_value"] } slotmap = { workspace = true, features = ["serde"] } strum = { workspace = true } -toml = { workspace = true } tracing = { workspace = true, features = ["attributes", "log"] } tree-sitter.workspace = true diff --git a/crates/pgls_workspace/src/lib.rs b/crates/pgls_workspace/src/lib.rs index 5f8e34c0b3..f31b99bb30 100644 --- a/crates/pgls_workspace/src/lib.rs +++ b/crates/pgls_workspace/src/lib.rs @@ -7,7 +7,6 @@ pub mod configuration; pub mod diagnostics; pub mod dome; pub mod features; -pub mod migrations; pub mod settings; pub mod workspace; diff --git a/crates/pgls_workspace/src/migrations.rs b/crates/pgls_workspace/src/migrations.rs deleted file mode 100644 index b506fb3bee..0000000000 --- a/crates/pgls_workspace/src/migrations.rs +++ /dev/null @@ -1,184 +0,0 @@ -//! Migration tools: everything the language server knows about a tool is one [`MigrationTool`]. -//! -//! - How to find a project of the tool, and what `init` sets for it ([`project`]). -//! - Which files are migrations, and their sequence numbers, for `migrations.after`. -//! - How a file splits into up and down sections, and the diagnostics for its markers. -//! -//! The workspace runs a file with the tool of `migrations.sectionMarkers`, see -//! [`migration_tool`]. A document gets the tool from the settings of its file, and keeps the -//! sections of its current content. -//! -//! The tools: -//! - [`Plain`]: no tool, every file is one migration. -//! - [`Supabase`]: the Supabase CLI. Every file is one migration. -//! - [`Dbmate`] and [`Goose`]: up and down sections in one file, separated by marker comments. - -mod dbmate; -mod files; -mod goose; -mod markers; -pub mod project; -mod supabase; - -use std::path::Path; - -use pgls_analyser::MigrationSection; -use pgls_configuration::migrations::SectionMarkers; -use pgls_fs::FileSystem; -use pgls_text_size::TextRange; - -pub(crate) use files::Migration; -pub(crate) use markers::SectionMarkerDiagnostic; - -use dbmate::Dbmate; -use goose::Goose; -use project::{Project, Root}; -use supabase::Supabase; - -/// What the language server knows about a migration tool. Every method but the name and the -/// setting has a default that fits a tool that runs every file as one migration. -pub(crate) trait MigrationTool: Sync { - /// The name of the tool, as its users know it. - fn name(&self) -> &'static str; - - /// The value of `migrations.sectionMarkers` that makes the workspace run files with the tool. - fn section_markers(&self) -> SectionMarkers; - - /// The folder of the migrations in a project of the tool, relative to the project, if the - /// tool has one by default. - fn default_migrations_dir(&self) -> Option<&'static str> { - None - } - - /// The project of the tool in `root`, for `init`. Only tools with a reliable sign of their - /// projects find them. - fn detect(&self, _fs: &dyn FileSystem, _root: &Root) -> Option { - None - } - - /// Whether `directory` is the folder of the tool inside a project, like `supabase`, so that - /// `init` configures the project above it. - fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { - false - } - - /// Whether the file is a migration, given `migrations.migrationsDir`. Migration-only rules - /// don't run on other files. - fn is_migration(&self, path: &Path, migrations_dir: &Path) -> bool { - files::is_in_migrations_dir(path, migrations_dir) - } - - /// The migration of a file in the migrations folder, with its sequence number, if its name - /// is one of a migration. - fn migration(&self, path: &Path, migrations_dir: &Path) -> Option { - files::get_migration(path, migrations_dir) - } - - /// The sections of a migration file, in file order. A file without markers has none, and is - /// one migration. - /// - /// Markers are found line by line, like the migration tools do, without looking at the SQL. - fn sections(&self, _text: &str) -> Vec { - Vec::new() - } - - /// The problems with the markers of a file with the given sections and statements: - /// statements before the first marker, which the tools reject, and lines that look like - /// markers but that the tools read as plain comments. - fn marker_diagnostics( - &self, - _text: &str, - _sections: &[MigrationSection], - _statements: &[TextRange], - ) -> Vec { - Vec::new() - } -} - -/// Migration files without a tool: every file is one migration. -pub(crate) struct Plain; - -impl MigrationTool for Plain { - fn name(&self) -> &'static str { - "plain SQL" - } - - fn section_markers(&self) -> SectionMarkers { - SectionMarkers::None - } -} - -/// The tools, in the order `init` reports their projects. -const TOOLS: &[&dyn MigrationTool] = &[&Supabase, &Dbmate, &Goose, &Plain]; - -/// The tool that the workspace runs files with for `migrations.sectionMarkers`. Supabase runs -/// every file as one migration, so its projects use [`Plain`]. -pub(crate) fn migration_tool(markers: SectionMarkers) -> &'static dyn MigrationTool { - match markers { - SectionMarkers::None => &Plain, - SectionMarkers::Dbmate => &Dbmate, - SectionMarkers::Goose => &Goose, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use pgls_analyser::Direction; - - pub(super) fn sections(text: &str, markers: SectionMarkers) -> Vec<(&str, Direction, bool)> { - migration_tool(markers) - .sections(text) - .into_iter() - .map(|section| (&text[section.range], section.direction, section.transaction)) - .collect() - } - - pub(super) fn marker_problems(text: &str, markers: SectionMarkers) -> Vec<(&str, String)> { - let tool = migration_tool(markers); - let sections = tool.sections(text); - let statements: Vec = pgls_statement_splitter::split(text).ranges; - tool.marker_diagnostics(text, §ions, &statements) - .into_iter() - .map(|diagnostic| (&text[diagnostic.span], diagnostic.message.to_string())) - .collect() - } - - #[test] - fn no_markers() { - let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; - assert!(sections(text, SectionMarkers::None).is_empty()); - assert!(marker_problems(text, SectionMarkers::None).is_empty()); - assert!(sections("select 1;", SectionMarkers::Dbmate).is_empty()); - } - - #[test] - fn supabase_files_are_one_migration() { - let text = "-- migrate:up\nselect 1;\n-- +goose Down\n"; - assert!(Supabase.sections(text).is_empty()); - assert!( - Supabase - .marker_diagnostics(text, &[], &[TextRange::default()]) - .is_empty() - ); - } - - #[test] - fn selects_the_tool_of_the_section_markers() { - for markers in [ - SectionMarkers::None, - SectionMarkers::Dbmate, - SectionMarkers::Goose, - ] { - assert_eq!(migration_tool(markers).section_markers(), markers); - } - assert_eq!(migration_tool(SectionMarkers::None).name(), "plain SQL"); - assert_eq!(migration_tool(SectionMarkers::Dbmate).name(), "dbmate"); - assert_eq!(migration_tool(SectionMarkers::Goose).name(), "goose"); - // Supabase has no markers of its own, so its files run like plain ones. - assert_eq!( - migration_tool(Supabase.section_markers()).name(), - "plain SQL" - ); - } -} diff --git a/crates/pgls_workspace/src/migrations/files.rs b/crates/pgls_workspace/src/migrations/files.rs deleted file mode 100644 index 03ae50e49a..0000000000 --- a/crates/pgls_workspace/src/migrations/files.rs +++ /dev/null @@ -1,150 +0,0 @@ -//! Which files are migrations, and their sequence numbers: the defaults of -//! [`super::MigrationTool::is_migration`] and [`super::MigrationTool::migration`]. Supabase, -//! dbmate and goose name the files the same way, `_.sql`. - -use std::path::Path; - -#[derive(Debug)] -pub(crate) struct Migration { - pub(crate) sequence_number: u64, - #[allow(unused)] - pub(crate) name: String, -} - -/// Whether the path is inside the migrations directory. -pub(super) fn is_in_migrations_dir(path: &Path, migrations_dir: &Path) -> bool { - if path.starts_with(migrations_dir) { - return true; - } - path.canonicalize() - .ok() - .and_then(|canonical_child| { - migrations_dir - .canonicalize() - .ok() - .map(|canonical_dir| canonical_child.starts_with(&canonical_dir)) - }) - .unwrap_or(false) -} - -/// Get the migration associated with a path, if it is a migration file -pub(super) fn get_migration(path: &Path, migrations_dir: &Path) -> Option { - if !is_in_migrations_dir(path, migrations_dir) { - return None; - } - - // we are trying to match patterns used by popular migration tools - - // in the "root" pattern, all files are directly within the migrations directory - // and their names follow _.sql. - // this is used by supabase - let root_migration = path - .file_name() - .and_then(|os_str| os_str.to_str()) - .and_then(parse_migration_name); - - if root_migration.is_some() { - return root_migration; - } - - // in the "subdirectory" pattern, each migration is in a subdirectory named _ - // this is used by prisma and drizzle - path.parent() - .and_then(|parent| parent.file_name()) - .and_then(|os_str| os_str.to_str()) - .and_then(parse_migration_name) -} - -fn parse_migration_name(name: &str) -> Option { - let mut parts = name.splitn(2, '_'); - // remove leading zeros to support numeric - let sequence_number: u64 = parts.next()?.trim_start_matches('0').parse().ok()?; - let full_name = parts.next()?; - let name = full_name - .strip_suffix(".sql") - .unwrap_or(full_name) - .to_string(); - Some(Migration { - sequence_number, - 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.sequence_number, 1234567890); - assert_eq!(migration.name, "create_users"); - } - - #[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.sequence_number, 1234567890); - assert_eq!(migration.name, "create_users"); - } - - #[test] - fn test_get_migration_prefix_number() { - let temp_dir = setup(); - let migrations_dir = temp_dir.path().to_path_buf(); - let path = migrations_dir.join("000201_a_migration.sql"); - fs::write(&path, "").unwrap(); - - let migration = get_migration(&path, &migrations_dir); - - assert!(migration.is_some()); - let migration = migration.unwrap(); - assert_eq!(migration.sequence_number, 201); - assert_eq!(migration.name, "a_migration"); - } - - #[test] - fn test_get_migration_not_timestamp_in_filename() { - 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/crates/pgls_workspace/src/migrations/markers.rs b/crates/pgls_workspace/src/migrations/markers.rs deleted file mode 100644 index 7ee5346ce2..0000000000 --- a/crates/pgls_workspace/src/migrations/markers.rs +++ /dev/null @@ -1,128 +0,0 @@ -//! What the adapters of the migration tools share: reading a file line by line, building its -//! sections from the markers, and the diagnostics for misplaced markers. - -use pgls_analyser::{Direction, MigrationSection}; -use pgls_diagnostics::{Diagnostic, MessageAndDescription}; -use pgls_text_size::{TextRange, TextSize}; - -/// The lines of `text` without their line endings, with the range of the whole line including -/// its line ending. -pub(super) fn lines(text: &str) -> impl Iterator { - let mut offset = TextSize::from(0); - text.split_inclusive('\n').map(move |line| { - let range = TextRange::at(offset, TextSize::of(line)); - offset = range.end(); - (range, line.trim_end_matches(['\n', '\r'])) - }) -} - -/// The sections of a file, built from its markers in file order. -pub(super) struct SectionsBuilder { - sections: Vec, - end: TextSize, -} - -impl SectionsBuilder { - pub(super) fn new(text: &str) -> Self { - Self { - sections: Vec::new(), - end: TextSize::of(text), - } - } - - /// Starts a section at the marker at `start`. It ends where the next one starts, or at the - /// end of the file. - pub(super) fn start(&mut self, start: TextSize, direction: Direction, transaction: bool) { - if let Some(previous) = self.sections.last_mut() { - previous.range = TextRange::new(previous.range.start(), start); - } - self.sections.push(MigrationSection { - range: TextRange::new(start, self.end), - direction, - transaction, - queries: Vec::new(), - }); - } - - /// The section that the line being read is in. - pub(super) fn current(&mut self) -> Option<&mut MigrationSection> { - self.sections.last_mut() - } - - pub(super) fn finish(self) -> Vec { - self.sections - } -} - -/// A problem with the section markers of a migration file that makes the migration tool reject -/// the file, or ignore a marker. -#[derive(Debug, Diagnostic)] -#[diagnostic(category = "migrations/sectionMarkers", severity = Warning)] -pub(crate) struct SectionMarkerDiagnostic { - #[location(span)] - pub(super) span: TextRange, - #[message] - #[description] - pub(super) message: MessageAndDescription, -} - -impl SectionMarkerDiagnostic { - fn new(span: TextRange, message: &str) -> Self { - Self { - span, - message: MessageAndDescription::from(message.to_string()), - } - } -} - -/// The markers of a migration tool, as far as the diagnostics need them. -pub(super) struct MarkerSyntax { - /// The name of the tool. - pub(super) tool: &'static str, - /// The marker that starts the first section, like `-- migrate:up`. - pub(super) first_marker: &'static str, - /// Whether the tool reads the line as a marker. - pub(super) is_marker: fn(&str) -> bool, - /// What the tool does with a line that looks like a marker but isn't one. - pub(super) look_alike_message: fn(&str) -> &'static str, -} - -/// See [`super::MigrationTool::marker_diagnostics`]. -pub(super) fn marker_diagnostics( - syntax: &MarkerSyntax, - text: &str, - sections: &[MigrationSection], - statements: &[TextRange], -) -> Vec { - let is_marker = syntax.is_marker; - - let mut diagnostics = Vec::new(); - if let Some(first) = sections.first() { - let message = format!( - "{} rejects a migration with statements before the first `{}`.", - syntax.tool, syntax.first_marker - ); - diagnostics.extend( - statements - .iter() - .take_while(|statement| statement.start() < first.range.start()) - .map(|statement| SectionMarkerDiagnostic::new(*statement, &message)), - ); - } - - for (range, line) in lines(text) { - let looks_like_marker = || { - let trimmed = line.trim_start(); - is_marker(trimmed) || is_marker(&trimmed.to_ascii_lowercase()) - }; - if !is_marker(line) && looks_like_marker() { - let span = TextRange::at(range.start(), TextSize::of(line)); - diagnostics.push(SectionMarkerDiagnostic::new( - span, - (syntax.look_alike_message)(line), - )); - } - } - diagnostics.sort_by_key(|diagnostic| diagnostic.span.start()); - diagnostics -} diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 8f4bf9b6f8..a9f333db4a 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -25,8 +25,8 @@ use pgls_analyse::{AnalysisFilter, RuleCategories, RuleCategoriesBuilder, RuleFi #[cfg(feature = "db")] use pgls_analyser::StatementAnalysis; use pgls_analyser::{ - Analyser, AnalyserConfig, AnalyserParams, AnalysisResult, Direction, FileKind, - LinterDiagnostic, LinterOptions, MigrationSection, + Analyser, AnalyserConfig, AnalyserParams, AnalysisResult, FileKind, LinterDiagnostic, + LinterOptions, transaction_mode, }; use pgls_catalog::resolve::{ColumnSource, Reference, ResolveParams, resolve}; use pgls_catalog::{Catalog, CatalogBase, Origin, Session, TransactionMode}; @@ -34,10 +34,12 @@ use pgls_catalog::{Catalog, CatalogBase, Origin, Session, TransactionMode}; use pgls_catalog::Snapshot; use pgls_configuration::RuleSelector; use pgls_configuration::inlay_hints::ParameterNameHints; +use pgls_configuration::migrations::SectionMarkers; use pgls_diagnostics::{ Diagnostic, DiagnosticExt, Error, Severity, serde::Diagnostic as SDiagnostic, }; use pgls_fs::{ConfigName, PgLSPath}; +use pgls_migrations::{Direction, MigrationTool, Role, ToolId, Unit, Version}; use pgls_query_ext::diagnostics::SyntaxDiagnostic; use pgls_text_size::{TextRange, TextSize}; #[cfg(feature = "db")] @@ -64,7 +66,6 @@ use crate::{ on_hover::{OnHoverParams, OnHoverResult}, semantic_tokens::{PullSemanticTokensParams, PullSemanticTokensResult, SemanticToken}, }, - migrations::{self, MigrationTool}, settings::{Settings, WorkspaceSettings, WorkspaceSettingsHandle, WorkspaceSettingsHandleMut}, workspace::{AnalyserDiagnosticsMapper, WithCSTMapper}, }; @@ -243,7 +244,7 @@ impl WorkspaceServer { .and_then(|settings| settings.migrations.as_ref()) .map(|migrations| migrations.section_markers) .unwrap_or_default(); - migrations::migration_tool(section_markers) + tool_of_section_markers(section_markers) } /// Applies the changed settings of a project to its open documents. @@ -276,15 +277,14 @@ impl WorkspaceServer { /// Whether the file is a migration. Migration-only rules don't run on other files. fn file_kind(&self, settings: &Settings, path: &Path) -> FileKind { let Some((tool, migrations_dir)) = settings.migrations.as_ref().and_then(|migrations| { - let tool = migrations::migration_tool(migrations.section_markers); + let tool = tool_of_section_markers(migrations.section_markers); Some((tool, migrations.path.as_ref()?)) }) else { return FileKind::Unknown; }; - if tool.is_migration(path, migrations_dir) { - FileKind::Migration - } else { - FileKind::Other + match tool.file(path, migrations_dir).role { + Role::Migration => FileKind::Migration, + Role::Other => FileKind::Other, } } @@ -478,17 +478,16 @@ impl WorkspaceServer { .unwrap_or_default(); let sections = doc.migration_sections(); let search_path = snapshot.expand_search_path(&search_path); - let new_session = |section: Option<&MigrationSection>| { - Session::new(search_path.clone()).with_transaction_mode( - section.map_or(TransactionMode::Unknown, MigrationSection::transaction_mode), - ) + let new_session = |section: Option<&Unit>| { + Session::new(search_path.clone()) + .with_transaction_mode(section.map_or(TransactionMode::Unknown, transaction_mode)) }; // Editor features show what names most likely refer to, so a `DO` block or a statement // that doesn't parse doesn't turn them off for the rest of the file. let mut catalog = Catalog::new(Some(self.catalog_base(snapshot))).best_effort(); let mut session = new_session(None); - let mut section: Option<&MigrationSection> = None; + let mut section: Option<&Unit> = None; // The catalog after the up sections so far, while walking a down section. let mut up_catalog: Option = None; for (stmt, _, unparsable) in doc.iter(AnalyserDiagnosticsMapper) { @@ -889,6 +888,17 @@ impl WorkspaceServer { } } +/// The tool that the workspace runs files with for `migrations.sectionMarkers`. Supabase runs +/// every file as one migration, so its projects use plain SQL. +fn tool_of_section_markers(markers: SectionMarkers) -> &'static dyn MigrationTool { + match markers { + SectionMarkers::None => ToolId::Plain, + SectionMarkers::Dbmate => ToolId::Dbmate, + SectionMarkers::Goose => ToolId::Goose, + } + .tool() +} + /// Check whether a file is a migration before `migrations.after` fn is_ignored_by_migration_settings(settings: &Settings, path: &Path) -> bool { settings @@ -897,10 +907,11 @@ fn is_ignored_by_migration_settings(settings: &Settings, path: &Path) -> bool { .and_then(|migration_settings| { let ignore_before = migration_settings.after.as_ref()?; let migrations_dir = migration_settings.path.as_ref()?; - let migration = migrations::migration_tool(migration_settings.section_markers) - .migration(path, migrations_dir)?; + let version = tool_of_section_markers(migration_settings.section_markers) + .file(path, migrations_dir) + .version?; - Some(&migration.sequence_number <= ignore_before) + Some(version <= Version::Number(*ignore_before)) }) .unwrap_or(false) } @@ -1346,12 +1357,8 @@ impl Workspace for WorkspaceServer { let path = params.path.as_path().display().to_string(); diagnostics.extend( - doc.migration_tool() - .marker_diagnostics( - doc.get_document_content(), - doc.migration_sections(), - doc.statement_ranges(), - ) + doc.migration_script() + .marker_diagnostics(doc.statement_ranges()) .into_iter() .map(|diagnostic| SDiagnostic::new(Error::from(diagnostic).with_file_path(&path))), ); diff --git a/crates/pgls_workspace/src/workspace/server.tests.rs b/crates/pgls_workspace/src/workspace/server.tests.rs index 96ef2ffafa..68e030a1a7 100644 --- a/crates/pgls_workspace/src/workspace/server.tests.rs +++ b/crates/pgls_workspace/src/workspace/server.tests.rs @@ -324,6 +324,7 @@ fn section_markers_follow_settings_changes() { gitignore_matches: vec![], vcs_base_path: None, workspace_directory: None, + replace: true, }) .expect("Unable to update the settings"); }; diff --git a/crates/pgls_workspace/src/workspace/server/document.rs b/crates/pgls_workspace/src/workspace/server/document.rs index 6607f316d5..3ca4b6353b 100644 --- a/crates/pgls_workspace/src/workspace/server/document.rs +++ b/crates/pgls_workspace/src/workspace/server/document.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use pgls_analyser::{AnalysableStatement, MigrationSection, UnparsableStatement}; +use pgls_analyser::{AnalysableStatement, UnparsableStatement}; use pgls_diagnostics::{Diagnostic, DiagnosticExt, serde::Diagnostic as SDiagnostic}; use pgls_query_ext::{diagnostics::SyntaxDiagnostic, function_body::OffsetMap}; use pgls_suppressions::Suppressions; @@ -8,8 +8,10 @@ use pgls_text_size::{TextRange, TextSize}; use rustc_hash::FxHashSet; #[cfg(test)] -use crate::migrations::Plain; -use crate::{migrations::MigrationTool, workspace::ProjectKey}; +use pgls_migrations::Plain; +use pgls_migrations::{MigrationTool, Script, Unit}; + +use crate::workspace::ProjectKey; use super::{ pg_query::{PgQueryStore, convert_to_positional_params_with_metadata}, @@ -29,8 +31,8 @@ pub struct Document { project_key: ProjectKey, /// The migration tool that runs the file, from its settings. See [`Document::open`]. migration_tool: &'static dyn MigrationTool, - /// The sections of the content for `migration_tool`. - migration_sections: Vec, + /// How `migration_tool` runs the content. + migration_script: Script, } impl Document { @@ -52,7 +54,7 @@ impl Document { let ast_db = PgQueryStore::new(); let (ranges, diagnostics) = split_with_diagnostics(&content, None); let suppressions = Suppressions::from(content.as_str()).with_statements(ranges.clone()); - let migration_sections = migration_tool.sections(&content); + let migration_script = migration_tool.read(&content); Document { ranges, @@ -64,14 +66,14 @@ impl Document { suppressions, project_key, migration_tool, - migration_sections, + migration_script, } } pub fn update_content(&mut self, content: String, version: i32) { self.content = content; self.version = version; - self.migration_sections = self.migration_tool.sections(&self.content); + self.migration_script = self.migration_tool.read(&self.content); let (ranges, diagnostics) = split_with_diagnostics(&self.content, None); @@ -95,7 +97,7 @@ impl Document { /// Applies the migration tool of changed settings. pub(crate) fn set_migration_tool(&mut self, migration_tool: &'static dyn MigrationTool) { self.migration_tool = migration_tool; - self.migration_sections = migration_tool.sections(&self.content); + self.migration_script = migration_tool.read(&self.content); } pub(crate) fn project_key(&self) -> ProjectKey { @@ -107,10 +109,15 @@ impl Document { self.migration_tool } + /// How the migration tool runs the document. + pub(crate) fn migration_script(&self) -> &Script { + &self.migration_script + } + /// The migration sections of the document, like the up and down migrations of dbmate and /// goose. A tool without section markers has none. - pub fn migration_sections(&self) -> &[MigrationSection] { - &self.migration_sections + pub fn migration_sections(&self) -> &[Unit] { + &self.migration_script.units } pub fn suppressions(&self) -> &Suppressions { From 15e95ab338aa22c833be7c1676376151bd78a2cb Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 9 Nov 2026 08:07:36 +0000 Subject: [PATCH 4/5] chore: sort the pgls_workspace dependencies --- crates/pgls_workspace/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pgls_workspace/Cargo.toml b/crates/pgls_workspace/Cargo.toml index 4df2bea440..e3d8757eac 100644 --- a/crates/pgls_workspace/Cargo.toml +++ b/crates/pgls_workspace/Cargo.toml @@ -18,7 +18,6 @@ ignore = { workspace = true } json-strip-comments = { workspace = true } pgls_analyse = { workspace = true, features = ["serde"] } pgls_analyser = { workspace = true } -pgls_migrations = { workspace = true } pgls_catalog = { workspace = true } pgls_completions = { workspace = true } pgls_configuration = { workspace = true } @@ -30,6 +29,7 @@ pgls_hover = { workspace = true } pgls_inlay_hints = { workspace = true } pgls_lexer = { workspace = true } pgls_matcher = { workspace = true } +pgls_migrations = { workspace = true } pgls_pretty_print = { workspace = true } pgls_query = { workspace = true } pgls_query_ext = { workspace = true } From 86e123f8fddeb923f284c6837b0ff78aa17ecac8 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 9 Nov 2026 09:26:07 +0000 Subject: [PATCH 5/5] refactor(migrations): detect the tool's own folder in detect, and the history tables in database - `is_tool_directory` is gone: Supabase's `detect` also finds the project when `init` runs inside its `supabase` folder, and says so with `Detected::configuration_directory` (`..`). `init` takes the folder of the configuration from the project it found. - `MigrationTool::database` tells where the tool records the migrations it ran (`History::Table` or `History::Schema`). Completions rank the history of every tool low, instead of the hard-coded `supabase_migrations` schema; sqlx's `_sqlx_migrations` stays as an extra until sqlx is a tool. --- Cargo.lock | 1 + crates/pgls_cli/src/commands/init.rs | 13 +-- crates/pgls_completions/Cargo.toml | 1 + .../pgls_completions/src/providers/tables.rs | 28 ++++++ .../pgls_completions/src/relevance/scoring.rs | 22 +++-- crates/pgls_migrations/src/detect.rs | 66 +++++++------ crates/pgls_migrations/src/lib.rs | 22 ++++- crates/pgls_migrations/src/tools/dbmate.rs | 11 ++- crates/pgls_migrations/src/tools/goose.rs | 8 +- crates/pgls_migrations/src/tools/plain.rs | 7 +- crates/pgls_migrations/src/tools/supabase.rs | 92 +++++++++++-------- 11 files changed, 178 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4312080ca8..061ae20d25 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2383,6 +2383,7 @@ dependencies = [ "fuzzy-matcher", "insta", "pgls_catalog", + "pgls_migrations", "pgls_test_utils", "pgls_text_size", "pgls_treesitter", diff --git a/crates/pgls_cli/src/commands/init.rs b/crates/pgls_cli/src/commands/init.rs index 5f257992d1..c9e9935dbf 100644 --- a/crates/pgls_cli/src/commands/init.rs +++ b/crates/pgls_cli/src/commands/init.rs @@ -3,17 +3,18 @@ use pgls_configuration::PartialConfiguration; use pgls_configuration::migrations::{PartialMigrationsConfiguration, SectionMarkers}; use pgls_console::{ConsoleExt, markup}; use pgls_fs::ConfigName; -use pgls_migrations::{ - Detected, ToolId, - detect::{configuration_directory, find_projects}, -}; +use pgls_migrations::{Detected, ToolId, detect::find_projects}; use pgls_workspace::configuration::create_config; +use std::path::Path; pub(crate) fn init(mut session: CliSession) -> Result<(), CliDiagnostic> { let fs = &mut session.app.fs; let config = &mut PartialConfiguration::init(); - let project_directory = configuration_directory(&**fs); - let mut projects = find_projects(&**fs, project_directory); + let mut projects = find_projects(&**fs); + // Inside the folder of a tool, like `supabase`, the configuration goes into the project. + let project_directory = projects + .first() + .map_or(Path::new(""), |project| project.configuration_directory); let (project, ambiguous) = if projects.len() == 1 { (projects.pop(), Vec::new()) } else { diff --git a/crates/pgls_completions/Cargo.toml b/crates/pgls_completions/Cargo.toml index 4f14d22119..3da1d5aefe 100644 --- a/crates/pgls_completions/Cargo.toml +++ b/crates/pgls_completions/Cargo.toml @@ -13,6 +13,7 @@ version = "0.0.0" [dependencies] pgls_catalog = { workspace = true } +pgls_migrations = { workspace = true } pgls_text_size = { workspace = true, features = ["serde"] } pgls_treesitter.workspace = true diff --git a/crates/pgls_completions/src/providers/tables.rs b/crates/pgls_completions/src/providers/tables.rs index b9c8b36255..741687a617 100644 --- a/crates/pgls_completions/src/providers/tables.rs +++ b/crates/pgls_completions/src/providers/tables.rs @@ -262,6 +262,34 @@ mod tests { assert_eq!(kind, CompletionItemKind::Table); } + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] + async fn ranks_the_history_tables_of_migration_tools_low(pool: PgPool) { + let setup = r#" + create table goose_db_version (id serial primary key, version_id bigint); + create table goose_feathers_and_eggs (id serial primary key); + "#; + + let query = format!( + r#"select * from goose{}"#, + QueryWithCursorPosition::cursor_marker() + ); + + let (tree, cache) = get_test_deps(Some(setup), query.as_str().into(), &pool).await; + let params = get_test_params(&tree, &cache, query.as_str().into()); + let labels: Vec = complete(params) + .into_iter() + .map(|item| item.label) + .collect(); + + let position = |label: &str| labels.iter().position(|l| l == label); + // goose's table matches as well as the other one, and is shorter, but comes after it. + assert!( + position("goose_feathers_and_eggs") < position("goose_db_version"), + "{labels:?}" + ); + assert!(position("goose_db_version").is_some(), "{labels:?}"); + } + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] async fn suggests_tables_in_update(pool: PgPool) { let setup = r#" diff --git a/crates/pgls_completions/src/relevance/scoring.rs b/crates/pgls_completions/src/relevance/scoring.rs index 37ec466272..2e478da82d 100644 --- a/crates/pgls_completions/src/relevance/scoring.rs +++ b/crates/pgls_completions/src/relevance/scoring.rs @@ -1,5 +1,6 @@ use fuzzy_matcher::{FuzzyMatcher, skim::SkimMatcherV2}; +use pgls_migrations::{History, TOOLS}; use pgls_treesitter::context::{TreesitterContext, WrappingClause}; use crate::{providers::helper::word_at_cursor, sanitization}; @@ -472,15 +473,20 @@ impl CompletionScore<'_> { } } + /// Ranks the tables where migration tools record the migrations they ran low. fn check_is_not_wellknown_migration(&mut self, _ctx: &TreesitterContext) { - if let Some(table_name) = self.get_table_name() - && ["_sqlx_migrations"].contains(&table_name) - { - self.score -= 10; - } - - if let Some(schema_name) = self.get_schema_name() - && ["supabase_migrations"].contains(&schema_name) + let table_name = self.get_table_name(); + let schema_name = self.get_schema_name(); + let is_history = |history: &History| match history { + History::Table(name) => table_name == Some(*name), + History::Schema(name) => schema_name == Some(*name), + }; + if TOOLS + .iter() + .filter_map(|tool| tool.database().history) + .any(|history| is_history(&history)) + // sqlx records its migrations here, but isn't a migration tool of ours yet. + || table_name == Some("_sqlx_migrations") { self.score -= 10; } diff --git a/crates/pgls_migrations/src/detect.rs b/crates/pgls_migrations/src/detect.rs index 4f2403474b..7e6b9e47cc 100644 --- a/crates/pgls_migrations/src/detect.rs +++ b/crates/pgls_migrations/src/detect.rs @@ -14,6 +14,11 @@ const SKIPPED_DIRECTORIES: &[&str] = &["node_modules", "target", "supabase"]; pub struct Detected { /// The tool. pub tool: ToolId, + /// The folder of the configuration file, relative to the working directory: `..` when + /// `init` runs in the folder of the tool inside a project, like `supabase`, so that the + /// configuration applies to the whole project like the tool's does, and the editor finds it + /// when it opens the project. The other paths are relative to it. + pub configuration_directory: &'static Path, /// The folder of the tool in the project, like `supabase` or `apps/web/supabase`, relative to /// the folder of the configuration file, with `/` as the separator. pub directory: String, @@ -41,6 +46,7 @@ impl Detected { pub(crate) fn new(tool: &dyn MigrationTool, root: &Root, directory: &str) -> Self { Self { tool: tool.id(), + configuration_directory: Path::new(""), directory: root.relative(directory), migrations_dir: tool .default_dir() @@ -70,35 +76,22 @@ impl Root { pub(crate) fn relative(&self, relative: &str) -> String { format!("{}{relative}", self.prefix) } -} -/// The folder of the configuration, relative to the working directory: the folder above it if -/// the working directory is the folder of a tool inside a project, like `supabase`, so that the -/// configuration applies to the whole project like the tool's does, and the editor finds it -/// when it opens the project. -pub fn configuration_directory(fs: &dyn FileSystem) -> &'static Path { - let working_directory = fs.working_directory().unwrap_or_default(); - if TOOLS - .iter() - .any(|tool| tool.is_tool_directory(fs, &working_directory)) - { - Path::new("..") - } else { - Path::new("") + /// Whether the folder is the working directory, where `init` runs. + pub(crate) fn is_working_directory(&self) -> bool { + self.prefix.is_empty() } } -/// The projects of the migration tools for the configuration in `configuration_directory`, -/// relative to the working directory: the ones in it, or else, if it is the working directory, -/// the ones in the folders one or two levels below it, as in a monorepo (`supabase/`, -/// `db/supabase/`, `apps/web/supabase/`). -pub fn find_projects(fs: &dyn FileSystem, configuration_directory: &Path) -> Vec { +/// The projects of the migration tools for `init`: the ones in the working directory, or the +/// one whose tool's folder it is (see [`Detected::configuration_directory`]), or else the ones in +/// the folders one or two levels below it, as in a monorepo (`supabase/`, `db/supabase/`, +/// `apps/web/supabase/`). +pub fn find_projects(fs: &dyn FileSystem) -> Vec { let working_directory = fs.working_directory().unwrap_or_default(); let detect = |relative: &str| { let root = Root { - path: working_directory - .join(configuration_directory) - .join(relative), + path: working_directory.join(relative), prefix: if relative.is_empty() { String::new() } else { @@ -111,7 +104,7 @@ pub fn find_projects(fs: &dyn FileSystem, configuration_directory: &Path) -> Vec .collect::>() }; let found = detect(""); - if !found.is_empty() || !configuration_directory.as_os_str().is_empty() { + if !found.is_empty() { return found; } let mut found = Vec::new(); @@ -169,7 +162,7 @@ mod tests { fn find(dir: &TempDir) -> Vec<(String, &'static str, Option)> { let fs = OsFileSystem::new(dir.path().to_path_buf()); - find_projects(&fs, Path::new("")) + find_projects(&fs) .into_iter() .map(|project| { ( @@ -192,8 +185,9 @@ mod tests { let dir = TempDir::new().unwrap(); write(&dir, "supabase/config.toml", "[db]\nport = 6543\n"); let fs = OsFileSystem::new(dir.path().to_path_buf()); - let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); + let [project] = find_projects(&fs).try_into().unwrap(); assert_eq!(project.tool, ToolId::Supabase); + assert_eq!(project.configuration_directory, Path::new("")); assert_eq!(project.directory, "supabase"); assert_eq!( project.migrations_dir.as_deref(), @@ -212,7 +206,7 @@ mod tests { "-- migrate:up\ncreate table users (id int);\n\n-- migrate:down\ndrop table users;\n", ); let fs = OsFileSystem::new(dir.path().to_path_buf()); - let [project] = find_projects(&fs, Path::new("")).try_into().unwrap(); + let [project] = find_projects(&fs).try_into().unwrap(); assert_eq!(project.tool, ToolId::Dbmate); assert_eq!(project.directory, "db"); assert_eq!(project.migrations_dir.as_deref(), Some("db/migrations")); @@ -280,9 +274,23 @@ mod tests { fn configures_the_project_above_the_supabase_folder() { let dir = TempDir::new().unwrap(); write(&dir, "supabase/config.toml", "[db]\nport = 6543\n"); + // dbmate's project above it doesn't count: `init` configures the Supabase project. + write(&dir, "db/migrations/1_init.sql", "-- migrate:up\n"); let fs = OsFileSystem::new(dir.path().join("supabase")); - assert_eq!(configuration_directory(&fs), Path::new("..")); - let fs = OsFileSystem::new(dir.path().to_path_buf()); - assert_eq!(configuration_directory(&fs), Path::new("")); + let [project] = find_projects(&fs).try_into().unwrap(); + assert_eq!(project.tool, ToolId::Supabase); + assert_eq!(project.configuration_directory, Path::new("..")); + assert_eq!(project.directory, "supabase"); + assert_eq!( + project.migrations_dir.as_deref(), + Some("supabase/migrations") + ); + assert_eq!(project.database.map(|database| database.port), Some(6543)); + + // A folder named `supabase` without a `[db]` section is no project of its own. + let other = TempDir::new().unwrap(); + write(&other, "supabase/config.toml", "project_id = \"test\"\n"); + let fs = OsFileSystem::new(other.path().join("supabase")); + assert!(find_projects(&fs).is_empty()); } } diff --git a/crates/pgls_migrations/src/lib.rs b/crates/pgls_migrations/src/lib.rs index 89066ad76c..2638a9c62f 100644 --- a/crates/pgls_migrations/src/lib.rs +++ b/crates/pgls_migrations/src/lib.rs @@ -45,9 +45,8 @@ pub trait MigrationTool: Sync { /// projects. fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option; - /// Whether `directory` is the folder of the tool inside a project, like `supabase`, so that - /// `init` configures the project above it. - fn is_tool_directory(&self, fs: &dyn FileSystem, directory: &Path) -> bool; + /// What the tool keeps in the database it migrates. + fn database(&self) -> Database; /// The folder of the migrations in a project of the tool, relative to the project, if the /// tool has one by default. @@ -91,6 +90,23 @@ impl ToolId { /// The tools, in the order `init` reports their projects. pub const TOOLS: &[&dyn MigrationTool] = &[&Supabase, &Dbmate, &Goose, &Plain]; +/// What a migration tool keeps in the database it migrates. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Database { + /// Where the tool records the migrations it ran, if it does. + pub history: Option, +} + +/// Where a migration tool records the migrations it ran. Completions rank it low, since a +/// migration rarely queries it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum History { + /// A table of this name, in any schema. + Table(&'static str), + /// Every table of this schema. + Schema(&'static str), +} + /// What the path of a file says. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FileInfo { diff --git a/crates/pgls_migrations/src/tools/dbmate.rs b/crates/pgls_migrations/src/tools/dbmate.rs index 0b21fe55d9..34c36aabaa 100644 --- a/crates/pgls_migrations/src/tools/dbmate.rs +++ b/crates/pgls_migrations/src/tools/dbmate.rs @@ -10,7 +10,8 @@ use std::path::Path; use pgls_fs::{FileSystem, OpenOptions}; use crate::{ - Detected, Direction, FileInfo, MigrationTool, Root, Script, ToolId, Version, detect, + Database, Detected, Direction, FileInfo, History, MigrationTool, Root, Script, ToolId, Version, + detect, directives::{self, lines}, naming, units::UnitsBuilder, @@ -55,9 +56,11 @@ impl MigrationTool for Dbmate { .then(|| Detected::new(self, root, DBMATE_DIRECTORY)) } - /// dbmate has no folder of its own to run `init` in. - fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { - false + /// The default of `--migrations-table`. + fn database(&self) -> Database { + Database { + history: Some(History::Table("schema_migrations")), + } } fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { diff --git a/crates/pgls_migrations/src/tools/goose.rs b/crates/pgls_migrations/src/tools/goose.rs index 931b02d5cb..f0c8d36512 100644 --- a/crates/pgls_migrations/src/tools/goose.rs +++ b/crates/pgls_migrations/src/tools/goose.rs @@ -11,7 +11,7 @@ use pgls_fs::FileSystem; use pgls_text_size::TextRange; use crate::{ - Detected, Direction, FileInfo, MigrationTool, Root, Script, ToolId, Version, + Database, Detected, Direction, FileInfo, History, MigrationTool, Root, Script, ToolId, Version, directives::{self, lines}, naming, units::UnitsBuilder, @@ -33,8 +33,10 @@ impl MigrationTool for Goose { None } - fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { - false + fn database(&self) -> Database { + Database { + history: Some(History::Table("goose_db_version")), + } } fn default_dir(&self) -> Option<&'static str> { diff --git a/crates/pgls_migrations/src/tools/plain.rs b/crates/pgls_migrations/src/tools/plain.rs index 8b995130d2..ab6927c9e5 100644 --- a/crates/pgls_migrations/src/tools/plain.rs +++ b/crates/pgls_migrations/src/tools/plain.rs @@ -4,7 +4,7 @@ use std::path::Path; use pgls_fs::FileSystem; -use crate::{Detected, FileInfo, MigrationTool, Root, Script, ToolId, Version, naming}; +use crate::{Database, Detected, FileInfo, MigrationTool, Root, Script, ToolId, Version, naming}; pub struct Plain; @@ -22,8 +22,9 @@ impl MigrationTool for Plain { None } - fn is_tool_directory(&self, _fs: &dyn FileSystem, _directory: &Path) -> bool { - false + /// Plain SQL files are run by hand, which records nothing. + fn database(&self) -> Database { + Database { history: None } } fn default_dir(&self) -> Option<&'static str> { diff --git a/crates/pgls_migrations/src/tools/supabase.rs b/crates/pgls_migrations/src/tools/supabase.rs index b2d856a68a..5b3bb7ce04 100644 --- a/crates/pgls_migrations/src/tools/supabase.rs +++ b/crates/pgls_migrations/src/tools/supabase.rs @@ -7,7 +7,8 @@ use std::path::Path; use pgls_fs::{FileSystem, OpenOptions}; use crate::{ - Detected, FileInfo, LocalDatabase, MigrationTool, Root, Script, ToolId, Version, naming, + Database, Detected, FileInfo, History, LocalDatabase, MigrationTool, Root, Script, ToolId, + Version, naming, }; /// The folder of a project managed by the Supabase CLI, and its configuration file in it. @@ -33,48 +34,33 @@ impl MigrationTool for Supabase { } /// A `supabase` folder with a `config.toml`, and the local database of `supabase status`. + /// + /// Run inside the `supabase` folder itself, which has a `config.toml` with a `[db]` section, + /// `init` configures the project above it. fn detect(&self, fs: &dyn FileSystem, root: &Root) -> Option { let path = root.path(SUPABASE_DIRECTORY).join(SUPABASE_CONFIG_FILE); - if !fs.path_is_file(&path) { - return None; + if fs.path_is_file(&path) { + return Some(self.project(root, read(fs, &path))); } - let mut contents = String::new(); - let port = fs - .open_with_options(&path, OpenOptions::default().read(true)) - .and_then(|mut file| file.read_to_string(&mut contents)) - .ok() - .and_then(|_| supabase_db_port(&contents)); - let mut project = Detected::new(self, root, SUPABASE_DIRECTORY); - if port.is_none() { - project.warning = Some(format!( - "Could not read the database port from {}/{SUPABASE_CONFIG_FILE}, using the default port {SUPABASE_DEFAULT_DB_PORT}.", - project.directory - )); - } - project.database = Some(LocalDatabase { - host: "127.0.0.1", - port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), - username: "postgres", - password: "postgres", - database: "postgres", - }); - Some(project) - } - /// The folder is named `supabase` and has a `config.toml` with a `[db]` section. - fn is_tool_directory(&self, fs: &dyn FileSystem, directory: &Path) -> bool { - let mut contents = String::new(); - directory.file_name() == Some(OsStr::new(SUPABASE_DIRECTORY)) - && fs - .open_with_options( - &directory.join(SUPABASE_CONFIG_FILE), - OpenOptions::default().read(true), - ) - .and_then(|mut file| file.read_to_string(&mut contents)) - .is_ok() + let path = root.path(SUPABASE_CONFIG_FILE); + let contents = read(fs, &path)?; + let in_supabase_directory = root.is_working_directory() + && path.parent().and_then(Path::file_name) == Some(OsStr::new(SUPABASE_DIRECTORY)) && contents .parse::() - .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)) + .is_ok_and(|config| config.get("db").is_some_and(toml::Value::is_table)); + in_supabase_directory.then(|| Detected { + configuration_directory: Path::new(".."), + ..self.project(root, Some(contents)) + }) + } + + fn database(&self) -> Database { + Database { + // The Supabase CLI keeps its migrations and seed files in the schema. + history: Some(History::Schema("supabase_migrations")), + } } fn file(&self, path: &Path, migrations_dir: &Path) -> FileInfo { @@ -91,6 +77,38 @@ impl MigrationTool for Supabase { } } +impl Supabase { + /// The project with the configuration `contents`, if it could be read. Its folders are + /// relative to `root`. + fn project(&self, root: &Root, contents: Option) -> Detected { + let port = contents.as_deref().and_then(supabase_db_port); + let mut project = Detected::new(self, root, SUPABASE_DIRECTORY); + if port.is_none() { + project.warning = Some(format!( + "Could not read the database port from {}/{SUPABASE_CONFIG_FILE}, using the default port {SUPABASE_DEFAULT_DB_PORT}.", + project.directory + )); + } + project.database = Some(LocalDatabase { + host: "127.0.0.1", + port: port.unwrap_or(SUPABASE_DEFAULT_DB_PORT), + username: "postgres", + password: "postgres", + database: "postgres", + }); + project + } +} + +/// The contents of the file at `path`, if it can be read. +fn read(fs: &dyn FileSystem, path: &Path) -> Option { + let mut contents = String::new(); + fs.open_with_options(path, OpenOptions::default().read(true)) + .and_then(|mut file| file.read_to_string(&mut contents)) + .ok()?; + Some(contents) +} + /// Returns `[db] port` of a Supabase configuration, or the default port if it is not set. /// Returns `None` if the configuration is not valid TOML or the port is not a valid port. fn supabase_db_port(contents: &str) -> Option {