Skip to content

Commit 255942b

Browse files
sbdchdsbdchd
authored
linter: fixes for prefer-bigint-over-int, prefer-bigint-over-small-int (#618)
- prefer-identity - require-concurrent-index-creation - require-concurrent-index-deletion
1 parent 7fa3e33 commit 255942b

13 files changed

+566
-71
lines changed

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

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode;
44
use squawk_syntax::{Parse, SourceFile, ast};
55

66
use crate::identifier::Identifier;
7-
use crate::{Linter, Rule, Violation};
7+
use crate::{Edit, Fix, Linter, Rule, Violation};
88

99
use crate::visitors::check_not_allowed_types;
1010
use crate::visitors::is_not_valid_int_type;
@@ -13,23 +13,50 @@ use lazy_static::lazy_static;
1313

1414
lazy_static! {
1515
static ref INT_TYPES: HashSet<Identifier> = HashSet::from([
16+
Identifier::new("int"),
1617
Identifier::new("integer"),
1718
Identifier::new("int4"),
1819
Identifier::new("serial"),
1920
Identifier::new("serial4"),
2021
]);
2122
}
2223

24+
fn int_to_bigint_replacement(int_type: &str) -> &'static str {
25+
match int_type.to_lowercase().as_str() {
26+
"int" | "integer" => "bigint",
27+
"int4" => "int8",
28+
"serial" => "bigserial",
29+
"serial4" => "serial8",
30+
_ => "bigint",
31+
}
32+
}
33+
34+
fn create_bigint_fix(ty: &ast::Type) -> Option<Fix> {
35+
if let Some(type_name) = ty.syntax().first_token() {
36+
let i64 = int_to_bigint_replacement(type_name.text());
37+
let edit = Edit::replace(ty.syntax().text_range(), i64);
38+
Some(Fix::new(
39+
format!("Replace with a 64-bit integer type: `{i64}`"),
40+
vec![edit],
41+
))
42+
} else {
43+
None
44+
}
45+
}
46+
2347
fn check_ty_for_big_int(ctx: &mut Linter, ty: Option<ast::Type>) {
2448
if let Some(ty) = ty {
2549
if is_not_valid_int_type(&ty, &INT_TYPES) {
50+
let fix = create_bigint_fix(&ty);
51+
2652
ctx.report(
2753
Violation::for_node(
2854
Rule::PreferBigintOverInt,
2955
"Using 32-bit integer fields can result in hitting the max `int` limit.".into(),
3056
ty.syntax(),
3157
)
32-
.help("Use 64-bit integer values instead to prevent hitting this limit."),
58+
.help("Use 64-bit integer values instead to prevent hitting this limit.")
59+
.fix(fix),
3360
);
3461
};
3562
}
@@ -43,14 +70,51 @@ pub(crate) fn prefer_bigint_over_int(ctx: &mut Linter, parse: &Parse<SourceFile>
4370

4471
#[cfg(test)]
4572
mod test {
46-
use insta::assert_debug_snapshot;
73+
use insta::{assert_debug_snapshot, assert_snapshot};
74+
75+
use crate::{
76+
Rule,
77+
test_utils::{fix_sql, lint},
78+
};
79+
80+
fn fix(sql: &str) -> String {
81+
fix_sql(sql, Rule::PreferBigintOverInt)
82+
}
4783

48-
use crate::Rule;
49-
use crate::test_utils::lint;
84+
#[test]
85+
fn fix_int_types() {
86+
assert_snapshot!(fix("create table users (id int);"), @"create table users (id bigint);");
87+
assert_snapshot!(fix("create table users (id integer);"), @"create table users (id bigint);");
88+
assert_snapshot!(fix("create table users (id int4);"), @"create table users (id int8);");
89+
assert_snapshot!(fix("create table users (id serial);"), @"create table users (id bigserial);");
90+
assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id serial8);");
91+
}
92+
93+
#[test]
94+
fn fix_mixed_case() {
95+
assert_snapshot!(fix("create table users (id INT);"), @"create table users (id bigint);");
96+
assert_snapshot!(fix("create table users (id INTEGER);"), @"create table users (id bigint);");
97+
assert_snapshot!(fix("create table users (id Int4);"), @"create table users (id int8);");
98+
assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id bigserial);");
99+
assert_snapshot!(fix("create table users (id SERIAL4);"), @"create table users (id serial8);");
100+
}
101+
102+
#[test]
103+
fn fix_multiple_columns() {
104+
assert_snapshot!(fix("create table users (id int, count integer, version serial);"), @"create table users (id bigint, count bigint, version bigserial);");
105+
}
106+
107+
#[test]
108+
fn fix_with_constraints() {
109+
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);");
110+
}
50111

