linter: simplify violation creation

crates/squawk_linter/src/ignore.rs

@@ -102,7 +102,6 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
                                 Rule::UnusedIgnore,
                                 format!("unknown name {trimmed}"),
                                 range,
-                                None,
                             ));
                         }
 

crates/squawk_linter/src/lib.rs

@@ -219,16 +219,6 @@ impl fmt::Display for Rule {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Violation {
-    // TODO: should this be String instead?
-    pub code: Rule,
-    pub message: String,
-    pub text_range: TextRange,
-    pub help: Option<String>,
-    pub fix: Option<Fix>,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Fix {
     pub title: String,
@@ -258,14 +248,19 @@ impl Edit {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Violation {
+    // TODO: should this be String instead?
+    pub code: Rule,
+    pub message: String,
+    pub text_range: TextRange,
+    pub help: Option<String>,
+    pub fix: Option<Fix>,
+}
+
 impl Violation {
     #[must_use]
-    pub fn for_node(
-        code: Rule,
-        message: String,
-        node: &SyntaxNode,
-        help: impl Into<Option<String>>,
-    ) -> Self {
+    pub fn for_node(code: Rule, message: String, node: &SyntaxNode) -> Self {
         let range = node.text_range();
 
         let start = node
@@ -280,31 +275,30 @@ impl Violation {
             code,
             text_range: TextRange::new(start, range.end()),
             message,
-            help: help.into(),
+            help: None,
             fix: None,
         }
     }
 
     #[must_use]
-    pub fn for_range(
-        code: Rule,
-        message: String,
-        text_range: TextRange,
-        help: impl Into<Option<String>>,
-    ) -> Self {
+    pub fn for_range(code: Rule, message: String, text_range: TextRange) -> Self {
         Self {
             code,
             text_range,
             message,
-            help: help.into(),
+            help: None,
             fix: None,
         }
     }
 
-    fn with_fix(mut self, fix: Option<Fix>) -> Violation {
+    fn fix(mut self, fix: Option<Fix>) -> Violation {
         self.fix = fix;
         self
     }
+    fn help(mut self, help: impl Into<String>) -> Violation {
+        self.help = Some(help.into());
+        self
+    }
 }
 
 #[derive(Default)]

crates/squawk_linter/src/rules/adding_field_with_default.rs

@@ -73,20 +73,24 @@ pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse<SourceFi
                                 if is_const_expr(&expr) || is_non_volatile(&expr) {
                                     continue;
                                 }
-                                ctx.report(Violation::for_node(
-                                    Rule::AddingFieldWithDefault,
-                                    message.into(),
-                                    expr.syntax(),
-                                    help.to_string(),
-                                ))
+                                ctx.report(
+                                    Violation::for_node(
+                                        Rule::AddingFieldWithDefault,
+                                        message.into(),
+                                        expr.syntax(),
+                                    )
+                                    .help(help),
+                                )
                             }
                             ast::Constraint::GeneratedConstraint(generated) => {
-                                ctx.report(Violation::for_node(
-                                    Rule::AddingFieldWithDefault,
-                                    message.into(),
-                                    generated.syntax(),
-                                    help.to_string(),
-                                ));
+                                ctx.report(
+                                    Violation::for_node(
+                                        Rule::AddingFieldWithDefault,
+                                        message.into(),
+                                        generated.syntax(),
+                                    )
+                                    .help(help),
+                                );
                             }
                             _ => (),
                         }

crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs

@@ -25,12 +25,14 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
                                 ast::Constraint::ForeignKeyConstraint(_)
                                     | ast::Constraint::ReferencesConstraint(_)
                             ) {
-                                ctx.report(Violation::for_node(
-                                    Rule::AddingForeignKeyConstraint,
-                                    message.into(),
-                                    constraint.syntax(),
-                                    help.to_string(),
-                                ))
+                                ctx.report(
+                                    Violation::for_node(
+                                        Rule::AddingForeignKeyConstraint,
+                                        message.into(),
+                                        constraint.syntax(),
+                                    )
+                                    .help(help),
+                                )
                             }
                         }
                     }
@@ -41,12 +43,14 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
                                 ast::Constraint::ForeignKeyConstraint(_)
                                     | ast::Constraint::ReferencesConstraint(_)
                             ) {
-                                ctx.report(Violation::for_node(
-                                    Rule::AddingForeignKeyConstraint,
-                                    message.into(),
-                                    constraint.syntax(),
-                                    help.to_string(),
-                                ))
+                                ctx.report(
+                                    Violation::for_node(
+                                        Rule::AddingForeignKeyConstraint,
+                                        message.into(),
+                                        constraint.syntax(),
+                                    )
+                                    .help(help),
+                                )
                             }
                         }
                     }

