From d77ec5733d06d174197b2507556d403533f3a68b Mon Sep 17 00:00:00 2001 From: Danilo Cabello Date: Sun, 8 Mar 2026 10:35:57 -0400 Subject: [PATCH 1/5] linter: add require-enum-value-ordering rule Add a new lint rule that flags ALTER TYPE ... ADD VALUE statements missing BEFORE or AFTER, which silently append to the end of the enum and can cause unexpected ordering. Co-Authored-By: Claude Opus 4.6 --- crates/squawk_linter/src/lib.rs | 7 ++ crates/squawk_linter/src/rules/mod.rs | 2 + .../src/rules/require_enum_value_ordering.rs | 105 ++++++++++++++++++ ..._value_if_not_exists_without_ordering.snap | 11 ++ ..._test__err_add_value_without_ordering.snap | 11 ++ docs/docs/require-enum-value-ordering.md | 28 +++++ docs/sidebars.js | 1 + docs/src/pages/index.js | 5 + 8 files changed, 170 insertions(+) create mode 100644 crates/squawk_linter/src/rules/require_enum_value_ordering.rs create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap create mode 100644 docs/docs/require-enum-value-ordering.md diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 9da89fbe..275d974f 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -55,6 +55,7 @@ use rules::require_concurrent_index_creation; use rules::require_concurrent_index_deletion; use rules::require_timeout_settings; use rules::transaction_nesting; +use rules::require_enum_value_ordering; // xtask:new-rule:rule-import #[derive(Debug, PartialEq, Clone, Copy, Hash, Eq, Sequence)] @@ -90,6 +91,7 @@ pub enum Rule { BanTruncateCascade, RequireTimeoutSettings, BanUncommittedTransaction, + RequireEnumValueOrdering, // xtask:new-rule:error-name } @@ -132,6 +134,7 @@ impl TryFrom<&str> for Rule { "ban-truncate-cascade" => Ok(Rule::BanTruncateCascade), "require-timeout-settings" => Ok(Rule::RequireTimeoutSettings), "ban-uncommitted-transaction" => Ok(Rule::BanUncommittedTransaction), + "require-enum-value-ordering" => Ok(Rule::RequireEnumValueOrdering), // xtask:new-rule:str-name _ => Err(format!("Unknown violation name: {s}")), } @@ -194,6 +197,7 @@ impl fmt::Display for Rule { Rule::BanTruncateCascade => "ban-truncate-cascade", Rule::RequireTimeoutSettings => "require-timeout-settings", Rule::BanUncommittedTransaction => "ban-uncommitted-transaction", + Rule::RequireEnumValueOrdering => "require-enum-value-ordering", // xtask:new-rule:variant-to-name }; write!(f, "{val}") @@ -421,6 +425,9 @@ impl Linter { if self.rules.contains(&Rule::BanUncommittedTransaction) { ban_uncommitted_transaction(self, file); } + if self.rules.contains(&Rule::RequireEnumValueOrdering) { + require_enum_value_ordering(self, &file); + } // xtask:new-rule:rule-call // locate any ignores in the file diff --git a/crates/squawk_linter/src/rules/mod.rs b/crates/squawk_linter/src/rules/mod.rs index 1bb4e019..ee7c687f 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -28,6 +28,7 @@ pub(crate) mod require_concurrent_index_creation; pub(crate) mod require_concurrent_index_deletion; pub(crate) mod require_timeout_settings; pub(crate) mod transaction_nesting; +pub(crate) mod require_enum_value_ordering; // xtask:new-rule:mod-decl pub(crate) use adding_field_with_default::adding_field_with_default; @@ -60,4 +61,5 @@ pub(crate) use require_concurrent_index_creation::require_concurrent_index_creat pub(crate) use require_concurrent_index_deletion::require_concurrent_index_deletion; pub(crate) use require_timeout_settings::require_timeout_settings; pub(crate) use transaction_nesting::transaction_nesting; +pub(crate) use require_enum_value_ordering::require_enum_value_ordering; // xtask:new-rule:export diff --git a/crates/squawk_linter/src/rules/require_enum_value_ordering.rs b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs new file mode 100644 index 00000000..3956526d --- /dev/null +++ b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs @@ -0,0 +1,105 @@ +use squawk_syntax::{ + Parse, SourceFile, SyntaxKind, + ast::{self, AstNode}, +}; + +use crate::{Linter, Rule, Violation}; + +pub(crate) fn require_enum_value_ordering(ctx: &mut Linter, parse: &Parse) { + let file = parse.tree(); + for stmt in file.stmts() { + if let ast::Stmt::AlterType(alter_type) = stmt { + let syntax = alter_type.syntax(); + + let mut has_add = false; + let mut has_value = false; + let mut has_before_or_after = false; + + for child in syntax.children_with_tokens() { + match child.kind() { + SyntaxKind::ADD_KW => has_add = true, + SyntaxKind::VALUE_KW if has_add => has_value = true, + SyntaxKind::BEFORE_KW | SyntaxKind::AFTER_KW if has_value => { + has_before_or_after = true; + } + _ => {} + } + } + + if has_add && has_value && !has_before_or_after { + ctx.report( + Violation::for_node( + Rule::RequireEnumValueOrdering, + "ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering.".into(), + syntax, + ) + .help("Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';"), + ); + } + } + } +} + +#[cfg(test)] +mod test { + use insta::assert_snapshot; + + use crate::Rule; + use crate::test_utils::{lint_errors, lint_ok}; + + #[test] + fn err_add_value_without_ordering() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE 'new_value'; +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireEnumValueOrdering)); + } + + #[test] + fn err_add_value_if_not_exists_without_ordering() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; +"#; + assert_snapshot!(lint_errors(sql, Rule::RequireEnumValueOrdering)); + } + + #[test] + fn ok_add_value_before() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; +"#; + lint_ok(sql, Rule::RequireEnumValueOrdering); + } + + #[test] + fn ok_add_value_after() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE 'new_value' AFTER 'existing_value'; +"#; + lint_ok(sql, Rule::RequireEnumValueOrdering); + } + + #[test] + fn ok_add_value_if_not_exists_before() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value'; +"#; + lint_ok(sql, Rule::RequireEnumValueOrdering); + } + + #[test] + fn ok_rename_value() { + let sql = r#" +ALTER TYPE my_enum RENAME VALUE 'old' TO 'new'; +"#; + lint_ok(sql, Rule::RequireEnumValueOrdering); + } + + #[test] + fn ok_add_attribute() { + let sql = r#" +ALTER TYPE my_type ADD ATTRIBUTE name text; +"#; + lint_ok(sql, Rule::RequireEnumValueOrdering); + } +} diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap new file mode 100644 index 00000000..63326b74 --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap @@ -0,0 +1,11 @@ +--- +source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs +assertion_line: 63 +expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)" +--- +warning[require-enum-value-ordering]: ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. + ╭▸ +2 │ ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; + │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + │ + ╰ help: Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap new file mode 100644 index 00000000..2c58923d --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap @@ -0,0 +1,11 @@ +--- +source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs +assertion_line: 55 +expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)" +--- +warning[require-enum-value-ordering]: ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. + ╭▸ +2 │ ALTER TYPE my_enum ADD VALUE 'new_value'; + │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + │ + ╰ help: Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; diff --git a/docs/docs/require-enum-value-ordering.md b/docs/docs/require-enum-value-ordering.md new file mode 100644 index 00000000..19a67d3e --- /dev/null +++ b/docs/docs/require-enum-value-ordering.md @@ -0,0 +1,28 @@ + +--- +id: require-enum-value-ordering +title: require-enum-value-ordering +--- + +## problem + +Using `ALTER TYPE ... ADD VALUE` without specifying `BEFORE` or `AFTER` appends the new value to the end of the enum type. This can lead to unexpected ordering, especially when enum values have a meaningful order (e.g., status workflows, severity levels). + +```sql +-- bad +ALTER TYPE status ADD VALUE 'pending_review'; +``` + +## solution + +Explicitly specify where the new value should be inserted using `BEFORE` or `AFTER`. + +```sql +-- good +ALTER TYPE status ADD VALUE 'pending_review' BEFORE 'approved'; +ALTER TYPE status ADD VALUE 'pending_review' AFTER 'submitted'; +``` + +## links + +- https://www.postgresql.org/docs/current/sql-altertype.html diff --git a/docs/sidebars.js b/docs/sidebars.js index 873997b8..c4cd1592 100644 --- a/docs/sidebars.js +++ b/docs/sidebars.js @@ -34,6 +34,7 @@ module.exports = { "syntax-error", "require-timeout-settings", "ban-uncommitted-transaction", + "require-enum-value-ordering", // xtask:new-rule:error-name ], }, diff --git a/docs/src/pages/index.js b/docs/src/pages/index.js index 8e36419d..b9b9b5b7 100644 --- a/docs/src/pages/index.js +++ b/docs/src/pages/index.js @@ -202,6 +202,11 @@ const rules = [ tags: ["schema"], description: "Ensure all transactions are committed", }, + { + name: "require-enum-value-ordering", + tags: ["schema"], + description: "Require BEFORE or AFTER when adding enum values", + }, // xtask:new-rule:rule-doc-meta ] From cc74755a944219e410d7293b4ade00a769dcdb2f Mon Sep 17 00:00:00 2001 From: Danilo Cabello Date: Sun, 8 Mar 2026 10:45:44 -0400 Subject: [PATCH 2/5] fix: sort imports and remove redundant borrow Co-Authored-By: Claude Opus 4.6 --- crates/squawk_linter/src/lib.rs | 4 ++-- crates/squawk_linter/src/rules/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 275d974f..e72654b6 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -53,9 +53,9 @@ use rules::renaming_column; use rules::renaming_table; use rules::require_concurrent_index_creation; use rules::require_concurrent_index_deletion; +use rules::require_enum_value_ordering; use rules::require_timeout_settings; use rules::transaction_nesting; -use rules::require_enum_value_ordering; // xtask:new-rule:rule-import #[derive(Debug, PartialEq, Clone, Copy, Hash, Eq, Sequence)] @@ -426,7 +426,7 @@ impl Linter { ban_uncommitted_transaction(self, file); } if self.rules.contains(&Rule::RequireEnumValueOrdering) { - require_enum_value_ordering(self, &file); + require_enum_value_ordering(self, file); } // xtask:new-rule:rule-call diff --git a/crates/squawk_linter/src/rules/mod.rs b/crates/squawk_linter/src/rules/mod.rs index ee7c687f..1cc5fe98 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -26,9 +26,9 @@ pub(crate) mod renaming_column; pub(crate) mod renaming_table; pub(crate) mod require_concurrent_index_creation; pub(crate) mod require_concurrent_index_deletion; +pub(crate) mod require_enum_value_ordering; pub(crate) mod require_timeout_settings; pub(crate) mod transaction_nesting; -pub(crate) mod require_enum_value_ordering; // xtask:new-rule:mod-decl pub(crate) use adding_field_with_default::adding_field_with_default; @@ -59,7 +59,7 @@ pub(crate) use renaming_column::renaming_column; pub(crate) use renaming_table::renaming_table; pub(crate) use require_concurrent_index_creation::require_concurrent_index_creation; pub(crate) use require_concurrent_index_deletion::require_concurrent_index_deletion; +pub(crate) use require_enum_value_ordering::require_enum_value_ordering; pub(crate) use require_timeout_settings::require_timeout_settings; pub(crate) use transaction_nesting::transaction_nesting; -pub(crate) use require_enum_value_ordering::require_enum_value_ordering; // xtask:new-rule:export From b0d7d2674f0453db72728bb3e411b7f9bb061068 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 9 Mar 2026 20:03:15 -0400 Subject: [PATCH 3/5] update grammar & cleanup --- .../src/rules/require_enum_value_ordering.rs | 63 ++++++---- ..._value_if_not_exists_without_ordering.snap | 10 +- ..._test__err_add_value_without_ordering.snap | 10 +- .../src/generated/syntax_kind.rs | 2 + crates/squawk_parser/src/grammar.rs | 18 ++- .../squawk_syntax/src/ast/generated/nodes.rs | 116 +++++++++++++++++- crates/squawk_syntax/src/postgresql.ungram | 12 +- 7 files changed, 186 insertions(+), 45 deletions(-) diff --git a/crates/squawk_linter/src/rules/require_enum_value_ordering.rs b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs index 3956526d..e9f5122b 100644 --- a/crates/squawk_linter/src/rules/require_enum_value_ordering.rs +++ b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs @@ -1,41 +1,34 @@ use squawk_syntax::{ - Parse, SourceFile, SyntaxKind, + Parse, SourceFile, ast::{self, AstNode}, }; -use crate::{Linter, Rule, Violation}; +use crate::{Edit, Fix, Linter, Rule, Violation}; + +fn create_fix(add_value: &ast::AddValue) -> Option { + let literal = add_value.literal()?; + let insert_at = literal.syntax().text_range().end(); + let edit = Edit::insert(" BEFORE 'existing_value'", insert_at); + Some(Fix::new("Insert `BEFORE` clause", vec![edit])) +} pub(crate) fn require_enum_value_ordering(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); for stmt in file.stmts() { - if let ast::Stmt::AlterType(alter_type) = stmt { - let syntax = alter_type.syntax(); - - let mut has_add = false; - let mut has_value = false; - let mut has_before_or_after = false; - - for child in syntax.children_with_tokens() { - match child.kind() { - SyntaxKind::ADD_KW => has_add = true, - SyntaxKind::VALUE_KW if has_add => has_value = true, - SyntaxKind::BEFORE_KW | SyntaxKind::AFTER_KW if has_value => { - has_before_or_after = true; - } - _ => {} - } - } - - if has_add && has_value && !has_before_or_after { - ctx.report( + if let ast::Stmt::AlterType(alter_type) = stmt + && let Some(add_value) = alter_type.add_value() + && add_value.value_position().is_none() + { + let fix = create_fix(&add_value); + ctx.report( Violation::for_node( Rule::RequireEnumValueOrdering, - "ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering.".into(), - syntax, + "ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering.".into(), + add_value.syntax(), ) - .help("Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';"), + .help("Add `BEFORE` or `AFTER` to specify the position of the new enum value.") + .fix(fix), ); - } } } } @@ -45,7 +38,7 @@ mod test { use insta::assert_snapshot; use crate::Rule; - use crate::test_utils::{lint_errors, lint_ok}; + use crate::test_utils::{fix_sql, lint_errors, lint_ok}; #[test] fn err_add_value_without_ordering() { @@ -63,6 +56,22 @@ ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; assert_snapshot!(lint_errors(sql, Rule::RequireEnumValueOrdering)); } + #[test] + fn fix_add_value_without_ordering() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE 'new_value'; +"#; + assert_snapshot!(fix_sql(sql, Rule::RequireEnumValueOrdering), @"ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';"); + } + + #[test] + fn fix_add_value_if_not_exists_without_ordering() { + let sql = r#" +ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; +"#; + assert_snapshot!(fix_sql(sql, Rule::RequireEnumValueOrdering), @"ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value';"); + } + #[test] fn ok_add_value_before() { let sql = r#" diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap index 63326b74..6fd0e4d1 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_if_not_exists_without_ordering.snap @@ -1,11 +1,13 @@ --- source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs -assertion_line: 63 expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)" --- -warning[require-enum-value-ordering]: ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. +warning[require-enum-value-ordering]: ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. ╭▸ 2 │ ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; - │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ │ - ╰ help: Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; + ├ help: Add `BEFORE` or `AFTER` to specify the position of the new enum value. + ╭╴ +2 │ ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value'; + ╰╴ +++++++++++++++++++++++ diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap index 2c58923d..0522e3b3 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap @@ -1,11 +1,13 @@ --- source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs -assertion_line: 55 expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)" --- -warning[require-enum-value-ordering]: ALTER TYPE ... ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. +warning[require-enum-value-ordering]: ADD VALUE without BEFORE or AFTER appends the value to the end of the enum, which may result in unexpected ordering. ╭▸ 2 │ ALTER TYPE my_enum ADD VALUE 'new_value'; - │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + │ ━━━━━━━━━━━━━━━━━━━━━ │ - ╰ help: Add BEFORE or AFTER to specify the position of the new enum value. Example: ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; + ├ help: Add `BEFORE` or `AFTER` to specify the position of the new enum value. + ╭╴ +2 │ ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; + ╰╴ +++++++++++++++++++++++ diff --git a/crates/squawk_parser/src/generated/syntax_kind.rs b/crates/squawk_parser/src/generated/syntax_kind.rs index 82bca450..0f9e1296 100644 --- a/crates/squawk_parser/src/generated/syntax_kind.rs +++ b/crates/squawk_parser/src/generated/syntax_kind.rs @@ -557,6 +557,7 @@ pub enum SyntaxKind { ADD_GENERATED, ADD_OP_CLASS_OPTIONS, ADD_VALUE, + AFTER_VALUE, AGGREGATE, ALIAS, ALL_FN, @@ -622,6 +623,7 @@ pub enum SyntaxKind { ATTRIBUTE_OPTION, ATTRIBUTE_VALUE, AT_TIME_ZONE, + BEFORE_VALUE, BEGIN, BEGIN_FUNC_OPTION, BEGIN_FUNC_OPTION_LIST, diff --git a/crates/squawk_parser/src/grammar.rs b/crates/squawk_parser/src/grammar.rs index ad3727d5..2b67f55d 100644 --- a/crates/squawk_parser/src/grammar.rs +++ b/crates/squawk_parser/src/grammar.rs @@ -8003,9 +8003,7 @@ fn alter_type(p: &mut Parser<'_>) -> CompletedMarker { p.expect(VALUE_KW); opt_if_not_exists(p); string_literal(p); - if p.eat(BEFORE_KW) || p.eat(AFTER_KW) { - string_literal(p); - } + opt_value_position(p); m.complete(p, ADD_VALUE); } _ => p.error("expected ALTER TYPE option"), @@ -8013,6 +8011,20 @@ fn alter_type(p: &mut Parser<'_>) -> CompletedMarker { m.complete(p, ALTER_TYPE) } +fn opt_value_position(p: &mut Parser<'_>) { + let m = p.start(); + let kind = if p.eat(BEFORE_KW) { + BEFORE_VALUE + } else if p.eat(AFTER_KW) { + AFTER_VALUE + } else { + m.abandon(p); + return; + }; + string_literal(p); + m.complete(p, kind); +} + // ALTER USER role_specification [ WITH ] option [ ... ] // where option can be: // SUPERUSER | NOSUPERUSER diff --git a/crates/squawk_syntax/src/ast/generated/nodes.rs b/crates/squawk_syntax/src/ast/generated/nodes.rs index ae0322c0..5314d316 100644 --- a/crates/squawk_syntax/src/ast/generated/nodes.rs +++ b/crates/squawk_syntax/src/ast/generated/nodes.rs @@ -168,20 +168,31 @@ impl AddValue { support::child(&self.syntax) } #[inline] + pub fn value_position(&self) -> Option { + support::child(&self.syntax) + } + #[inline] pub fn add_token(&self) -> Option { support::token(&self.syntax, SyntaxKind::ADD_KW) } #[inline] - pub fn after_token(&self) -> Option { - support::token(&self.syntax, SyntaxKind::AFTER_KW) + pub fn value_token(&self) -> Option { + support::token(&self.syntax, SyntaxKind::VALUE_KW) } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct AfterValue { + pub(crate) syntax: SyntaxNode, +} +impl AfterValue { #[inline] - pub fn before_token(&self) -> Option { - support::token(&self.syntax, SyntaxKind::BEFORE_KW) + pub fn literal(&self) -> Option { + support::child(&self.syntax) } #[inline] - pub fn value_token(&self) -> Option { - support::token(&self.syntax, SyntaxKind::VALUE_KW) + pub fn after_token(&self) -> Option { + support::token(&self.syntax, SyntaxKind::AFTER_KW) } } @@ -2160,6 +2171,21 @@ impl AttributeValue { } } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct BeforeValue { + pub(crate) syntax: SyntaxNode, +} +impl BeforeValue { + #[inline] + pub fn literal(&self) -> Option { + support::child(&self.syntax) + } + #[inline] + pub fn before_token(&self) -> Option { + support::token(&self.syntax, SyntaxKind::BEFORE_KW) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Begin { pub(crate) syntax: SyntaxNode, @@ -17763,6 +17789,12 @@ pub enum Type { TimeType(TimeType), } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ValuePosition { + AfterValue(AfterValue), + BeforeValue(BeforeValue), +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum WithQuery { CompoundSelect(CompoundSelect), @@ -17883,6 +17915,24 @@ impl AstNode for AddValue { &self.syntax } } +impl AstNode for AfterValue { + #[inline] + fn can_cast(kind: SyntaxKind) -> bool { + kind == SyntaxKind::AFTER_VALUE + } + #[inline] + fn cast(syntax: SyntaxNode) -> Option { + if Self::can_cast(syntax.kind()) { + Some(Self { syntax }) + } else { + None + } + } + #[inline] + fn syntax(&self) -> &SyntaxNode { + &self.syntax + } +} impl AstNode for Aggregate { #[inline] fn can_cast(kind: SyntaxKind) -> bool { @@ -19053,6 +19103,24 @@ impl AstNode for AttributeValue { &self.syntax } } +impl AstNode for BeforeValue { + #[inline] + fn can_cast(kind: SyntaxKind) -> bool { + kind == SyntaxKind::BEFORE_VALUE + } + #[inline] + fn cast(syntax: SyntaxNode) -> Option { + if Self::can_cast(syntax.kind()) { + Some(Self { syntax }) + } else { + None + } + } + #[inline] + fn syntax(&self) -> &SyntaxNode { + &self.syntax + } +} impl AstNode for Begin { #[inline] fn can_cast(kind: SyntaxKind) -> bool { @@ -34297,6 +34365,42 @@ impl From for Type { Type::TimeType(node) } } +impl AstNode for ValuePosition { + #[inline] + fn can_cast(kind: SyntaxKind) -> bool { + matches!(kind, SyntaxKind::AFTER_VALUE | SyntaxKind::BEFORE_VALUE) + } + #[inline] + fn cast(syntax: SyntaxNode) -> Option { + let res = match syntax.kind() { + SyntaxKind::AFTER_VALUE => ValuePosition::AfterValue(AfterValue { syntax }), + SyntaxKind::BEFORE_VALUE => ValuePosition::BeforeValue(BeforeValue { syntax }), + _ => { + return None; + } + }; + Some(res) + } + #[inline] + fn syntax(&self) -> &SyntaxNode { + match self { + ValuePosition::AfterValue(it) => &it.syntax, + ValuePosition::BeforeValue(it) => &it.syntax, + } + } +} +impl From for ValuePosition { + #[inline] + fn from(node: AfterValue) -> ValuePosition { + ValuePosition::AfterValue(node) + } +} +impl From for ValuePosition { + #[inline] + fn from(node: BeforeValue) -> ValuePosition { + ValuePosition::BeforeValue(node) + } +} impl AstNode for WithQuery { #[inline] fn can_cast(kind: SyntaxKind) -> bool { diff --git a/crates/squawk_syntax/src/postgresql.ungram b/crates/squawk_syntax/src/postgresql.ungram index 59f1157a..fae16493 100644 --- a/crates/squawk_syntax/src/postgresql.ungram +++ b/crates/squawk_syntax/src/postgresql.ungram @@ -2148,7 +2148,17 @@ AlterAttribute = 'alter' 'attribute' (Cascade | Restrict)? AddValue = - 'add' 'value' IfNotExists? Literal (('before' | 'after') Literal)? + 'add' 'value' IfNotExists? Literal ValuePosition? + +ValuePosition = + BeforeValue +| AfterValue + +BeforeValue = + 'before' Literal + +AfterValue = + 'after' Literal AlterUser = 'alter' 'user' RoleRef From 228ac7520320b94a19c013978489c40527744aa7 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 9 Mar 2026 20:12:33 -0400 Subject: [PATCH 4/5] update docs --- .../src/rules/require_enum_value_ordering.rs | 32 ------------------- docs/docs/require-enum-value-ordering.md | 29 +++++++++++++++-- 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/crates/squawk_linter/src/rules/require_enum_value_ordering.rs b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs index e9f5122b..3d5bdba2 100644 --- a/crates/squawk_linter/src/rules/require_enum_value_ordering.rs +++ b/crates/squawk_linter/src/rules/require_enum_value_ordering.rs @@ -64,14 +64,6 @@ ALTER TYPE my_enum ADD VALUE 'new_value'; assert_snapshot!(fix_sql(sql, Rule::RequireEnumValueOrdering), @"ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';"); } - #[test] - fn fix_add_value_if_not_exists_without_ordering() { - let sql = r#" -ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value'; -"#; - assert_snapshot!(fix_sql(sql, Rule::RequireEnumValueOrdering), @"ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value';"); - } - #[test] fn ok_add_value_before() { let sql = r#" @@ -84,30 +76,6 @@ ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value'; fn ok_add_value_after() { let sql = r#" ALTER TYPE my_enum ADD VALUE 'new_value' AFTER 'existing_value'; -"#; - lint_ok(sql, Rule::RequireEnumValueOrdering); - } - - #[test] - fn ok_add_value_if_not_exists_before() { - let sql = r#" -ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value'; -"#; - lint_ok(sql, Rule::RequireEnumValueOrdering); - } - - #[test] - fn ok_rename_value() { - let sql = r#" -ALTER TYPE my_enum RENAME VALUE 'old' TO 'new'; -"#; - lint_ok(sql, Rule::RequireEnumValueOrdering); - } - - #[test] - fn ok_add_attribute() { - let sql = r#" -ALTER TYPE my_type ADD ATTRIBUTE name text; "#; lint_ok(sql, Rule::RequireEnumValueOrdering); } diff --git a/docs/docs/require-enum-value-ordering.md b/docs/docs/require-enum-value-ordering.md index 19a67d3e..ce17b17e 100644 --- a/docs/docs/require-enum-value-ordering.md +++ b/docs/docs/require-enum-value-ordering.md @@ -1,4 +1,3 @@ - --- id: require-enum-value-ordering title: require-enum-value-ordering @@ -6,11 +5,25 @@ title: require-enum-value-ordering ## problem -Using `ALTER TYPE ... ADD VALUE` without specifying `BEFORE` or `AFTER` appends the new value to the end of the enum type. This can lead to unexpected ordering, especially when enum values have a meaningful order (e.g., status workflows, severity levels). +Using `ALTER TYPE ... ADD VALUE` without specifying `BEFORE` or `AFTER` appends the new value to the end of the enum type. + +This can lead to unexpected ordering, especially when enum values have a meaningful order (e.g., workflow status, severity, priority). ```sql +-- existing type +CREATE TYPE status AS ENUM ( + 'draft', + 'submitted', + 'approved', + 'rejected' +); +-- existing query to find all not yet approved +select * from papers where status < 'approved'; + -- bad ALTER TYPE status ADD VALUE 'pending_review'; + +-- now our query isn't including any of the 'pending_review' items! ``` ## solution @@ -18,8 +31,20 @@ ALTER TYPE status ADD VALUE 'pending_review'; Explicitly specify where the new value should be inserted using `BEFORE` or `AFTER`. ```sql +-- existing type +CREATE TYPE status AS ENUM ( + 'draft', + 'submitted', + 'approved', + 'rejected' +); + +-- existing query to find all not yet approved +select * from papers where status < 'approved'; + -- good ALTER TYPE status ADD VALUE 'pending_review' BEFORE 'approved'; +-- or ALTER TYPE status ADD VALUE 'pending_review' AFTER 'submitted'; ``` From a3330eeba354383dfaac994c155d301b4f081f0e Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 9 Mar 2026 20:17:38 -0400 Subject: [PATCH 5/5] update --- .../tests/snapshots/tests__alter_type_ok.snap | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/squawk_parser/tests/snapshots/tests__alter_type_ok.snap b/crates/squawk_parser/tests/snapshots/tests__alter_type_ok.snap index 684929e2..ac02acce 100644 --- a/crates/squawk_parser/tests/snapshots/tests__alter_type_ok.snap +++ b/crates/squawk_parser/tests/snapshots/tests__alter_type_ok.snap @@ -229,10 +229,11 @@ SOURCE_FILE LITERAL STRING "'v'" WHITESPACE " " - BEFORE_KW "before" - WHITESPACE " " - LITERAL - STRING "'w'" + BEFORE_VALUE + BEFORE_KW "before" + WHITESPACE " " + LITERAL + STRING "'w'" SEMICOLON ";" WHITESPACE "\n" ALTER_TYPE @@ -265,10 +266,11 @@ SOURCE_FILE LITERAL STRING "'v'" WHITESPACE " " - AFTER_KW "after" - WHITESPACE " " - LITERAL - STRING "'w'" + AFTER_VALUE + AFTER_KW "after" + WHITESPACE " " + LITERAL + STRING "'w'" SEMICOLON ";" WHITESPACE "\n\n" COMMENT "-- rename_value"