51112
#[test]
52113
fn err() {
53114
let sql = r#"
115+
create table users (
116+
id int
117+
);
54118
create table users (
55119
id integer
56120
);
@@ -66,13 +130,13 @@ create table users (
66130
"#;
67131
let errors = lint(sql, Rule::PreferBigintOverInt);
68132
assert_ne!(errors.len(), 0);
69-
assert_eq!(errors.len(), 4);
133+
assert_eq!(errors.len(), 5);
70134
assert_eq!(
71135
errors
72136
.iter()
73137
.filter(|x| x.code == Rule::PreferBigintOverInt)
74138
.count(),
75-
4
139+
5
76140
);
77141
assert_debug_snapshot!(errors);
78142
}

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

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode;
44
use squawk_syntax::{Parse, SourceFile, ast};
55

66
use crate::identifier::Identifier;
7-
use crate::{Linter, Rule, Violation};
7+
use crate::{Edit, Fix, Linter, Rule, Violation};
88

99
use crate::visitors::check_not_allowed_types;
1010
use crate::visitors::is_not_valid_int_type;
@@ -20,16 +20,42 @@ lazy_static! {
2020
]);
2121
}
2222

23+
fn smallint_to_bigint(smallint_type: &str) -> &'static str {
24+
match smallint_type.to_lowercase().as_str() {
25+
"smallint" => "bigint",
26+
"int2" => "int8",
27+
"smallserial" => "bigserial",
28+
"serial2" => "serial8",
29+
_ => "bigint",
30+
}
31+
}
32+
33+
fn create_bigint_fix(ty: &ast::Type) -> Option<Fix> {
34+
if let Some(type_name) = ty.syntax().first_token() {
35+
let i64 = smallint_to_bigint(type_name.text());
36+
let edit = Edit::replace(ty.syntax().text_range(), i64);
37+
Some(Fix::new(
38+
format!("Replace with a 64-bit integer type: `{i64}`"),
39+
vec![edit],
40+
))
41+
} else {
42+
None
43+
}
44+
}
45+
2346
fn check_ty_for_small_int(ctx: &mut Linter, ty: Option<ast::Type>) {
2447
if let Some(ty) = ty {
2548
if is_not_valid_int_type(&ty, &SMALL_INT_TYPES) {
49+
let fix = create_bigint_fix(&ty);
50+
2651
ctx.report(
2752
Violation::for_node(
2853
Rule::PreferBigintOverSmallint,
2954
"Using 16-bit integer fields can result in hitting the max `int` limit.".into(),
3055
ty.syntax(),
3156
)
32-
.help("Use 64-bit integer values instead to prevent hitting this limit."),
57+
.help("Use 64-bit integer values instead to prevent hitting this limit.")
58+
.fix(fix),
3359
);
3460
};
3561
}
@@ -42,10 +68,42 @@ pub(crate) fn prefer_bigint_over_smallint(ctx: &mut Linter, parse: &Parse<Source
4268

4369
#[cfg(test)]
4470
mod test {
45-
use insta::assert_debug_snapshot;
71+
use insta::{assert_debug_snapshot, assert_snapshot};
72+
73+
use crate::{
74+
Rule,
75+
test_utils::{fix_sql, lint},
76+
};
4677

47-
use crate::Rule;
48-
use crate::test_utils::lint;
78+
fn fix(sql: &str) -> String {
79+
fix_sql(sql, Rule::PreferBigintOverSmallint)
80+
}
81+
82+
#[test]
83+
fn fix_smallint_types() {
84+
assert_snapshot!(fix("create table users (id smallint);"), @"create table users (id bigint);");
85+
assert_snapshot!(fix("create table users (id int2);"), @"create table users (id int8);");
86+
assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id bigserial);");
87+
assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id serial8);");
88+
}
89+
90+
#[test]
91+
fn fix_mixed_case() {
92+
assert_snapshot!(fix("create table users (id SMALLINT);"), @"create table users (id bigint);");
93+
assert_snapshot!(fix("create table users (id Int2);"), @"create table users (id int8);");
94+
assert_snapshot!(fix("create table users (id SmallSerial);"), @"create table users (id bigserial);");
95+
assert_snapshot!(fix("create table users (id SERIAL2);"), @"create table users (id serial8);");
96+
}
97+
98+
#[test]
99+
fn fix_multiple_columns() {
100+
assert_snapshot!(fix("create table users (id smallint, count int2, version smallserial);"), @"create table users (id bigint, count int8, version bigserial);");
101+
}
102+
103+
#[test]
104+
fn fix_with_constraints() {
105+
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);");
106+
}
49107

50108
#[test]
51109
fn err() {

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

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use squawk_syntax::{
55
ast::{self, AstNode},
66
};
77

8-
use crate::{Linter, Rule, Violation, identifier::Identifier};
8+
use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier};
99

1010
use lazy_static::lazy_static;
1111

@@ -22,17 +22,39 @@ lazy_static! {
2222
]);
2323
}
2424

