diff --git a/crates/squawk_linter/src/rules/prefer_text_field.rs b/crates/squawk_linter/src/rules/prefer_text_field.rs index c90ede25..846b7945 100644 --- a/crates/squawk_linter/src/rules/prefer_text_field.rs +++ b/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 { + 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) { 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) { #[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);"); + } } diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap index e7f672c3..e4cb45fb 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap +++ b/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", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap index e7a14f0f..f0ab270e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap +++ b/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", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap index d83aa957..4aa3b57e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap +++ b/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", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap index c7b20817..19133abb 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap +++ b/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", + ), + }, + ], + }, + ), }, ]