linter: fixes for prefer-bigint-over-int, prefer-bigint-over-small-int

crates/squawk_linter/src/rules/prefer_bigint_over_int.rs

@@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode;
 use squawk_syntax::{Parse, SourceFile, ast};
 
 use crate::identifier::Identifier;
-use crate::{Linter, Rule, Violation};
+use crate::{Edit, Fix, Linter, Rule, Violation};
 
 use crate::visitors::check_not_allowed_types;
 use crate::visitors::is_not_valid_int_type;
@@ -13,23 +13,50 @@ use lazy_static::lazy_static;
 
 lazy_static! {
     static ref INT_TYPES: HashSet<Identifier> = HashSet::from([
+        Identifier::new("int"),
         Identifier::new("integer"),
         Identifier::new("int4"),
         Identifier::new("serial"),
         Identifier::new("serial4"),
     ]);
 }
 
+fn int_to_bigint_replacement(int_type: &str) -> &'static str {
+    match int_type.to_lowercase().as_str() {
+        "int" | "integer" => "bigint",
+        "int4" => "int8",
+        "serial" => "bigserial",
+        "serial4" => "serial8",
+        _ => "bigint",
+    }
+}
+
+fn create_bigint_fix(ty: &ast::Type) -> Option<Fix> {
+    if let Some(type_name) = ty.syntax().first_token() {
+        let i64 = int_to_bigint_replacement(type_name.text());
+        let edit = Edit::replace(ty.syntax().text_range(), i64);
+        Some(Fix::new(
+            format!("Replace with a 64-bit integer type: `{i64}`"),
+            vec![edit],
+        ))
+    } else {
+        None
+    }
+}
+
 fn check_ty_for_big_int(ctx: &mut Linter, ty: Option<ast::Type>) {
     if let Some(ty) = ty {
         if is_not_valid_int_type(&ty, &INT_TYPES) {
+            let fix = create_bigint_fix(&ty);
+
             ctx.report(
                 Violation::for_node(
                     Rule::PreferBigintOverInt,
                     "Using 32-bit integer fields can result in hitting the max `int` limit.".into(),
                     ty.syntax(),
                 )
-                .help("Use 64-bit integer values instead to prevent hitting this limit."),
+                .help("Use 64-bit integer values instead to prevent hitting this limit.")
+                .fix(fix),
             );
         };
     }
@@ -43,14 +70,51 @@ pub(crate) fn prefer_bigint_over_int(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,
+        test_utils::{fix_sql, lint},
+    };
+
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::PreferBigintOverInt)
+    }
 
-    use crate::Rule;
-    use crate::test_utils::lint;
+    #[test]
+    fn fix_int_types() {
+        assert_snapshot!(fix("create table users (id int);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id integer);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id int4);"), @"create table users (id int8);");
+        assert_snapshot!(fix("create table users (id serial);"), @"create table users (id bigserial);");
+        assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id serial8);");
+    }
+
+    #[test]
+    fn fix_mixed_case() {
+        assert_snapshot!(fix("create table users (id INT);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id INTEGER);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id Int4);"), @"create table users (id int8);");
+        assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id bigserial);");
+        assert_snapshot!(fix("create table users (id SERIAL4);"), @"create table users (id serial8);");
+    }
+
+    #[test]
+    fn fix_multiple_columns() {
+        assert_snapshot!(fix("create table users (id int, count integer, version serial);"), @"create table users (id bigint, count bigint, version bigserial);");
+    }
+
+    #[test]
+    fn fix_with_constraints() {
+        assert_snapshot!(fix("create table users (id serial primary key, score int not null);"), @"create table users (id bigserial primary key, score bigint not null);");
+    }
 
     #[test]
     fn err() {
         let sql = r#"
+create table users (
+    id int
+);
 create table users (
     id integer
 );
@@ -66,13 +130,13 @@ create table users (
         "#;
         let errors = lint(sql, Rule::PreferBigintOverInt);
         assert_ne!(errors.len(), 0);
-        assert_eq!(errors.len(), 4);
+        assert_eq!(errors.len(), 5);
         assert_eq!(
             errors
                 .iter()
                 .filter(|x| x.code == Rule::PreferBigintOverInt)
                 .count(),
-            4
+            5
         );
         assert_debug_snapshot!(errors);
     }

crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs

@@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode;
 use squawk_syntax::{Parse, SourceFile, ast};
 
 use crate::identifier::Identifier;
-use crate::{Linter, Rule, Violation};
+use crate::{Edit, Fix, Linter, Rule, Violation};
 
 use crate::visitors::check_not_allowed_types;
 use crate::visitors::is_not_valid_int_type;
@@ -20,16 +20,42 @@ lazy_static! {
     ]);
 }
 