crates/squawk_linter/src/rules/adding_not_null_field.rs

@@ -21,9 +21,7 @@ pub(crate) fn adding_not_null_field(ctx: &mut Linter, parse: &Parse<SourceFile>)
                             "Setting a column `NOT NULL` blocks reads while the table is scanned."
                                 .into(),
                             option.syntax(),
-                            "Make the field nullable and use a `CHECK` constraint instead."
-                                .to_string(),
-                        ));
+                        ).help("Make the field nullable and use a `CHECK` constraint instead."));
                     }
                 }
             }

crates/squawk_linter/src/rules/adding_primary_key_constraint.rs

@@ -18,12 +18,14 @@ pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
                             add_constraint.constraint()
                         {
                             if primary_key_constraint.using_index().is_none() {
-                                ctx.report(Violation::for_node(
-                                    Rule::AddingSerialPrimaryKeyField,
-                                    message.to_string(),
-                                    primary_key_constraint.syntax(),
-                                    help.to_string(),
-                                ));
+                                ctx.report(
+                                    Violation::for_node(
+                                        Rule::AddingSerialPrimaryKeyField,
+                                        message.to_string(),
+                                        primary_key_constraint.syntax(),
+                                    )
+                                    .help(help),
+                                );
                             }
                         }
                     }
@@ -33,12 +35,14 @@ pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
                                 constraint
                             {
                                 if primary_key_constraint.using_index().is_none() {
-                                    ctx.report(Violation::for_node(
-                                        Rule::AddingSerialPrimaryKeyField,
-                                        message.to_string(),
-                                        primary_key_constraint.syntax(),
-                                        help.to_string(),
-                                    ));
+                                    ctx.report(
+                                        Violation::for_node(
+                                            Rule::AddingSerialPrimaryKeyField,
+                                            message.to_string(),
+                                            primary_key_constraint.syntax(),
+                                        )
+                                        .help(help),
+                                    );
                                 }
                             }
                         }

crates/squawk_linter/src/rules/adding_required_field.rs

@@ -19,8 +19,7 @@ pub(crate) fn adding_required_field(ctx: &mut Linter, parse: &Parse<SourceFile>)
                             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(),
-                            "Make the field nullable or add a non-VOLATILE DEFAULT".to_string(),
-                        ));
+                        ).help("Make the field nullable or add a non-VOLATILE DEFAULT"));
                     }
                 }
             }

crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs

@@ -16,7 +16,6 @@ pub(crate) fn ban_alter_domain_with_add_constraint(ctx: &mut Linter, parse: &Par
                     Rule::BanAlterDomainWithAddConstraint,
                         "Domains with constraints have poor support for online migrations. Use table and column constraints instead.".into(),
                         add_constraint.syntax(),
-                        None,
                     ))
             }
         }

crates/squawk_linter/src/rules/ban_char_field.rs

@@ -24,7 +24,6 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) {
                 Rule::BanCharField,
                 "Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
                 path_type.syntax(),
-                None,
             ));
         }
     }
@@ -36,7 +35,6 @@ fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) {
             Rule::BanCharField,
             "Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.".into(),
             char_type.syntax(),
-            None,
         ));
     }
 }

crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs

@@ -26,8 +26,7 @@ pub(crate) fn ban_concurrent_index_creation_in_transaction(
                             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(),
-                            "Build the index outside any transactions.".to_string(),
-                        ));
+                        ).help("Build the index outside any transactions."));
                     }
                 }
             }
@@ -45,7 +44,10 @@ pub(crate) fn ban_concurrent_index_creation_in_transaction(
 mod test {
     use insta::assert_debug_snapshot;
 
-    use crate::{Rule, test_utils::{lint, lint_with_assume_in_transaction}};
+    use crate::{
+        Rule,
+        test_utils::{lint, lint_with_assume_in_transaction},
+    };
 
     #[test]
     fn ban_concurrent_index_creation_in_transaction_err() {
@@ -77,7 +79,8 @@ mod test {
   CREATE UNIQUE INDEX CONCURRENTLY "field_name_idx" ON "table_name" ("field_name");
   ALTER TABLE "table_name" ADD CONSTRAINT "field_name_id" UNIQUE USING INDEX "field_name_idx";
     "#;
-        let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
+        let errors =
+            lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
         assert_ne!(errors.len(), 0);
         assert_debug_snapshot!(errors);
     }
@@ -88,7 +91,8 @@ mod test {
   -- run index creation in a standalone migration
   CREATE UNIQUE INDEX CONCURRENTLY "field_name_idx" ON "table_name" ("field_name");
         "#;
-        let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
+        let errors =
+            lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
         assert_eq!(errors.len(), 0);
     }
 
@@ -101,7 +105,8 @@ mod test {
   BEGIN;
   ALTER TABLE "table_name" ADD CONSTRAINT "field_name_id" UNIQUE USING INDEX "field_name_idx";
     "#;
-        let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
+        let errors =
+            lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
         assert_eq!(errors.len(), 0);
     }
 }

crates/squawk_linter/src/rules/ban_create_domain_with_constraint.rs

@@ -27,7 +27,6 @@ pub(crate) fn ban_create_domain_with_constraint(ctx: &mut Linter, parse: &Parse<
                 Rule::BanCreateDomainWithConstraint,
                     "Domains with constraints have poor support for online migrations. Use table and column constraints instead.".into(),
                     range,
-                    None,
                 ))
             }
         }

