add new "ban-drop-column" rule (#132)

fixes #128

Author: chdsbd

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - link to website for tty reporter when there are lint errors. (#120)
 - `DROP INDEX` support for "require-concurrent-index-creation" and "prefer-robust-stmts". (#124)
 - github comment now includes Squawk's version number. (#131)
+- new `ban-drop-column` rule (#132)
 
 ### Changed
 

docs/docs/ban-drop-column.md

@@ -0,0 +1,6 @@
+---
+id: ban-drop-column
+title: ban-drop-column
+---
+
+Dropping a column may break existing clients.

linter/src/lib.rs

@@ -9,9 +9,10 @@ extern crate lazy_static;
 use crate::errors::CheckSQLError;
 use crate::rules::{
     adding_field_with_default, adding_foreign_key_constraint, adding_not_nullable_field,
-    adding_primary_key_constraint, ban_char_type, ban_drop_database, changing_column_type,
-    constraint_missing_not_valid, disallow_unique_constraint, prefer_robust_stmts,
-    prefer_text_field, renaming_column, renaming_table, require_concurrent_index_creation,
+    adding_primary_key_constraint, ban_char_type, ban_drop_column, ban_drop_database,
+    changing_column_type, constraint_missing_not_valid, disallow_unique_constraint,
+    prefer_robust_stmts, prefer_text_field, renaming_column, renaming_table,
+    require_concurrent_index_creation,
 };
 use crate::violations::{RuleViolation, RuleViolationKind, ViolationMessage};
 use squawk_parser::ast::RootStmt;
@@ -92,6 +93,16 @@ lazy_static! {
             ),
         ]
     },
+    SquawkRule {
+        id: "ban-drop-column".into(),
+        name: RuleViolationKind::BanDropColumn,
+        func: ban_drop_column,
+        messages: vec![
+            ViolationMessage::Note(
+                "Dropping a column may break existing clients.".into()
+            ),
+        ],
+    },
     SquawkRule {
         id: "ban-drop-database".into(),
         name: RuleViolationKind::BanDropDatabase,
@@ -234,7 +245,7 @@ lazy_static! {
                 "Create the index CONCURRENTLY.".into()
             ),
         ],
-    },
+    }
     ];
 
 }

linter/src/rules/ban_drop_column.rs

@@ -0,0 +1,51 @@
+use crate::violations::{RuleViolation, RuleViolationKind};
+use squawk_parser::ast::{AlterTableCmds, AlterTableType, ObjectType, RootStmt, Stmt};
+
+#[must_use]
+pub fn ban_drop_column(tree: &[RootStmt]) -> Vec<RuleViolation> {
+    let mut errs = vec![];
+    for RootStmt::RawStmt(raw_stmt) in tree {
+        match &raw_stmt.stmt {
+            Stmt::RenameStmt(stmt) => match stmt.rename_type {
+                ObjectType::Table => {
+                    errs.push(RuleViolation::new(
+                        RuleViolationKind::RenamingTable,
+                        raw_stmt.into(),
+                        None,
+                    ));
+                }
+                _ => continue,
+            },
+            Stmt::AlterTableStmt(stmt) => {
+                for cmd in &stmt.cmds {
+                    if let AlterTableCmds::AlterTableCmd(cmd) = cmd {
+                        if cmd.subtype == AlterTableType::DropColumn {
+                            errs.push(RuleViolation::new(
+                                RuleViolationKind::BanDropColumn,
+                                raw_stmt.into(),
+                                None,
+                            ))
+                        }
+                    }
+                }
+            }
+            _ => continue,
+        }
+    }
+    errs
+}
+
+#[cfg(test)]
+mod test_rules {
+    use crate::check_sql;
+    use insta::assert_debug_snapshot;
+
+    #[test]
+    fn test_drop_column() {
+        let sql = r#"
+ALTER TABLE "bar_tbl" DROP COLUMN "foo_col" CASCADE;
+        "#;
+
+        assert_debug_snapshot!(check_sql(sql, &["prefer-robust-stmts".into()]));
+    }
+}

linter/src/rules/mod.rs

@@ -20,6 +20,8 @@ pub mod renaming_column;
 pub use renaming_column::*;
 pub mod renaming_table;
 pub use renaming_table::*;
+pub mod ban_drop_column;
+pub use ban_drop_column::*;
 pub mod require_concurrent_index_creation;
 pub use require_concurrent_index_creation::*;
 pub mod ban_char_field;

linter/src/rules/snapshots/squawk_linter__rules__ban_drop_column__test_rules__drop_column.snap

@@ -0,0 +1,22 @@
+---
+source: linter/src/rules/ban_drop_column.rs
+expression: "check_sql(sql, &[\"prefer-robust-stmts\".into()])"
+---
+Ok(
+    [
+        RuleViolation {
+            kind: BanDropColumn,
+            span: Span {
+                start: 0,
+                len: Some(
+                    52,
+                ),
+            },
+            messages: [
+                Note(
+                    "Dropping a column may break existing clients.",
+                ),
+            ],
+        },
+    ],
+)

linter/src/rules/snapshots/squawk_linter__rules__renaming_table__test_rules__renaming_table.snap

@@ -18,5 +18,19 @@ Ok(
                 ),
             ],
         },
+        RuleViolation {
+            kind: RenamingTable,
+            span: Span {
+                start: 0,
+                len: Some(
+                    52,
+                ),
+            },
+            messages: [
+                Note(
+                    "Renaming a table may break existing clients.",
+                ),
+            ],
+        },
     ],
 )

linter/src/violations.rs

@@ -18,6 +18,7 @@ pub enum RuleViolationKind {
     PreferTextField,
     PreferRobustStmts,
     BanCharField,
+    BanDropColumn,
 }
 
 impl std::fmt::Display for RuleViolationKind {