+fn smallint_to_bigint(smallint_type: &str) -> &'static str {
+    match smallint_type.to_lowercase().as_str() {
+        "smallint" => "bigint",
+        "int2" => "int8",
+        "smallserial" => "bigserial",
+        "serial2" => "serial8",
+        _ => "bigint",
+    }
+}
+
+fn create_bigint_fix(ty: &ast::Type) -> Option<Fix> {
+    if let Some(type_name) = ty.syntax().first_token() {
+        let i64 = smallint_to_bigint(type_name.text());
+        let edit = Edit::replace(ty.syntax().text_range(), i64);
+        Some(Fix::new(
+            format!("Replace with a 64-bit integer type: `{i64}`"),
+            vec![edit],
+        ))
+    } else {
+        None
+    }
+}
+
 fn check_ty_for_small_int(ctx: &mut Linter, ty: Option<ast::Type>) {
     if let Some(ty) = ty {
         if is_not_valid_int_type(&ty, &SMALL_INT_TYPES) {
+            let fix = create_bigint_fix(&ty);
+
             ctx.report(
                 Violation::for_node(
                     Rule::PreferBigintOverSmallint,
                     "Using 16-bit integer fields can result in hitting the max `int` limit.".into(),
                     ty.syntax(),
                 )
-                .help("Use 64-bit integer values instead to prevent hitting this limit."),
+                .help("Use 64-bit integer values instead to prevent hitting this limit.")
+                .fix(fix),
             );
         };
     }
@@ -42,10 +68,42 @@ pub(crate) fn prefer_bigint_over_smallint(ctx: &mut Linter, parse: &Parse<Source
 
 #[cfg(test)]
 mod test {
-    use insta::assert_debug_snapshot;
+    use insta::{assert_debug_snapshot, assert_snapshot};
+
+    use crate::{
+        Rule,
+        test_utils::{fix_sql, lint},
+    };
 
-    use crate::Rule;
-    use crate::test_utils::lint;
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::PreferBigintOverSmallint)
+    }
+
+    #[test]
+    fn fix_smallint_types() {
+        assert_snapshot!(fix("create table users (id smallint);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id int2);"), @"create table users (id int8);");
+        assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id bigserial);");
+        assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id serial8);");
+    }
+
+    #[test]
+    fn fix_mixed_case() {
+        assert_snapshot!(fix("create table users (id SMALLINT);"), @"create table users (id bigint);");
+        assert_snapshot!(fix("create table users (id Int2);"), @"create table users (id int8);");
+        assert_snapshot!(fix("create table users (id SmallSerial);"), @"create table users (id bigserial);");
+        assert_snapshot!(fix("create table users (id SERIAL2);"), @"create table users (id serial8);");
+    }
+
+    #[test]
+    fn fix_multiple_columns() {
+        assert_snapshot!(fix("create table users (id smallint, count int2, version smallserial);"), @"create table users (id bigint, count int8, version bigserial);");
+    }
+
+    #[test]
+    fn fix_with_constraints() {
+        assert_snapshot!(fix("create table users (id smallserial primary key, score smallint not null);"), @"create table users (id bigserial primary key, score bigint not null);");
+    }
 
     #[test]
     fn err() {

crates/squawk_linter/src/rules/prefer_identity.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 lazy_static::lazy_static;
 
@@ -22,17 +22,39 @@ lazy_static! {
     ]);
 }
 
