feat(linting): customize severity per rule (#430)

Author: juleswritescode

Cargo.lock

@@ -24,6 +24,7 @@ macro_rules! declare_lint_rule {
     ( $( #[doc = $doc:literal] )+ $vis:vis $id:ident {
         version: $version:literal,
         name: $name:tt,
+        severity: $severity:expr_2021,
         $( $key:ident: $value:expr_2021, )*
     } ) => {
 
@@ -32,6 +33,7 @@ macro_rules! declare_lint_rule {
             $vis $id {
                 version: $version,
                 name: $name,
+                severity: $severity,
                 $( $key: $value, )*
             }
         );
@@ -53,6 +55,7 @@ macro_rules! declare_rule {
         ( $( #[doc = $doc:literal] )+ $vis:vis $id:ident {
         version: $version:literal,
         name: $name:tt,
+        severity: $severity:expr_2021,
         $( $key:ident: $value:expr_2021, )*
     } ) => {
         $( #[doc = $doc] )*
@@ -61,7 +64,7 @@ macro_rules! declare_rule {
         impl $crate::RuleMeta for $id {
             type Group = super::Group;
             const METADATA: $crate::RuleMetadata =
-                $crate::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )) $( .$key($value) )*;
+                $crate::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* ), $severity) $( .$key($value) )*;
         }
     }
 }

crates/pgt_analyse/src/macros.rs

@@ -3,7 +3,7 @@ use pgt_console::{MarkupBuf, markup};
 use pgt_diagnostics::advice::CodeSuggestionAdvice;
 use pgt_diagnostics::{
     Advices, Category, Diagnostic, DiagnosticTags, Location, LogCategory, MessageAndDescription,
-    Visit,
+    Severity, Visit,
 };
 use pgt_text_size::TextRange;
 use std::cmp::Ordering;
@@ -31,17 +31,25 @@ pub struct RuleMetadata {
     pub recommended: bool,
     /// The source URL of the rule
     pub sources: &'static [RuleSource],
+    /// The default severity of the rule
+    pub severity: Severity,
 }
 
 impl RuleMetadata {
-    pub const fn new(version: &'static str, name: &'static str, docs: &'static str) -> Self {
+    pub const fn new(
+        version: &'static str,
+        name: &'static str,
+        docs: &'static str,
+        severity: Severity,
+    ) -> Self {
         Self {
             deprecated: None,
             version,
             name,
             docs,
             sources: &[],
             recommended: false,
+            severity,
         }
     }
 

crates/pgt_analyse/src/rule.rs

@@ -54,9 +54,11 @@ Let's say we want to create a new **lint** rule called `useMyRuleName`, follow t
 1. Run the command
 
    ```shell
-   just new-lintrule safety useMyRuleName
+   just new-lintrule safety useMyRuleName (<severity>)
    ```
 
+   Where severity is optional but can be "info", "warn", or "error" (default).
+
    The script will generate a bunch of files inside the `pgt_analyser` crate.
    Among the other files, you'll find a file called `use_my_new_rule_name.rs` inside the `pgt_analyser/lib/src/lint/safety` folder. You'll implement your rule in this file.
 
@@ -187,6 +189,7 @@ declare_lint_rule! {
     pub(crate) ExampleRule {
         version: "next",
         name: "myRuleName",
+        severity: Severity::Error,
         recommended: false,
     }
 }
@@ -206,6 +209,7 @@ declare_lint_rule! {
     pub(crate) ExampleRule {
         version: "next",
         name: "myRuleName",
+        severity: Severity::Error,
         recommended: false,
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }
@@ -228,6 +232,7 @@ declare_lint_rule! {
     pub(crate) ExampleRule {
         version: "next",
         name: "myRuleName",
+        severity: Severity::Error,
         recommended: false,
     }
 }
@@ -280,6 +285,7 @@ declare_lint_rule! {
         version: "next",
         name: "banDropColumn",
         recommended: true,
+        severity: Severity::Error,
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }
 }
@@ -351,6 +357,7 @@ declare_lint_rule! {
         version: "next",
         name: "banDropColumn",
         recommended: true,
+        severity: Severity::Error,
         deprecated: true,
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }

crates/pgt_analyser/CONTRIBUTING.md

@@ -12,10 +12,11 @@ repository.workspace = true
 version              = "0.0.0"
 
 [dependencies]
-pgt_analyse   = { workspace = true }
-pgt_console   = { workspace = true }
-pgt_query_ext = { workspace = true }
-serde         = { workspace = true }
+pgt_analyse     = { workspace = true }
+pgt_console     = { workspace = true }
+pgt_diagnostics = { workspace = true }
+pgt_query_ext   = { workspace = true }
+serde           = { workspace = true }
 
 [dev-dependencies]
 insta           = { version = "1.42.1" }

crates/pgt_analyser/Cargo.toml

@@ -3,6 +3,8 @@
 use pgt_analyse::declare_lint_group;
 pub mod adding_required_field;
 pub mod ban_drop_column;
+pub mod ban_drop_database;
 pub mod ban_drop_not_null;
 pub mod ban_drop_table;
-declare_lint_group! { pub Safety { name : "safety" , rules : [self :: adding_required_field :: AddingRequiredField , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable ,] } }
+pub mod ban_truncate_cascade;
+declare_lint_group! { pub Safety { name : "safety" , rules : [self :: adding_required_field :: AddingRequiredField , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_database :: BanDropDatabase , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable , self :: ban_truncate_cascade :: BanTruncateCascade ,] } }

crates/pgt_analyser/src/lint/safety.rs

@@ -1,5 +1,6 @@
 use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
 use pgt_console::markup;
+use pgt_diagnostics::Severity;
 
 declare_lint_rule! {
     /// Adding a new column that is NOT NULL and has no default value to an existing table effectively makes it required.
@@ -17,6 +18,7 @@ declare_lint_rule! {
     pub AddingRequiredField {
         version: "next",
         name: "addingRequiredField",
+        severity: Severity::Error,
         recommended: false,
         sources: &[RuleSource::Squawk("adding-required-field")],
     }

crates/pgt_analyser/src/lint/safety/adding_required_field.rs

@@ -1,5 +1,6 @@
 use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
 use pgt_console::markup;
+use pgt_diagnostics::Severity;
 
 declare_lint_rule! {
     /// Dropping a column may break existing clients.
@@ -19,6 +20,7 @@ declare_lint_rule! {
     pub BanDropColumn {
         version: "next",
         name: "banDropColumn",
+        severity: Severity::Warning,
         recommended: true,
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }

crates/pgt_analyser/src/lint/safety/ban_drop_column.rs

@@ -0,0 +1,39 @@
+use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
+use pgt_console::markup;
+use pgt_diagnostics::Severity;
+
+declare_lint_rule! {
+    /// Dropping a database may break existing clients (and everything else, really).
+    ///
+    /// Make sure that you really want to drop it.
+    pub BanDropDatabase {
+        version: "next",
+        name: "banDropDatabase",
+        severity: Severity::Warning,
+        recommended: false,
+        sources: &[RuleSource::Squawk("ban-drop-database")],
+    }
+}
+
+impl Rule for BanDropDatabase {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = vec![];
+
+        if let pgt_query_ext::NodeEnum::DropdbStmt(_) = &ctx.stmt() {
+            diagnostics.push(
+                RuleDiagnostic::new(
+                    rule_category!(),
+                    None,
+                    markup! {
+                        "Dropping a database may break existing clients."
+                    },
+                )
+                .detail(None, "You probably don't want to drop your database."),
+            );
+        }
+
+        diagnostics
+    }
+}

crates/pgt_analyser/src/lint/safety/ban_drop_database.rs

@@ -1,5 +1,6 @@
 use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
 use pgt_console::markup;
+use pgt_diagnostics::Severity;
 
 declare_lint_rule! {
     /// Dropping a NOT NULL constraint may break existing clients.
@@ -18,6 +19,7 @@ declare_lint_rule! {
     pub BanDropNotNull {
         version: "next",
         name: "banDropNotNull",
+        severity: Severity::Warning,
         recommended: true,
         sources: &[RuleSource::Squawk("ban-drop-not-null")],
 

crates/pgt_analyser/src/lint/safety/ban_drop_not_null.rs

@@ -1,5 +1,6 @@
 use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
 use pgt_console::markup;
+use pgt_diagnostics::Severity;
 
 declare_lint_rule! {
     /// Dropping a table may break existing clients.
@@ -18,6 +19,7 @@ declare_lint_rule! {
     pub BanDropTable {
         version: "next",
         name: "banDropTable",
+        severity: Severity::Warning,
         recommended: true,
         sources: &[RuleSource::Squawk("ban-drop-table")],
     }

crates/pgt_analyser/src/lint/safety/ban_drop_table.rs

@@ -0,0 +1,51 @@
+use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
+use pgt_console::markup;
+use pgt_diagnostics::Severity;
+use pgt_query_ext::protobuf::DropBehavior;
+
+declare_lint_rule! {
+    /// Using `TRUNCATE`'s `CASCADE` option will truncate any tables that are also foreign-keyed to the specified tables.
+    ///
+    /// So if you had tables with foreign-keys like:
+    ///
+    /// `a <- b <- c`
+    ///
+    /// and ran:
+    ///
+    /// `truncate a cascade;`
+    ///
+    /// You'd end up with a, b, & c all being truncated!
+    ///
+    /// Instead, you can manually specify the tables you want.
+    ///
+    /// `truncate a, b;`
+    pub BanTruncateCascade {
+        version: "next",
+        name: "banTruncateCascade",
+        severity: Severity::Error,
+        recommended: false,
+        sources: &[RuleSource::Squawk("ban-truncate-cascade")],
+    }
+}
+
+impl Rule for BanTruncateCascade {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = Vec::new();
+
+        if let pgt_query_ext::NodeEnum::TruncateStmt(stmt) = &ctx.stmt() {
+            if stmt.behavior() == DropBehavior::DropCascade {
+                diagnostics.push(RuleDiagnostic::new(
+                            rule_category!(),
+                            None,
+                            markup! {
+                                "The `CASCADE` option will also truncate any tables that are foreign-keyed to the specified tables."
+                            },
+                        ).detail(None, "Do not use the `CASCADE` option. Instead, specify manually what you want: `TRUNCATE a, b;`."));
+            }
+        }
+
+        diagnostics
+    }
+}

crates/pgt_analyser/src/lint/safety/ban_truncate_cascade.rs

@@ -5,6 +5,10 @@ pub type AddingRequiredField =
     <lint::safety::adding_required_field::AddingRequiredField as pgt_analyse::Rule>::Options;
 pub type BanDropColumn =
     <lint::safety::ban_drop_column::BanDropColumn as pgt_analyse::Rule>::Options;
+pub type BanDropDatabase =
+    <lint::safety::ban_drop_database::BanDropDatabase as pgt_analyse::Rule>::Options;
 pub type BanDropNotNull =
     <lint::safety::ban_drop_not_null::BanDropNotNull as pgt_analyse::Rule>::Options;
 pub type BanDropTable = <lint::safety::ban_drop_table::BanDropTable as pgt_analyse::Rule>::Options;
+pub type BanTruncateCascade =
+    <lint::safety::ban_truncate_cascade::BanTruncateCascade as pgt_analyse::Rule>::Options;

crates/pgt_analyser/src/options.rs

@@ -0,0 +1,2 @@
+-- expect_only_lint/safety/banDropDatabase
+drop database all_users;

crates/pgt_analyser/tests/specs/safety/banDropDatabase/basic.sql

@@ -0,0 +1,16 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+---
+# Input
+```
+-- expect_only_lint/safety/banDropDatabase
+drop database all_users;
+```
+
+# Diagnostics
+lint/safety/banDropDatabase ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Dropping a database may break existing clients.
+  
+  i You probably don't want to drop your database.

crates/pgt_analyser/tests/specs/safety/banDropDatabase/basic.sql.snap

@@ -0,0 +1,2 @@
+-- expect_only_lint/safety/banTruncateCascade
+truncate a cascade;

crates/pgt_analyser/tests/specs/safety/banTruncateCascade/basic.sql

@@ -0,0 +1,16 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+---
+# Input
+```
+-- expect_only_lint/safety/banTruncateCascade
+truncate a cascade;
+```
+
+# Diagnostics
+lint/safety/banTruncateCascade ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × The `CASCADE` option will also truncate any tables that are foreign-keyed to the specified tables.
+  
+  i Do not use the `CASCADE` option. Instead, specify manually what you want: `TRUNCATE a, b;`.