25+
fn replace_serial(serial_type: &str) -> &'static str {
26+
match serial_type.to_lowercase().as_str() {
27+
"serial" | "serial4" => "integer generated by default as identity",
28+
"serial2" | "smallserial" => "smallint generated by default as identity",
29+
"serial8" | "bigserial" => "bigint generated by default as identity",
30+
_ => "integer generated by default as identity",
31+
}
32+
}
33+
34+
fn create_identity_fix(ty: &ast::Type) -> Option<Fix> {
35+
if let Some(type_name) = ty.syntax().first_token() {
36+
let text = replace_serial(type_name.text());
37+
let edit = Edit::replace(ty.syntax().text_range(), text);
38+
Some(Fix::new("Replace with IDENTITY column", vec![edit]))
39+
} else {
40+
None
41+
}
42+
}
43+
2544
fn check_ty_for_serial(ctx: &mut Linter, ty: Option<ast::Type>) {
2645
if let Some(ty) = ty {
2746
if is_not_valid_int_type(&ty, &SERIAL_TYPES) {
47+
let fix = create_identity_fix(&ty);
48+
2849
ctx.report(
2950
Violation::for_node(
3051
Rule::PreferIdentity,
3152
"Serial types make schema, dependency, and permission management difficult."
3253
.into(),
3354
ty.syntax(),
3455
)
35-
.help("Use an `IDENTITY` column instead."),
56+
.help("Use an `IDENTITY` column instead.")
57+
.fix(fix),
3658
);
3759
};
3860
}
@@ -45,10 +67,32 @@ pub(crate) fn prefer_identity(ctx: &mut Linter, parse: &Parse<SourceFile>) {
4567

4668
#[cfg(test)]
4769
mod test {
48-
use insta::assert_debug_snapshot;
70+
use insta::{assert_debug_snapshot, assert_snapshot};
71+
72+
use crate::{
73+
Rule,
74+
test_utils::{fix_sql, lint},
75+
};
4976

50-
use crate::Rule;
51-
use crate::test_utils::lint;
77+
fn fix(sql: &str) -> String {
78+
fix_sql(sql, Rule::PreferIdentity)
79+
}
80+
81+
#[test]
82+
fn fix_serial_types() {
83+
assert_snapshot!(fix("create table users (id serial);"), @"create table users (id integer generated by default as identity);");
84+
assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id smallint generated by default as identity);");
85+
assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id integer generated by default as identity);");
86+
assert_snapshot!(fix("create table users (id serial8);"), @"create table users (id bigint generated by default as identity);");
87+
assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id smallint generated by default as identity);");
88+
assert_snapshot!(fix("create table users (id bigserial);"), @"create table users (id bigint generated by default as identity);");
89+
}
90+
91+
#[test]
92+
fn fix_mixed_case() {
93+
assert_snapshot!(fix("create table users (id BIGSERIAL);"), @"create table users (id bigint generated by default as identity);");
94+
assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id integer generated by default as identity);");
95+
}
5296

5397
#[test]
5498
fn err() {

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

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,20 @@ use squawk_syntax::{
33
ast::{self, AstNode},
44
};
55

6-
use crate::{Linter, Rule, Violation, identifier::Identifier};
6+
use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier};
77

88
use super::constraint_missing_not_valid::tables_created_in_transaction;
99

10+
fn concurrently_fix(create_index: &ast::CreateIndex) -> Option<Fix> {
11+
if let Some(index_token) = create_index.index_token() {
12+
let at = index_token.text_range().end();
13+
let edit = Edit::insert(" concurrently", at);
14+
Some(Fix::new("Add `concurrently`", vec![edit]))
15+
} else {
16+
None
17+
}
18+
}
19+
1020
pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse<SourceFile>) {
1121
let file = parse.tree();
1222
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<
2131
if create_index.concurrently_token().is_none()
2232
&& !tables_created.contains(&Identifier::new(&table_name.text()))
2333
{
34+
let fix = concurrently_fix(&create_index);
35+
2436
ctx.report(Violation::for_node(
2537
Rule::RequireConcurrentIndexCreation,
2638
"During normal index creation, table updates are blocked, but reads are still allowed.".into(),
2739
create_index.syntax(),
28-
).help("Use `CONCURRENTLY` to avoid blocking writes."));
40+
)
41+
.help("Use `concurrently` to avoid blocking writes.")
42+
.fix(fix));
2943
}
3044
}
3145
}
@@ -34,13 +48,29 @@ pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse<
3448

3549
#[cfg(test)]
3650
mod test {
37-
use insta::assert_debug_snapshot;
51+
use insta::{assert_debug_snapshot, assert_snapshot};
3852

3953
use crate::{
4054
Rule,
41-
test_utils::{lint, lint_with_assume_in_transaction},
55+
test_utils::{fix_sql, lint, lint_with_assume_in_transaction},
4256
};
4357

58+
fn fix(sql: &str) -> String {
59+
fix_sql(sql, Rule::RequireConcurrentIndexCreation)
60+
}
61+
62+
#[test]
63+
fn fix_add_concurrently_named_index() {
64+
assert_snapshot!(fix("CREATE INDEX i ON t (c);"), @"CREATE INDEX concurrently i ON t (c);");
65+
}
66+
67+
#[test]
68+
fn fix_add_concurrently_unnamed_index() {
69+
assert_snapshot!(fix("
70+
CREATE INDEX ON t (a);
71+
"), @"CREATE INDEX concurrently ON t (a);");
72+
}
73+
4474
/// ```sql
4575
/// -- instead of
4676
/// CREATE INDEX "field_name_idx" ON "table_name" ("field_name");

0 commit comments

Comments
 (0)