diff --git a/crates/squawk_linter/src/rules/ban_char_field.rs b/crates/squawk_linter/src/rules/ban_char_field.rs index 35e614d0..4f51629a 100644 --- a/crates/squawk_linter/src/rules/ban_char_field.rs +++ b/crates/squawk_linter/src/rules/ban_char_field.rs @@ -1,12 +1,13 @@ use std::collections::HashSet; +use rowan::TextRange; use squawk_syntax::{ Parse, SourceFile, TokenText, ast::{self, AstNode}, }; -use crate::{Linter, Rule, Violation}; -use crate::{identifier::Identifier, visitors::check_not_allowed_types}; +use crate::visitors::check_not_allowed_types; +use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier}; use lazy_static::lazy_static; @@ -22,6 +23,17 @@ fn is_char_type(x: TokenText<'_>) -> bool { CHAR_TYPES.contains(&Identifier::new(&x.to_string())) } +fn create_fix(range: TextRange, args: Option) -> Fix { + if let Some(args_list) = args { + let end = args_list.syntax().text_range().start(); + let edit = Edit::replace(TextRange::new(range.start(), end), "varchar"); + Fix::new(format!("Replace with `varchar`"), vec![edit]) + } else { + let edit = Edit::replace(range, "text"); + Fix::new(format!("Replace with `text`"), vec![edit]) + } +} + fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) { if let Some(name_ref) = path_type .path() @@ -29,22 +41,24 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) { .and_then(|x| x.name_ref()) { if is_char_type(name_ref.text()) { + let fix = create_fix(name_ref.syntax().text_range(), path_type.arg_list()); ctx.report(Violation::for_node( Rule::BanCharField, "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(), path_type.syntax(), - )); + ).fix(Some(fix))); } } } fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) { if is_char_type(char_type.text()) { + let fix = create_fix(char_type.syntax().text_range(), char_type.arg_list()); ctx.report(Violation::for_node( Rule::BanCharField, - "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.".into(), + "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(), char_type.syntax(), - )); + ).fix(Some(fix))); } } @@ -76,10 +90,58 @@ pub(crate) fn ban_char_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, + test_utils::{fix_sql, lint}, + }; + + fn fix(sql: &str) -> String { + fix_sql(sql, Rule::BanCharField) + } + + #[test] + fn fix_char_without_length() { + assert_snapshot!(fix("CREATE TABLE t (c char);"), @"CREATE TABLE t (c text);"); + assert_snapshot!(fix("CREATE TABLE t (c character);"), @"CREATE TABLE t (c text);"); + assert_snapshot!(fix("CREATE TABLE t (c bpchar);"), @"CREATE TABLE t (c text);"); + } + + #[test] + fn fix_char_with_length() { + assert_snapshot!(fix("CREATE TABLE t (c char(100));"), @"CREATE TABLE t (c varchar(100));"); + assert_snapshot!(fix("CREATE TABLE t (c character(255));"), @"CREATE TABLE t (c varchar(255));"); + assert_snapshot!(fix("CREATE TABLE t (c bpchar(50));"), @"CREATE TABLE t (c varchar(50));"); + + assert_snapshot!(fix(r#"CREATE TABLE t (c "char"(100));"#), @"CREATE TABLE t (c varchar(100));"); + assert_snapshot!(fix(r#"CREATE TABLE t (c "character"(255));"#), @"CREATE TABLE t (c varchar(255));"); + assert_snapshot!(fix(r#"CREATE TABLE t (c "bpchar"(50));"#), @"CREATE TABLE t (c varchar(50));"); + } - use crate::Rule; - use crate::test_utils::lint; + #[test] + fn fix_mixed_case() { + assert_snapshot!(fix("CREATE TABLE t (c CHAR);"), @"CREATE TABLE t (c text);"); + assert_snapshot!(fix("CREATE TABLE t (c CHARACTER(100));"), @"CREATE TABLE t (c varchar(100));"); + assert_snapshot!(fix("CREATE TABLE t (c Char(50));"), @"CREATE TABLE t (c varchar(50));"); + } + + #[test] + fn fix_array_types() { + assert_snapshot!(fix("CREATE TABLE t (c char[]);"), @"CREATE TABLE t (c text[]);"); + assert_snapshot!(fix("CREATE TABLE t (c character(100)[]);"), @"CREATE TABLE t (c varchar(100)[]);"); + } + + #[test] + fn fix_alter_table() { + assert_snapshot!(fix("ALTER TABLE t ADD COLUMN c char;"), @"ALTER TABLE t ADD COLUMN c text;"); + assert_snapshot!(fix("ALTER TABLE t ADD COLUMN c character(100);"), @"ALTER TABLE t ADD COLUMN c varchar(100);"); + } + + #[test] + fn fix_multiple_columns() { + assert_snapshot!(fix("CREATE TABLE t (a char, b character(100), c bpchar(50));"), @"CREATE TABLE t (a text, b varchar(100), c varchar(50));"); + } #[test] fn creating_table_with_char_errors() { diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__all_the_types.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__all_the_types.snap index d751a492..59d81fdd 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__all_the_types.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__all_the_types.snap @@ -5,44 +5,116 @@ expression: errors [ Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 59..68, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `varchar`", + edits: [ + Edit { + text_range: 59..63, + text: Some( + "varchar", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 76..90, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `varchar`", + edits: [ + Edit { + text_range: 76..85, + text: Some( + "varchar", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 98..102, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 98..102, + text: Some( + "text", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 110..119, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 110..119, + text: Some( + "text", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 265..280, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 276..280, + text: Some( + "text", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 288..292, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 288..292, + text: Some( + "text", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__alter_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__alter_table_err.snap index ec0fe2c8..7d34714d 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__alter_table_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__alter_table_err.snap @@ -5,9 +5,21 @@ expression: errors [ Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 28..32, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 28..32, + text: Some( + "text", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__array_char_type_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__array_char_type_err.snap index 9b1cfdc7..f91d14fa 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__array_char_type_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__array_char_type_err.snap @@ -5,9 +5,21 @@ expression: errors [ Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 22..26, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 22..26, + text: Some( + "text", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__case_insensitive.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__case_insensitive.snap index 9b1cfdc7..f91d14fa 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__case_insensitive.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__case_insensitive.snap @@ -5,9 +5,21 @@ expression: errors [ Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 22..26, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 22..26, + text: Some( + "text", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__creating_table_with_char_errors.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__creating_table_with_char_errors.snap index c070a32f..c1b4c4b1 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__creating_table_with_char_errors.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__creating_table_with_char_errors.snap @@ -5,30 +5,78 @@ expression: errors [ Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 77..86, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `varchar`", + edits: [ + Edit { + text_range: 77..81, + text: Some( + "varchar", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 108..122, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `varchar`", + edits: [ + Edit { + text_range: 108..117, + text: Some( + "varchar", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 147..151, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 147..151, + text: Some( + "text", + ), + }, + ], + }, + ), }, Violation { code: BanCharField, - message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", + message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 174..183, help: None, - fix: None, + fix: Some( + Fix { + title: "Replace with `text`", + edits: [ + Edit { + text_range: 174..183, + text: Some( + "text", + ), + }, + ], + }, + ), }, ]