linter: fixes for prefer-text-field (#621)

Author: sbdchd

crates/squawk_linter/src/rules/prefer_text_field.rs

@@ -5,7 +5,7 @@ use squawk_syntax::{
     ast::{self, AstNode},
 };
 
-use crate::{Linter, Rule, Violation, identifier::Identifier};
+use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier};
 
 use crate::visitors::check_not_allowed_types;
 
@@ -49,14 +49,38 @@ fn is_not_allowed_varchar(ty: &ast::Type) -> bool {
     }
 }
 
+fn create_varchar_to_text_fix(ty: &ast::Type) -> Option<Fix> {
+    match ty {
+        ast::Type::PathType(path_type) => {
+            // we'll replace the entire path type, including args
+            // so: `"varchar"(100)` becomes `text`
+            let edit = Edit::replace(path_type.syntax().text_range(), "text");
+            Some(Fix::new("Replace with `text`", vec![edit]))
+        }
+        ast::Type::CharType(char_type) => {
+            // we'll replace the entire char type, including args
+            // so: `varchar(100)` becomes `text`
+            let edit = Edit::replace(char_type.syntax().text_range(), "text");
+            Some(Fix::new("Replace with `text`", vec![edit]))
+        }
+        ast::Type::ArrayType(array_type) => {
+            let ty = array_type.ty()?;
+            let edit = Edit::replace(ty.syntax().text_range(), "text");
+            Some(Fix::new("Replace with `text`", vec![edit]))
+        }
+        _ => None,
+    }
+}
+
 fn check_ty_for_varchar(ctx: &mut Linter, ty: Option<ast::Type>) {
     if let Some(ty) = ty {
         if is_not_allowed_varchar(&ty) {
+            let fix = create_varchar_to_text_fix(&ty);
             ctx.report(Violation::for_node(
                 Rule::PreferTextField,
                "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.".to_string(),
                 ty.syntax(),
-            ).help("Use a `TEXT` field with a `CHECK` constraint."));
+            ).help("Use a `TEXT` field with a `CHECK` constraint.").fix(fix));
         };
     }
 }
@@ -68,10 +92,16 @@ pub(crate) fn prefer_text_field(ctx: &mut Linter, parse: &Parse<SourceFile>) {
 
 #[cfg(test)]
 mod test {
-    use insta::assert_debug_snapshot;
+    use insta::{assert_debug_snapshot, assert_snapshot};
 
-    use crate::Rule;
-    use crate::test_utils::lint;
+    use crate::{
+        Rule,
+        test_utils::{fix_sql, lint},
+    };
+
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::PreferTextField)
+    }
 
     /// Changing a column of varchar(255) to varchar(1000) requires an ACCESS
     /// EXCLUSIVE lock
@@ -162,4 +192,41 @@ COMMIT;
         let errors = lint(sql, Rule::PreferTextField);
         assert_eq!(errors.len(), 0);
     }
+
+    #[test]
+    fn fix_varchar_with_length() {
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(100));"), @"CREATE TABLE t (c text);");
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(255));"), @"CREATE TABLE t (c text);");
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(50));"), @"CREATE TABLE t (c text);");
+    }
+
+    #[test]
+    fn fix_mixed_case_varchar() {
+        assert_snapshot!(fix("CREATE TABLE t (c VARCHAR(100));"), @"CREATE TABLE t (c text);");
+        assert_snapshot!(fix("CREATE TABLE t (c Varchar(50));"), @"CREATE TABLE t (c text);");
+        assert_snapshot!(fix("CREATE TABLE t (c VarChar(255));"), @"CREATE TABLE t (c text);");
+    }
+
+    #[test]
+    fn fix_varchar_arrays() {
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(100)[]);"), @"CREATE TABLE t (c text[]);");
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(255)[5]);"), @"CREATE TABLE t (c text[5]);");
+        assert_snapshot!(fix("CREATE TABLE t (c varchar(50)[3][4]);"), @"CREATE TABLE t (c text[3][4]);");
+    }
+
+    #[test]
+    fn fix_alter_table() {
+        assert_snapshot!(fix("ALTER TABLE t ADD COLUMN c varchar(100);"), @"ALTER TABLE t ADD COLUMN c text;");
+        assert_snapshot!(fix("ALTER TABLE t ALTER COLUMN c TYPE varchar(256);"), @"ALTER TABLE t ALTER COLUMN c TYPE text;");
+    }
+
+    #[test]
+    fn fix_multiple_varchar_columns() {
+        assert_snapshot!(fix("CREATE TABLE t (a varchar(100), b varchar(255), c varchar(50));"), @"CREATE TABLE t (a text, b text, c text);");
+    }
+
+    #[test]
+    fn fix_pgcatalog_varchar() {
+        assert_snapshot!(fix("CREATE TABLE t (c pg_catalog.varchar(100));"), @"CREATE TABLE t (c text);");
+    }
 }

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

@@ -10,6 +10,18 @@ expression: errors
         help: Some(
             "Use a `TEXT` field with a `CHECK` constraint.",
         ),
-        fix: None,
+        fix: Some(
+            Fix {
+                title: "Replace with `text`",
+                edits: [
+                    Edit {
+                        text_range: 56..68,
+                        text: Some(
+                            "text",
+                        ),
+                    },
+                ],
+            },
+        ),
     },
 ]

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

@@ -10,6 +10,18 @@ expression: errors
         help: Some(
             "Use a `TEXT` field with a `CHECK` constraint.",
         ),
-        fix: None,
+        fix: Some(
+            Fix {
+                title: "Replace with `text`",
+                edits: [
+                    Edit {
+                        text_range: 69..92,
+                        text: Some(
+                            "text",
+                        ),
+                    },
+                ],
+            },
+        ),
     },
 ]

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

@@ -10,6 +10,18 @@ expression: errors
         help: Some(
             "Use a `TEXT` field with a `CHECK` constraint.",
         ),
-        fix: None,
+        fix: Some(
+            Fix {
+                title: "Replace with `text`",
+                edits: [
+                    Edit {
+                        text_range: 111..123,
+                        text: Some(
+                            "text",
+                        ),
+                    },
+                ],
+            },
+        ),
     },
 ]

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

@@ -10,6 +10,18 @@ expression: errors
         help: Some(
             "Use a `TEXT` field with a `CHECK` constraint.",
         ),
-        fix: None,
+        fix: Some(
+            Fix {
+                title: "Replace with `text`",
+                edits: [
+                    Edit {
+                        text_range: 89..102,
+                        text: Some(
+                            "text",
+                        ),
+                    },
+                ],
+            },
+        ),
     },
 ]