From 6e3ca98610c153acccc972d2caaec06883abd843 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 5 May 2025 22:59:47 -0400 Subject: [PATCH 1/5] v2: refactor linter --- crates/squawk_linter/Cargo.toml | 1 + crates/squawk_linter/src/ignore.rs | 32 +- crates/squawk_linter/src/ignore_index.rs | 10 +- crates/squawk_linter/src/lib.rs | 472 +++++------------- .../src/rules/adding_field_with_default.rs | 21 +- .../rules/adding_foreign_key_constraint.rs | 22 +- .../src/rules/adding_not_null_field.rs | 17 +- .../rules/adding_primary_key_constraint.rs | 22 +- .../src/rules/adding_required_field.rs | 6 +- .../ban_alter_domain_with_add_constraint.rs | 4 +- .../squawk_linter/src/rules/ban_char_field.rs | 6 +- ...oncurrent_index_creation_in_transaction.rs | 8 +- .../ban_create_domain_with_constraint.rs | 4 +- .../src/rules/ban_drop_column.rs | 4 +- .../src/rules/ban_drop_database.rs | 4 +- .../src/rules/ban_drop_not_null.rs | 6 +- .../squawk_linter/src/rules/ban_drop_table.rs | 4 +- .../src/rules/changing_column_type.rs | 4 +- .../src/rules/constraint_missing_not_valid.rs | 12 +- .../src/rules/disallow_unique_constraint.rs | 32 +- .../squawk_linter/src/rules/prefer_big_int.rs | 10 +- .../src/rules/prefer_bigint_over_int.rs | 10 +- .../src/rules/prefer_bigint_over_smallint.rs | 10 +- .../src/rules/prefer_identity.rs | 12 +- .../src/rules/prefer_robust_stmts.rs | 16 +- .../src/rules/prefer_text_field.rs | 6 +- .../src/rules/prefer_timestamptz.rs | 14 +- .../src/rules/renaming_column.rs | 4 +- .../squawk_linter/src/rules/renaming_table.rs | 4 +- .../require_concurrent_index_creation.rs | 8 +- .../require_concurrent_index_deletion.rs | 12 +- ...ith_default__test__arbitrary_func_err.snap | 4 +- ...t__test__default_random_with_args_err.snap | 4 +- ...ith_default__test__default_uuid_error.snap | 4 +- ...__test__default_uuid_error_multi_stmt.snap | 4 +- ...ault__test__default_volatile_func_err.snap | 4 +- ...h_default__test__generated_stored_err.snap | 4 +- ...ng_not_null_field__test__set_not_null.snap | 8 +- ...y_constraint__test__plain_primary_key.snap | 4 +- ..._constraint__test__serial_primary_key.snap | 4 +- ...field__test__not_null_without_default.snap | 4 +- ...domain_with_add_constraint__test__err.snap | 2 +- ...__ban_char_field__test__all_the_types.snap | 12 +- ...ban_char_field__test__alter_table_err.snap | 2 +- ...char_field__test__array_char_type_err.snap | 2 +- ...test__creating_table_with_char_errors.snap | 8 +- ...on__test__assuming_in_transaction_err.snap | 6 +- ...ent_index_creation_in_transaction_err.snap | 6 +- ...ate_domain_with_constraint__test__err.snap | 2 +- ...__test__err_with_multiple_constraints.snap | 2 +- ...er__rules__ban_drop_column__test__err.snap | 2 +- ...rop_database__test__ban_drop_database.snap | 6 +- ...__rules__ban_drop_not_null__test__err.snap | 4 +- ...ter__rules__ban_drop_table__test__err.snap | 6 +- ...anging_column_type__test__another_err.snap | 4 +- ...ules__changing_column_type__test__err.snap | 2 +- ...id__test__adding_check_constraint_err.snap | 2 +- ...issing_not_valid__test__adding_fk_err.snap | 2 +- ...valid_validate_assume_transaction_err.snap | 8 +- ...t__not_valid_validate_transaction_err.snap | 8 +- ..._transaction_with_explicit_commit_err.snap | 8 +- ...t__test__adding_unique_constraint_err.snap | 4 +- ...ique_constraint_inline_add_column_err.snap | 4 +- ...nstraint_inline_add_column_unique_err.snap | 4 +- ...int__test__alter_table_add_column_err.snap | 4 +- ...st__alter_table_alter_column_type_err.snap | 4 +- ...ble_alter_column_type_with_quotes_err.snap | 4 +- ..._big_int__test__create_table_many_err.snap | 8 +- ...ter__rules__prefer_big_int__test__err.snap | 32 +- ...es__prefer_bigint_over_int__test__err.snap | 16 +- ...refer_bigint_over_smallint__test__err.snap | 16 +- ...er__rules__prefer_identity__test__err.snap | 36 +- ...ts__test__alter_table_drop_column_err.snap | 2 +- ...test__alter_table_drop_constraint_err.snap | 2 +- ...r_robust_stmts__test__alter_table_err.snap | 2 +- ...__test__create_index_concurrently_err.snap | 4 +- ...ate_index_concurrently_muli_stmts_err.snap | 8 +- ...create_index_concurrently_unnamed_err.snap | 4 +- ..._robust_stmts__test__create_table_err.snap | 2 +- ..._test__disable_row_level_security_err.snap | 2 +- ...tmts__test__double_add_after_drop_err.snap | 2 +- ...er_robust_stmts__test__drop_index_err.snap | 2 +- ...__test__enable_row_level_security_err.snap | 2 +- ...vel_security_without_exists_check_err.snap | 2 +- ...eld__test__adding_column_non_text_err.snap | 4 +- ...eate_table_with_pgcatalog_varchar_err.snap | 4 +- ...__test__create_table_with_varchar_err.snap | 4 +- ...ield__test__increase_varchar_size_err.snap | 4 +- ..._test__alter_table_with_timestamp_err.snap | 8 +- ...test__create_table_with_timestamp_err.snap | 8 +- ...er__rules__renaming_column__test__err.snap | 2 +- ...ter__rules__renaming_table__test__err.snap | 2 +- ...st__adding_index_non_concurrently_err.snap | 2 +- ...__drop_index_missing_concurrently_err.snap | 6 +- ...nter__version__test_pg_version__parse.snap | 12 + crates/squawk_linter/src/version.rs | 146 ++++++ 96 files changed, 660 insertions(+), 643 deletions(-) create mode 100644 crates/squawk_linter/src/snapshots/squawk_linter__version__test_pg_version__parse.snap create mode 100644 crates/squawk_linter/src/version.rs diff --git a/crates/squawk_linter/Cargo.toml b/crates/squawk_linter/Cargo.toml index 600aaa3f..6121ab2b 100644 --- a/crates/squawk_linter/Cargo.toml +++ b/crates/squawk_linter/Cargo.toml @@ -15,6 +15,7 @@ lazy_static.workspace = true insta.workspace = true enum-iterator.workspace = true line-index.workspace = true +serde_plain.workspace = true [lints] diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 0fb50618..1f25b1ac 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -3,12 +3,12 @@ use std::collections::HashSet; use rowan::{NodeOrToken, TextRange, TextSize}; use squawk_syntax::{SyntaxKind, SyntaxNode, SyntaxToken}; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Linter, Rule, Violation}; #[derive(Debug)] pub struct Ignore { pub range: TextRange, - pub violation_names: HashSet, + pub violation_names: HashSet, } fn comment_body(token: &SyntaxToken) -> Option<(&str, TextRange)> { @@ -71,7 +71,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { if x.is_empty() { continue; } - if let Ok(violation_name) = ErrorCode::try_from(x.trim()) { + if let Ok(violation_name) = Rule::try_from(x.trim()) { set.insert(violation_name); } else { let without_start = x.trim_start(); @@ -85,10 +85,10 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { let range = TextRange::new(start, end); ctx.report(Violation::new( - ErrorCode::UnusedIgnore, + Rule::UnusedIgnore, format!("unknown name {trimmed}"), range, - vec![], + None, )); } @@ -108,7 +108,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { #[cfg(test)] mod test { - use crate::{find_ignores, ErrorCode, Linter, Violation}; + use crate::{find_ignores, Linter, Rule, Violation}; #[test] fn single_ignore() { @@ -123,7 +123,7 @@ alter table t drop column c cascade; assert_eq!(linter.ignores.len(), 1); let ignore = &linter.ignores[0]; - assert!(ignore.violation_names.contains(&ErrorCode::BanDropColumn)); + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); } #[test] @@ -140,7 +140,7 @@ alter table t drop column c cascade; assert_eq!(linter.ignores.len(), 1); let ignore = &linter.ignores[0]; - assert!(ignore.violation_names.contains(&ErrorCode::BanDropColumn)); + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); } #[test] @@ -157,9 +157,9 @@ alter table t drop column c cascade; assert_eq!(linter.ignores.len(), 1); let ignore = &linter.ignores[0]; - assert!(ignore.violation_names.contains(&ErrorCode::BanDropColumn)); - assert!(ignore.violation_names.contains(&ErrorCode::RenamingColumn)); - assert!(ignore.violation_names.contains(&ErrorCode::BanDropDatabase)); + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(ignore.violation_names.contains(&Rule::RenamingColumn)); + assert!(ignore.violation_names.contains(&Rule::BanDropDatabase)); } #[test] @@ -176,9 +176,9 @@ alter table t drop column c cascade; assert_eq!(linter.ignores.len(), 1); let ignore = &linter.ignores[0]; - assert!(ignore.violation_names.contains(&ErrorCode::BanDropColumn)); - assert!(ignore.violation_names.contains(&ErrorCode::RenamingColumn)); - assert!(ignore.violation_names.contains(&ErrorCode::BanDropDatabase)); + assert!(ignore.violation_names.contains(&Rule::BanDropColumn)); + assert!(ignore.violation_names.contains(&Rule::RenamingColumn)); + assert!(ignore.violation_names.contains(&Rule::BanDropDatabase)); } #[test] @@ -199,7 +199,7 @@ create table users ( "#; let parse = squawk_syntax::SourceFile::parse(sql); - let errors: Vec<&Violation> = linter.lint(parse, sql); + let errors = linter.lint(parse, sql); assert_eq!(errors.len(), 0); } @@ -209,7 +209,7 @@ create table users ( let sql = r#"alter table t add column c char;"#; let parse = squawk_syntax::SourceFile::parse(sql); - let errors: Vec<&Violation> = linter.lint(parse, sql); + let errors = linter.lint(parse, sql); assert_eq!(errors.len(), 1); } } diff --git a/crates/squawk_linter/src/ignore_index.rs b/crates/squawk_linter/src/ignore_index.rs index b4b0c447..309b934a 100644 --- a/crates/squawk_linter/src/ignore_index.rs +++ b/crates/squawk_linter/src/ignore_index.rs @@ -6,10 +6,10 @@ use std::{ use line_index::LineIndex; use rowan::TextRange; -use crate::{ErrorCode, Ignore}; +use crate::{Ignore, Rule}; pub(crate) struct IgnoreIndex { - line_to_ignored_names: HashMap>, + line_to_ignored_names: HashMap>, line_index: LineIndex, } @@ -30,7 +30,7 @@ impl fmt::Debug for IgnoreIndex { impl IgnoreIndex { pub(crate) fn new(text: &str, ignores: &[Ignore]) -> Self { let line_index = LineIndex::new(text); - let mut line_to_ignored_names: HashMap> = HashMap::new(); + let mut line_to_ignored_names: HashMap> = HashMap::new(); for ignore in ignores { let line = line_index.line_col(ignore.range.start()).line; line_to_ignored_names.insert(line, ignore.violation_names.clone()); @@ -42,12 +42,12 @@ impl IgnoreIndex { } } - pub(crate) fn contains(&self, range: TextRange, error_name: ErrorCode) -> bool { + pub(crate) fn contains(&self, range: TextRange, item: Rule) -> bool { // TODO: hmmm basically we want to ensure that either it's on the line before or it's inside the start of the node. we parse stuff so that the comment ends up inside the node :/ let line = self.line_index.line_col(range.start()).line; for line in [line, if line == 0 { 0 } else { line - 1 }] { if let Some(set) = self.line_to_ignored_names.get(&line) { - if set.contains(&error_name) { + if set.contains(&item) { return true; } } diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index 7209a899..7be9e356 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -12,8 +12,11 @@ use serde::{Deserialize, Serialize}; use squawk_syntax::{Parse, SourceFile}; +pub use version::Version; + mod ignore; mod ignore_index; +mod version; mod rules; mod text; @@ -27,6 +30,7 @@ use rules::ban_char_field; use rules::ban_concurrent_index_creation_in_transaction; use rules::ban_create_domain_with_constraint; use rules::ban_drop_column; +use rules::ban_drop_database; use rules::ban_drop_not_null; use rules::ban_drop_table; use rules::changing_column_type; @@ -45,10 +49,8 @@ use rules::require_concurrent_index_creation; use rules::require_concurrent_index_deletion; // xtask:new-lint:rule-import -use rules::ban_drop_database; - -#[derive(Debug, PartialEq, Clone, Copy, Serialize, Hash, Eq, Deserialize)] -pub enum ErrorCode { +#[derive(Debug, PartialEq, Clone, Copy, Serialize, Hash, Eq, Deserialize, Sequence)] +pub enum Rule { #[serde(rename = "require-concurrent-index-creation")] RequireConcurrentIndexCreation, #[serde(rename = "require-concurrent-index-deletion")] @@ -110,380 +112,130 @@ pub enum ErrorCode { // xtask:new-lint:error-name } -impl TryFrom<&str> for ErrorCode { +impl TryFrom<&str> for Rule { type Error = String; fn try_from(s: &str) -> Result { match s { - "require-concurrent-index-creation" => Ok(ErrorCode::RequireConcurrentIndexCreation), - "require-concurrent-index-deletion" => Ok(ErrorCode::RequireConcurrentIndexDeletion), - "constraint-missing-not-valid" => Ok(ErrorCode::ConstraintMissingNotValid), - "adding-field-with-default" => Ok(ErrorCode::AddingFieldWithDefault), - "adding-foreign-key-constraint" => Ok(ErrorCode::AddingForeignKeyConstraint), - "changing-column-type" => Ok(ErrorCode::ChangingColumnType), - "adding-not-nullable-field" => Ok(ErrorCode::AddingNotNullableField), - "adding-serial-primary-key-field" => Ok(ErrorCode::AddingSerialPrimaryKeyField), - "renaming-column" => Ok(ErrorCode::RenamingColumn), - "renaming-table" => Ok(ErrorCode::RenamingTable), - "disallowed-unique-constraint" => Ok(ErrorCode::DisallowedUniqueConstraint), - "ban-drop-database" => Ok(ErrorCode::BanDropDatabase), - "prefer-big-int" => Ok(ErrorCode::PreferBigInt), - "prefer-bigint-over-int" => Ok(ErrorCode::PreferBigintOverInt), - "prefer-bigint-over-smallint" => Ok(ErrorCode::PreferBigintOverSmallint), - "prefer-identity" => Ok(ErrorCode::PreferIdentity), - "prefer-robust-stmts" => Ok(ErrorCode::PreferRobustStmts), - "prefer-text-field" => Ok(ErrorCode::PreferTextField), - "prefer-timestamptz" => Ok(ErrorCode::PreferTimestampTz), - "ban-char-field" => Ok(ErrorCode::BanCharField), - "ban-drop-column" => Ok(ErrorCode::BanDropColumn), - "ban-drop-table" => Ok(ErrorCode::BanDropTable), - "ban-drop-not-null" => Ok(ErrorCode::BanDropNotNull), - "transaction-nesting" => Ok(ErrorCode::TransactionNesting), - "adding-required-field" => Ok(ErrorCode::AddingRequiredField), + "require-concurrent-index-creation" => Ok(Rule::RequireConcurrentIndexCreation), + "require-concurrent-index-deletion" => Ok(Rule::RequireConcurrentIndexDeletion), + "constraint-missing-not-valid" => Ok(Rule::ConstraintMissingNotValid), + "adding-field-with-default" => Ok(Rule::AddingFieldWithDefault), + "adding-foreign-key-constraint" => Ok(Rule::AddingForeignKeyConstraint), + "changing-column-type" => Ok(Rule::ChangingColumnType), + "adding-not-nullable-field" => Ok(Rule::AddingNotNullableField), + "adding-serial-primary-key-field" => Ok(Rule::AddingSerialPrimaryKeyField), + "renaming-column" => Ok(Rule::RenamingColumn), + "renaming-table" => Ok(Rule::RenamingTable), + "disallowed-unique-constraint" => Ok(Rule::DisallowedUniqueConstraint), + "ban-drop-database" => Ok(Rule::BanDropDatabase), + "prefer-big-int" => Ok(Rule::PreferBigInt), + "prefer-bigint-over-int" => Ok(Rule::PreferBigintOverInt), + "prefer-bigint-over-smallint" => Ok(Rule::PreferBigintOverSmallint), + "prefer-identity" => Ok(Rule::PreferIdentity), + "prefer-robust-stmts" => Ok(Rule::PreferRobustStmts), + "prefer-text-field" => Ok(Rule::PreferTextField), + "prefer-timestamptz" => Ok(Rule::PreferTimestampTz), + "ban-char-field" => Ok(Rule::BanCharField), + "ban-drop-column" => Ok(Rule::BanDropColumn), + "ban-drop-table" => Ok(Rule::BanDropTable), + "ban-drop-not-null" => Ok(Rule::BanDropNotNull), + "transaction-nesting" => Ok(Rule::TransactionNesting), + "adding-required-field" => Ok(Rule::AddingRequiredField), "ban-concurrent-index-creation-in-transaction" => { - Ok(ErrorCode::BanConcurrentIndexCreationInTransaction) - } - "ban-create-domain-with-constraint" => Ok(ErrorCode::BanCreateDomainWithConstraint), - "ban-alter-domain-with-add-constraint" => { - Ok(ErrorCode::BanAlterDomainWithAddConstraint) + Ok(Rule::BanConcurrentIndexCreationInTransaction) } + "ban-create-domain-with-constraint" => Ok(Rule::BanCreateDomainWithConstraint), + "ban-alter-domain-with-add-constraint" => Ok(Rule::BanAlterDomainWithAddConstraint), // xtask:new-lint:str-name _ => Err(format!("Unknown violation name: {}", s)), } } } -impl fmt::Display for ErrorCode { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let val = match &self { - ErrorCode::RequireConcurrentIndexCreation => "require-concurrent-index-creation", - ErrorCode::RequireConcurrentIndexDeletion => "require-concurrent-index-deletion", - ErrorCode::ConstraintMissingNotValid => "constraint-missing-not-valid", - ErrorCode::AddingFieldWithDefault => "adding-field-with-default", - ErrorCode::AddingForeignKeyConstraint => "adding-foreign-key-constraint", - ErrorCode::ChangingColumnType => "changing-column-type", - ErrorCode::AddingNotNullableField => "adding-not-nullable-field", - ErrorCode::AddingSerialPrimaryKeyField => "adding-serial-primary-key-field", - ErrorCode::RenamingColumn => "renaming-column", - ErrorCode::RenamingTable => "renaming-table", - ErrorCode::DisallowedUniqueConstraint => "disallowed-unique-constraint", - ErrorCode::BanDropDatabase => "ban-drop-database", - ErrorCode::PreferBigInt => "prefer-big-int", - ErrorCode::PreferBigintOverInt => "prefer-bigint-over-int", - ErrorCode::PreferBigintOverSmallint => "prefer-bigint-over-smallint", - ErrorCode::PreferIdentity => "prefer-identity", - ErrorCode::PreferRobustStmts => "prefer-robust-stmts", - ErrorCode::PreferTextField => "prefer-text-field", - ErrorCode::PreferTimestampTz => "prefer-timestamp-tz", - ErrorCode::BanCharField => "ban-char-field", - ErrorCode::BanDropColumn => "ban-drop-column", - ErrorCode::BanDropTable => "ban-drop-table", - ErrorCode::BanDropNotNull => "ban-drop-not-null", - ErrorCode::TransactionNesting => "transaction-nesting", - ErrorCode::AddingRequiredField => "adding-required-field", - ErrorCode::BanConcurrentIndexCreationInTransaction => { - "ban-concurrent-index-creation-in-transaction" - } - ErrorCode::BanCreateDomainWithConstraint => "ban-create-domain-with-constraint", - ErrorCode::UnusedIgnore => "unused-ignore", - ErrorCode::BanAlterDomainWithAddConstraint => "ban-alter-domain-with-add-constraint", - }; - write!(f, "{}", val) - } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnknownRuleName { + val: String, } -impl ErrorCode { - pub fn meta(&self) -> ViolationMeta { - match self { - ErrorCode::RequireConcurrentIndexCreation => ViolationMeta::new( - "Require Concurrent Index Creation", - [ - ViolationMessage::Note("Creating an index blocks writes."), - ViolationMessage::Help("Create the index CONCURRENTLY."), - ] - ), - ErrorCode::RequireConcurrentIndexDeletion => ViolationMeta::new( - "Require Concurrent Index Deletion", - [ - ViolationMessage::Note("Deleting an index blocks selects, inserts, updates, and deletes on the index's table."), - ViolationMessage::Help("Delete the index CONCURRENTLY."), - ] - ), - ErrorCode::ConstraintMissingNotValid => ViolationMeta::new( - "Constraint Missing Not Valid", - [ - ViolationMessage::Note("Requires a table scan to verify constraint and an ACCESS EXCLUSIVE lock which blocks reads."), - ViolationMessage::Help("Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction."), - ] - ), - ErrorCode::AddingFieldWithDefault => ViolationMeta::new( - "Adding Field With Default", - [ - ViolationMessage::Note("Adding a field with a VOLATILE DEFAULT requires a table rewrite with an ACCESS EXCLUSIVE lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite."), - ViolationMessage::Help("Add the field as nullable, then set a default, backfill, and remove nullabilty."), - ] - ), - ErrorCode::AddingForeignKeyConstraint => ViolationMeta::new( - "Adding Foreign Key Constraint", - [ - ViolationMessage::Note("Requires a table scan of the table you're altering and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to both tables while your table is scanned."), - ViolationMessage::Help("Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction."), - ] - ), - ErrorCode::ChangingColumnType => ViolationMeta::new( - "Changing Column Type", - [ - ViolationMessage::Note("Requires an ACCESS EXCLUSIVE lock on the table which blocks reads."), - ViolationMessage::Note("Changing the type may break existing clients."), - ] - ), - ErrorCode::AddingNotNullableField => ViolationMeta::new( - "Adding Not Nullable Field", - [ - ViolationMessage::Note("Adding a NOT NULL field requires exclusive locks and table rewrites."), - ViolationMessage::Help("Make the field nullable."), - ] - ), - ErrorCode::AddingSerialPrimaryKeyField => ViolationMeta::new( - "Adding Serial Primary Key Field", - [ - ViolationMessage::Note("Adding a PRIMARY KEY constraint results in locks and table rewrites"), - ViolationMessage::Help("Add the PRIMARY KEY constraint USING an index."), - ] - ), - ErrorCode::RenamingColumn => ViolationMeta::new( - "Renaming Column", - [ViolationMessage::Note("Renaming a column may break existing clients.")] - ), - ErrorCode::RenamingTable => ViolationMeta::new( - "Renaming Table", - [ViolationMessage::Note("Renaming a table may break existing clients.")] - ), - ErrorCode::DisallowedUniqueConstraint => ViolationMeta::new( - "Disallowed Unique Constraint", - [ - ViolationMessage::Note("Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads."), - ViolationMessage::Help("Create an index CONCURRENTLY and create the constraint using the index."), - ] - ), - ErrorCode::BanDropDatabase => ViolationMeta::new( - "Ban Drop Database", - [ViolationMessage::Note("Dropping a database may break existing clients.")] - ), - ErrorCode::PreferBigInt => ViolationMeta::new( - "Prefer Big Int", - [ - ViolationMessage::Note("Hitting the max 32 bit integer is possible and may break your application."), - ViolationMessage::Help("Use 64bit integer values instead to prevent hitting this limit."), - ] - ), - ErrorCode::PreferBigintOverSmallint => ViolationMeta::new( - "Prefer Bigint Over Smallint", - [ - ViolationMessage::Note("Hitting the max 16 bit integer is possible and may break your application."), - ViolationMessage::Help("Use 64bit integer values instead to prevent hitting this limit."), - ] - ), - ErrorCode::PreferIdentity => ViolationMeta::new( - "Prefer Identity", - [ - ViolationMessage::Note("Serial types have confusing behaviors that make schema management difficult."), - ViolationMessage::Help("Use identity columns instead for more features and better usability."), - ] - ), - ErrorCode::PreferRobustStmts => ViolationMeta::new( - "Prefer Robust Statements", - [ViolationMessage::Help("Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.")] - ), - ErrorCode::PreferTextField => ViolationMeta::new( - "Prefer Text Field", - [ - ViolationMessage::Note("Changing the size of a varchar field requires an ACCESS EXCLUSIVE lock."), - ViolationMessage::Help("Use a text field with a check constraint."), - ] - ), - ErrorCode::PreferTimestampTz => ViolationMeta::new( - "Prefer Timestamp with Timezone", - [ - ViolationMessage::Note("A timestamp field without a timezone can lead to data loss, depending on your database session timezone."), - ViolationMessage::Help("Use timestamptz instead of timestamp for your column type."), - ] - ), - ErrorCode::BanCharField => ViolationMeta::new( - "Ban Char Field", - [ViolationMessage::Help("Use text or varchar instead.")] - ), - ErrorCode::BanDropColumn => ViolationMeta::new( - "Dropping columns not allowed", - [ViolationMessage::Note("Dropping a column may break existing clients.")] - ), - ErrorCode::BanDropTable => ViolationMeta::new( - "Ban Drop Table", - [ViolationMessage::Note("Dropping a table may break existing clients.")] - ), - ErrorCode::BanDropNotNull => ViolationMeta::new( - "Ban Drop Not Null", - [ViolationMessage::Note("Dropping a NOT NULL constraint may break existing clients.")] - ), - ErrorCode::TransactionNesting => ViolationMeta::new( - "Transaction Nesting", - [ - ViolationMessage::Note("There is an existing transaction already in progress."), - ViolationMessage::Help("COMMIT the previous transaction before issuing a BEGIN or START TRANSACTION statement."), - ] - ), - ErrorCode::AddingRequiredField => ViolationMeta::new( - "Adding Required Field", - [ - ViolationMessage::Note("Adding a NOT NULL field without a DEFAULT will fail for a populated table."), - ViolationMessage::Help("Make the field nullable or add a non-VOLATILE DEFAULT (Postgres 11+)."), - ] - ), - ErrorCode::BanConcurrentIndexCreationInTransaction => ViolationMeta::new( - "Ban Concurrent Index Creation in Transaction", - [ - ViolationMessage::Note("Concurrent index creation is not allowed inside a transaction."), - ViolationMessage::Help("Build the index outside any transactions."), - ] - ), - ErrorCode::PreferBigintOverInt => ViolationMeta::new( - "Prefer Big Int Over Int", - [ - ViolationMessage::Note( - "Hitting the max 32 bit integer is possible and may break your application." - ), - ViolationMessage::Help( - "Use 64bit integer values instead to prevent hitting this limit." - ), - ] - ), - ErrorCode::BanCreateDomainWithConstraint => ViolationMeta::new( - "Ban Create Domains with Constraints", - [ - ViolationMessage::Note( - "Domains with constraints have poor support for online migrations", - ), - ] - ), - ErrorCode::BanAlterDomainWithAddConstraint => ViolationMeta::new( - "Ban Alter Domain With Add Constraints", - [ - ViolationMessage::Note( - "Domains with constraints have poor support for online migrations", - ) - ] - ), - ErrorCode::UnusedIgnore => ViolationMeta::new("Unused linter ignore", []) - } +impl std::fmt::Display for UnknownRuleName { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "invalid rule name {}", self.val) } } -#[derive(Debug)] -pub enum ViolationMessage<'a> { - Note(&'a str), - Help(&'a str), -} - -#[derive(Debug)] -pub struct ViolationMeta<'a> { - /// A description of the rule that's used when rendering the error message - /// in on the CLI. It should be a slightly expanded version of the [`ViolationName`] - pub title: String, - /// Messages rendered for each error to provide context and offer advice on how to fix. - pub messages: Vec>, +impl std::str::FromStr for Rule { + type Err = UnknownRuleName; + fn from_str(s: &str) -> Result { + serde_plain::from_str(s).map_err(|_| UnknownRuleName { val: s.to_string() }) + } } -impl<'a> ViolationMeta<'a> { - pub fn new( - title: impl Into, - messages: impl Into>>, - ) -> ViolationMeta<'a> { - ViolationMeta { - title: title.into(), - messages: messages.into(), - } +impl fmt::Display for Rule { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let val = match &self { + Rule::RequireConcurrentIndexCreation => "require-concurrent-index-creation", + Rule::RequireConcurrentIndexDeletion => "require-concurrent-index-deletion", + Rule::ConstraintMissingNotValid => "constraint-missing-not-valid", + Rule::AddingFieldWithDefault => "adding-field-with-default", + Rule::AddingForeignKeyConstraint => "adding-foreign-key-constraint", + Rule::ChangingColumnType => "changing-column-type", + Rule::AddingNotNullableField => "adding-not-nullable-field", + Rule::AddingSerialPrimaryKeyField => "adding-serial-primary-key-field", + Rule::RenamingColumn => "renaming-column", + Rule::RenamingTable => "renaming-table", + Rule::DisallowedUniqueConstraint => "disallowed-unique-constraint", + Rule::BanDropDatabase => "ban-drop-database", + Rule::PreferBigInt => "prefer-big-int", + Rule::PreferBigintOverInt => "prefer-bigint-over-int", + Rule::PreferBigintOverSmallint => "prefer-bigint-over-smallint", + Rule::PreferIdentity => "prefer-identity", + Rule::PreferRobustStmts => "prefer-robust-stmts", + Rule::PreferTextField => "prefer-text-field", + Rule::PreferTimestampTz => "prefer-timestamp-tz", + Rule::BanCharField => "ban-char-field", + Rule::BanDropColumn => "ban-drop-column", + Rule::BanDropTable => "ban-drop-table", + Rule::BanDropNotNull => "ban-drop-not-null", + Rule::TransactionNesting => "transaction-nesting", + Rule::AddingRequiredField => "adding-required-field", + Rule::BanConcurrentIndexCreationInTransaction => { + "ban-concurrent-index-creation-in-transaction" + } + Rule::BanCreateDomainWithConstraint => "ban-create-domain-with-constraint", + Rule::UnusedIgnore => "unused-ignore", + Rule::BanAlterDomainWithAddConstraint => "ban-alter-domain-with-add-constraint", + }; + write!(f, "{}", val) } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Violation { - pub code: ErrorCode, + // TODO: should this be String instead? + pub code: Rule, pub message: String, pub text_range: TextRange, - pub messages: Vec, + pub help: Option, } impl Violation { #[must_use] - pub(crate) fn new( - code: ErrorCode, + pub fn new( + code: Rule, message: String, text_range: TextRange, - messages: impl Into>>, + help: impl Into>, ) -> Self { Self { code, text_range, message, - messages: messages.into().unwrap_or_default(), - } - } -} - -#[derive(Debug, PartialEq, PartialOrd, Clone, Copy)] -pub struct Version { - major: i32, - minor: Option, - patch: Option, -} - -impl Version { - #[must_use] - pub(crate) fn new( - major: i32, - minor: impl Into>, - patch: impl Into>, - ) -> Self { - Self { - major, - minor: minor.into(), - patch: patch.into(), + help: help.into(), } } } -#[derive(PartialEq, Eq, Hash, Copy, Clone, Sequence)] -pub enum Rule { - AddingFieldWithDefault, - AddingForeignKeyConstraint, - AddingNotNullField, - AddingPrimaryKeyConstraint, - AddingRequiredField, - BanDropDatabase, - BanCharField, - BanConcurrentIndexCreationInTransaction, - BanDropColumn, - BanDropNotNull, - BanDropTable, - ChangingColumnType, - ConstraintMissingNotValid, - DisallowUniqueConstraint, - PreferBigInt, - PreferBigintOverInt, - PreferBigintOverSmallint, - PreferIdentity, - PreferRobustStmts, - PreferTextField, - PreferTimestamptz, - RenamingColumn, - RenamingTable, - RequireConcurrentIndexCreation, - RequireConcurrentIndexDeletion, - BanCreateDomainWithConstraint, - BanAlterDomainWithAddConstraint, - // xtask:new-lint:name -} - pub struct LinterSettings { pub pg_version: Version, pub assume_in_transaction: bool, @@ -510,17 +262,17 @@ impl Linter { } #[must_use] - pub fn lint(&mut self, file: Parse, text: &str) -> Vec<&Violation> { + pub fn lint(&mut self, file: Parse, text: &str) -> Vec { if self.rules.contains(&Rule::AddingFieldWithDefault) { adding_field_with_default(self, &file); } if self.rules.contains(&Rule::AddingForeignKeyConstraint) { adding_foreign_key_constraint(self, &file); } - if self.rules.contains(&Rule::AddingNotNullField) { + if self.rules.contains(&Rule::AddingNotNullableField) { adding_not_null_field(self, &file); } - if self.rules.contains(&Rule::AddingPrimaryKeyConstraint) { + if self.rules.contains(&Rule::AddingSerialPrimaryKeyField) { adding_primary_key_constraint(self, &file); } if self.rules.contains(&Rule::AddingRequiredField) { @@ -553,7 +305,7 @@ impl Linter { if self.rules.contains(&Rule::ConstraintMissingNotValid) { constraint_missing_not_valid(self, &file); } - if self.rules.contains(&Rule::DisallowUniqueConstraint) { + if self.rules.contains(&Rule::DisallowedUniqueConstraint) { disallow_unique_constraint(self, &file); } if self.rules.contains(&Rule::PreferBigInt) { @@ -574,7 +326,7 @@ impl Linter { if self.rules.contains(&Rule::PreferTextField) { prefer_text_field(self, &file); } - if self.rules.contains(&Rule::PreferTimestamptz) { + if self.rules.contains(&Rule::PreferTimestampTz) { prefer_timestamptz(self, &file); } if self.rules.contains(&Rule::RenamingColumn) { @@ -603,18 +355,19 @@ impl Linter { self.errors(text) } - fn errors(&mut self, text: &str) -> Vec<&Violation> { - // ensure we order them by where they appear in the file - self.errors.sort_by_key(|x| x.text_range.start()); - + fn errors(&mut self, text: &str) -> Vec { let ignore_index = IgnoreIndex::new(text, &self.ignores); - // TODO: we should have errors for when there was an ignore but that - // ignore didn't actually ignore anything - - self.errors + let mut errors: Vec = self + .errors .iter() + // TODO: we should have errors for when there was an ignore but that + // ignore didn't actually ignore anything .filter(|err| !ignore_index.contains(err.text_range, err.code)) - .collect::>() + .cloned() + .collect::>(); + // ensure we order them by where they appear in the file + errors.sort_by_key(|x| x.text_range.start()); + errors } pub fn with_all_rules() -> Self { @@ -622,6 +375,21 @@ impl Linter { Linter::from(rules) } + pub fn without_rules(exclude: &[Rule]) -> Self { + let all_rules = all::().collect::>(); + let mut exclude_set = HashSet::with_capacity(exclude.len()); + for e in exclude { + exclude_set.insert(e); + } + + let rules = all_rules + .into_iter() + .filter(|x| !exclude_set.contains(x)) + .collect::>(); + + Linter::from(rules) + } + pub fn from(rules: impl Into>) -> Self { Self { errors: vec![], diff --git a/crates/squawk_linter/src/rules/adding_field_with_default.rs b/crates/squawk_linter/src/rules/adding_field_with_default.rs index ba8a6fa1..f9323281 100644 --- a/crates/squawk_linter/src/rules/adding_field_with_default.rs +++ b/crates/squawk_linter/src/rules/adding_field_with_default.rs @@ -5,7 +5,7 @@ use squawk_syntax::ast; use squawk_syntax::ast::{AstNode, HasArgList}; use squawk_syntax::{ast::HasModuleItem, Parse, SourceFile}; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; fn is_const_expr(expr: &ast::Expr) -> bool { match expr { @@ -52,6 +52,9 @@ fn is_non_volatile(expr: &ast::Expr) -> bool { const NON_VOLATILE_BUILT_IN_FUNCTIONS: &str = include_str!("non_volatile_built_in_functions.txt"); pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse) { + let message = + "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock."; + let help = "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead."; let file = parse.tree(); // TODO: use match_ast! like in #api_walkthrough for item in file.items() { @@ -68,22 +71,18 @@ pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse { ctx.report(Violation::new( - ErrorCode::AddingFieldWithDefault, - "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.".into(), + Rule::AddingFieldWithDefault, + message.into(), generated.syntax().text_range(), - vec![ - "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.".into(), - ], + help.to_string(), )); } _ => (), diff --git a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs index f801612e..d32ff1bb 100644 --- a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs +++ b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs @@ -3,9 +3,11 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse) { + let message = "Adding a foreign key constraint requires a table scan and a `SHARE ROW EXCLUSIVE` lock on both tables, which blocks writes to each table."; + let help = "Add `NOT VALID` to the constraint in one transaction and then VALIDATE the constraint in a separate transaction."; let file = parse.tree(); // TODO: use match_ast! like in #api_walkthrough for item in file.items() { @@ -24,10 +26,10 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse) { if ctx.settings.pg_version >= Version::new(11, 0, 0) { @@ -20,11 +20,12 @@ pub(crate) fn adding_not_null_field(ctx: &mut Linter, parse: &Parse) if matches!(option, ast::AlterColumnOption::SetNotNull(_)) { ctx.report(Violation::new( - ErrorCode::AddingNotNullableField, - "Setting a column NOT NULL blocks reads while the table is scanned." + Rule::AddingNotNullableField, + "Setting a column `NOT NULL` blocks reads while the table is scanned." .into(), option.syntax().text_range(), - vec!["Use a check constraint instead.".into()], + "Make the field nullable and use a `CHECK` constraint instead." + .to_string(), )); } } @@ -45,7 +46,7 @@ mod test { ALTER TABLE "core_recipe" ALTER COLUMN "foo" SET NOT NULL; "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::AddingNotNullField]); + let mut linter = Linter::from([Rule::AddingNotNullableField]); linter.settings.pg_version = Version::new(10, 0, 0); let errors = linter.lint(file, sql); assert!(!errors.is_empty()); @@ -63,7 +64,7 @@ ALTER TABLE "core_recipe" ALTER COLUMN "foo" DROP DEFAULT; COMMIT; "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::AddingNotNullField]); + let mut linter = Linter::from([Rule::AddingNotNullableField]); let errors = linter.lint(file, sql); assert!(errors.is_empty()); } @@ -75,7 +76,7 @@ COMMIT; ALTER TABLE "core_recipe" ADD COLUMN "foo" integer NOT NULL; "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::AddingNotNullField]); + let mut linter = Linter::from([Rule::AddingNotNullableField]); let errors = linter.lint(file, sql); assert!(errors.is_empty()); } @@ -91,7 +92,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer NOT NULL DEFAULT 10; COMMIT; "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::AddingNotNullField]); + let mut linter = Linter::from([Rule::AddingNotNullableField]); let errors = linter.lint(file, sql); assert!(errors.is_empty()); } diff --git a/crates/squawk_linter/src/rules/adding_primary_key_constraint.rs b/crates/squawk_linter/src/rules/adding_primary_key_constraint.rs index 58b9616b..7e3663fa 100644 --- a/crates/squawk_linter/src/rules/adding_primary_key_constraint.rs +++ b/crates/squawk_linter/src/rules/adding_primary_key_constraint.rs @@ -3,9 +3,11 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse) { + let message = "Adding a primary key constraint requires an `ACCESS EXCLUSIVE` lock that will block all reads and writes to the table while the primary key index is built."; + let help = "Add the `PRIMARY KEY` constraint `USING` an index."; let file = parse.tree(); for item in file.items() { if let ast::Item::AlterTable(alter_table) = item { @@ -17,10 +19,10 @@ pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -16,10 +16,10 @@ pub(crate) fn adding_required_field(ctx: &mut Linter, parse: &Parse) } if has_not_null_and_no_default_constraint(add_column.constraints()) { ctx.report(Violation::new( - ErrorCode::AddingRequiredField, + Rule::AddingRequiredField, "Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.".into(), add_column.syntax().text_range(), - None, + "Make the field nullable or add a non-VOLATILE DEFAULT".to_string(), )); } } diff --git a/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs b/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs index b21b2413..496fdd00 100644 --- a/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs +++ b/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_alter_domain_with_add_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -13,7 +13,7 @@ pub(crate) fn ban_alter_domain_with_add_constraint(ctx: &mut Linter, parse: &Par for action in actions { if let ast::AlterDomainAction::AddConstraint(add_constraint) = action { ctx.report(Violation::new( - ErrorCode::BanAlterDomainWithAddConstraint, + Rule::BanAlterDomainWithAddConstraint, "Domains with constraints have poor support for online migrations. Use table and column constraints instead.".into(), add_constraint.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/ban_char_field.rs b/crates/squawk_linter/src/rules/ban_char_field.rs index 4cfb2e13..1021cead 100644 --- a/crates/squawk_linter/src/rules/ban_char_field.rs +++ b/crates/squawk_linter/src/rules/ban_char_field.rs @@ -4,7 +4,7 @@ use squawk_syntax::{ }; use crate::prefer_big_int::check_not_allowed_types; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; fn is_char_type(x: TokenText<'_>) -> bool { if x == "char" || x == "character" || x == "bpchar" { @@ -21,7 +21,7 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) { { if is_char_type(name_ref.text()) { ctx.report(Violation::new( - ErrorCode::BanCharField, + Rule::BanCharField, "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(), path_type.syntax().text_range(), None, @@ -33,7 +33,7 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) { fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) { if is_char_type(char_type.text()) { ctx.report(Violation::new( - ErrorCode::BanCharField, + Rule::BanCharField, "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.".into(), char_type.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs index d7d42393..ac4c96c5 100644 --- a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs +++ b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_concurrent_index_creation_in_transaction( ctx: &mut Linter, @@ -26,10 +26,10 @@ pub(crate) fn ban_concurrent_index_creation_in_transaction( if in_transaction { if let Some(concurrently) = create_index.concurrently_token() { errors.push(Violation::new( - ErrorCode::BanConcurrentIndexCreationInTransaction, - "While regular index creation can happen inside a transaction, this is not allowed when the CONCURRENTLY option is used.".into(), + Rule::BanConcurrentIndexCreationInTransaction, + "While regular index creation can happen inside a transaction, this is not allowed when the `CONCURRENTLY` option is used.".into(), concurrently.text_range(), - None, + "Build the index outside any transactions.".to_string(), )); } } diff --git a/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs b/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs index 77ad1c3b..b424269a 100644 --- a/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs +++ b/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs @@ -4,7 +4,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_create_domain_with_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -24,7 +24,7 @@ pub(crate) fn ban_create_domain_with_constraint(ctx: &mut Linter, parse: &Parse< }); if let Some(range) = range { ctx.report(Violation::new( - ErrorCode::BanCreateDomainWithConstraint, + Rule::BanCreateDomainWithConstraint, "Domains with constraints have poor support for online migrations. Use table and column constraints instead.".into(), range, None, diff --git a/crates/squawk_linter/src/rules/ban_drop_column.rs b/crates/squawk_linter/src/rules/ban_drop_column.rs index 08cc4113..165d6141 100644 --- a/crates/squawk_linter/src/rules/ban_drop_column.rs +++ b/crates/squawk_linter/src/rules/ban_drop_column.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_drop_column(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -12,7 +12,7 @@ pub(crate) fn ban_drop_column(ctx: &mut Linter, parse: &Parse) { for action in alter_table.actions() { if let ast::AlterTableAction::DropColumn(drop_column) = action { ctx.report(Violation::new( - ErrorCode::BanDropColumn, + Rule::BanDropColumn, "Dropping a column may break existing clients.".into(), drop_column.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/ban_drop_database.rs b/crates/squawk_linter/src/rules/ban_drop_database.rs index 1799571d..7fae68ff 100644 --- a/crates/squawk_linter/src/rules/ban_drop_database.rs +++ b/crates/squawk_linter/src/rules/ban_drop_database.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; /// Brad's Rule aka ban dropping database statements. pub(crate) fn ban_drop_database(ctx: &mut Linter, parse: &Parse) { @@ -11,7 +11,7 @@ pub(crate) fn ban_drop_database(ctx: &mut Linter, parse: &Parse) { for item in file.items() { if let ast::Item::DropDatabase(drop_database) = item { ctx.report(Violation::new( - ErrorCode::BanDropDatabase, + Rule::BanDropDatabase, "Dropping a database may break existing clients.".into(), drop_database.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/ban_drop_not_null.rs b/crates/squawk_linter/src/rules/ban_drop_not_null.rs index 9f1f06a9..8e3f61e9 100644 --- a/crates/squawk_linter/src/rules/ban_drop_not_null.rs +++ b/crates/squawk_linter/src/rules/ban_drop_not_null.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_drop_not_null(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -15,8 +15,8 @@ pub(crate) fn ban_drop_not_null(ctx: &mut Linter, parse: &Parse) { alter_column.option() { ctx.report(Violation::new( - ErrorCode::BanDropNotNull, - "Dropping a NOT NULL constraint may break existing clients.".into(), + Rule::BanDropNotNull, + "Dropping a `NOT NULL` constraint may break existing clients.".into(), drop_not_null.syntax().text_range(), None, )); diff --git a/crates/squawk_linter/src/rules/ban_drop_table.rs b/crates/squawk_linter/src/rules/ban_drop_table.rs index 1ca25248..038d6680 100644 --- a/crates/squawk_linter/src/rules/ban_drop_table.rs +++ b/crates/squawk_linter/src/rules/ban_drop_table.rs @@ -3,14 +3,14 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn ban_drop_table(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); for item in file.items() { if let ast::Item::DropTable(drop_table) = item { ctx.report(Violation::new( - ErrorCode::BanDropTable, + Rule::BanDropTable, "Dropping a table may break existing clients.".into(), drop_table.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/changing_column_type.rs b/crates/squawk_linter/src/rules/changing_column_type.rs index ff91c4ee..0d2178f4 100644 --- a/crates/squawk_linter/src/rules/changing_column_type.rs +++ b/crates/squawk_linter/src/rules/changing_column_type.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn changing_column_type(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -13,7 +13,7 @@ pub(crate) fn changing_column_type(ctx: &mut Linter, parse: &Parse) if let ast::AlterTableAction::AlterColumn(alter_column) = action { if let Some(ast::AlterColumnOption::SetType(set_type)) = alter_column.option() { ctx.report(Violation::new( - ErrorCode::ChangingColumnType, + Rule::ChangingColumnType, "Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.".into(), set_type.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs b/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs index d42356ba..2854d24f 100644 --- a/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs +++ b/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs @@ -5,7 +5,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, ErrorCode, Linter, Violation}; +use crate::{text::trim_quotes, Rule, Linter, Violation}; pub fn tables_created_in_transaction( assume_in_transaction: bool, @@ -58,12 +58,10 @@ fn not_valid_validate_in_transaction( { ctx.report( Violation::new( - ErrorCode::ConstraintMissingNotValid, - "Using NOT VALID and VALIDATE CONSTRAINT in the same transaction will block all reads while the constraint is validated.".into(), + Rule::ConstraintMissingNotValid, + "Using `NOT VALID` and `VALIDATE CONSTRAINT` in the same transaction will block all reads while the constraint is validated.".into(), validate_constraint.syntax().text_range(), - vec![ - "Add constraint as NOT VALID in one transaction and VALIDATE CONSTRAINT in a separate transaction.".into(), - ] + "Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction.".to_string(), )) } } @@ -130,7 +128,7 @@ pub(crate) fn constraint_missing_not_valid(ctx: &mut Linter, parse: &Parse) { + let message = "Adding a `UNIQUE` constraint requires an `ACCESS EXCLUSIVE` lock which blocks reads and writes to the table while the index is built."; + let help = "Create an index CONCURRENTLY and create the constraint using the index."; let file = parse.tree(); let tables_created = tables_created_in_transaction(ctx.settings.assume_in_transaction, &file); for item in file.items() { @@ -30,10 +32,10 @@ pub(crate) fn disallow_unique_constraint(ctx: &mut Linter, parse: &Parse) { if let Some(ty) = ty { if is_not_valid_int_type(&ty, &SMALL_INT_TYPES) { ctx.report(Violation::new( - ErrorCode::PreferBigInt, + Rule::PreferBigInt, "Using 32-bit integer fields can result in hitting the max `int` limit.".into(), ty.syntax().text_range(), - None, + "Use 64-bit integer values instead to prevent hitting this limit.".to_string(), )); }; } @@ -111,7 +111,7 @@ pub(crate) fn prefer_big_int(ctx: &mut Linter, parse: &Parse) { mod test { use insta::assert_debug_snapshot; - use crate::{ErrorCode, Linter, Rule}; + use crate::{Rule, Linter}; #[test] fn err() { @@ -149,7 +149,7 @@ create table users ( assert_eq!( errors .iter() - .filter(|x| x.code == ErrorCode::PreferBigInt) + .filter(|x| x.code == Rule::PreferBigInt) .count(), 8 ); 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 2d3e28f8..f372cce9 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use squawk_syntax::ast::AstNode; use squawk_syntax::{ast, Parse, SourceFile}; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; use crate::prefer_big_int::check_not_allowed_types; use crate::prefer_big_int::is_not_valid_int_type; @@ -19,10 +19,10 @@ 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) { ctx.report(Violation::new( - ErrorCode::PreferBigintOverInt, + Rule::PreferBigintOverInt, "Using 32-bit integer fields can result in hitting the max `int` limit.".into(), ty.syntax().text_range(), - None, + "Use 64-bit integer values instead to prevent hitting this limit.".to_string(), )); }; } @@ -38,7 +38,7 @@ pub(crate) fn prefer_bigint_over_int(ctx: &mut Linter, parse: &Parse mod test { use insta::assert_debug_snapshot; - use crate::{ErrorCode, Linter, Rule}; + use crate::{Rule, Linter}; #[test] fn err() { @@ -64,7 +64,7 @@ create table users ( assert_eq!( errors .iter() - .filter(|x| x.code == ErrorCode::PreferBigintOverInt) + .filter(|x| x.code == Rule::PreferBigintOverInt) .count(), 4 ); 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 277ddae3..f54425ef 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use squawk_syntax::ast::AstNode; use squawk_syntax::{ast, Parse, SourceFile}; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; use crate::prefer_big_int::check_not_allowed_types; use crate::prefer_big_int::is_not_valid_int_type; @@ -19,10 +19,10 @@ 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) { ctx.report(Violation::new( - ErrorCode::PreferBigintOverSmallint, + Rule::PreferBigintOverSmallint, "Using 16-bit integer fields can result in hitting the max `int` limit.".into(), ty.syntax().text_range(), - None, + "Use 64-bit integer values instead to prevent hitting this limit.".to_string(), )); }; } @@ -37,7 +37,7 @@ pub(crate) fn prefer_bigint_over_smallint(ctx: &mut Linter, parse: &Parse) { if let Some(ty) = ty { if is_not_valid_int_type(&ty, &SERIAL_TYPES) { ctx.report(Violation::new( - ErrorCode::PreferIdentity, - "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.".into(), + Rule::PreferIdentity, + "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.".into(), ty.syntax().text_range(), - None, + "Use Identity columns instead.".to_string(), )); }; } @@ -44,7 +44,7 @@ pub(crate) fn prefer_identity(ctx: &mut Linter, parse: &Parse) { mod test { use insta::assert_debug_snapshot; - use crate::{ErrorCode, Linter, Rule}; + use crate::{Linter, Rule}; #[test] fn err() { @@ -76,7 +76,7 @@ create table users ( assert_eq!( errors .iter() - .filter(|x| x.code == ErrorCode::PreferIdentity) + .filter(|x| x.code == Rule::PreferIdentity) .count(), 6 ); diff --git a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs index 1cc39781..3eb71a9f 100644 --- a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs +++ b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs @@ -5,7 +5,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, ErrorCode, Linter, Violation}; +use crate::{text::trim_quotes, Rule, Linter, Violation}; #[derive(PartialEq)] enum Constraint { @@ -92,7 +92,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { } ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), action.syntax().text_range(), None, @@ -104,17 +104,17 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { && (create_index.concurrently_token().is_some() || !inside_transaction) => { ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), create_index.syntax().text_range(), - vec!["Use an explicit name for a concurrently created index".into()], + "Use an explicit name for a concurrently created index".to_string(), )); } ast::Item::CreateTable(create_table) if create_table.if_not_exists().is_none() && !inside_transaction => { ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), create_table.syntax().text_range(), None, @@ -124,7 +124,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { if drop_index.if_exists().is_none() && !inside_transaction => { ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), drop_index.syntax().text_range(), None, @@ -134,7 +134,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { if drop_table.if_exists().is_none() && !inside_transaction => { ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), drop_table.syntax().text_range(), None, @@ -144,7 +144,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { if drop_type.if_exists().is_none() && !inside_transaction => { ctx.report(Violation::new( - ErrorCode::PreferRobustStmts, + Rule::PreferRobustStmts, "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), drop_type.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/prefer_text_field.rs b/crates/squawk_linter/src/rules/prefer_text_field.rs index 08342d6d..829e1f7d 100644 --- a/crates/squawk_linter/src/rules/prefer_text_field.rs +++ b/crates/squawk_linter/src/rules/prefer_text_field.rs @@ -5,7 +5,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, ErrorCode, Linter, Violation}; +use crate::{text::trim_quotes, Rule, Linter, Violation}; use crate::prefer_big_int::check_not_allowed_types; @@ -51,10 +51,10 @@ fn check_ty_for_varchar(ctx: &mut Linter, ty: Option) { if let Some(ty) = ty { if is_not_allowed_varchar(&ty) { ctx.report(Violation::new( - ErrorCode::PreferTextField, + Rule::PreferTextField, "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.".to_string(), ty.syntax().text_range(), - None, + "Use a `text` field with a `check` constraint.".to_string(), )); }; } diff --git a/crates/squawk_linter/src/rules/prefer_timestamptz.rs b/crates/squawk_linter/src/rules/prefer_timestamptz.rs index 8677a238..a2a1b45a 100644 --- a/crates/squawk_linter/src/rules/prefer_timestamptz.rs +++ b/crates/squawk_linter/src/rules/prefer_timestamptz.rs @@ -4,7 +4,7 @@ use squawk_syntax::{ }; use crate::{prefer_big_int::check_not_allowed_types, text::trim_quotes}; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub fn is_not_allowed_timestamp(ty: &ast::Type) -> bool { match ty { @@ -47,10 +47,10 @@ fn check_ty_for_timestamp(ctx: &mut Linter, ty: Option) { if let Some(ty) = ty { if is_not_allowed_timestamp(&ty) { ctx.report(Violation::new( - ErrorCode::PreferTimestampTz, + Rule::PreferTimestampTz, "When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.".into(), ty.syntax().text_range(), - None, + "Use timestamptz instead of timestamp for your column type.".to_string(), )); }; } @@ -80,7 +80,7 @@ create table app.accounts ); "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::PreferTimestamptz]); + let mut linter = Linter::from([Rule::PreferTimestampTz]); let errors = linter.lint(file, sql); assert_ne!(errors.len(), 0); assert_debug_snapshot!(errors); @@ -95,7 +95,7 @@ alter table app.accounts alter column created_ts type timestamp without time zone; "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::PreferTimestamptz]); + let mut linter = Linter::from([Rule::PreferTimestampTz]); let errors = linter.lint(file, sql); assert_ne!(errors.len(), 0); assert_debug_snapshot!(errors); @@ -114,7 +114,7 @@ create table app.accounts ); "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::PreferTimestamptz]); + let mut linter = Linter::from([Rule::PreferTimestampTz]); let errors = linter.lint(file, sql); assert_eq!(errors.len(), 0); } @@ -132,7 +132,7 @@ create table app.accounts ); "#; let file = squawk_syntax::SourceFile::parse(sql); - let mut linter = Linter::from([Rule::PreferTimestamptz]); + let mut linter = Linter::from([Rule::PreferTimestampTz]); let errors = linter.lint(file, sql); assert_eq!(errors.len(), 0); } diff --git a/crates/squawk_linter/src/rules/renaming_column.rs b/crates/squawk_linter/src/rules/renaming_column.rs index 37c00d3b..cc3fc8e5 100644 --- a/crates/squawk_linter/src/rules/renaming_column.rs +++ b/crates/squawk_linter/src/rules/renaming_column.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn renaming_column(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -12,7 +12,7 @@ pub(crate) fn renaming_column(ctx: &mut Linter, parse: &Parse) { for action in alter_table.actions() { if let ast::AlterTableAction::RenameColumn(rename_column) = action { ctx.report(Violation::new( - ErrorCode::RenamingColumn, + Rule::RenamingColumn, "Renaming a column may break existing clients.".into(), rename_column.syntax().text_range(), None, diff --git a/crates/squawk_linter/src/rules/renaming_table.rs b/crates/squawk_linter/src/rules/renaming_table.rs index 73113dc5..262d89bc 100644 --- a/crates/squawk_linter/src/rules/renaming_table.rs +++ b/crates/squawk_linter/src/rules/renaming_table.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Rule, Linter, Violation}; pub(crate) fn renaming_table(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -12,7 +12,7 @@ pub(crate) fn renaming_table(ctx: &mut Linter, parse: &Parse) { for action in alter_table.actions() { if let ast::AlterTableAction::RenameTable(rename_table) = action { ctx.report(Violation::new( - ErrorCode::RenamingTable, + Rule::RenamingTable, "Renaming a table may break existing clients.".into(), rename_table.syntax().text_range(), None, 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 cd3041a4..0d2ee596 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, ErrorCode, Linter, Violation}; +use crate::{text::trim_quotes, Rule, Linter, Violation}; use super::constraint_missing_not_valid::tables_created_in_transaction; @@ -21,10 +21,8 @@ pub(crate) fn require_concurrent_index_creation(ctx: &mut Linter, parse: &Parse< && !tables_created.contains(trim_quotes(table_name.text().as_str())) { ctx.report(Violation::new( - ErrorCode::RequireConcurrentIndexCreation, -"During a normal index creation, table updates are blocked, but reads are still allowed. `CONCURRENTLY` avoids locking the table against writes during index creation.".into(), - - + Rule::RequireConcurrentIndexCreation, + "During a normal index creation, table updates are blocked, but reads are still allowed. `CONCURRENTLY` avoids locking the table against writes during index creation.".into(), create_index.syntax().text_range(), None, )); 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 875a95f6..314537ae 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_deletion.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{ErrorCode, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn require_concurrent_index_deletion(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); @@ -11,10 +11,10 @@ pub(crate) fn require_concurrent_index_deletion(ctx: &mut Linter, parse: &Parse< if let ast::Item::DropIndex(drop_index) = item { if drop_index.concurrently_token().is_none() { ctx.report(Violation::new( - ErrorCode::RequireConcurrentIndexDeletion, -"A normal `DROP INDEX` acquires an `ACCESS EXCLUSIVE` lock on the table, blocking other accesses until the index drop can be completed.".into(), + 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().text_range(), - None, + "Drop the index `CONCURRENTLY`.".to_string(), )); } } @@ -25,7 +25,7 @@ pub(crate) fn require_concurrent_index_deletion(ctx: &mut Linter, parse: &Parse< mod test { use insta::assert_debug_snapshot; - use crate::{ErrorCode, Linter, Rule}; + use crate::{Linter, Rule}; #[test] fn drop_index_missing_concurrently_err() { @@ -37,7 +37,7 @@ mod test { let mut linter = Linter::from([Rule::RequireConcurrentIndexDeletion]); let errors = linter.lint(file, sql); assert_eq!(errors.len(), 1); - assert_eq!(errors[0].code, ErrorCode::RequireConcurrentIndexDeletion); + assert_eq!(errors[0].code, Rule::RequireConcurrentIndexDeletion); assert_debug_snapshot!(errors); } diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap index 0d76ee71..f271e43a 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 74..83, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap index fea9efa4..829200f7 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 80..88, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap index 01326346..5b2b77cd 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 60..66, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap index 047703a5..848a93b3 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 56..62, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap index 3efb5b3f..2ee281e2 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 76..84, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap index 951b257a..d9242981 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap @@ -7,8 +7,8 @@ expression: errors code: AddingFieldWithDefault, message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", text_range: 40..78, - messages: [ + help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_not_null_field__test__set_not_null.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_not_null_field__test__set_not_null.snap index 269d3ecf..64f3b9ea 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_not_null_field__test__set_not_null.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_not_null_field__test__set_not_null.snap @@ -5,10 +5,10 @@ expression: errors [ Violation { code: AddingNotNullableField, - message: "Setting a column NOT NULL blocks reads while the table is scanned.", + message: "Setting a column `NOT NULL` blocks reads while the table is scanned.", text_range: 46..58, - messages: [ - "Use a check constraint instead.", - ], + help: Some( + "Make the field nullable and use a `CHECK` constraint instead.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__plain_primary_key.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__plain_primary_key.snap index a71a4f98..31900665 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__plain_primary_key.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__plain_primary_key.snap @@ -7,6 +7,8 @@ expression: errors code: AddingSerialPrimaryKeyField, message: "Adding a primary key constraint requires an `ACCESS EXCLUSIVE` lock that will block all reads and writes to the table while the primary key index is built.", text_range: 23..39, - messages: [], + help: Some( + "Add the `PRIMARY KEY` constraint `USING` an index.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__serial_primary_key.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__serial_primary_key.snap index 00defb2b..5d3a6838 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__serial_primary_key.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_primary_key_constraint__test__serial_primary_key.snap @@ -7,6 +7,8 @@ expression: errors code: AddingSerialPrimaryKeyField, message: "Adding a primary key constraint requires an `ACCESS EXCLUSIVE` lock that will block all reads and writes to the table while the primary key index is built.", text_range: 43..54, - messages: [], + help: Some( + "Add the `PRIMARY KEY` constraint `USING` an index.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_required_field__test__not_null_without_default.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_required_field__test__not_null_without_default.snap index 939eb3b5..3c78474e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_required_field__test__not_null_without_default.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_required_field__test__not_null_without_default.snap @@ -7,6 +7,8 @@ expression: errors code: AddingRequiredField, message: "Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.", text_range: 22..58, - messages: [], + help: Some( + "Make the field nullable or add a non-VOLATILE DEFAULT", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_alter_domain_with_add_constraint__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_alter_domain_with_add_constraint__test__err.snap index 1104e1f9..c09b190c 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_alter_domain_with_add_constraint__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_alter_domain_with_add_constraint__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: BanAlterDomainWithAddConstraint, message: "Domains with constraints have poor support for online migrations. Use table and column constraints instead.", text_range: 31..79, - messages: [], + help: None, }, ] 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 2bcc9a03..f3aa674d 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 @@ -7,36 +7,36 @@ expression: errors code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 59..68, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 76..90, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 98..102, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 110..119, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 265..280, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 288..292, - messages: [], + help: None, }, ] 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 7c5a941c..5eaf71d8 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 @@ -7,6 +7,6 @@ expression: errors code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 28..32, - messages: [], + help: None, }, ] 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 f486d0d3..e4ff2781 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 @@ -7,6 +7,6 @@ expression: errors code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 22..26, - messages: [], + help: None, }, ] 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 b813748f..fc5421ee 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 @@ -7,24 +7,24 @@ expression: errors code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 77..86, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 108..122, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 147..151, - messages: [], + help: None, }, Violation { code: BanCharField, message: "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.", text_range: 174..183, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__assuming_in_transaction_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__assuming_in_transaction_err.snap index 69fd9180..a6508a0f 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__assuming_in_transaction_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__assuming_in_transaction_err.snap @@ -5,8 +5,10 @@ expression: errors [ Violation { code: BanConcurrentIndexCreationInTransaction, - message: "While regular index creation can happen inside a transaction, this is not allowed when the CONCURRENTLY option is used.", + message: "While regular index creation can happen inside a transaction, this is not allowed when the `CONCURRENTLY` option is used.", text_range: 39..51, - messages: [], + help: Some( + "Build the index outside any transactions.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__ban_concurrent_index_creation_in_transaction_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__ban_concurrent_index_creation_in_transaction_err.snap index eef7e768..e1173894 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__ban_concurrent_index_creation_in_transaction_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_concurrent_index_creation_in_transaction__test__ban_concurrent_index_creation_in_transaction_err.snap @@ -5,8 +5,10 @@ expression: errors [ Violation { code: BanConcurrentIndexCreationInTransaction, - message: "While regular index creation can happen inside a transaction, this is not allowed when the CONCURRENTLY option is used.", + message: "While regular index creation can happen inside a transaction, this is not allowed when the `CONCURRENTLY` option is used.", text_range: 59..71, - messages: [], + help: Some( + "Build the index outside any transactions.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err.snap index 062476c8..d67c0c1d 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: BanCreateDomainWithConstraint, message: "Domains with constraints have poor support for online migrations. Use table and column constraints instead.", text_range: 46..63, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err_with_multiple_constraints.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err_with_multiple_constraints.snap index 47dcc10e..da9972b9 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err_with_multiple_constraints.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test__err_with_multiple_constraints.snap @@ -7,6 +7,6 @@ expression: errors code: BanCreateDomainWithConstraint, message: "Domains with constraints have poor support for online migrations. Use table and column constraints instead.", text_range: 22..48, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_column__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_column__test__err.snap index e23bddfa..d9158092 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_column__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_column__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: BanDropColumn, message: "Dropping a column may break existing clients.", text_range: 23..52, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_database__test__ban_drop_database.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_database__test__ban_drop_database.snap index 998372ec..17a8ed09 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_database__test__ban_drop_database.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_database__test__ban_drop_database.snap @@ -7,18 +7,18 @@ expression: errors code: BanDropDatabase, message: "Dropping a database may break existing clients.", text_range: 9..35, - messages: [], + help: None, }, Violation { code: BanDropDatabase, message: "Dropping a database may break existing clients.", text_range: 45..81, - messages: [], + help: None, }, Violation { code: BanDropDatabase, message: "Dropping a database may break existing clients.", text_range: 91..127, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_not_null__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_not_null__test__err.snap index 4b2ffd58..860f5451 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_not_null__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_not_null__test__err.snap @@ -5,8 +5,8 @@ expression: errors [ Violation { code: BanDropNotNull, - message: "Dropping a NOT NULL constraint may break existing clients.", + message: "Dropping a `NOT NULL` constraint may break existing clients.", text_range: 46..59, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_table__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_table__test__err.snap index bf40fd30..c966fdea 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_table__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__ban_drop_table__test__err.snap @@ -7,18 +7,18 @@ expression: errors code: BanDropTable, message: "Dropping a table may break existing clients.", text_range: 1..24, - messages: [], + help: None, }, Violation { code: BanDropTable, message: "Dropping a table may break existing clients.", text_range: 26..59, - messages: [], + help: None, }, Violation { code: BanDropTable, message: "Dropping a table may break existing clients.", text_range: 61..94, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__another_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__another_err.snap index 94f8ffff..229b1062 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__another_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__another_err.snap @@ -7,12 +7,12 @@ expression: errors code: ChangingColumnType, message: "Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.", text_range: 88..131, - messages: [], + help: None, }, Violation { code: ChangingColumnType, message: "Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.", text_range: 178..205, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__err.snap index 65473fe3..0073ae44 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__changing_column_type__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: ChangingColumnType, message: "Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.", text_range: 92..121, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_check_constraint_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_check_constraint_err.snap index f52b2f84..e4ecb32a 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_check_constraint_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_check_constraint_err.snap @@ -7,6 +7,6 @@ expression: errors code: ConstraintMissingNotValid, message: "By default new constraints require a table scan and block writes to the table while that scan occurs.", text_range: 38..94, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_fk_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_fk_err.snap index c9cb5cd5..70418553 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_fk_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__adding_fk_err.snap @@ -7,6 +7,6 @@ expression: errors code: ConstraintMissingNotValid, message: "By default new constraints require a table scan and block writes to the table while that scan occurs.", text_range: 40..114, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_assume_transaction_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_assume_transaction_err.snap index cb28dfb9..a2c13a2e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_assume_transaction_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_assume_transaction_err.snap @@ -5,10 +5,10 @@ expression: errors [ Violation { code: ConstraintMissingNotValid, - message: "Using NOT VALID and VALIDATE CONSTRAINT in the same transaction will block all reads while the constraint is validated.", + message: "Using `NOT VALID` and `VALIDATE CONSTRAINT` in the same transaction will block all reads while the constraint is validated.", text_range: 134..163, - messages: [ - "Add constraint as NOT VALID in one transaction and VALIDATE CONSTRAINT in a separate transaction.", - ], + help: Some( + "Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_transaction_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_transaction_err.snap index bc72d2a7..ae177fee 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_transaction_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_transaction_err.snap @@ -5,10 +5,10 @@ expression: errors [ Violation { code: ConstraintMissingNotValid, - message: "Using NOT VALID and VALIDATE CONSTRAINT in the same transaction will block all reads while the constraint is validated.", + message: "Using `NOT VALID` and `VALIDATE CONSTRAINT` in the same transaction will block all reads while the constraint is validated.", text_range: 141..170, - messages: [ - "Add constraint as NOT VALID in one transaction and VALIDATE CONSTRAINT in a separate transaction.", - ], + help: Some( + "Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_with_assume_in_transaction_with_explicit_commit_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_with_assume_in_transaction_with_explicit_commit_err.snap index cb28dfb9..a2c13a2e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_with_assume_in_transaction_with_explicit_commit_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__constraint_missing_not_valid__test__not_valid_validate_with_assume_in_transaction_with_explicit_commit_err.snap @@ -5,10 +5,10 @@ expression: errors [ Violation { code: ConstraintMissingNotValid, - message: "Using NOT VALID and VALIDATE CONSTRAINT in the same transaction will block all reads while the constraint is validated.", + message: "Using `NOT VALID` and `VALIDATE CONSTRAINT` in the same transaction will block all reads while the constraint is validated.", text_range: 134..163, - messages: [ - "Add constraint as NOT VALID in one transaction and VALIDATE CONSTRAINT in a separate transaction.", - ], + help: Some( + "Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__adding_unique_constraint_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__adding_unique_constraint_err.snap index f7f8d32b..da344db7 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__adding_unique_constraint_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__adding_unique_constraint_err.snap @@ -7,6 +7,8 @@ expression: errors code: DisallowedUniqueConstraint, message: "Adding a `UNIQUE` constraint requires an `ACCESS EXCLUSIVE` lock which blocks reads and writes to the table while the index is built.", text_range: 28..80, - messages: [], + help: Some( + "Create an index CONCURRENTLY and create the constraint using the index.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_err.snap index 4f784138..f1dd4873 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_err.snap @@ -7,6 +7,8 @@ expression: errors code: DisallowedUniqueConstraint, message: "Adding a `UNIQUE` constraint requires an `ACCESS EXCLUSIVE` lock which blocks reads and writes to the table while the index is built.", text_range: 37..69, - messages: [], + help: Some( + "Create an index CONCURRENTLY and create the constraint using the index.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_unique_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_unique_err.snap index 6b96c7fe..0acc94e3 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_unique_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__disallow_unique_constraint__test__unique_constraint_inline_add_column_unique_err.snap @@ -7,6 +7,8 @@ expression: errors code: DisallowedUniqueConstraint, message: "Adding a `UNIQUE` constraint requires an `ACCESS EXCLUSIVE` lock which blocks reads and writes to the table while the index is built.", text_range: 37..43, - messages: [], + help: Some( + "Create an index CONCURRENTLY and create the constraint using the index.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_add_column_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_add_column_err.snap index 11e530a3..3ccfc537 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_add_column_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_add_column_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 28..35, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_err.snap index 79e843cd..627bf246 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 35..42, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_with_quotes_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_with_quotes_err.snap index 4a698fcf..37439978 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_with_quotes_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__alter_table_alter_column_type_with_quotes_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 35..44, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__create_table_many_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__create_table_many_err.snap index e6fdb760..6dfe6ead 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__create_table_many_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__create_table_many_err.snap @@ -7,12 +7,16 @@ expression: errors code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 30..37, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 47..53, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__err.snap index b922a468..4e259503 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_big_int__test__err.snap @@ -7,48 +7,64 @@ expression: errors code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 29..37, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 69..73, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 105..112, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 144..148, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 180..186, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 218..225, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 257..264, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 296..307, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] 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 ca6e5e0d..69408f63 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 @@ -7,24 +7,32 @@ expression: errors code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 29..36, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 68..72, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 104..110, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverInt, message: "Using 32-bit integer fields can result in hitting the max `int` limit.", text_range: 142..149, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] 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 15c1c290..5028a58c 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 @@ -7,24 +7,32 @@ expression: errors code: PreferBigintOverSmallint, message: "Using 16-bit integer fields can result in hitting the max `int` limit.", text_range: 29..37, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverSmallint, message: "Using 16-bit integer fields can result in hitting the max `int` limit.", text_range: 69..73, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverSmallint, message: "Using 16-bit integer fields can result in hitting the max `int` limit.", text_range: 105..116, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, Violation { code: PreferBigintOverSmallint, message: "Using 16-bit integer fields can result in hitting the max `int` limit.", text_range: 148..155, - messages: [], + help: Some( + "Use 64-bit integer values instead to prevent hitting this limit.", + ), }, ] 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 af7027c3..db10e813 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 @@ -5,38 +5,50 @@ expression: errors [ Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 29..35, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 67..74, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 106..113, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 145..152, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 184..195, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, Violation { code: PreferIdentity, - message: "Serial types make permissions and schema management difficult. Identity columns are standard SQL and have more features and better usability.", + message: "Serial types make schema, dependency, and permission management difficult. Use Identity columns instead.", text_range: 227..236, - messages: [], + help: Some( + "Use Identity columns instead.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap index 4c571169..e18b8c61 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 54..75, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap index 16223bdf..57b0af67 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 63..93, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_err.snap index 386a33b9..d37568cd 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 63..98, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_err.snap index f336aa71..04c9fb41 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_err.snap @@ -7,8 +7,8 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 40..108, - messages: [ + help: Some( "Use an explicit name for a concurrently created index", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_muli_stmts_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_muli_stmts_err.snap index 36322423..cc254e82 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_muli_stmts_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_muli_stmts_err.snap @@ -7,16 +7,16 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 1..57, - messages: [ + help: Some( "Use an explicit name for a concurrently created index", - ], + ), }, Violation { code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 59..115, - messages: [ + help: Some( "Use an explicit name for a concurrently created index", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_unnamed_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_unnamed_err.snap index 98161f4b..99b97f52 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_unnamed_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_index_concurrently_unnamed_err.snap @@ -7,8 +7,8 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 40..96, - messages: [ + help: Some( "Use an explicit name for a concurrently created index", - ], + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_table_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_table_err.snap index 9f86e90a..4d7fd451 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_table_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__create_table_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 11..122, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap index 61304942..1bb33be7 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 63..89, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap index f87b67c4..0b837fb7 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 240..298, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__drop_index_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__drop_index_err.snap index 8618647f..fc971399 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__drop_index_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__drop_index_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 40..75, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap index 26d66eca..e4608142 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 63..88, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap index b4b79ee2..8d1a1e8c 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap @@ -7,6 +7,6 @@ expression: errors code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 53..78, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap index bf23edd6..c83f24a4 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__adding_column_non_text_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferTextField, message: "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.", text_range: 56..68, - messages: [], + help: Some( + "Use a `text` field with a `check` constraint.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap index 9719e0a1..6192597f 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_pgcatalog_varchar_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferTextField, message: "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.", text_range: 69..92, - messages: [], + help: Some( + "Use a `text` field with a `check` constraint.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap index 2970531d..5f2f3052 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__create_table_with_varchar_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferTextField, message: "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.", text_range: 111..123, - messages: [], + help: Some( + "Use a `text` field with a `check` constraint.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap index e4137151..7578baad 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_text_field__test__increase_varchar_size_err.snap @@ -7,6 +7,8 @@ expression: errors code: PreferTextField, message: "Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.", text_range: 89..102, - messages: [], + help: Some( + "Use a `text` field with a `check` constraint.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__alter_table_with_timestamp_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__alter_table_with_timestamp_err.snap index 9913c5a9..e5d68a61 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__alter_table_with_timestamp_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__alter_table_with_timestamp_err.snap @@ -7,12 +7,16 @@ expression: errors code: PreferTimestampTz, message: "When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.", text_range: 56..65, - messages: [], + help: Some( + "Use timestamptz instead of timestamp for your column type.", + ), }, Violation { code: PreferTimestampTz, message: "When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.", text_range: 125..152, - messages: [], + help: Some( + "Use timestamptz instead of timestamp for your column type.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__create_table_with_timestamp_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__create_table_with_timestamp_err.snap index 4bbc5ea7..d913322e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__create_table_with_timestamp_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_timestamptz__test__create_table_with_timestamp_err.snap @@ -7,12 +7,16 @@ expression: errors code: PreferTimestampTz, message: "When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.", text_range: 43..52, - messages: [], + help: Some( + "Use timestamptz instead of timestamp for your column type.", + ), }, Violation { code: PreferTimestampTz, message: "When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.", text_range: 99..126, - messages: [], + help: Some( + "Use timestamptz instead of timestamp for your column type.", + ), }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_column__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_column__test__err.snap index e8f4343c..2c4bed06 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_column__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_column__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: RenamingColumn, message: "Renaming a column may break existing clients.", text_range: 26..74, - messages: [], + help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_table__test__err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_table__test__err.snap index d94c68e2..7461e6fa 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_table__test__err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__renaming_table__test__err.snap @@ -7,6 +7,6 @@ expression: errors code: RenamingTable, message: "Renaming a table may break existing clients.", text_range: 26..52, - messages: [], + help: None, }, ] 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 219b5629..316e678f 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 @@ -7,6 +7,6 @@ expression: errors code: RequireConcurrentIndexCreation, message: "During a normal index creation, table updates are blocked, but reads are still allowed. `CONCURRENTLY` avoids locking the table against writes during index creation.", text_range: 15..75, - messages: [], + help: 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 688b409f..ab820050 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 @@ -5,8 +5,10 @@ expression: errors [ Violation { code: RequireConcurrentIndexDeletion, - message: "A normal `DROP INDEX` acquires an `ACCESS EXCLUSIVE` lock on the table, blocking other accesses until the index drop can be completed.", + message: "A normal `DROP INDEX` acquires an `ACCESS EXCLUSIVE` lock on the table, blocking other accesses until the index drop can complete.", text_range: 19..56, - messages: [], + help: Some( + "Drop the index `CONCURRENTLY`.", + ), }, ] diff --git a/crates/squawk_linter/src/snapshots/squawk_linter__version__test_pg_version__parse.snap b/crates/squawk_linter/src/snapshots/squawk_linter__version__test_pg_version__parse.snap new file mode 100644 index 00000000..981753e8 --- /dev/null +++ b/crates/squawk_linter/src/snapshots/squawk_linter__version__test_pg_version__parse.snap @@ -0,0 +1,12 @@ +--- +source: crates/squawk_linter/src/version.rs +expression: "Version::from_str(\"test\").unwrap_err()" +--- +InvalidNumber( + InvalidNumber { + version: "test", + e: ParseIntError { + kind: InvalidDigit, + }, + }, +) diff --git a/crates/squawk_linter/src/version.rs b/crates/squawk_linter/src/version.rs new file mode 100644 index 00000000..13fe80e9 --- /dev/null +++ b/crates/squawk_linter/src/version.rs @@ -0,0 +1,146 @@ +use std::num::ParseIntError; +use std::str::FromStr; + +use serde::{de::Error, Deserialize, Deserializer}; + +#[derive(Debug, PartialEq, PartialOrd, Clone, Copy)] +pub struct Version { + major: i32, + minor: Option, + patch: Option, +} + +impl Version { + #[must_use] + pub(crate) fn new( + major: i32, + minor: impl Into>, + patch: impl Into>, + ) -> Self { + Self { + major, + minor: minor.into(), + patch: patch.into(), + } + } +} + +// Allow us to deserialize our version from a string in .squawk.toml. +// from https://stackoverflow.com/a/46755370/ +impl<'de> Deserialize<'de> for Version { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s: &str = Deserialize::deserialize(deserializer)?; + Version::from_str(s).map_err(D::Error::custom) + } +} + +#[derive(Debug, PartialEq)] +pub struct InvalidNumber { + pub version: String, + pub e: ParseIntError, +} + +#[derive(Debug, PartialEq)] +pub struct EmptyVersion { + pub version: String, +} + +#[derive(Debug, PartialEq)] +pub enum ParseVersionError { + EmptyVersion(EmptyVersion), + InvalidNumber(InvalidNumber), +} + +fn parse_int(s: &str) -> Result { + Ok(s.parse().map_err(|e| { + ParseVersionError::InvalidNumber(InvalidNumber { + version: s.to_string(), + e, + }) + }))? +} + +impl std::fmt::Display for ParseVersionError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match *self { + Self::EmptyVersion(ref err) => { + write!(f, "Empty version number provided: {:?}", err.version) + } + Self::InvalidNumber(ref err) => { + write!( + f, + "Invalid number in version: {:?}. Parse error: {}", + err.version, err.e + ) + } + } + } +} + +impl FromStr for Version { + type Err = ParseVersionError; + + fn from_str(s: &str) -> Result { + let version_pieces: Vec<&str> = s.split('.').collect(); + + if version_pieces.is_empty() { + return Err(ParseVersionError::EmptyVersion(EmptyVersion { + version: s.to_string(), + })); + } + let major = parse_int(version_pieces[0])?; + + let minor: Option = if version_pieces.len() > 1 { + Some(parse_int(version_pieces[1])?) + } else { + None + }; + let patch: Option = if version_pieces.len() > 2 { + Some(parse_int(version_pieces[2])?) + } else { + None + }; + + Ok(Version { + major, + minor, + patch, + }) + } +} + +#[cfg(test)] +mod test_pg_version { + #![allow(clippy::neg_cmp_op_on_partial_ord)] + use insta::assert_debug_snapshot; + + use super::*; + #[test] + fn eq() { + assert_eq!(Version::new(10, None, None), Version::new(10, None, None)); + } + #[test] + fn gt() { + assert!(Version::new(10, Some(1), None) > Version::new(10, None, None)); + assert!(Version::new(10, None, Some(1)) > Version::new(10, None, None)); + assert!(Version::new(10, None, Some(1)) > Version::new(9, None, None)); + + assert!(!(Version::new(10, None, None) > Version::new(10, None, None))); + } + #[test] + fn parse() { + assert_eq!( + Version::from_str("10.1"), + Ok(Version::new(10, Some(1), None)) + ); + assert_eq!(Version::from_str("10"), Ok(Version::new(10, None, None))); + assert_eq!( + Version::from_str("10.2.1"), + Ok(Version::new(10, Some(2), Some(1))) + ); + assert_debug_snapshot!(Version::from_str("test").unwrap_err()); + } +} From b7183cfebe9dcbd6c664f54cd3b1c35c02631b65 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 5 May 2025 23:02:30 -0400 Subject: [PATCH 2/5] fix --- Cargo.lock | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 025b7987..39df923b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2468,9 +2468,9 @@ dependencies = [ [[package]] name = "serde_plain" -version = "1.0.0" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95455e7e29fada2052e72170af226fbe368a4ca33dee847875325d9fdb133858" +checksum = "9ce1fc6db65a611022b23a0dec6975d63fb80a302cb3388835ff02c097258d50" dependencies = [ "serde", ] @@ -2678,6 +2678,7 @@ dependencies = [ "line-index", "rowan", "serde", + "serde_plain", "squawk_syntax", ] From 075ee696baf583af4f67921a04624e851e99ad7d Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 5 May 2025 23:05:18 -0400 Subject: [PATCH 3/5] fix --- Cargo.lock | 13 --- crates/squawk_cli/Cargo.toml | 24 ----- crates/squawk_cli/src/main.rs | 191 ---------------------------------- 3 files changed, 228 deletions(-) delete mode 100644 crates/squawk_cli/Cargo.toml delete mode 100644 crates/squawk_cli/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 39df923b..62d550d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2647,19 +2647,6 @@ dependencies = [ "serde_repr", ] -[[package]] -name = "squawk_cli" -version = "0.0.0" -dependencies = [ - "annotate-snippets", - "anyhow", - "atty", - "clap 4.5.37", - "enum-iterator", - "squawk_lexer", - "squawk_linter", - "squawk_syntax", -] [[package]] name = "squawk_lexer" diff --git a/crates/squawk_cli/Cargo.toml b/crates/squawk_cli/Cargo.toml deleted file mode 100644 index 0646a6bc..00000000 --- a/crates/squawk_cli/Cargo.toml +++ /dev/null @@ -1,24 +0,0 @@ -[package] -name = "squawk_cli" -version = "0.0.0" -description = "TBD" -default-run = "squawk_cli" - -authors.workspace = true -edition.workspace = true -license.workspace = true -rust-version.workspace = true - -[dependencies] -clap.workspace = true -anyhow.workspace = true -enum-iterator.workspace = true -annotate-snippets.workspace = true -atty.workspace = true - -squawk_syntax.workspace = true -squawk_lexer.workspace = true -squawk_linter.workspace = true - -[lints] -workspace = true diff --git a/crates/squawk_cli/src/main.rs b/crates/squawk_cli/src/main.rs deleted file mode 100644 index 9944b69b..00000000 --- a/crates/squawk_cli/src/main.rs +++ /dev/null @@ -1,191 +0,0 @@ -use std::{ - io::{self, Read}, - process::ExitCode, -}; - -use annotate_snippets::{Level, Renderer, Snippet}; -use anyhow::Result; -use atty::Stream; -use clap::{error::ErrorKind, CommandFactory, Parser, ValueEnum}; -use squawk_linter::Violation; -use squawk_syntax::syntax_error::SyntaxError; -use std::{fs, path::PathBuf}; - -#[derive(ValueEnum, Clone, Debug)] -enum Mode { - Parse, - Lex, - Lint, -} - -/// Dump Parse/Lex Data -#[derive(Parser, Debug)] -#[command(version, about, long_about = None)] -struct Args { - /// SQL to dump - #[arg(short, long, conflicts_with = "file")] - sql: Option, - - /// Path to read SQL - #[arg(short, long, conflicts_with = "sql")] - file: Option, - - /// Either Parser debug output or Lexer debug output. - #[arg(short, long, default_value = "parse")] - mode: Mode, - - #[arg(short, long, default_value_t = false)] - verbose: bool, - - /// Assume the SQL is being run within a transaction. No explicit begin, - /// commit required. - #[arg(short, long)] - assume_in_transaction: bool, -} - -fn read_stdin() -> Result { - let mut buffer = String::new(); - let stdin = io::stdin(); - let mut handle = stdin.lock(); - handle.read_to_string(&mut buffer)?; - Ok(buffer) -} - -fn read_sql(arg_sql: Option, file: &Option) -> Result { - let is_stdin = !atty::is(Stream::Stdin); - if is_stdin { - read_stdin() - } else if let Some(path) = &file { - Ok(fs::read_to_string(path)?) - } else if let Some(sql) = arg_sql { - Ok(sql) - } else { - let err = Args::command().error( - ErrorKind::ArgumentConflict, - "--sql, --file, or stdin must be provided.", - ); - err.exit() - } -} - -fn main() -> Result { - let args = Args::parse(); - let sql = read_sql(args.sql, &args.file)?; - let filename = args - .file - .map(|x| x.display().to_string()) - .unwrap_or("stdin".to_string()); - match args.mode { - Mode::Lex => { - let tokens = squawk_lexer::tokenize(&sql); - let mut start = 0; - for token in tokens { - if args.verbose { - let content = &sql[start as usize..(start + token.len) as usize]; - start += token.len; - println!("{:?} @ {:?}", content, token.kind); - } else { - println!("{:?}", token); - } - } - Ok(ExitCode::SUCCESS) - } - Mode::Parse => { - let parse = squawk_syntax::SourceFile::parse(&sql); - if args.verbose { - println!("{}\n---", parse.syntax_node()); - } - print!("{:#?}", parse.syntax_node()); - let errors = parse.errors(); - if !errors.is_empty() { - let mut snap = "---".to_string(); - for syntax_error in &errors { - let range = syntax_error.range(); - let text = syntax_error.message(); - // split into there own lines so that we can just grep - // for error without hitting this part - snap += "\n"; - snap += "ERROR"; - if range.start() == range.end() { - snap += &format!("@{:?} {:?}", range.start(), text); - } else { - snap += &format!("@{:?}:{:?} {:?}", range.start(), range.end(), text); - } - } - println!("{}", snap); - - render_syntax_errors(&errors, &filename, &sql); - - return Ok(ExitCode::FAILURE); - } - Ok(ExitCode::SUCCESS) - } - Mode::Lint => { - let mut linter = squawk_linter::Linter::with_all_rules(); - linter.settings.assume_in_transaction = args.assume_in_transaction; - let parse = squawk_syntax::SourceFile::parse(&sql); - - if args.verbose { - println!("{}\n---", parse.syntax_node()); - // print!("{:#?}\n---", parse.syntax_node()); - } - - let errors = linter.lint(parse, &sql); - - if errors.is_empty() { - Ok(ExitCode::SUCCESS) - } else { - render_lint_errors(&errors, &filename, &sql); - println!(); - println!("Find detailed examples and solutions for each rule at https://squawkhq.com/docs/rules"); - println!( - "Found {} issue in 1 file (checked 1 source file)", - errors.len() - ); - Ok(ExitCode::FAILURE) - } - } - } -} - -fn render_syntax_errors(errors: &[SyntaxError], filename: &str, sql: &str) { - let renderer = Renderer::styled(); - for err in errors { - let text = err.message(); - let span = err.range().into(); - let message = Level::Warning.title(text).id("syntax-error").snippet( - Snippet::source(sql) - .origin(filename) - .fold(true) - .annotation(Level::Error.span(span)), - ); - println!("{}", renderer.render(message)); - } -} - -fn render_lint_errors(errors: &Vec<&Violation>, filename: &str, sql: &str) { - let renderer = Renderer::styled(); - for err in errors { - let meta = err.code.meta(); - let footers = err.messages.iter().map(|e| Level::Help.title(e)); - // TODO: we need to figure out error messages, they shouldn't be in two places - let prebuilt_footers = meta.messages.into_iter().map(|x| match x { - squawk_linter::ViolationMessage::Note(x) => Level::Note.title(x), - squawk_linter::ViolationMessage::Help(x) => Level::Help.title(x), - }); - let error_name = err.code.to_string(); - let message = Level::Warning - .title(&meta.title) - .id(&error_name) - .snippet( - Snippet::source(sql) - .origin(filename) - .fold(true) - .annotation(Level::Error.span(err.text_range.into())), - ) - .footers(footers) - .footers(prebuilt_footers); - - println!("{}", renderer.render(message)); - } -} From 848e943258cf7077d93d2e9c732b1bd3e13d0638 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 5 May 2025 23:14:53 -0400 Subject: [PATCH 4/5] fmt --- crates/squawk_linter/src/ignore.rs | 2 +- crates/squawk_linter/src/rules/adding_field_with_default.rs | 2 +- .../squawk_linter/src/rules/adding_foreign_key_constraint.rs | 4 ++-- crates/squawk_linter/src/rules/adding_required_field.rs | 2 +- .../src/rules/ban_alter_domain_with_add_constraint.rs | 2 +- crates/squawk_linter/src/rules/ban_char_field.rs | 2 +- .../src/rules/ban_concurrent_index_creation_in_transaction.rs | 2 +- .../src/rules/ban_create_domain_with_constraint.rs | 2 +- crates/squawk_linter/src/rules/ban_drop_column.rs | 2 +- crates/squawk_linter/src/rules/ban_drop_database.rs | 2 +- crates/squawk_linter/src/rules/ban_drop_not_null.rs | 2 +- crates/squawk_linter/src/rules/ban_drop_table.rs | 2 +- crates/squawk_linter/src/rules/changing_column_type.rs | 2 +- .../squawk_linter/src/rules/constraint_missing_not_valid.rs | 2 +- crates/squawk_linter/src/rules/prefer_big_int.rs | 4 ++-- crates/squawk_linter/src/rules/prefer_bigint_over_int.rs | 4 ++-- crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs | 4 ++-- crates/squawk_linter/src/rules/prefer_robust_stmts.rs | 2 +- crates/squawk_linter/src/rules/prefer_text_field.rs | 2 +- crates/squawk_linter/src/rules/renaming_column.rs | 2 +- crates/squawk_linter/src/rules/renaming_table.rs | 2 +- .../src/rules/require_concurrent_index_creation.rs | 2 +- 22 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/squawk_linter/src/ignore.rs b/crates/squawk_linter/src/ignore.rs index 1f25b1ac..dc3a4fb0 100644 --- a/crates/squawk_linter/src/ignore.rs +++ b/crates/squawk_linter/src/ignore.rs @@ -108,7 +108,7 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) { #[cfg(test)] mod test { - use crate::{find_ignores, Linter, Rule, Violation}; + use crate::{find_ignores, Linter, Rule}; #[test] fn single_ignore() { diff --git a/crates/squawk_linter/src/rules/adding_field_with_default.rs b/crates/squawk_linter/src/rules/adding_field_with_default.rs index f9323281..7eeea117 100644 --- a/crates/squawk_linter/src/rules/adding_field_with_default.rs +++ b/crates/squawk_linter/src/rules/adding_field_with_default.rs @@ -5,7 +5,7 @@ use squawk_syntax::ast; use squawk_syntax::ast::{AstNode, HasArgList}; use squawk_syntax::{ast::HasModuleItem, Parse, SourceFile}; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; fn is_const_expr(expr: &ast::Expr) -> bool { match expr { diff --git a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs index d32ff1bb..e5ab248a 100644 --- a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs +++ b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse) { let message = "Adding a foreign key constraint requires a table scan and a `SHARE ROW EXCLUSIVE` lock on both tables, which blocks writes to each table."; @@ -59,7 +59,7 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs b/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs index 496fdd00..64907c7a 100644 --- a/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs +++ b/crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_alter_domain_with_add_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/ban_char_field.rs b/crates/squawk_linter/src/rules/ban_char_field.rs index 1021cead..49e40da1 100644 --- a/crates/squawk_linter/src/rules/ban_char_field.rs +++ b/crates/squawk_linter/src/rules/ban_char_field.rs @@ -4,7 +4,7 @@ use squawk_syntax::{ }; use crate::prefer_big_int::check_not_allowed_types; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; fn is_char_type(x: TokenText<'_>) -> bool { if x == "char" || x == "character" || x == "bpchar" { diff --git a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs index ac4c96c5..13e72add 100644 --- a/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs +++ b/crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_concurrent_index_creation_in_transaction( ctx: &mut Linter, diff --git a/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs b/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs index b424269a..7866aad9 100644 --- a/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs +++ b/crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs @@ -4,7 +4,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_create_domain_with_constraint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/ban_drop_column.rs b/crates/squawk_linter/src/rules/ban_drop_column.rs index 165d6141..fecbc7a5 100644 --- a/crates/squawk_linter/src/rules/ban_drop_column.rs +++ b/crates/squawk_linter/src/rules/ban_drop_column.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_drop_column(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/ban_drop_database.rs b/crates/squawk_linter/src/rules/ban_drop_database.rs index 7fae68ff..0a0497dc 100644 --- a/crates/squawk_linter/src/rules/ban_drop_database.rs +++ b/crates/squawk_linter/src/rules/ban_drop_database.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; /// Brad's Rule aka ban dropping database statements. pub(crate) fn ban_drop_database(ctx: &mut Linter, parse: &Parse) { diff --git a/crates/squawk_linter/src/rules/ban_drop_not_null.rs b/crates/squawk_linter/src/rules/ban_drop_not_null.rs index 8e3f61e9..e0b4896e 100644 --- a/crates/squawk_linter/src/rules/ban_drop_not_null.rs +++ b/crates/squawk_linter/src/rules/ban_drop_not_null.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_drop_not_null(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/ban_drop_table.rs b/crates/squawk_linter/src/rules/ban_drop_table.rs index 038d6680..7935660f 100644 --- a/crates/squawk_linter/src/rules/ban_drop_table.rs +++ b/crates/squawk_linter/src/rules/ban_drop_table.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn ban_drop_table(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/changing_column_type.rs b/crates/squawk_linter/src/rules/changing_column_type.rs index 0d2178f4..709cc047 100644 --- a/crates/squawk_linter/src/rules/changing_column_type.rs +++ b/crates/squawk_linter/src/rules/changing_column_type.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn changing_column_type(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs b/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs index 2854d24f..bfc49f74 100644 --- a/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs +++ b/crates/squawk_linter/src/rules/constraint_missing_not_valid.rs @@ -5,7 +5,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, Rule, Linter, Violation}; +use crate::{text::trim_quotes, Linter, Rule, Violation}; pub fn tables_created_in_transaction( assume_in_transaction: bool, diff --git a/crates/squawk_linter/src/rules/prefer_big_int.rs b/crates/squawk_linter/src/rules/prefer_big_int.rs index cc8f2c04..711d7849 100644 --- a/crates/squawk_linter/src/rules/prefer_big_int.rs +++ b/crates/squawk_linter/src/rules/prefer_big_int.rs @@ -5,7 +5,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, Rule, Linter, Violation}; +use crate::{text::trim_quotes, Linter, Rule, Violation}; use lazy_static::lazy_static; lazy_static! { @@ -111,7 +111,7 @@ pub(crate) fn prefer_big_int(ctx: &mut Linter, parse: &Parse) { mod test { use insta::assert_debug_snapshot; - use crate::{Rule, Linter}; + use crate::{Linter, Rule}; #[test] fn err() { 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 f372cce9..75222435 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_int.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use squawk_syntax::ast::AstNode; use squawk_syntax::{ast, Parse, SourceFile}; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; use crate::prefer_big_int::check_not_allowed_types; use crate::prefer_big_int::is_not_valid_int_type; @@ -38,7 +38,7 @@ pub(crate) fn prefer_bigint_over_int(ctx: &mut Linter, parse: &Parse mod test { use insta::assert_debug_snapshot; - use crate::{Rule, Linter}; + use crate::{Linter, Rule}; #[test] fn err() { 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 f54425ef..7fe28b0c 100644 --- a/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs +++ b/crates/squawk_linter/src/rules/prefer_bigint_over_smallint.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use squawk_syntax::ast::AstNode; use squawk_syntax::{ast, Parse, SourceFile}; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; use crate::prefer_big_int::check_not_allowed_types; use crate::prefer_big_int::is_not_valid_int_type; @@ -37,7 +37,7 @@ pub(crate) fn prefer_bigint_over_smallint(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); diff --git a/crates/squawk_linter/src/rules/renaming_table.rs b/crates/squawk_linter/src/rules/renaming_table.rs index 262d89bc..095d15c3 100644 --- a/crates/squawk_linter/src/rules/renaming_table.rs +++ b/crates/squawk_linter/src/rules/renaming_table.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{Rule, Linter, Violation}; +use crate::{Linter, Rule, Violation}; pub(crate) fn renaming_table(ctx: &mut Linter, parse: &Parse) { let file = parse.tree(); 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 0d2ee596..0daf1865 100644 --- a/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs +++ b/crates/squawk_linter/src/rules/require_concurrent_index_creation.rs @@ -3,7 +3,7 @@ use squawk_syntax::{ Parse, SourceFile, }; -use crate::{text::trim_quotes, Rule, Linter, Violation}; +use crate::{text::trim_quotes, Linter, Rule, Violation}; use super::constraint_missing_not_valid::tables_created_in_transaction; From 66c1de2a830b6f4c693610ae66d3ab322b14aa0a Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Mon, 5 May 2025 23:15:26 -0400 Subject: [PATCH 5/5] fix --- crates/squawk_wasm/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/squawk_wasm/src/lib.rs b/crates/squawk_wasm/src/lib.rs index dc67d7c5..cc61d6d1 100644 --- a/crates/squawk_wasm/src/lib.rs +++ b/crates/squawk_wasm/src/lib.rs @@ -110,12 +110,18 @@ pub fn lint(text: String) -> Result { let end = line_index .to_wide(line_index::WideEncoding::Utf16, end) .unwrap(); + + let messages = match x.help { + Some(help) => vec![help], + None => vec![], + }; + LintError { code: x.code.to_string(), range_start: x.text_range.start().into(), range_end: x.text_range.end().into(), message: x.message.clone(), - messages: x.messages.clone(), + messages, // parser errors should be error severity: Severity::Warning, start_line_number: start.line,