Skip to content

Commit 580a215

Browse files
psteinroepsteinroe
committed
chore: add normalize coverage test and document comment limitation
- Add test to verify all AST nodes with location fields are handled in normalize.rs - Document known limitation: comments are removed during formatting - Clean up unused scripts and files
1 parent 408cf38 commit 580a215

92 files changed

Lines changed: 3001 additions & 14937 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 860 additions & 827 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/pgls_analyser/src/lint/safety/add_serial_column.rs‎

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,16 @@ impl LinterRule for AddSerialColumn {
4646

4747
if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
4848
for cmd in &stmt.cmds {
49-
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
50-
if cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn {
51-
if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
52-
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
53-
{
54-
// Check for SERIAL types
55-
if let Some(type_name) = &col_def.type_name {
56-
let type_str = get_type_name(type_name);
57-
if is_serial_type(&type_str) {
58-
diagnostics.push(
49+
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node
50+
&& cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn
51+
&& let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
52+
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
53+
{
54+
// Check for SERIAL types
55+
if let Some(type_name) = &col_def.type_name {
56+
let type_str = get_type_name(type_name);
57+
if is_serial_type(&type_str) {
58+
diagnostics.push(
5959
LinterDiagnostic::new(
6060
rule_category!(),
6161
None,
@@ -66,26 +66,22 @@ impl LinterRule for AddSerialColumn {
6666
.detail(None, "SERIAL types require rewriting the entire table with an ACCESS EXCLUSIVE lock, blocking all reads and writes.")
6767
.note("SERIAL types cannot be added to existing tables without a full table rewrite. Consider using a non-serial type with a sequence instead."),
6868
);
69-
continue;
70-
}
71-
}
69+
continue;
70+
}
71+
}
7272

73-
// Check for GENERATED ALWAYS AS ... STORED
74-
let has_stored_generated =
75-
col_def.constraints.iter().any(|constraint| {
76-
if let Some(pgls_query::NodeEnum::Constraint(c)) =
77-
&constraint.node
78-
{
79-
c.contype()
80-
== pgls_query::protobuf::ConstrType::ConstrGenerated
81-
&& c.generated_when == "a" // 'a' = ALWAYS
82-
} else {
83-
false
84-
}
85-
});
73+
// Check for GENERATED ALWAYS AS ... STORED
74+
let has_stored_generated = col_def.constraints.iter().any(|constraint| {
75+
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node {
76+
c.contype() == pgls_query::protobuf::ConstrType::ConstrGenerated
77+
&& c.generated_when == "a" // 'a' = ALWAYS
78+
} else {
79+
false
80+
}
81+
});
8682

87-
if has_stored_generated {
88-
diagnostics.push(
83+
if has_stored_generated {
84+
diagnostics.push(
8985
LinterDiagnostic::new(
9086
rule_category!(),
9187
None,
@@ -96,8 +92,6 @@ impl LinterRule for AddSerialColumn {
9692
.detail(None, "GENERATED ... STORED columns require rewriting the entire table with an ACCESS EXCLUSIVE lock, blocking all reads and writes.")
9793
.note("GENERATED ... STORED columns cannot be added to existing tables without a full table rewrite."),
9894
);
99-
}
100-
}
10195
}
10296
}
10397
}

‎crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs‎

Lines changed: 44 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -50,31 +50,29 @@ impl LinterRule for AddingFieldWithDefault {
5050

5151
if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
5252
for cmd in &stmt.cmds {
53-
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
54-
if cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn {
55-
if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
56-
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
57-
{
58-
let has_default = col_def.constraints.iter().any(|constraint| {
59-
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node
60-
{
61-
c.contype() == pgls_query::protobuf::ConstrType::ConstrDefault
62-
} else {
63-
false
64-
}
65-
});
53+
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node
54+
&& cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn
55+
&& let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
56+
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
57+
{
58+
let has_default = col_def.constraints.iter().any(|constraint| {
59+
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node {
60+
c.contype() == pgls_query::protobuf::ConstrType::ConstrDefault
61+
} else {
62+
false
63+
}
64+
});
6665

67-
let has_generated = col_def.constraints.iter().any(|constraint| {
68-
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node
69-
{
70-
c.contype() == pgls_query::protobuf::ConstrType::ConstrGenerated
71-
} else {
72-
false
73-
}
74-
});
66+
let has_generated = col_def.constraints.iter().any(|constraint| {
67+
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node {
68+
c.contype() == pgls_query::protobuf::ConstrType::ConstrGenerated
69+
} else {
70+
false
71+
}
72+
});
7573

76-
if has_generated {
77-
diagnostics.push(
74+
if has_generated {
75+
diagnostics.push(
7876
LinterDiagnostic::new(
7977
rule_category!(),
8078
None,
@@ -85,23 +83,26 @@ impl LinterRule for AddingFieldWithDefault {
8583
.detail(None, "This operation requires an ACCESS EXCLUSIVE lock and rewrites the entire table.")
8684
.note("Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead."),
8785
);
88-
} else if has_default {
89-
// For PG 11+, check if the default is volatile
90-
if pg_version.is_some_and(|v| v >= 11) {
91-
// Check if default is non-volatile
92-
let is_safe_default = col_def.constraints.iter().any(|constraint| {
93-
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node {
94-
if c.contype() == pgls_query::protobuf::ConstrType::ConstrDefault {
95-
if let Some(raw_expr) = &c.raw_expr {
96-
return is_safe_default_expr(&raw_expr.node.as_ref().map(|n| Box::new(n.clone())), ctx.schema_cache());
97-
}
98-
}
99-
}
100-
false
101-
});
86+
} else if has_default {
87+
// For PG 11+, check if the default is volatile
88+
if pg_version.is_some_and(|v| v >= 11) {
89+
// Check if default is non-volatile
90+
let is_safe_default = col_def.constraints.iter().any(|constraint| {
91+
if let Some(pgls_query::NodeEnum::Constraint(c)) = &constraint.node
92+
&& c.contype()
93+
== pgls_query::protobuf::ConstrType::ConstrDefault
94+
&& let Some(raw_expr) = &c.raw_expr
95+
{
96+
return is_safe_default_expr(
97+
&raw_expr.node.as_ref().map(|n| Box::new(n.clone())),
98+
ctx.schema_cache(),
99+
);
100+
}
101+
false
102+
});
102103

103-
if !is_safe_default {
104-
diagnostics.push(
104+
if !is_safe_default {
105+
diagnostics.push(
105106
LinterDiagnostic::new(
106107
rule_category!(),
107108
None,
@@ -112,10 +113,10 @@ impl LinterRule for AddingFieldWithDefault {
112113
.detail(None, "Even in PostgreSQL 11+, volatile default values require a full table rewrite.")
113114
.note("Add the column without a default, then set the default in a separate statement."),
114115
);
115-
}
116-
} else {
117-
// Pre PG 11, all defaults cause rewrites
118-
diagnostics.push(
116+
}
117+
} else {
118+
// Pre PG 11, all defaults cause rewrites
119+
diagnostics.push(
119120
LinterDiagnostic::new(
120121
rule_category!(),
121122
None,
@@ -126,8 +127,6 @@ impl LinterRule for AddingFieldWithDefault {
126127
.detail(None, "This operation requires an ACCESS EXCLUSIVE lock and rewrites the entire table.")
127128
.note("Add the column without a default, then set the default in a separate statement."),
128129
);
129-
}
130-
}
131130
}
132131
}
133132
}

‎crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@ impl LinterRule for AddingForeignKeyConstraint {
5656
pgls_query::protobuf::AlterTableType::AtAddConstraint => {
5757
if let Some(pgls_query::NodeEnum::Constraint(constraint)) =
5858
cmd.def.as_ref().and_then(|d| d.node.as_ref())
59-
{
60-
if let Some(diagnostic) =
59+
&& let Some(diagnostic) =
6160
check_foreign_key_constraint(constraint, false)
62-
{
63-
diagnostics.push(diagnostic);
64-
}
61+
{
62+
diagnostics.push(diagnostic);
6563
}
6664
}
6765
pgls_query::protobuf::AlterTableType::AtAddColumn => {
@@ -72,12 +70,10 @@ impl LinterRule for AddingForeignKeyConstraint {
7270
for constraint in &col_def.constraints {
7371
if let Some(pgls_query::NodeEnum::Constraint(constr)) =
7472
&constraint.node
75-
{
76-
if let Some(diagnostic) =
73+
&& let Some(diagnostic) =
7774
check_foreign_key_constraint(constr, true)
78-
{
79-
diagnostics.push(diagnostic);
80-
}
75+
{
76+
diagnostics.push(diagnostic);
8177
}
8278
}
8379
}

‎crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,17 @@ impl LinterRule for AddingNotNullField {
5858

5959
if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
6060
for cmd in &stmt.cmds {
61-
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
62-
if cmd.subtype() == pgls_query::protobuf::AlterTableType::AtSetNotNull {
63-
diagnostics.push(LinterDiagnostic::new(
61+
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node
62+
&& cmd.subtype() == pgls_query::protobuf::AlterTableType::AtSetNotNull
63+
{
64+
diagnostics.push(LinterDiagnostic::new(
6465
rule_category!(),
6566
None,
6667
markup! {
6768
"Setting a column NOT NULL blocks reads while the table is scanned."
6869
},
6970
).detail(None, "This operation requires an ACCESS EXCLUSIVE lock and a full table scan to verify all rows.")
7071
.note("Use a CHECK constraint with NOT VALID instead, then validate it in a separate transaction."));
71-
}
7272
}
7373
}
7474
}

‎crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,10 @@ impl LinterRule for AddingPrimaryKeyConstraint {
5454
pgls_query::protobuf::AlterTableType::AtAddConstraint => {
5555
if let Some(pgls_query::NodeEnum::Constraint(constraint)) =
5656
cmd.def.as_ref().and_then(|d| d.node.as_ref())
57-
{
58-
if let Some(diagnostic) =
57+
&& let Some(diagnostic) =
5958
check_for_primary_key_constraint(constraint)
60-
{
61-
diagnostics.push(diagnostic);
62-
}
59+
{
60+
diagnostics.push(diagnostic);
6361
}
6462
}
6563
// Check for ADD COLUMN with PRIMARY KEY
@@ -70,12 +68,10 @@ impl LinterRule for AddingPrimaryKeyConstraint {
7068
for constraint in &col_def.constraints {
7169
if let Some(pgls_query::NodeEnum::Constraint(constr)) =
7270
&constraint.node
73-
{
74-
if let Some(diagnostic) =
71+
&& let Some(diagnostic) =
7572
check_for_primary_key_constraint(constr)
76-
{
77-
diagnostics.push(diagnostic);
78-
}
73+
{
74+
diagnostics.push(diagnostic);
7975
}
8076
}
8177
}

‎crates/pgls_analyser/src/lint/safety/adding_required_field.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ impl LinterRule for AddingRequiredField {
4242
}
4343

4444
for cmd in &stmt.cmds {
45-
if let Some(pgls_query::NodeEnum::AlterTableCmd(alter_table_cmd)) = &cmd.node {
46-
if alter_table_cmd.subtype()
45+
if let Some(pgls_query::NodeEnum::AlterTableCmd(alter_table_cmd)) = &cmd.node
46+
&& alter_table_cmd.subtype()
4747
== pgls_query::protobuf::AlterTableType::AtAddColumn
48-
{
49-
diagnostics.push(
48+
{
49+
diagnostics.push(
5050
LinterDiagnostic::new(
5151
rule_category!(),
5252
None,
@@ -60,7 +60,6 @@ impl LinterRule for AddingRequiredField {
6060
",
6161
),
6262
);
63-
}
6463
}
6564
}
6665
}

‎crates/pgls_analyser/src/lint/safety/ban_char_field.rs‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,27 +49,24 @@ impl LinterRule for BanCharField {
4949

5050
if let pgls_query::NodeEnum::CreateStmt(stmt) = &ctx.stmt() {
5151
for table_elt in &stmt.table_elts {
52-
if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) = &table_elt.node {
53-
if let Some(diagnostic) = check_column_for_char_type(col_def) {
54-
diagnostics.push(diagnostic);
55-
}
52+
if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) = &table_elt.node
53+
&& let Some(diagnostic) = check_column_for_char_type(col_def)
54+
{
55+
diagnostics.push(diagnostic);
5656
}
5757
}
5858
}
5959

6060
// Also check ALTER TABLE ADD COLUMN statements
6161
if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
6262
for cmd in &stmt.cmds {
63-
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node {
64-
if cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn {
65-
if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
66-
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
67-
{
68-
if let Some(diagnostic) = check_column_for_char_type(col_def) {
69-
diagnostics.push(diagnostic);
70-
}
71-
}
72-
}
63+
if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node
64+
&& cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddColumn
65+
&& let Some(pgls_query::NodeEnum::ColumnDef(col_def)) =
66+
&cmd.def.as_ref().and_then(|d| d.node.as_ref())
67+
&& let Some(diagnostic) = check_column_for_char_type(col_def)
68+
{
69+
diagnostics.push(diagnostic);
7370
}
7471
}
7572
}

0 commit comments

Comments
 (0)