From 9397d0235dbc489b1f2e7237d912c6a000228b4d Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 22 Aug 2025 20:03:30 -0400 Subject: [PATCH 1/3] linter: fixes for prefer-bigint-over-int, prefer-bigint-over-small-int - prefer-identity - require-concurrent-index-creation - require-concurrent-index-deletion --- .../src/rules/prefer_bigint_over_int.rs | 78 +++++++++++++-- .../src/rules/prefer_bigint_over_smallint.rs | 68 ++++++++++++- .../src/rules/prefer_identity.rs | 54 +++++++++- .../require_concurrent_index_creation.rs | 38 ++++++- .../require_concurrent_index_deletion.rs | 49 +++++++++- ...es__prefer_bigint_over_int__test__err.snap | 85 ++++++++++++++-- ...refer_bigint_over_smallint__test__err.snap | 56 ++++++++++- ...er__rules__prefer_identity__test__err.snap | 98 +++++++++++++++++-- ...prefer_identity__test__ok_when_quoted.snap | 28 +++++- ...st__adding_index_non_concurrently_err.snap | 16 ++- ...__drop_index_missing_concurrently_err.snap | 14 ++- crates/squawk_linter/src/test_utils.rs | 4 +- 12 files changed, 536 insertions(+), 52 deletions(-) diff --git a/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs b/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs index 47c1dfdf..bfd45adc 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs @@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode; use squawk_syntax::{Parse, SourceFile, ast}; use crate::identifier::Identifier; -use crate::{Linter, Rule, Violation}; +use crate::{Edit, Fix, Linter, Rule, Violation}; use crate::visitors::check_not_allowed_types; use crate::visitors::is_not_valid_int_type; @@ -13,6 +13,7 @@ use lazy_static::lazy_static; lazy_static! { static ref INT_TYPES: HashSet = HashSet::from([ + Identifier::new("int"), Identifier::new("integer"), Identifier::new("int4"), Identifier::new("serial"), @@ -20,16 +21,42 @@ lazy_static! { ]); } +fn int_to_bigint_replacement(int_type: &str) -> &'static str { + match int_type.to_lowercase().as_str() { + "int" | "integer" => "bigint", + "int4" => "int8", + "serial" => "bigserial", + "serial4" => "serial8", + _ => "bigint", + } +} + +fn create_bigint_fix(ty: &ast::Type) -> Option { + if let Some(type_name) = ty.syntax().first_token() { + let i64 = int_to_bigint_replacement(type_name.text()); + let edit = Edit::replace(ty.syntax().text_range(), i64); + Some(Fix::new( + format!("Replace with a 64-bit integer type: `{i64}`"), + vec![edit], + )) + } else { + None + } +} + fn check_ty_for_big_int(ctx: &mut Linter, ty: Option) { if let Some(ty) = ty { if is_not_valid_int_type(&ty, &INT_TYPES) { + let fix = create_bigint_fix(&ty); + ctx.report( Violation::for_node( Rule::PreferBigintOverInt, "Using 32-bit integer fields can result in hitting the max `int` limit.".into(), ty.syntax(), ) - .help("Use 64-bit integer values instead to prevent hitting this limit."), + .help("Use 64-bit integer values instead to prevent hitting this limit.") + .fix(fix), ); }; } @@ -43,14 +70,51 @@ pub(crate) fn prefer_bigint_over_int(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::PreferBigintOverInt) + } - use crate::Rule; - use crate::test_utils::lint; + #[test] + fn fix_int_types() { + assert_snapshot!(fix("create table users (id int);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id integer);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id int4);"), @"create table users (id int8);"); + assert_snapshot!(fix("create table users (id serial);"), @"create table users (id bigserial);"); + assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id serial8);"); + } + + #[test] + fn fix_mixed_case() { + assert_snapshot!(fix("create table users (id INT);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id INTEGER);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id Int4);"), @"create table users (id int8);"); + assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id bigserial);"); + assert_snapshot!(fix("create table users (id SERIAL4);"), @"create table users (id serial8);"); + } + + #[test] + fn fix_multiple_columns() { + assert_snapshot!(fix("create table users (id int, count integer, version serial);"), @"create table users (id bigint, count bigint, version bigserial);"); + } + + #[test] + fn fix_with_constraints() { + 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);"); + } #[test] fn err() { let sql = r#" +create table users ( + id int +); create table users ( id integer ); @@ -66,13 +130,13 @@ create table users ( "#; let errors = lint(sql, Rule::PreferBigintOverInt); assert_ne!(errors.len(), 0); - assert_eq!(errors.len(), 4); + assert_eq!(errors.len(), 5); assert_eq!( errors .iter() .filter(|x| x.code == Rule::PreferBigintOverInt) .count(), - 4 + 5 ); assert_debug_snapshot!(errors); } diff --git a/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs b/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs index c1903718..20f7e026 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs @@ -4,7 +4,7 @@ use squawk_syntax::ast::AstNode; use squawk_syntax::{Parse, SourceFile, ast}; use crate::identifier::Identifier; -use crate::{Linter, Rule, Violation}; +use crate::{Edit, Fix, Linter, Rule, Violation}; use crate::visitors::check_not_allowed_types; use crate::visitors::is_not_valid_int_type; @@ -20,16 +20,42 @@ lazy_static! { ]); } +fn smallint_to_bigint(smallint_type: &str) -> &'static str { + match smallint_type.to_lowercase().as_str() { + "smallint" => "bigint", + "int2" => "int8", + "smallserial" => "bigserial", + "serial2" => "serial8", + _ => "bigint", + } +} + +fn create_bigint_fix(ty: &ast::Type) -> Option { + if let Some(type_name) = ty.syntax().first_token() { + let i64 = smallint_to_bigint(type_name.text()); + let edit = Edit::replace(ty.syntax().text_range(), i64); + Some(Fix::new( + format!("Replace with a 64-bit integer type: `{i64}`"), + vec![edit], + )) + } else { + None + } +} + fn check_ty_for_small_int(ctx: &mut Linter, ty: Option) { if let Some(ty) = ty { if is_not_valid_int_type(&ty, &SMALL_INT_TYPES) { + let fix = create_bigint_fix(&ty); + ctx.report( Violation::for_node( Rule::PreferBigintOverSmallint, "Using 16-bit integer fields can result in hitting the max `int` limit.".into(), ty.syntax(), ) - .help("Use 64-bit integer values instead to prevent hitting this limit."), + .help("Use 64-bit integer values instead to prevent hitting this limit.") + .fix(fix), ); }; } @@ -42,10 +68,42 @@ pub(crate) fn prefer_bigint_over_smallint(ctx: &mut Linter, parse: &Parse String { + fix_sql(sql, Rule::PreferBigintOverSmallint) + } + + #[test] + fn fix_smallint_types() { + assert_snapshot!(fix("create table users (id smallint);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id int2);"), @"create table users (id int8);"); + assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id bigserial);"); + assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id serial8);"); + } + + #[test] + fn fix_mixed_case() { + assert_snapshot!(fix("create table users (id SMALLINT);"), @"create table users (id bigint);"); + assert_snapshot!(fix("create table users (id Int2);"), @"create table users (id int8);"); + assert_snapshot!(fix("create table users (id SmallSerial);"), @"create table users (id bigserial);"); + assert_snapshot!(fix("create table users (id SERIAL2);"), @"create table users (id serial8);"); + } + + #[test] + fn fix_multiple_columns() { + assert_snapshot!(fix("create table users (id smallint, count int2, version smallserial);"), @"create table users (id bigint, count int8, version bigserial);"); + } + + #[test] + fn fix_with_constraints() { + 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);"); + } #[test] fn err() { diff --git a/crates/squawk_linter/src/rules/prefer_identity.rs b/crates/squawk_linter/src/rules/prefer_identity.rs index 606bf56d..8822bd36 100644 --- a/crates/squawk_linter/src/rules/prefer_identity.rs +++ b/crates/squawk_linter/src/rules/prefer_identity.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 lazy_static::lazy_static; @@ -22,9 +22,30 @@ lazy_static! { ]); } +fn replace_serial(serial_type: &str) -> &'static str { + match serial_type.to_lowercase().as_str() { + "serial" | "serial4" => "integer generated by default as identity", + "serial2" | "smallserial" => "smallint generated by default as identity", + "serial8" | "bigserial" => "bigint generated by default as identity", + _ => "integer generated by default as identity", + } +} + +fn create_identity_fix(ty: &ast::Type) -> Option { + if let Some(type_name) = ty.syntax().first_token() { + let text = replace_serial(type_name.text()); + let edit = Edit::replace(ty.syntax().text_range(), text); + Some(Fix::new("Replace with IDENTITY column", vec![edit])) + } else { + None + } +} + fn check_ty_for_serial(ctx: &mut Linter, ty: Option) { if let Some(ty) = ty { if is_not_valid_int_type(&ty, &SERIAL_TYPES) { + let fix = create_identity_fix(&ty); + ctx.report( Violation::for_node( Rule::PreferIdentity, @@ -32,7 +53,8 @@ fn check_ty_for_serial(ctx: &mut Linter, ty: Option) { .into(), ty.syntax(), ) - .help("Use an `IDENTITY` column instead."), + .help("Use an `IDENTITY` column instead.") + .fix(fix), ); }; } @@ -45,10 +67,32 @@ pub(crate) fn prefer_identity(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}, + }; - use crate::Rule; - use crate::test_utils::lint; + fn fix(sql: &str) -> String { + fix_sql(sql, Rule::PreferIdentity) + } + + #[test] + fn fix_serial_types() { + assert_snapshot!(fix("create table users (id serial);"), @"create table users (id integer generated by default as identity);"); + assert_snapshot!(fix("create table users (id serial2);"), @"create table users (id smallint generated by default as identity);"); + assert_snapshot!(fix("create table users (id serial4);"), @"create table users (id integer generated by default as identity);"); + assert_snapshot!(fix("create table users (id serial8);"), @"create table users (id bigint generated by default as identity);"); + assert_snapshot!(fix("create table users (id smallserial);"), @"create table users (id smallint generated by default as identity);"); + assert_snapshot!(fix("create table users (id bigserial);"), @"create table users (id bigint generated by default as identity);"); + } + + #[test] + fn fix_mixed_case() { + assert_snapshot!(fix("create table users (id BIGSERIAL);"), @"create table users (id bigint generated by default as identity);"); + assert_snapshot!(fix("create table users (id Serial);"), @"create table users (id integer generated by default as identity);"); + } #[test] fn err() { diff --git a/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs b/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs index 03cea33e..d27e61a5 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs @@ -3,10 +3,20 @@ use squawk_syntax::{ ast::{self, AstNode}, }; -use crate::{Linter, Rule, Violation, identifier::Identifier}; +use crate::{Edit, Fix, Linter, Rule, Violation, identifier::Identifier}; use super::constraint_missing_not_valid::tables_created_in_transaction; +fn concurrently_fix(create_index: &ast::CreateIndex) -> Option { + if let Some(index_token) = create_index.index_token() { + let at = index_token.text_range().end(); + let edit = Edit::insert(" concurrently", at); + Some(Fix::new("Add `concurrently`", vec![edit])) + } else { + None + } +} + pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); 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< if create_index.concurrently_token().is_none() && !tables_created.contains(&Identifier::new(&table_name.text())) { + let fix = concurrently_fix(&create_index); + ctx.report(Violation::for_node( Rule::RequireConcurrentIndexCreation, "During normal index creation, table updates are blocked, but reads are still allowed.".into(), create_index.syntax(), - ).help("Use `CONCURRENTLY` to avoid blocking writes.")); + ) + .help("Use `concurrently` to avoid blocking writes.") + .fix(fix)); } } } @@ -34,13 +48,29 @@ pub(crate) fn require_concurrent_index_creation(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::{lint, lint_with_assume_in_transaction}, + test_utils::{fix_sql, lint, lint_with_assume_in_transaction}, }; + fn fix(sql: &str) -> String { + fix_sql(sql, Rule::RequireConcurrentIndexCreation) + } + + #[test] + fn fix_add_concurrently_named_index() { + assert_snapshot!(fix("CREATE INDEX i ON t (c);"), @"CREATE INDEX concurrently i ON t (c);"); + } + + #[test] + fn fix_add_concurrently_unnamed_index() { + assert_snapshot!(fix(" +CREATE INDEX ON t (a); +"), @"CREATE INDEX concurrently ON t (a);"); + } + /// ```sql /// -- instead of /// CREATE INDEX "field_name_idx" ON "table_name" ("field_name"); diff --git a/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs b/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs index 01d8dd41..07613c9f 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs @@ -3,18 +3,30 @@ use squawk_syntax::{ ast::{self, AstNode}, }; -use crate::{Linter, Rule, Violation}; +use crate::{Edit, Fix, Linter, Rule, Violation}; + +fn concurrently_fix(drop_index: &ast::DropIndex) -> Option { + if let Some(index_token) = drop_index.index_token() { + let at = index_token.text_range().end(); + let edit = Edit::insert(" concurrently", at); + Some(Fix::new("Add `concurrently`", vec![edit])) + } else { + None + } +} pub(crate) fn require_concurrent_index_deletion(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); for stmt in file.stmts() { if let ast::Stmt::DropIndex(drop_index) = stmt { if drop_index.concurrently_token().is_none() { + let fix = concurrently_fix(&drop_index); + ctx.report(Violation::for_node( Rule::RequireConcurrentIndexDeletion, "A normal `DROP INDEX` acquires an `ACCESS EXCLUSIVE` lock on the table, blocking other accesses until the index drop can complete.".into(), drop_index.syntax(), - ).help("Drop the index `CONCURRENTLY`.")); + ).help("Drop the index `CONCURRENTLY`.").fix(fix)); } } } @@ -22,10 +34,37 @@ pub(crate) fn require_concurrent_index_deletion(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::RequireConcurrentIndexDeletion) + } + + #[test] + fn fix_add_concurrently_simple() { + let sql = "drop index i;"; + let result = fix(sql); + assert_snapshot!(result, @"drop index concurrently i;"); + } + + #[test] + fn fix_add_concurrently_if_exists() { + let sql = r#"DROP INDEX IF EXISTS "field_name_idx";"#; + let result = fix(sql); + assert_snapshot!(result, @r#"DROP INDEX concurrently IF EXISTS "field_name_idx";"#); + } + + #[test] + fn fix_add_concurrently_multiple_indexes() { + let sql = r#"DROP INDEX "idx1", "idx2";"#; + let result = fix(sql); + assert_snapshot!(result, @r#"DROP INDEX concurrently "idx1", "idx2";"#); + } #[test] fn drop_index_missing_concurrently_err() { diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_int__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_int__test__err.snap index 56acd739..b0950a0a 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_int__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_int__test__err.snap @@ -6,37 +6,106 @@ expression: errors Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", - text_range: 29..36, + text_range: 29..32, help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `bigint`", + edits: [ + Edit { + text_range: 29..32, + text: Some( + "bigint", + ), + }, + ], + }, + ), + }, + Violation { + code: PreferBigintOverInt, + message: "Using 32-bit integer fields can result in hitting the max `int` limit.", + text_range: 64..71, + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `bigint`", + edits: [ + Edit { + text_range: 64..71, + text: Some( + "bigint", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", - text_range: 68..72, + text_range: 103..107, help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `int8`", + edits: [ + Edit { + text_range: 103..107, + text: Some( + "int8", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", - text_range: 104..110, + text_range: 139..145, help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `bigserial`", + edits: [ + Edit { + text_range: 139..145, + text: Some( + "bigserial", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", - text_range: 142..149, + text_range: 177..184, help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `serial8`", + edits: [ + Edit { + text_range: 177..184, + text: Some( + "serial8", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_smallint__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_smallint__test__err.snap index 1b758e72..bcd7fac5 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_smallint__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_bigint_over_smallint__test__err.snap @@ -10,7 +10,19 @@ expression: errors help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `bigint`", + edits: [ + Edit { + text_range: 29..37, + text: Some( + "bigint", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverSmallint, @@ -19,7 +31,19 @@ expression: errors help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `int8`", + edits: [ + Edit { + text_range: 69..73, + text: Some( + "int8", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverSmallint, @@ -28,7 +52,19 @@ expression: errors help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `bigserial`", + edits: [ + Edit { + text_range: 105..116, + text: Some( + "bigserial", + ), + }, + ], + }, + ), }, Violation { code: PreferBigintOverSmallint, @@ -37,6 +73,18 @@ expression: errors help: Some( "Use 64-bit integer values instead to prevent hitting this limit.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with a 64-bit integer type: `serial8`", + edits: [ + Edit { + text_range: 148..155, + text: Some( + "serial8", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__err.snap index 492516e3..4ed7068e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__err.snap @@ -10,7 +10,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 29..35, + text: Some( + "integer generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -19,7 +31,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 67..74, + text: Some( + "smallint generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -28,7 +52,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 106..113, + text: Some( + "integer generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -37,7 +73,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 145..152, + text: Some( + "bigint generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -46,7 +94,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 184..195, + text: Some( + "smallint generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -55,7 +115,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 227..236, + text: Some( + "bigint generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -64,6 +136,18 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 268..277, + text: Some( + "bigint generated by default as identity", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__ok_when_quoted.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__ok_when_quoted.snap index 08d31d86..74612939 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__ok_when_quoted.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_identity__test__ok_when_quoted.snap @@ -10,7 +10,19 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 29..37, + text: Some( + "integer generated by default as identity", + ), + }, + ], + }, + ), }, Violation { code: PreferIdentity, @@ -19,6 +31,18 @@ expression: errors help: Some( "Use an `IDENTITY` column instead.", ), - fix: None, + fix: Some( + Fix { + title: "Replace with IDENTITY column", + edits: [ + Edit { + text_range: 69..80, + text: Some( + "integer generated by default as identity", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_creation__test__adding_index_non_concurrently_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_creation__test__adding_index_non_concurrently_err.snap index a9972c9e..5575d2f9 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_creation__test__adding_index_non_concurrently_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_creation__test__adding_index_non_concurrently_err.snap @@ -8,8 +8,20 @@ expression: errors message: "During normal index creation, table updates are blocked, but reads are still allowed.", text_range: 15..75, help: Some( - "Use `CONCURRENTLY` to avoid blocking writes.", + "Use `concurrently` to avoid blocking writes.", + ), + fix: Some( + Fix { + title: "Add `concurrently`", + edits: [ + Edit { + text_range: 27..27, + text: Some( + " concurrently", + ), + }, + ], + }, ), - fix: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_deletion__test__drop_index_missing_concurrently_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_deletion__test__drop_index_missing_concurrently_err.snap index 3b3e1df7..62689296 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_deletion__test__drop_index_missing_concurrently_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__require_concurrent_index_deletion__test__drop_index_missing_concurrently_err.snap @@ -10,6 +10,18 @@ expression: errors help: Some( "Drop the index `CONCURRENTLY`.", ), - fix: None, + fix: Some( + Fix { + title: "Add `concurrently`", + edits: [ + Edit { + text_range: 29..29, + text: Some( + " concurrently", + ), + }, + ], + }, + ), }, ] diff --git a/crates/squawk_linter/src/test_utils.rs b/crates/squawk_linter/src/test_utils.rs index f0620a1b..9c8ae315 100644 --- a/crates/squawk_linter/src/test_utils.rs +++ b/crates/squawk_linter/src/test_utils.rs @@ -39,8 +39,8 @@ pub(crate) fn fix_sql(sql: &str, rule: Rule) -> String { let file = squawk_syntax::SourceFile::parse(&result); assert_eq!( - file.errors().len(), - 0, + file.errors(), + vec![], "Shouldn't introduce any syntax errors" ); let mut linter = Linter::from([rule]); From c6ebc18c8612f6a1f599a791bbc530c53428dc9c Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 22 Aug 2025 20:16:09 -0400 Subject: [PATCH 2/3] fix-ignore --- crates/squawk_server/src/ignore.rs | 46 ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/crates/squawk_server/src/ignore.rs b/crates/squawk_server/src/ignore.rs index 40bac135..9ba8079c 100644 --- a/crates/squawk_server/src/ignore.rs +++ b/crates/squawk_server/src/ignore.rs @@ -28,13 +28,18 @@ pub(crate) fn ignore_line_edit( match previous_line_token.kind() { SyntaxKind::COMMENT if is_ignore_comment(&previous_line_token) => { - let (_str, range, _ignore_kind) = + let (_str, ignore_comment_range, _ignore_kind) = squawk_linter::ignore::ignore_rule_info(&previous_line_token)?; - Some(Edit::insert(format!(" {rule_name},"), range.start())) + Some(Edit::insert( + format!(" {rule_name},"), + ignore_comment_range.start(), + )) } + + // TODO: we need to handle indenting correctly _ => Some(Edit::insert( format!("-- {IGNORE_LINE_TEXT} {rule_name}\n"), - violation.text_range.start(), + line_index.line(violation_line.line)?.start(), )), } } @@ -90,22 +95,25 @@ create table c ( }) .collect::>(); insta::assert_snapshot!(apply_text_edits(sql, ignore_line_edits), @r" - -- squawk-ignore prefer-robust-stmts - create table a ( - a int - ); - - -- an existing comment that shouldn't get in the way of us adding a new ignore - -- squawk-ignore prefer-robust-stmts - create table b ( - b int - ); - - -- squawk-ignore prefer-robust-stmts, prefer-text-field - create table c ( - b int - ); - "); + -- squawk-ignore prefer-robust-stmts + create table a ( + -- squawk-ignore prefer-bigint-over-int + a int + ); + + -- an existing comment that shouldn't get in the way of us adding a new ignore + -- squawk-ignore prefer-robust-stmts + create table b ( + -- squawk-ignore prefer-bigint-over-int + b int + ); + + -- squawk-ignore prefer-robust-stmts, prefer-text-field + create table c ( + -- squawk-ignore prefer-bigint-over-int + b int + ); + "); } #[test] From 733dc85332a309c4554703596489bc4b95ccbd3b Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 22 Aug 2025 20:18:24 -0400 Subject: [PATCH 3/3] fix --- crates/squawk_server/src/ignore.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/squawk_server/src/ignore.rs b/crates/squawk_server/src/ignore.rs index 9ba8079c..c05db397 100644 --- a/crates/squawk_server/src/ignore.rs +++ b/crates/squawk_server/src/ignore.rs @@ -142,8 +142,11 @@ create table c ( }) .collect::>(); insta::assert_snapshot!(apply_text_edits(sql, ignore_line_edits), @r" + -- squawk-ignore-file prefer-bigint-over-int -- squawk-ignore-file prefer-robust-stmts + -- squawk-ignore-file prefer-bigint-over-int -- squawk-ignore-file prefer-robust-stmts + -- squawk-ignore-file prefer-bigint-over-int -- squawk-ignore-file prefer-robust-stmts -- some existing comment