Skip to content

Commit 6a1ae04

Browse files
psteinroepsteinroe
committed
progress
1 parent 55474eb commit 6a1ae04

File tree

13 files changed

+327
-17
lines changed

13 files changed

+327
-17
lines changed

‎agentic/port_squawk_rules.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ DONE:
7777
- prefer_bigint_over_int ✓ (ported from Squawk)
7878
- prefer_bigint_over_smallint ✓ (ported from Squawk)
7979
- prefer_identity ✓ (ported from Squawk)
80+
- prefer_jsonb ✓ (new rule added)
8081
- prefer_text_field ✓ (ported from Squawk)
8182
- prefer_timestamptz ✓ (ported from Squawk)
8283
- disallow_unique_constraint ✓ (ported from Squawk)

‎crates/pgt_analyse/src/rule.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,8 @@ impl RuleDiagnostic {
291291
pub enum RuleSource {
292292
/// Rules from [Squawk](https://squawkhq.com)
293293
Squawk(&'static str),
294+
/// Rules from [Eugene](https://github.com/kaaveland/eugene)
295+
Eugene(&'static str),
294296
}
295297

296298
impl PartialEq for RuleSource {
@@ -303,6 +305,7 @@ impl std::fmt::Display for RuleSource {
303305
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
304306
match self {
305307
Self::Squawk(_) => write!(f, "Squawk"),
308+
Self::Eugene(_) => write!(f, "Eugene"),
306309
}
307310
}
308311
}
@@ -325,18 +328,23 @@ impl RuleSource {
325328
pub fn as_rule_name(&self) -> &'static str {
326329
match self {
327330
Self::Squawk(rule_name) => rule_name,
331+
Self::Eugene(rule_name) => rule_name,
328332
}
329333
}
330334

331335
pub fn to_namespaced_rule_name(&self) -> String {
332336
match self {
333337
Self::Squawk(rule_name) => format!("squawk/{rule_name}"),
338+
Self::Eugene(rule_name) => format!("eugene/{rule_name}"),
334339
}
335340
}
336341

337342
pub fn to_rule_url(&self) -> String {
338343
match self {
339344
Self::Squawk(rule_name) => format!("https://squawkhq.com/docs/{rule_name}"),
345+
Self::Eugene(rule_name) => {
346+
format!("https://kaveland.no/eugene/hints/{rule_name}/index.html")
347+
}
340348
}
341349
}
342350

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub mod prefer_big_int;
2020
pub mod prefer_bigint_over_int;
2121
pub mod prefer_bigint_over_smallint;
2222
pub mod prefer_identity;
23+
pub mod prefer_jsonb;
2324
pub mod prefer_robust_stmts;
2425
pub mod prefer_text_field;
2526
pub mod prefer_timestamptz;
@@ -28,4 +29,4 @@ pub mod renaming_table;
2829
pub mod require_concurrent_index_creation;
2930
pub mod require_concurrent_index_deletion;
3031
pub mod transaction_nesting;
31-
declare_lint_group! { pub Safety { name : "safety" , rules : [self :: adding_field_with_default :: AddingFieldWithDefault , self :: adding_foreign_key_constraint :: AddingForeignKeyConstraint , self :: adding_not_null_field :: AddingNotNullField , self :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint , self :: adding_required_field :: AddingRequiredField , self :: ban_char_field :: BanCharField , self :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction , 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 , self :: changing_column_type :: ChangingColumnType , self :: constraint_missing_not_valid :: ConstraintMissingNotValid , self :: disallow_unique_constraint :: DisallowUniqueConstraint , self :: prefer_big_int :: PreferBigInt , self :: prefer_bigint_over_int :: PreferBigintOverInt , self :: prefer_bigint_over_smallint :: PreferBigintOverSmallint , self :: prefer_identity :: PreferIdentity , self :: prefer_robust_stmts :: PreferRobustStmts , self :: prefer_text_field :: PreferTextField , self :: prefer_timestamptz :: PreferTimestamptz , self :: renaming_column :: RenamingColumn , self :: renaming_table :: RenamingTable , self :: require_concurrent_index_creation :: RequireConcurrentIndexCreation , self :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion , self :: transaction_nesting :: TransactionNesting ,] } }
32+
declare_lint_group! { pub Safety { name : "safety" , rules : [self :: adding_field_with_default :: AddingFieldWithDefault , self :: adding_foreign_key_constraint :: AddingForeignKeyConstraint , self :: adding_not_null_field :: AddingNotNullField , self :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint , self :: adding_required_field :: AddingRequiredField , self :: ban_char_field :: BanCharField , self :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction , 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 , self :: changing_column_type :: ChangingColumnType , self :: constraint_missing_not_valid :: ConstraintMissingNotValid , self :: disallow_unique_constraint :: DisallowUniqueConstraint , self :: prefer_big_int :: PreferBigInt , self :: prefer_bigint_over_int :: PreferBigintOverInt , self :: prefer_bigint_over_smallint :: PreferBigintOverSmallint , self :: prefer_identity :: PreferIdentity , self :: prefer_jsonb :: PreferJsonb , self :: prefer_robust_stmts :: PreferRobustStmts , self :: prefer_text_field :: PreferTextField , self :: prefer_timestamptz :: PreferTimestamptz , self :: renaming_column :: RenamingColumn , self :: renaming_table :: RenamingTable , self :: require_concurrent_index_creation :: RequireConcurrentIndexCreation , self :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion , self :: transaction_nesting :: TransactionNesting ,] } }

‎crates/pgt_analyser/src/lint/safety/prefer_jsonb.rs‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
2+
use pgt_console::markup;
3+
use pgt_diagnostics::Severity;
4+
5+
declare_lint_rule! {
6+
/// Prefer JSONB over JSON types.
7+
///
8+
/// JSONB is the binary JSON data type in PostgreSQL that is more efficient for most use cases.
9+
/// Unlike JSON, JSONB stores data in a decomposed binary format which provides several advantages:
10+
/// - Significantly faster query performance for operations like indexing and searching
11+
/// - Support for indexing (GIN indexes)
12+
/// - Duplicate keys are automatically removed
13+
/// - Keys are sorted
14+
///
15+
/// The only reasons to use JSON instead of JSONB are:
16+
/// - You need to preserve exact input formatting (whitespace, key order)
17+
/// - You need to preserve duplicate keys
18+
/// - You have very specific performance requirements where JSON's lack of parsing overhead matters
19+
///
20+
/// ## Examples
21+
///
22+
/// ### Invalid
23+
///
24+
/// ```sql,expect_diagnostic
25+
/// CREATE TABLE users (
26+
/// data json
27+
/// );
28+
/// ```
29+
///
30+
/// ```sql,expect_diagnostic
31+
/// ALTER TABLE users ADD COLUMN metadata json;
32+
/// ```
33+
///
34+
/// ```sql,expect_diagnostic
35+
/// ALTER TABLE users ALTER COLUMN data TYPE json;
36+
/// ```
37+
///
38+
/// ### Valid
39+
///
40+
/// ```sql
41+
/// CREATE TABLE users (
42+
/// data jsonb
43+
/// );
44+
/// ```
45+
///
46+
/// ```sql
47+
/// ALTER TABLE users ADD COLUMN metadata jsonb;
48+
/// ```
49+
///
50+
/// ```sql
51+
/// ALTER TABLE users ALTER COLUMN data TYPE jsonb;
52+
/// ```
53+
///
54+
pub PreferJsonb {
55+
version: "next",
56+
name: "preferJsonb",
57+
severity: Severity::Warning,
58+
recommended: false,
59+
sources: &[RuleSource::Eugene("E3")],
60+
}
61+
}
62+
63+
impl Rule for PreferJsonb {
64+
type Options = ();
65+
66+
fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
67+
let mut diagnostics = Vec::new();
68+
69+
match &ctx.stmt() {
70+
pgt_query::NodeEnum::CreateStmt(stmt) => {
71+
for table_elt in &stmt.table_elts {
72+
if let Some(pgt_query::NodeEnum::ColumnDef(col_def)) = &table_elt.node {
73+
check_column_def(&mut diagnostics, col_def);
74+
}
75+
}
76+
}
77+
pgt_query::NodeEnum::AlterTableStmt(stmt) => {
78+
for cmd in &stmt.cmds {
79+
if let Some(pgt_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
80+
if cmd.subtype() == pgt_query::protobuf::AlterTableType::AtAddColumn
81+
|| cmd.subtype()
82+
== pgt_query::protobuf::AlterTableType::AtAlterColumnType
83+
{
84+
if let Some(pgt_query::NodeEnum::ColumnDef(col_def)) =
85+
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
86+
{
87+
check_column_def(&mut diagnostics, col_def);
88+
}
89+
}
90+
}
91+
}
92+
}
93+
_ => {}
94+
}
95+
96+
diagnostics
97+
}
98+
}
99+
100+
fn check_column_def(
101+
diagnostics: &mut Vec<RuleDiagnostic>,
102+
col_def: &pgt_query::protobuf::ColumnDef,
103+
) {
104+
let Some(type_name) = &col_def.type_name else {
105+
return;
106+
};
107+
108+
for name_node in &type_name.names {
109+
let Some(pgt_query::NodeEnum::String(name)) = &name_node.node else {
110+
continue;
111+
};
112+
113+
let type_name_lower = name.sval.to_lowercase();
114+
if type_name_lower != "json" {
115+
continue;
116+
}
117+
118+
diagnostics.push(
119+
RuleDiagnostic::new(
120+
rule_category!(),
121+
None,
122+
markup! {
123+
"Prefer JSONB over JSON for better performance and functionality."
124+
},
125+
)
126+
.detail(None, "JSON stores exact text representation while JSONB stores parsed binary format. JSONB is faster for queries, supports indexing, and removes duplicate keys.")
127+
.note("Consider using JSONB instead unless you specifically need to preserve formatting or duplicate keys."),
128+
);
129+
}
130+
}

‎crates/pgt_analyser/src/options.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub type PreferBigintOverInt =
3030
pub type PreferBigintOverSmallint = < lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint as pgt_analyse :: Rule > :: Options ;
3131
pub type PreferIdentity =
3232
<lint::safety::prefer_identity::PreferIdentity as pgt_analyse::Rule>::Options;
33+
pub type PreferJsonb = <lint::safety::prefer_jsonb::PreferJsonb as pgt_analyse::Rule>::Options;
3334
pub type PreferRobustStmts =
3435
<lint::safety::prefer_robust_stmts::PreferRobustStmts as pgt_analyse::Rule>::Options;
3536
pub type PreferTextField =

‎crates/pgt_analyser/tests/specs/safety/preferJsonb/basic.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- expect_only_lint/safety/preferJsonb
2+
-- select 1;

‎crates/pgt_configuration/src/analyser/linter/rules.rs‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ pub struct Safety {
207207
#[doc = "Prefer using IDENTITY columns over serial columns."]
208208
#[serde(skip_serializing_if = "Option::is_none")]
209209
pub prefer_identity: Option<RuleConfiguration<pgt_analyser::options::PreferIdentity>>,
210+
#[doc = "Prefer JSONB over JSON types."]
211+
#[serde(skip_serializing_if = "Option::is_none")]
212+
pub prefer_jsonb: Option<RuleConfiguration<pgt_analyser::options::PreferJsonb>>,
210213
#[doc = "Prefer statements with guards for robustness in migrations."]
211214
#[serde(skip_serializing_if = "Option::is_none")]
212215
pub prefer_robust_stmts: Option<RuleConfiguration<pgt_analyser::options::PreferRobustStmts>>,
@@ -256,6 +259,7 @@ impl Safety {
256259
"preferBigintOverInt",
257260
"preferBigintOverSmallint",
258261
"preferIdentity",
262+
"preferJsonb",
259263
"preferRobustStmts",
260264
"preferTextField",
261265
"preferTimestamptz",
@@ -303,6 +307,7 @@ impl Safety {
303307
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]),
304308
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]),
305309
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]),
310+
RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27]),
306311
];
307312
#[doc = r" Retrieves the recommended rules"]
308313
pub(crate) fn is_recommended_true(&self) -> bool {
@@ -414,46 +419,51 @@ impl Safety {
414419
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]));
415420
}
416421
}
417-
if let Some(rule) = self.prefer_robust_stmts.as_ref() {
422+
if let Some(rule) = self.prefer_jsonb.as_ref() {
418423
if rule.is_enabled() {
419424
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19]));
420425
}
421426
}
422-
if let Some(rule) = self.prefer_text_field.as_ref() {
427+
if let Some(rule) = self.prefer_robust_stmts.as_ref() {
423428
if rule.is_enabled() {
424429
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20]));
425430
}
426431
}
427-
if let Some(rule) = self.prefer_timestamptz.as_ref() {
432+
if let Some(rule) = self.prefer_text_field.as_ref() {
428433
if rule.is_enabled() {
429434
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]));
430435
}
431436
}
432-
if let Some(rule) = self.renaming_column.as_ref() {
437+
if let Some(rule) = self.prefer_timestamptz.as_ref() {
433438
if rule.is_enabled() {
434439
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]));
435440
}
436441
}
437-
if let Some(rule) = self.renaming_table.as_ref() {
442+
if let Some(rule) = self.renaming_column.as_ref() {
438443
if rule.is_enabled() {
439444
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]));
440445
}
441446
}
442-
if let Some(rule) = self.require_concurrent_index_creation.as_ref() {
447+
if let Some(rule) = self.renaming_table.as_ref() {
443448
if rule.is_enabled() {
444449
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]));
445450
}
446451
}
447-
if let Some(rule) = self.require_concurrent_index_deletion.as_ref() {
452+
if let Some(rule) = self.require_concurrent_index_creation.as_ref() {
448453
if rule.is_enabled() {
449454
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]));
450455
}
451456
}
452-
if let Some(rule) = self.transaction_nesting.as_ref() {
457+
if let Some(rule) = self.require_concurrent_index_deletion.as_ref() {
453458
if rule.is_enabled() {
454459
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]));
455460
}
456461
}
462+
if let Some(rule) = self.transaction_nesting.as_ref() {
463+
if rule.is_enabled() {
464+
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27]));
465+
}
466+
}
457467
index_set
458468
}
459469
pub(crate) fn get_disabled_rules(&self) -> FxHashSet<RuleFilter<'static>> {
@@ -553,46 +563,51 @@ impl Safety {
553563
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]));
554564
}
555565
}
556-
if let Some(rule) = self.prefer_robust_stmts.as_ref() {
566+
if let Some(rule) = self.prefer_jsonb.as_ref() {
557567
if rule.is_disabled() {
558568
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19]));
559569
}
560570
}
561-
if let Some(rule) = self.prefer_text_field.as_ref() {
571+
if let Some(rule) = self.prefer_robust_stmts.as_ref() {
562572
if rule.is_disabled() {
563573
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20]));
564574
}
565575
}
566-
if let Some(rule) = self.prefer_timestamptz.as_ref() {
576+
if let Some(rule) = self.prefer_text_field.as_ref() {
567577
if rule.is_disabled() {
568578
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]));
569579
}
570580
}
571-
if let Some(rule) = self.renaming_column.as_ref() {
581+
if let Some(rule) = self.prefer_timestamptz.as_ref() {
572582
if rule.is_disabled() {
573583
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]));
574584
}
575585
}
576-
if let Some(rule) = self.renaming_table.as_ref() {
586+
if let Some(rule) = self.renaming_column.as_ref() {
577587
if rule.is_disabled() {
578588
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]));
579589
}
580590
}
581-
if let Some(rule) = self.require_concurrent_index_creation.as_ref() {
591+
if let Some(rule) = self.renaming_table.as_ref() {
582592
if rule.is_disabled() {
583593
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]));
584594
}
585595
}
586-
if let Some(rule) = self.require_concurrent_index_deletion.as_ref() {
596+
if let Some(rule) = self.require_concurrent_index_creation.as_ref() {
587597
if rule.is_disabled() {
588598
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]));
589599
}
590600
}
591-
if let Some(rule) = self.transaction_nesting.as_ref() {
601+
if let Some(rule) = self.require_concurrent_index_deletion.as_ref() {
592602
if rule.is_disabled() {
593603
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]));
594604
}
595605
}
606+
if let Some(rule) = self.transaction_nesting.as_ref() {
607+
if rule.is_disabled() {
608+
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27]));
609+
}
610+
}
596611
index_set
597612
}
598613
#[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"]
@@ -641,6 +656,7 @@ impl Safety {
641656
"preferBigintOverInt" => Severity::Warning,
642657
"preferBigintOverSmallint" => Severity::Warning,
643658
"preferIdentity" => Severity::Warning,
659+
"preferJsonb" => Severity::Warning,
644660
"preferRobustStmts" => Severity::Warning,
645661
"preferTextField" => Severity::Warning,
646662
"preferTimestamptz" => Severity::Warning,
@@ -733,6 +749,10 @@ impl Safety {
733749
.prefer_identity
734750
.as_ref()
735751
.map(|conf| (conf.level(), conf.get_options())),
752+
"preferJsonb" => self
753+
.prefer_jsonb
754+
.as_ref()
755+
.map(|conf| (conf.level(), conf.get_options())),
736756
"preferRobustStmts" => self
737757
.prefer_robust_stmts
738758
.as_ref()

‎crates/pgt_diagnostics_categories/src/categories.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ define_categories! {
3232
"lint/safety/preferBigintOverInt": "https://pgtools.dev/latest/rules/prefer-bigint-over-int",
3333
"lint/safety/preferBigintOverSmallint": "https://pgtools.dev/latest/rules/prefer-bigint-over-smallint",
3434
"lint/safety/preferIdentity": "https://pgtools.dev/latest/rules/prefer-identity",
35+
"lint/safety/preferJsonb": "https://pgtools.dev/latest/rules/prefer-jsonb",
3536
"lint/safety/preferRobustStmts": "https://pgtools.dev/latest/rules/prefer-robust-stmts",
3637
"lint/safety/preferTextField": "https://pgtools.dev/latest/rules/prefer-text-field",
3738
"lint/safety/preferTimestamptz": "https://pgtools.dev/latest/rules/prefer-timestamptz",

‎docs/reference/rule_sources.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ Many rules are inspired by or directly ported from other tools. This page lists
33
## Exclusive rules
44
_No exclusive rules available._
55
## Rules from other sources
6+
### Eugene
7+
| Eugene Rule Name | Rule Name |
8+
| ---- | ---- |
9+
| [prefer-jsonb](https://github.com/kaaveland/eugene/blob/main/docs/docs/prefer-jsonb.md) |[preferJsonb](../rules/prefer-jsonb) |
610
### Squawk
711
| Squawk Rule Name | Rule Name |
812
| ---- | ---- |

‎docs/reference/rules.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Rules that detect potential safety issues in your code.
3131
| [preferBigintOverInt](./prefer-bigint-over-int) | Prefer BIGINT over INT/INTEGER types. | |
3232
| [preferBigintOverSmallint](./prefer-bigint-over-smallint) | Prefer BIGINT over SMALLINT types. | |
3333
| [preferIdentity](./prefer-identity) | Prefer using IDENTITY columns over serial columns. | |
34+
| [preferJsonb](./prefer-jsonb) | Prefer JSONB over JSON types. | |
3435
| [preferRobustStmts](./prefer-robust-stmts) | Prefer statements with guards for robustness in migrations. | |
3536
| [preferTextField](./prefer-text-field) | Prefer using TEXT over VARCHAR(n) types. | |
3637
| [preferTimestamptz](./prefer-timestamptz) | Prefer TIMESTAMPTZ over TIMESTAMP types. | |

0 commit comments

Comments
 (0)