From 7ac53e6d459aaa43e155f3179b7a3384346ec4b1 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Sat, 23 Aug 2025 19:12:16 -0400 Subject: [PATCH] linter: cleanup auto fix code to be more concise --- .../src/rules/prefer_bigint_over_int.rs | 17 ++--- .../src/rules/prefer_bigint_over_smallint.rs | 17 ++--- .../src/rules/prefer_identity.rs | 11 ++-- .../src/rules/prefer_robust_stmts.rs | 66 +++++++------------ .../src/rules/prefer_text_field.rs | 17 +++-- .../require_concurrent_index_creation.rs | 11 ++-- .../require_concurrent_index_deletion.rs | 11 ++-- crates/squawk_wasm/src/lib.rs | 5 +- 8 files changed, 59 insertions(+), 96 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 bfd45adc..feb11ade 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs @@ -32,16 +32,13 @@ fn int_to_bigint_replacement(int_type: &str) -> &'static str { } 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 - } + let 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], + )) } fn check_ty_for_big_int(ctx: &mut Linter, ty: Option) { 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 20f7e026..0ddd7ea2 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs @@ -31,16 +31,13 @@ fn smallint_to_bigint(smallint_type: &str) -> &'static str { } 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 - } + let 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], + )) } fn check_ty_for_small_int(ctx: &mut Linter, ty: Option) { diff --git a/crates/squawk_linter/src/rules/prefer_identity.rs b/crates/squawk_linter/src/rules/prefer_identity.rs index 8822bd36..7c33ac0e 100644 --- a/crates/squawk_linter/src/rules/prefer_identity.rs +++ b/crates/squawk_linter/src/rules/prefer_identity.rs @@ -32,13 +32,10 @@ fn replace_serial(serial_type: &str) -> &'static str { } 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 - } + let 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])) } fn check_ty_for_serial(ctx: &mut Linter, ty: Option) { diff --git a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs index 9ca3ec29..7b05e5c3 100644 --- a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs +++ b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs @@ -58,15 +58,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { continue; } - let fix = if let Some(constraint_token) = - drop_constraint.constraint_token() - { + let fix = drop_constraint.constraint_token().map(|constraint_token| { let at = constraint_token.text_range().end(); let edit = Edit::insert(" if exists", at); - Some(Fix::new("Insert `if exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if exists`", vec![edit]) + }); (ActionErrorMessage::IfExists, fix) } @@ -75,13 +71,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { continue; } - let fix = if let Some(column_token) = add_column.column_token() { + let fix = add_column.column_token().map(|column_token| { let at = column_token.text_range().end(); let edit = Edit::insert(" if not exists", at); - Some(Fix::new("Insert `if not exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if not exists`", vec![edit]) + }); (ActionErrorMessage::IfNotExists, fix) } ast::AlterTableAction::ValidateConstraint(validate_constraint) => { @@ -113,13 +107,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { continue; } - let fix = if let Some(column_token) = drop_column.column_token() { + let fix = drop_column.column_token().map(|column_token| { let at = column_token.text_range().end(); let edit = Edit::insert(" if exists", at); - Some(Fix::new("Insert `if exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if exists`", vec![edit]) + }); (ActionErrorMessage::IfExists, fix) } _ => (ActionErrorMessage::None, None), @@ -152,13 +144,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { && create_index.name().is_some() && (create_index.concurrently_token().is_some() || !inside_transaction) => { - let fix = if let Some(name) = create_index.name() { + let fix = create_index.name().map(|name| { let at = name.syntax().text_range().start(); let edit = Edit::insert("if not exists ", at); - Some(Fix::new("Insert `if not exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if not exists`", vec![edit]) + }); ctx.report(Violation::for_node( Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), @@ -168,13 +158,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { ast::Stmt::CreateTable(create_table) if create_table.if_not_exists().is_none() && !inside_transaction => { - let fix = if let Some(table_token) = create_table.table_token() { + let fix = create_table.table_token().map(|table_token| { let at = table_token.text_range().end(); let edit = Edit::insert(" if not exists", at); - Some(Fix::new("Insert `if not exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if not exists`", vec![edit]) + }); ctx.report(Violation::for_node( Rule::PreferRobustStmts, @@ -185,13 +173,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { ast::Stmt::DropIndex(drop_index) if drop_index.if_exists().is_none() && !inside_transaction => { - let fix = if let Some(first_index) = drop_index.paths().next() { + let fix = drop_index.paths().next().map(|first_index| { let at = first_index.syntax().text_range().start(); let edit = Edit::insert("if exists ", at); - Some(Fix::new("Insert `if exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if exists`", vec![edit]) + }); ctx.report(Violation::for_node( Rule::PreferRobustStmts, @@ -202,13 +188,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { ast::Stmt::DropTable(drop_table) if drop_table.if_exists().is_none() && !inside_transaction => { - let fix = if let Some(table_token) = drop_table.table_token() { + let fix = drop_table.table_token().map(|table_token| { let at = table_token.text_range().end(); let edit = Edit::insert(" if exists", at); - Some(Fix::new("Insert `if exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if exists`", vec![edit]) + }); ctx.report(Violation::for_node( Rule::PreferRobustStmts, "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.".into(), @@ -218,13 +202,11 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { ast::Stmt::DropType(drop_type) if drop_type.if_exists().is_none() && !inside_transaction => { - let fix = if let Some(type_token) = drop_type.type_token() { + let fix = drop_type.type_token().map(|type_token| { let at = type_token.text_range().end(); let edit = Edit::insert(" if exists", at); - Some(Fix::new("Insert `if exists`", vec![edit])) - } else { - None - }; + Fix::new("Insert `if exists`", vec![edit]) + }); ctx.report(Violation::for_node( Rule::PreferRobustStmts, diff --git a/crates/squawk_linter/src/rules/prefer_text_field.rs b/crates/squawk_linter/src/rules/prefer_text_field.rs index 846b7945..32cd0d58 100644 --- a/crates/squawk_linter/src/rules/prefer_text_field.rs +++ b/crates/squawk_linter/src/rules/prefer_text_field.rs @@ -50,26 +50,25 @@ fn is_not_allowed_varchar(ty: &ast::Type) -> bool { } fn create_varchar_to_text_fix(ty: &ast::Type) -> Option { - match ty { + let range = 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])) + path_type.syntax().text_range() } 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])) + char_type.syntax().text_range() } 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])) + ty.syntax().text_range() } - _ => None, - } + _ => return None, + }; + let edit = Edit::replace(range, "text"); + Some(Fix::new("Replace with `text`", vec![edit])) } fn check_ty_for_varchar(ctx: &mut Linter, ty: Option) { 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 d27e61a5..1f7de9e7 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs @@ -8,13 +8,10 @@ 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 - } + let 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])) } pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse) { 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 07613c9f..257f8a56 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs @@ -6,13 +6,10 @@ use squawk_syntax::{ 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 - } + let 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])) } pub(crate) fn require_concurrent_index_deletion(ctx: &mut Linter, parse: &Parse) { diff --git a/crates/squawk_wasm/src/lib.rs b/crates/squawk_wasm/src/lib.rs index 15e4986b..92046a97 100644 --- a/crates/squawk_wasm/src/lib.rs +++ b/crates/squawk_wasm/src/lib.rs @@ -128,10 +128,7 @@ pub fn lint(text: String) -> Result { .to_wide(line_index::WideEncoding::Utf16, end) .unwrap(); - let messages = match x.help { - Some(help) => vec![help], - None => vec![], - }; + let messages = x.help.into_iter().collect(); let fix = x.fix.map(|fix| { let edits = fix