Skip to content

feat(linting): customize severity per rule #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jun 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion crates/pgt_analyse/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, )*
} ) => {

Expand All @@ -32,6 +33,7 @@ macro_rules! declare_lint_rule {
$vis $id {
version: $version,
name: $name,
severity: $severity,
$( $key: $value, )*
}
);
Expand All @@ -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] )*
Expand All @@ -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) )*;
}
}
}
Expand Down
12 changes: 10 additions & 2 deletions crates/pgt_analyse/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}

Expand Down
9 changes: 8 additions & 1 deletion crates/pgt_analyser/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -187,6 +189,7 @@ declare_lint_rule! {
pub(crate) ExampleRule {
version: "next",
name: "myRuleName",
severity: Severity::Error,
recommended: false,
}
}
Expand All @@ -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")],
}
Expand All @@ -228,6 +232,7 @@ declare_lint_rule! {
pub(crate) ExampleRule {
version: "next",
name: "myRuleName",
severity: Severity::Error,
recommended: false,
}
}
Expand Down Expand Up @@ -280,6 +285,7 @@ declare_lint_rule! {
version: "next",
name: "banDropColumn",
recommended: true,
severity: Severity::Error,
sources: &[RuleSource::Squawk("ban-drop-column")],
}
}
Expand Down Expand Up @@ -351,6 +357,7 @@ declare_lint_rule! {
version: "next",
name: "banDropColumn",
recommended: true,
severity: Severity::Error,
deprecated: true,
sources: &[RuleSource::Squawk("ban-drop-column")],
}
Expand Down
9 changes: 5 additions & 4 deletions crates/pgt_analyser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 3 additions & 1 deletion crates/pgt_analyser/src/lint/safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ,] } }
2 changes: 2 additions & 0 deletions crates/pgt_analyser/src/lint/safety/adding_required_field.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -17,6 +18,7 @@ declare_lint_rule! {
pub AddingRequiredField {
version: "next",
name: "addingRequiredField",
severity: Severity::Error,
recommended: false,
sources: &[RuleSource::Squawk("adding-required-field")],
}
Expand Down
2 changes: 2 additions & 0 deletions crates/pgt_analyser/src/lint/safety/ban_drop_column.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -19,6 +20,7 @@ declare_lint_rule! {
pub BanDropColumn {
version: "next",
name: "banDropColumn",
severity: Severity::Warning,
recommended: true,
sources: &[RuleSource::Squawk("ban-drop-column")],
}
Expand Down
39 changes: 39 additions & 0 deletions crates/pgt_analyser/src/lint/safety/ban_drop_database.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 2 additions & 0 deletions crates/pgt_analyser/src/lint/safety/ban_drop_not_null.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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")],

Expand Down
2 changes: 2 additions & 0 deletions crates/pgt_analyser/src/lint/safety/ban_drop_table.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -18,6 +19,7 @@ declare_lint_rule! {
pub BanDropTable {
version: "next",
name: "banDropTable",
severity: Severity::Warning,
recommended: true,
sources: &[RuleSource::Squawk("ban-drop-table")],
}
Expand Down
51 changes: 51 additions & 0 deletions crates/pgt_analyser/src/lint/safety/ban_truncate_cascade.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
4 changes: 4 additions & 0 deletions crates/pgt_analyser/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- expect_only_lint/safety/banDropDatabase
drop database all_users;
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- expect_only_lint/safety/banTruncateCascade
truncate a cascade;
Original file line number Diff line number Diff line change
@@ -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;`.
Loading