+fn replace_serial(serial_type: &str) -> &'static str {
+    match serial_type.to_lowercase().as_str() {
+        "serial" | "serial4" => "integer generated by default as identity",
+        "serial2" | "smallserial" => "smallint generated by default as identity",
+        "serial8" | "bigserial" => "bigint generated by default as identity",
+        _ => "integer generated by default as identity",
+    }
+}
+
+fn create_identity_fix(ty: &ast::Type) -> Option<Fix> {
+    if let Some(type_name) = ty.syntax().first_token() {
+        let text = replace_serial(type_name.text());
+        let edit = Edit::replace(ty.syntax().text_range(), text);
+        Some(Fix::new("Replace with IDENTITY column", vec![edit]))
+    } else {
+        None
+    }
+}
+
 fn check_ty_for_serial(ctx: &mut Linter, ty: Option<ast::Type>) {
     if let Some(ty) = ty {
         if is_not_valid_int_type(&ty, &SERIAL_TYPES) {
+            let fix = create_identity_fix(&ty);
+
             ctx.report(
                 Violation::for_node(
                     Rule::PreferIdentity,
                     "Serial types make schema, dependency, and permission management difficult."
                         .into(),
                     ty.syntax(),
                 )
-                .help("Use an `IDENTITY` column instead."),
+                .help("Use an `IDENTITY` column instead.")
+                .fix(fix),
             );
         };
     }
@@ -45,10 +67,32 @@ pub(crate) fn prefer_identity(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,
+        test_utils::{fix_sql, lint},
+    };
 
-    use crate::Rule;
-    use crate::test_utils::lint;
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::PreferIdentity)
+    }
+
+    #[test]
+    fn fix_serial_types() {
+        assert_snapshot!(fix("create table users (id serial);"), @"create table users (id integer generated by default as identity);");
+        assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id smallint generated by default as identity);");
+        assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id integer generated by default as identity);");
+        assert_snapshot!(fix("create table users (id serial8);"), @"create table users (id bigint generated by default as identity);");
+        assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id smallint generated by default as identity);");
+        assert_snapshot!(fix("create table users (id bigserial);"), @"create table users (id bigint generated by default as identity);");
+    }
+
+    #[test]
+    fn fix_mixed_case() {
+        assert_snapshot!(fix("create table users (id BIGSERIAL);"), @"create table users (id bigint generated by default as identity);");
+        assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id integer generated by default as identity);");
+    }
 
     #[test]
     fn err() {

crates/squawk_linter/src/rules/require_concurrent_index_creation.rs

@@ -3,10 +3,20 @@ use squawk_syntax::{
     ast::{self, AstNode},
 };
 
-use crate::{Linter, Rule, Violation, identifier::Identifier};
+use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier};
 
 use super::constraint_missing_not_valid::tables_created_in_transaction;
 
+fn concurrently_fix(create_index: &ast::CreateIndex) -> Option<Fix> {
+    if let Some(index_token) = create_index.index_token() {
+        let at = index_token.text_range().end();
+        let edit = Edit::insert(" concurrently", at);
+        Some(Fix::new("Add `concurrently`", vec![edit]))
+    } else {
+        None
+    }
+}
+
 pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse<SourceFile>) {
     let file = parse.tree();
     let tables_created = tables_created_in_transaction(ctx.settings.assume_in_transaction, &file);
@@ -21,11 +31,15 @@ pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse<
                 if create_index.concurrently_token().is_none()
                     && !tables_created.contains(&Identifier::new(&table_name.text()))
                 {
+                    let fix = concurrently_fix(&create_index);
+
                     ctx.report(Violation::for_node(
                         Rule::RequireConcurrentIndexCreation,
                 "During normal index creation, table updates are blocked, but reads are still allowed.".into(),
                         create_index.syntax(),
-                    ).help("Use `CONCURRENTLY` to avoid blocking writes."));
+                    )
+                    .help("Use `concurrently` to avoid blocking writes.")
+                    .fix(fix));
                 }
             }
         }
@@ -34,13 +48,29 @@ pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse<
 
 #[cfg(test)]
 mod test {
-    use insta::assert_debug_snapshot;
+    use insta::{assert_debug_snapshot, assert_snapshot};
 
     use crate::{
         Rule,
-        test_utils::{lint, lint_with_assume_in_transaction},
+        test_utils::{fix_sql, lint, lint_with_assume_in_transaction},
     };
 
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::RequireConcurrentIndexCreation)
+    }
+
+    #[test]
+    fn fix_add_concurrently_named_index() {
+        assert_snapshot!(fix("CREATE INDEX i ON t (c);"), @"CREATE INDEX concurrently i ON t (c);");
+    }
+
+    #[test]
+    fn fix_add_concurrently_unnamed_index() {
+        assert_snapshot!(fix("
+CREATE INDEX ON t (a);
+"), @"CREATE INDEX concurrently ON t (a);");
+    }
+
     /// ```sql
     /// -- instead of
     /// CREATE INDEX "field_name_idx" ON "table_name" ("field_name");