linter: add require-enum-value-ordering rule

crates/squawk_linter/src/lib.rs

@@ -53,6 +53,7 @@ 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;
 // xtask:new-rule:rule-import
@@ -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

crates/squawk_linter/src/rules/mod.rs

@@ -26,6 +26,7 @@ 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;
 // xtask:new-rule:mod-decl
@@ -58,6 +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;
 // xtask:new-rule:export

crates/squawk_linter/src/rules/require_enum_value_ordering.rs

@@ -0,0 +1,82 @@
+use squawk_syntax::{
+    Parse, SourceFile,
+    ast::{self, AstNode},
+};
+
+use crate::{Edit, Fix, Linter, Rule, Violation};
+
+fn create_fix(add_value: &ast::AddValue) -> Option<Fix> {
+    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<SourceFile>) {
+    let file = parse.tree();
+    for stmt in file.stmts() {
+        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,
+                        "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.")
+                    .fix(fix),
+                );
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use insta::assert_snapshot;
+
+    use crate::Rule;
+    use crate::test_utils::{fix_sql, 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 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 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);
+    }
+}

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,13 @@
+---
+source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs
+expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)"
+---
+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.
+  ╭╴
+2 │ ALTER TYPE my_enum ADD VALUE IF NOT EXISTS 'new_value' BEFORE 'existing_value';
+  ╰╴                                                       +++++++++++++++++++++++

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_enum_value_ordering__test__err_add_value_without_ordering.snap

@@ -0,0 +1,13 @@
+---
+source: crates/squawk_linter/src/rules/require_enum_value_ordering.rs
+expression: "lint_errors(sql, Rule::RequireEnumValueOrdering)"
+---
+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.
+  ╭╴
+2 │ ALTER TYPE my_enum ADD VALUE 'new_value' BEFORE 'existing_value';
+  ╰╴                                         +++++++++++++++++++++++

crates/squawk_parser/src/grammar.rs

@@ -8003,16 +8003,28 @@ 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"),
     }
     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

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"