crates/squawk_linter/src/rules/ban_drop_column.rs

@@ -15,7 +15,6 @@ pub(crate) fn ban_drop_column(ctx: &mut Linter, parse: &Parse<SourceFile>) {
                         Rule::BanDropColumn,
                         "Dropping a column may break existing clients.".into(),
                         drop_column.syntax(),
-                        None,
                     ));
                 }
             }

crates/squawk_linter/src/rules/ban_drop_database.rs

@@ -14,7 +14,6 @@ pub(crate) fn ban_drop_database(ctx: &mut Linter, parse: &Parse<SourceFile>) {
                 Rule::BanDropDatabase,
                 "Dropping a database may break existing clients.".into(),
                 drop_database.syntax(),
-                None,
             ));
         }
     }

crates/squawk_linter/src/rules/ban_drop_not_null.rs

@@ -18,7 +18,6 @@ pub(crate) fn ban_drop_not_null(ctx: &mut Linter, parse: &Parse<SourceFile>) {
                             Rule::BanDropNotNull,
                             "Dropping a `NOT NULL` constraint may break existing clients.".into(),
                             drop_not_null.syntax(),
-                            None,
                         ));
                     }
                 }

crates/squawk_linter/src/rules/ban_drop_table.rs

@@ -13,7 +13,6 @@ pub(crate) fn ban_drop_table(ctx: &mut Linter, parse: &Parse<SourceFile>) {
                 Rule::BanDropTable,
                 "Dropping a table may break existing clients.".into(),
                 drop_table.syntax(),
-                None,
             ));
         }
     }

crates/squawk_linter/src/rules/ban_truncate_cascade.rs

@@ -14,8 +14,7 @@ pub(crate) fn ban_truncate_cascade(ctx: &mut Linter, parse: &Parse<SourceFile>)
                     Rule::BanTruncateCascade,
                     "Using `CASCADE` will recursively truncate any tables that foreign key to the referenced tables! So if you had foreign keys setup as `a <- b <- c` and truncated `a`, then `b` & `c` would also be truncated!".to_string(),
                     cascade.text_range(),
-                    "Remove the `CASCADE` and specify exactly which tables you want to truncate.".to_string(),
-                ));
+                ).help("Remove the `CASCADE` and specify exactly which tables you want to truncate."));
             }
         }
     }

crates/squawk_linter/src/rules/changing_column_type.rs

@@ -16,7 +16,6 @@ pub(crate) fn changing_column_type(ctx: &mut Linter, parse: &Parse<SourceFile>)
                             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(),
-                            None,
                         ));
                     }
                 }

crates/squawk_linter/src/rules/constraint_missing_not_valid.rs

@@ -61,8 +61,7 @@ fn not_valid_validate_in_transaction(
                                         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(),
-                                        "Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction.".to_string(),
-                                    ))
+                                    ).help("Add constraint as `NOT VALID` in one transaction and `VALIDATE CONSTRAINT` in a separate transaction."))
                                 }
                             }
                         }
@@ -131,8 +130,7 @@ pub(crate) fn constraint_missing_not_valid(ctx: &mut Linter, parse: &Parse<Sourc
                             Rule::ConstraintMissingNotValid,
                             "By default new constraints require a table scan and block writes to the table while that scan occurs.".into(),
                             add_constraint.syntax(),
-                            "Use `NOT VALID` with a later `VALIDATE CONSTRAINT` call.".to_string(),
-                        ));
+                        ).help("Use `NOT VALID` with a later `VALIDATE CONSTRAINT` call."));
                     }
                 }
             }
@@ -144,7 +142,10 @@ pub(crate) fn constraint_missing_not_valid(ctx: &mut Linter, parse: &Parse<Sourc
 mod test {
     use insta::assert_debug_snapshot;
 
-    use crate::{Rule, test_utils::{lint, lint_with_assume_in_transaction}};
+    use crate::{
+        Rule,
+        test_utils::{lint, lint_with_assume_in_transaction},
+    };
 
     #[test]
     fn not_valid_validate_transaction_err() {