Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 72 additions & 5 deletions crates/squawk_linter/src/rules/prefer_text_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
};
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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);");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
},
],
},
),
},
]
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
},
],
},
),
},
]
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
},
],
},
),
},
]
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
},
],
},
),
},
]
Loading