rebased

Author: juleswritescode

crates/pglt_analyser/src/lib.rs

@@ -92,8 +92,8 @@ mod tests {
             String::from_utf8(buffer).unwrap()
         }
 
-        const SQL: &str = r#"alter table test drop column id;"#;
-        let rule_filter = RuleFilter::Rule("safety", "banDropColumn");
+        const SQL: &str = r#"alter table test add column count int not null;"#;
+        let rule_filter = RuleFilter::Rule("safety", "addingRequiredField");
 
         let filter = AnalysisFilter {
             enabled_rules: Some(slice::from_ref(&rule_filter)),

crates/pglt_analyser/src/lint/safety.rs

@@ -1,7 +1,8 @@
 //! Generated file, do not edit by hand, see `xtask/codegen`
 
 use pglt_analyse::declare_lint_group;
+pub mod adding_required_field;
 pub mod ban_drop_column;
 pub mod ban_drop_not_null;
 pub mod ban_drop_table;
-declare_lint_group! { pub Safety { name : "safety" , rules : [self :: ban_drop_column :: BanDropColumn , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable ,] } }
+declare_lint_group! { pub Safety { name : "safety" , rules : [self :: adding_required_field :: AddingRequiredField , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable ,] } }

crates/pglt_analyser/src/lint/safety/adding_required_field.rs

@@ -0,0 +1,67 @@
+use pglt_analyse::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic, RuleSource};
+use pglt_console::markup;
+
+declare_lint_rule! {
+    /// Adding a new column that is NOT NULL and has no default value to an existing table effectively makes it required.
+    ///
+    /// This will fail immediately upon running for any populated table. Furthermore, old application code that is unaware of this column will fail to INSERT to this table.
+    ///
+    /// Make new columns optional initially by omitting the NOT NULL constraint until all existing data and application code has been updated. Once no NULL values are written to or persisted in the database, set it to NOT NULL.
+    /// Alternatively, if using Postgres version 11 or later, add a DEFAULT value that is not volatile. This allows the column to keep its NOT NULL constraint.
+    ///
+    /// ## Invalid
+    /// alter table test add column count int not null;
+    ///
+    /// ## Valid in Postgres >= 11
+    /// alter table test add column count int not null default 0;
+    pub AddingRequiredField {
+        version: "next",
+        name: "addingRequiredField",
+        recommended: false,
+        sources: &[RuleSource::Squawk("adding-required-field")],
+    }
+}
+
+impl Rule for AddingRequiredField {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = vec![];
+
+        if let pglt_query_ext::NodeEnum::AlterTableStmt(stmt) = ctx.stmt() {
+            // We are currently lacking a way to check if a `AtAddColumn` subtype sets a
+            // not null constraint – so we'll need to check the plain SQL.
+            let plain_sql = ctx.stmt().to_ref().deparse().unwrap().to_ascii_lowercase();
+            let is_nullable = !plain_sql.contains("not null");
+            let has_set_default = plain_sql.contains("default");
+            if is_nullable || has_set_default {
+                return diagnostics;
+            }
+
+            for cmd in &stmt.cmds {
+                if let Some(pglt_query_ext::NodeEnum::AlterTableCmd(alter_table_cmd)) = &cmd.node {
+                    if alter_table_cmd.subtype()
+                        == pglt_query_ext::protobuf::AlterTableType::AtAddColumn
+                    {
+                        diagnostics.push(
+                            RuleDiagnostic::new(
+                                rule_category!(),
+                                None,
+                                markup! {
+                                    "Adding a new column that is NOT NULL and has no default value to an existing table effectively makes it required."
+                                },
+                            )
+                            .detail(
+                                None,
+                                "Make new columns optional initially by omitting the NOT NULL constraint until all existing data and application code has been updated. Once no NULL values are written to or persisted in the database, set it to NOT NULL. Alternatively, if using Postgres version 11 or later, add a DEFAULT value that is not volatile. This allows the column to keep its NOT NULL constraint.
+                                ",
+                        ),
+                    );
+                    }
+                }
+            }
+        }
+
+        diagnostics
+    }
+}

crates/pglt_analyser/src/options.rs

@@ -1,6 +1,8 @@
 //! Generated file, do not edit by hand, see `xtask/codegen`
 
 use crate::lint;
+pub type AddingRequiredField =
+    <lint::safety::adding_required_field::AddingRequiredField as pglt_analyse::Rule>::Options;
 pub type BanDropColumn =
     <lint::safety::ban_drop_column::BanDropColumn as pglt_analyse::Rule>::Options;
 pub type BanDropNotNull =

crates/pglt_analyser/tests/specs/safety/addingRequiredField/basic.sql

@@ -0,0 +1,2 @@
+-- expect_only_lint/safety/addingRequiredField
+alter table test add column c int not null;

crates/pglt_analyser/tests/specs/safety/addingRequiredField/basic.sql.snap

@@ -0,0 +1,16 @@
+---
+source: crates/pglt_analyser/tests/rules_tests.rs
+expression: snapshot
+---
+# Input
+```
+-- expect_only_lint/safety/addingRequiredField
+alter table test add column c int not null;
+```
+
+# Diagnostics
+lint/safety/addingRequiredField ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Adding a new column that is NOT NULL and has no default value to an existing table effectively makes it required.
+  
+  i Make new columns optional initially by omitting the NOT NULL constraint until all existing data and application code has been updated. Once no NULL values are written to or persisted in the database, set it to NOT NULL. Alternatively, if using Postgres version 11 or later, add a DEFAULT value that is not volatile. This allows the column to keep its NOT NULL constraint.

crates/pglt_analyser/tests/specs/safety/addingRequiredField/with_default.sql

@@ -0,0 +1,3 @@
+-- expect_no_diagnostics
+alter table test
+add column c int not null default 0;

crates/pglt_analyser/tests/specs/safety/addingRequiredField/with_default.sql.snap

@@ -0,0 +1,10 @@
+---
+source: crates/pglt_analyser/tests/rules_tests.rs
+expression: snapshot
+---
+# Input
+```
+-- expect_no_diagnostics
+alter table test
+add column c int not null default 0;
+```

crates/pglt_analyser/tests/specs/safety/addingRequiredField/without_required.sql

@@ -0,0 +1,3 @@
+-- expect_no_diagnostics
+alter table test
+add column c int;

crates/pglt_analyser/tests/specs/safety/addingRequiredField/without_required.sql.snap

@@ -0,0 +1,10 @@
+---
+source: crates/pglt_analyser/tests/rules_tests.rs
+expression: snapshot
+---
+# Input
+```
+-- expect_no_diagnostics
+alter table test
+add column c int;
+```