chore: merge main

Author: psteinroe

crates/pglt_analyser/CONTRIBUTING.md

@@ -17,12 +17,14 @@ We follow a naming convention according to what the rule does:
 1. Forbid a concept
 
    ```block
-   no<Concept>
+   ban<Concept>
    ```
 
-   When a rule's sole intention is to **forbid a single concept** the rule should be named using the `no` prefix.
+   When a rule's sole intention is to **forbid a single concept** the rule should be named using the `ban` prefix.
 
-1. Mandate a concept
+   Example: "banDropColumn"
+
+2. Mandate a concept
 
    ```block
    use<Concept>
@@ -35,9 +37,10 @@ We follow a naming convention according to what the rule does:
 A rule should be informative to the user, and give as much explanation as possible.
 
 When writing a rule, you must adhere to the following **pillars**:
+
 1. Explain to the user the error. Generally, this is the message of the diagnostic.
-1. Explain to the user **why** the error is triggered. Generally, this is implemented with an additional node.
-1. Tell the user what they should do. Generally, this is implemented using a code action. If a code action is not applicable a note should tell the user what they should do to fix the error.
+2. Explain to the user **why** the error is triggered. Generally, this is implemented with an additional node.
+3. Tell the user what they should do. Generally, this is implemented using a code action. If a code action is not applicable a note should tell the user what they should do to fix the error.
 
 ### Create and implement the rule
 
@@ -53,10 +56,11 @@ Let's say we want to create a new **lint** rule called `useMyRuleName`, follow t
    ```shell
    just new-lintrule safety useMyRuleName
    ```
+
    The script will generate a bunch of files inside the `pglt_analyser` crate.
    Among the other files, you'll find a file called `use_my_new_rule_name.rs` inside the `pglt_analyser/lib/src/lint/safety` folder. You'll implement your rule in this file.
 
-1. The `Option` type doesn't have to be used, so it can be considered optional. However, it has to be defined as `type Option = ()`.
+1. The `Options` type doesn't have to be used, so it can be considered optional. However, it has to be defined as `type Options = ()`.
 1. Implement the `run` function: The function is called for every statement, and should return zero or more diagnostics. Follow the [pillars](#explain-a-rule-to-the-user) when writing the message of a diagnostic
 
 Don't forget to format your code with `just f` and lint with `just l`.
@@ -107,7 +111,7 @@ pub enum Behavior {
 ```
 
 Note that we use a boxed slice `Box<[Box<str>]>` instead of `Vec<String>`.
