Skip to content

Commit d1604fc

Browse files
sbdchdsbdchd
authored
linter: fixes fox ban-char-field (#620)
when there are args, like character(100), we convert to varchar(100) instead of text later we'll add an autofix for varchar(100) to convert it to text w/ a constraint
1 parent 255942b commit d1604fc

6 files changed

+251
-33
lines changed

‎crates/squawk_linter/src/rules/ban_char_field.rs‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use std::collections::HashSet;
22

3+
use rowan::TextRange;
34
use squawk_syntax::{
45
Parse, SourceFile, TokenText,
56
ast::{self, AstNode},
67
};
78

8-
use crate::{Linter, Rule, Violation};
9-
use crate::{identifier::Identifier, visitors::check_not_allowed_types};
9+
use crate::visitors::check_not_allowed_types;
10+
use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier};
1011

1112
use lazy_static::lazy_static;
1213

@@ -22,29 +23,42 @@ fn is_char_type(x: TokenText<'_>) -> bool {
2223
CHAR_TYPES.contains(&Identifier::new(&x.to_string()))
2324
}
2425

26+
fn create_fix(range: TextRange, args: Option<ast::ArgList>) -> Fix {
27+
if let Some(args_list) = args {
28+
let end = args_list.syntax().text_range().start();
29+
let edit = Edit::replace(TextRange::new(range.start(), end), "varchar");
30+
Fix::new(format!("Replace with `varchar`"), vec![edit])
31+
} else {
32+
let edit = Edit::replace(range, "text");
33+
Fix::new(format!("Replace with `text`"), vec![edit])
34+
}
35+
}
36+
2537
fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) {
2638
if let Some(name_ref) = path_type
2739
.path()
2840
.and_then(|x| x.segment())
2941
.and_then(|x| x.name_ref())
3042
{
3143
if is_char_type(name_ref.text()) {
44+
let fix = create_fix(name_ref.syntax().text_range(), path_type.arg_list());
3245
ctx.report(Violation::for_node(
3346
Rule::BanCharField,
3447
"Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
3548
path_type.syntax(),
36-
));
49+
).fix(Some(fix)));
3750
}
3851
}
3952
}
4053

4154
fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) {
4255
if is_char_type(char_type.text()) {
56+
let fix = create_fix(char_type.syntax().text_range(), char_type.arg_list());
4357
ctx.report(Violation::for_node(
4458
Rule::BanCharField,
45-
"Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.".into(),
59+
"Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
4660
char_type.syntax(),
47-
));
61+
).fix(Some(fix)));
4862
}
4963
}
5064