-This allows saving memory: [boxed slices and boxed str use 2 words instead of three words](https://nnethercote.github.io/perf-book/type-sizes.html#boxed-slices).
+This allows saving memory: [boxed slices and boxed str use two instead of three words](https://nnethercote.github.io/perf-book/type-sizes.html#boxed-slices).
 
 With these types in place, you can set the associated type `Options` of the rule:
 
@@ -127,6 +131,7 @@ The compiler should warn you that `MyRuleOptions` does not implement some requir
 We currently require implementing _serde_'s traits `Deserialize`/`Serialize`.
 
 Also, we use other `serde` macros to adjust the JSON configuration:
+
 - `rename_all = "snake_case"`: it renames all fields in camel-case, so they are in line with the naming style of the `pglsp.toml`.
 - `deny_unknown_fields`: it raises an error if the configuration contains extraneous fields.
 - `default`: it uses the `Default` value when the field is missing from `pglsp.toml`. This macro makes the field optional.
@@ -159,7 +164,6 @@ pub enum Behavior {
 
 Below, there are many tips and guidelines on how to create a lint rule using our infrastructure.
 
-
 #### `declare_lint_rule`
 
 This macro is used to declare an analyzer rule type, and implement the [RuleMeta] trait for it.
@@ -235,6 +239,7 @@ impl Rule for BanDropColumn {
 ### Document the rule
 
 The documentation needs to adhere to the following rules:
+
 - The **first** paragraph of the documentation is used as brief description of the rule, and it **must** be written in one single line. Breaking the paragraph in multiple lines will break the table content of the rules page.
 - The next paragraphs can be used to further document the rule with as many details as you see fit.
 - The documentation must have a `## Examples` header, followed by two headers: `### Invalid` and `### Valid`. `### Invalid` must go first because we need to show when the rule is triggered.
@@ -246,7 +251,7 @@ The documentation needs to adhere to the following rules:
 
 Here's an example of how the documentation could look like:
 
-```rust
+````rust
 declare_lint_rule! {
     /// Dropping a column may break existing clients.
     ///
@@ -269,7 +274,7 @@ declare_lint_rule! {
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }
 }
-```
+````
 
 This will cause the documentation generator to ensure the rule does emit
 exactly one diagnostic for this code, and to include a snapshot for the
@@ -294,15 +299,14 @@ Stage and commit your changes:
 > git commit -m 'feat(pglt_analyser): myRuleName'
 ```
 
-
 ### Deprecate a rule
 
 There are occasions when a rule must be deprecated, to avoid breaking changes. The reason
 of deprecation can be multiple.
 
 In order to do, the macro allows adding additional field to add the reason for deprecation
 
-```rust
+````rust
 use pglt_analyse::declare_lint_rule;
 
 declare_lint_rule! {
@@ -328,5 +332,4 @@ declare_lint_rule! {
         sources: &[RuleSource::Squawk("ban-drop-column")],
     }
 }
-```
-
+````

crates/pglt_analyser/src/lint/safety.rs

@@ -2,4 +2,5 @@
 
 use pglt_analyse::declare_lint_group;
 pub mod ban_drop_column;
-declare_lint_group! { pub Safety { name : "safety" , rules : [self :: ban_drop_column :: BanDropColumn ,] } }
+pub mod ban_drop_not_null;
+declare_lint_group! { pub Safety { name : "safety" , rules : [self :: ban_drop_column :: BanDropColumn , self :: ban_drop_not_null :: BanDropNotNull ,] } }

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

@@ -1,13 +1,6 @@
 use pglt_analyse::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic, RuleSource};
 use pglt_console::markup;
 
-#[derive(Clone, Debug, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
-// #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[serde(rename_all = "camelCase", deny_unknown_fields, default)]
-pub struct Options {
-    test: String,
-}
-
 declare_lint_rule! {
     /// Dropping a column may break existing clients.
     ///
@@ -19,7 +12,7 @@ declare_lint_rule! {
     ///
     /// ### Invalid
     ///
-    /// ```sql,expect_diagnostic
+    /// ```sql,ignore
     /// alter table test drop column id;
     /// ```
     ///
@@ -32,7 +25,7 @@ declare_lint_rule! {
 }
 
 impl Rule for BanDropColumn {
-    type Options = Options;
+    type Options = ();
 
     fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
         let mut diagnostics = Vec::new();
@@ -47,7 +40,7 @@ impl Rule for BanDropColumn {
                             markup! {
                                 "Dropping a column may break existing clients."
                             },
-                        ).detail(None, format!("[{}] You can leave the column as nullable or delete the column once queries no longer select or modify the column.", ctx.options().test)));
+                        ).detail(None, "You can leave the column as nullable or delete the column once queries no longer select or modify the column."));
                     }
                 }
             }

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

@@ -0,0 +1,51 @@
+use pglt_analyse::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic, RuleSource};
+use pglt_console::markup;
+
+declare_lint_rule! {
+    /// Dropping a NOT NULL constraint may break existing clients.
+    ///
+    /// Application code or code written in procedural languages like PL/SQL or PL/pgSQL may not expect NULL values for the column that was previously guaranteed to be NOT NULL and therefore may fail to process them correctly.
+    ///
+    /// You can consider using a marker value that represents NULL. Alternatively, create a new table allowing NULL values, copy the data from the old table, and create a view that filters NULL values.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```sql,ignore
+    /// alter table users alter column email drop not null;
+    /// ```
+    pub BanDropNotNull {
+        version: "next",
+        name: "banDropNotNull",
+        recommended: true,
+        sources: &[RuleSource::Squawk("ban-drop-not-null")],
+
+    }
+}
+
+impl Rule for BanDropNotNull {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = Vec::new();
+
+        if let pglt_query_ext::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
+            for cmd in &stmt.cmds {
+                if let Some(pglt_query_ext::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
+                    if cmd.subtype() == pglt_query_ext::protobuf::AlterTableType::AtDropNotNull {
+                        diagnostics.push(RuleDiagnostic::new(
+                            rule_category!(),
+                            None,
+                            markup! {
+                                "Dropping a NOT NULL constraint may break existing clients."
+                            },
+                        ).detail(None, "Consider using a marker value that represents NULL. Alternatively, create a new table allowing NULL values, copy the data from the old table, and create a view that filters NULL values."));
+                    }
+                }
+            }
+        }
+
+        diagnostics
+    }
+}