@@ -76,10 +90,58 @@ pub(crate) fn ban_char_field(ctx: &mut Linter, parse: &Parse<SourceFile>) {
7690

7791
#[cfg(test)]
7892
mod test {
79-
use insta::assert_debug_snapshot;
93+
use insta::{assert_debug_snapshot, assert_snapshot};
94+
95+
use crate::{
96+
Rule,
97+
test_utils::{fix_sql, lint},
98+
};
99+
100+
fn fix(sql: &str) -> String {
101+
fix_sql(sql, Rule::BanCharField)
102+
}
103+
104+
#[test]
105+
fn fix_char_without_length() {
106+
assert_snapshot!(fix("CREATE TABLE t (c char);"), @"CREATE TABLE t (c text);");
107+
assert_snapshot!(fix("CREATE TABLE t (c character);"), @"CREATE TABLE t (c text);");
108+
assert_snapshot!(fix("CREATE TABLE t (c bpchar);"), @"CREATE TABLE t (c text);");
109+
}
110+
111+
#[test]
112+
fn fix_char_with_length() {
113+
assert_snapshot!(fix("CREATE TABLE t (c char(100));"), @"CREATE TABLE t (c varchar(100));");
114+
assert_snapshot!(fix("CREATE TABLE t (c character(255));"), @"CREATE TABLE t (c varchar(255));");
115+
assert_snapshot!(fix("CREATE TABLE t (c bpchar(50));"), @"CREATE TABLE t (c varchar(50));");
116+
117+
assert_snapshot!(fix(r#"CREATE TABLE t (c "char"(100));"#), @"CREATE TABLE t (c varchar(100));");
118+
assert_snapshot!(fix(r#"CREATE TABLE t (c "character"(255));"#), @"CREATE TABLE t (c varchar(255));");
119+
assert_snapshot!(fix(r#"CREATE TABLE t (c "bpchar"(50));"#), @"CREATE TABLE t (c varchar(50));");
120+
}
80121

81-
use crate::Rule;
82-
use crate::test_utils::lint;
122+
#[test]
123+
fn fix_mixed_case() {
124+
assert_snapshot!(fix("CREATE TABLE t (c CHAR);"), @"CREATE TABLE t (c text);");
125+
assert_snapshot!(fix("CREATE TABLE t (c CHARACTER(100));"), @"CREATE TABLE t (c varchar(100));");
126+
assert_snapshot!(fix("CREATE TABLE t (c Char(50));"), @"CREATE TABLE t (c varchar(50));");
127+
}
128+
129+
#[test]
130+
fn fix_array_types() {
131+
assert_snapshot!(fix("CREATE TABLE t (c char[]);"), @"CREATE TABLE t (c text[]);");
132+
assert_snapshot!(fix("CREATE TABLE t (c character(100)[]);"), @"CREATE TABLE t (c varchar(100)[]);");
133+
}
134+
135+
#[test]
136+
fn fix_alter_table() {
137+
assert_snapshot!(fix("ALTER TABLE t ADD COLUMN c char;"), @"ALTER TABLE t ADD COLUMN c text;");
138+
assert_snapshot!(fix("ALTER TABLE t ADD COLUMN c character(100);"), @"ALTER TABLE t ADD COLUMN c varchar(100);");
139+
}
140+
141+
#[test]
142+
fn fix_multiple_columns() {
143+
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));");
144+
}
83145

84146
#[test]
85147
fn creating_table_with_char_errors() {

‎crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__all_the_types.snap‎

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,116 @@ expression: errors
55
[
66
Violation {
77
code: BanCharField,
8-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
8+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
99
text_range: 59..68,
1010
help: None,
11-
fix: None,
11+
fix: Some(
12+
Fix {
13+
title: "Replace with `varchar`",
14+
edits: [
15+
Edit {
16+
text_range: 59..63,
17+
text: Some(
18+
"varchar",
19+
),
20+
},
21+
],
22+
},
23+
),
1224
},
1325
Violation {
1426
code: BanCharField,
15-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
27+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
1628
text_range: 76..90,
1729
help: None,
18-
fix: None,
30+
fix: Some(
31+
Fix {
32+
title: "Replace with `varchar`",
33+
edits: [
34+
Edit {
35+
text_range: 76..85,
36+
text: Some(
37+
"varchar",
38+
),
39+
},
40+
],
41+
},
42+
),
1943
},
2044
Violation {
2145
code: BanCharField,
22-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
46+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
2347
text_range: 98..102,
2448
help: None,
25-
fix: None,
49+
fix: Some(
50+
Fix {
51+
title: "Replace with `text`",
52+
edits: [
53+
Edit {
54+
text_range: 98..102,
55+
text: Some(
56+
"text",
57+
),
58+
},
59+
],
60+
},
61+
),
2662
},
2763
Violation {
2864
code: BanCharField,
29-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
65+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
3066
text_range: 110..119,
3167
help: None,
32-
fix: None,
68+
fix: Some(
69+
Fix {
70+
title: "Replace with `text`",
71+
edits: [
72+
Edit {
73+
text_range: 110..119,
74+
text: Some(
75+
"text",
76+
),
77+
},
78+
],
79+
},
80+
),
3381
},
3482
Violation {
3583
code: BanCharField,
3684
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
3785
text_range: 265..280,
3886
help: None,
39-
fix: None,
87+
fix: Some(
88+
Fix {
89+
title: "Replace with `text`",
90+
edits: [
91+
Edit {
92+
text_range: 276..280,
93+
text: Some(
94+
"text",
95+
),
96+
},
97+
],
98+
},
99+
),
40100
},
41101
Violation {
42102
code: BanCharField,
43-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
103+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
44104
text_range: 288..292,
45105
help: None,
46-
fix: None,
106+
fix: Some(
107+
Fix {
108+
title: "Replace with `text`",
109+
edits: [
110+
Edit {
111+
text_range: 288..292,
112+
text: Some(
113+
"text",
114+
),
115+
},
116+
],
117+
},
118+
),
47119
},
48120
]

‎crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__alter_table_err.snap‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,21 @@ expression: errors
55
[
66
Violation {
77
code: BanCharField,
8-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
8+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
99
text_range: 28..32,
1010
help: None,
11-
fix: None,
11+
fix: Some(
12+
Fix {
13+
title: "Replace with `text`",
14+
edits: [
15+
Edit {
16+
text_range: 28..32,
17+
text: Some(
18+
"text",
19+
),
20+
},
21+
],
22+
},
23+
),
1224
},
1325
]

‎crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__array_char_type_err.snap‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,21 @@ expression: errors
55
[
66
Violation {
77
code: BanCharField,
8-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
8+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
99
text_range: 22..26,
1010
help: None,
11-
fix: None,
11+
fix: Some(
12+
Fix {
13+
title: "Replace with `text`",
14+
edits: [
15+
Edit {
16+
text_range: 22..26,
17+
text: Some(
18+
"text",
19+
),
20+
},
21+
],
22+
},
23+
),
1224
},
1325
]

‎crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_char_field__test__case_insensitive.snap‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,21 @@ expression: errors
55
[
66
Violation {
77
code: BanCharField,
8-
message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.",
8+
message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.",
99
text_range: 22..26,
1010
help: None,
11-
fix: None,
11+
fix: Some(
12+
Fix {
13+
title: "Replace with `text`",
14+
edits: [
15+
Edit {
16+
text_range: 22..26,
17+
text: Some(
18+
"text",
19+
),
20+
},
21+
],
22+
},
23+
),
1224
},
1325
]

0 commit comments

Comments
 (0)