crates/pglt_analyser/src/options.rs

@@ -3,3 +3,5 @@
 use crate::lint;
 pub type BanDropColumn =
     <lint::safety::ban_drop_column::BanDropColumn as pglt_analyse::Rule>::Options;
+pub type BanDropNotNull =
+    <lint::safety::ban_drop_not_null::BanDropNotNull as pglt_analyse::Rule>::Options;

crates/pglt_cli/src/cli_options.rs

@@ -18,6 +18,10 @@ pub struct CliOptions {
     #[bpaf(long("use-server"), switch, fallback(false))]
     pub use_server: bool,
 
+    /// Skip connecting to the database and only run checks that don't require a database connection.
+    #[bpaf(long("skip-db"), switch, fallback(false))]
+    pub skip_db: bool,
+
     /// Print additional diagnostics, and some diagnostics show more information. Also, print out what files were processed and which ones were modified.
     #[bpaf(long("verbose"), switch, fallback(false))]
     pub verbose: bool,

crates/pglt_cli/src/commands/mod.rs

@@ -35,8 +35,10 @@ pub enum PgltCommand {
     Check {
         #[bpaf(external(partial_configuration), hide_usage, optional)]
         configuration: Option<PartialConfiguration>,
+
         #[bpaf(external, hide_usage)]
         cli_options: CliOptions,
+
         /// Use this option when you want to format code piped from `stdin`, and print the output to `stdout`.
         ///
         /// The file doesn't need to exist on disk, what matters is the extension of the file. Based on the extension, we know how to check the code.
@@ -286,6 +288,7 @@ pub(crate) trait CommandRunner: Sized {
             configuration,
             vcs_base_path,
             gitignore_matches,
+            skip_db: cli_options.skip_db,
         })?;
 
         let execution = self.get_execution(cli_options, console, workspace)?;

crates/pglt_cli/src/execute/process_file/check.rs

@@ -28,6 +28,7 @@ pub(crate) fn check_with_guard<'ctx>(
             let (only, skip) = (Vec::new(), Vec::new());
 
             let max_diagnostics = ctx.remaining_diagnostics.load(Ordering::Relaxed);
+
             let pull_diagnostics_result = workspace_file
                 .guard()
                 .pull_diagnostics(

crates/pglt_cli/src/execute/traverse.rs

@@ -123,6 +123,7 @@ pub(crate) fn traverse(
     let skipped = skipped.load(Ordering::Relaxed);
     let suggested_fixes_skipped = printer.skipped_fixes();
     let diagnostics_not_printed = printer.not_printed_diagnostics();
+
     Ok(TraverseResult {
         summary: TraversalSummary {
             changed,
@@ -381,6 +382,7 @@ impl<'ctx> DiagnosticsPrinter<'ctx> {
                 }
             }
         }
+
         diagnostics_to_print
     }
 }

crates/pglt_cli/src/reporter/gitlab.rs

@@ -13,8 +13,8 @@ use std::{
 };
 
 pub struct GitLabReporter {
-    pub execution: Execution,
-    pub diagnostics: DiagnosticsPayload,
+    pub(crate) execution: Execution,
+    pub(crate) diagnostics: DiagnosticsPayload,
 }
 
 impl Reporter for GitLabReporter {