feat(lint): add linter rules for migration and naming conventions

* Implement migration safety rules:
  - MIG001: Warn on CREATE INDEX without CONCURRENT.
  - MIG002: Warn on ALTER TABLE ADD COLUMN NOT NULL without DEFAULT.
  - MIG003: Warn on ALTER TABLE ADD CONSTRAINT without NOT VALID.
* Implement naming conventions rules:
  - NAM001: Warn on non-snake_case table names.
  - NAM002: Warn on non-snake_case column names in CREATE TABLE.
  - NAM003: Warn on non-snake_case function names.
* Add test fixtures for all new rules.
This commit is contained in:
2026-06-27 22:15:02 +02:00
parent 932c83dbad
commit 7fb76bae3d
16 changed files with 1032 additions and 12 deletions
+119
View File
@@ -0,0 +1,119 @@
// Package lint provides the rule engine and built-in rule packs for PgTidy.
//
// Usage:
//
// eng := lint.New()
// diags, err := eng.Check(sql, filename)
package lint
import (
"sort"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/hein/pgtidy/pkg/diagnostics"
"github.com/hein/pgtidy/pkg/pgast"
)
// Rule is implemented by each lint rule.
type Rule interface {
// ID returns the stable rule identifier (e.g. "MIG001").
ID() string
// Severity returns the default severity for findings from this rule.
Severity() diagnostics.Severity
// Check inspects the parsed statement list and returns any findings.
// src is the original SQL string, used for location lookups.
Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic
}
// Engine runs a set of rules against SQL input.
type Engine struct {
rules []Rule
}
// New returns an Engine loaded with all built-in rules.
func New() *Engine {
e := &Engine{}
e.registerAll()
return e
}
// Custom returns an empty Engine; use Register to add rules.
func Custom() *Engine {
return &Engine{}
}
func (e *Engine) registerAll() {
e.Register(ruleMIG001{})
e.Register(ruleMIG002{})
e.Register(ruleMIG003{})
e.Register(ruleCOR001{})
e.Register(ruleCOR002{})
e.Register(ruleCOR003{})
e.Register(ruleNAM001{})
e.Register(ruleNAM002{})
e.Register(ruleNAM003{})
}
// Register adds a rule to the engine.
func (e *Engine) Register(r Rule) {
e.rules = append(e.rules, r)
}
// Rules returns a copy of the registered rule slice (for testing/inspection).
func (e *Engine) Rules() []Rule {
out := make([]Rule, len(e.rules))
copy(out, e.rules)
return out
}
// Check parses sql and runs all registered rules. Diagnostics are sorted by
// (line, col, ruleID). A parse error is returned as a single diagnostic with
// ruleID "PARSE".
func (e *Engine) Check(sql, file string) ([]diagnostics.Diagnostic, error) {
result, err := pgast.Parse(sql)
if err != nil {
line, col := parseErrLocation(err, sql)
return []diagnostics.Diagnostic{{
RuleID: "PARSE",
Severity: diagnostics.SeverityError,
Message: err.Error(),
File: file,
Line: line,
Col: col,
}}, nil
}
var all []diagnostics.Diagnostic
for _, r := range e.rules {
found := r.Check(result.Stmts, sql)
for i := range found {
found[i].File = file
}
all = append(all, found...)
}
sort.Slice(all, func(i, j int) bool {
if all[i].Line != all[j].Line {
return all[i].Line < all[j].Line
}
if all[i].Col != all[j].Col {
return all[i].Col < all[j].Col
}
return all[i].RuleID < all[j].RuleID
})
return all, nil
}
// parseErrLocation extracts a (line, col) from a pg_query parse error.
// The error message format is "syntax error at or near ... (location N)" where
// N is a 1-based character offset.
func parseErrLocation(err error, sql string) (line, col int) {
// pg_query errors expose Location via a type assertion.
type locErr interface{ GetCursorPos() int }
if le, ok := err.(locErr); ok {
pos := le.GetCursorPos() - 1 // convert to 0-based
return pgast.LocationToLineCol(sql, pos)
}
return 1, 1
}
+125
View File
@@ -0,0 +1,125 @@
package lint_test
import (
"os"
"path/filepath"
"testing"
"github.com/hein/pgtidy/pkg/diagnostics"
"github.com/hein/pgtidy/pkg/lint"
)
func fixtureDir() string {
return filepath.Join("..", "..", "testdata", "lint")
}
func checkFile(t *testing.T, name string) []diagnostics.Diagnostic {
t.Helper()
src, err := os.ReadFile(filepath.Join(fixtureDir(), name))
if err != nil {
t.Fatalf("read %s: %v", name, err)
}
eng := lint.New()
diags, err := eng.Check(string(src), name)
if err != nil {
t.Fatalf("Check(%s): %v", name, err)
}
return diags
}
func ruleIDs(diags []diagnostics.Diagnostic) map[string]int {
m := make(map[string]int)
for _, d := range diags {
m[d.RuleID]++
}
return m
}
func TestMigrationViolations(t *testing.T) {
diags := checkFile(t, "migration_violations.sql")
ids := ruleIDs(diags)
cases := []struct {
id string
count int
}{
{"MIG001", 1}, // one non-concurrent index
{"MIG002", 1}, // one NOT NULL + no DEFAULT
{"MIG003", 2}, // one FK + one CHECK without NOT VALID
}
for _, c := range cases {
if got := ids[c.id]; got != c.count {
t.Errorf("rule %s: want %d findings, got %d", c.id, c.count, got)
}
}
}
func TestMigrationClean(t *testing.T) {
diags := checkFile(t, "migration_clean.sql")
ids := ruleIDs(diags)
for _, id := range []string{"MIG001", "MIG002", "MIG003"} {
if n := ids[id]; n != 0 {
t.Errorf("rule %s: want 0 findings on clean fixture, got %d", id, n)
}
}
}
func TestCorrectnessViolations(t *testing.T) {
diags := checkFile(t, "correctness_violations.sql")
ids := ruleIDs(diags)
cases := []struct {
id string
count int
}{
{"COR001", 1},
{"COR002", 1},
{"COR003", 1},
}
for _, c := range cases {
if got := ids[c.id]; got != c.count {
t.Errorf("rule %s: want %d findings, got %d", c.id, c.count, got)
}
}
}
func TestNamingViolations(t *testing.T) {
diags := checkFile(t, "naming_violations.sql")
ids := ruleIDs(diags)
cases := []struct {
id string
count int
}{
{"NAM001", 1}, // UserAccounts
{"NAM002", 2}, // userId, emailAddress
{"NAM003", 1}, // GetUserById
}
for _, c := range cases {
if got := ids[c.id]; got != c.count {
t.Errorf("rule %s: want %d findings, got %d", c.id, c.count, got)
}
}
}
func TestParseError(t *testing.T) {
eng := lint.New()
diags, err := eng.Check("SELECT FROM WHERE", "test.sql")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(diags) == 0 || diags[0].RuleID != "PARSE" {
t.Errorf("expected a PARSE diagnostic, got %v", diags)
}
}
func TestCustomEngine(t *testing.T) {
eng := lint.Custom()
eng.Register(lint.New().Rules()[0]) // register first rule only
// Just confirm it doesn't panic and returns results.
diags, err := eng.Check("SELECT 1", "stdin")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
_ = diags
}
+127
View File
@@ -0,0 +1,127 @@
package lint
import (
"fmt"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/hein/pgtidy/pkg/diagnostics"
"github.com/hein/pgtidy/pkg/pgast"
)
// COR001 — SELECT *.
// Selecting all columns by wildcard is fragile: column additions/removals
// break callers silently. Enumerate the columns you need explicitly.
type ruleCOR001 struct{}
func (ruleCOR001) ID() string { return "COR001" }
func (ruleCOR001) Severity() diagnostics.Severity { return diagnostics.SeverityHint }
func (ruleCOR001) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
walkSelectStar(raw.Stmt, src, &out)
}
return out
}
func walkSelectStar(node *pg_query.Node, src string, out *[]diagnostics.Diagnostic) {
if node == nil {
return
}
sel, ok := node.GetNode().(*pg_query.Node_SelectStmt)
if !ok {
return
}
checkSelectStmt(sel.SelectStmt, src, out)
}
func checkSelectStmt(s *pg_query.SelectStmt, src string, out *[]diagnostics.Diagnostic) {
if s == nil {
return
}
for _, t := range s.TargetList {
rt, ok := t.GetNode().(*pg_query.Node_ResTarget)
if !ok {
continue
}
cr, ok := rt.ResTarget.Val.GetNode().(*pg_query.Node_ColumnRef)
if !ok {
continue
}
for _, f := range cr.ColumnRef.Fields {
if _, isStar := f.GetNode().(*pg_query.Node_AStar); isStar {
line, col := pgast.LocationToLineCol(src, int(cr.ColumnRef.Location))
*out = append(*out, diagnostics.Diagnostic{
RuleID: "COR001",
Severity: diagnostics.SeverityHint,
Message: "SELECT * is fragile; enumerate the columns explicitly",
Line: line,
Col: col,
})
}
}
}
// Recurse into set-operation branches (UNION, INTERSECT, EXCEPT).
checkSelectStmt(s.Larg, src, out)
checkSelectStmt(s.Rarg, src, out)
}
// COR002 — UPDATE without WHERE.
// An UPDATE with no WHERE clause modifies every row in the table.
type ruleCOR002 struct{}
func (ruleCOR002) ID() string { return "COR002" }
func (ruleCOR002) Severity() diagnostics.Severity { return diagnostics.SeverityWarning }
func (ruleCOR002) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
u, ok := raw.Stmt.GetNode().(*pg_query.Node_UpdateStmt)
if !ok {
continue
}
if u.UpdateStmt.WhereClause != nil {
continue
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "COR002",
Severity: diagnostics.SeverityWarning,
Message: fmt.Sprintf("UPDATE %q has no WHERE clause; this modifies every row", relName(u.UpdateStmt.Relation)),
Line: line,
Col: col,
})
}
return out
}
// COR003 — DELETE without WHERE.
// A DELETE with no WHERE clause removes every row from the table.
// Use TRUNCATE if you intend a full wipe; it is faster and explicit.
type ruleCOR003 struct{}
func (ruleCOR003) ID() string { return "COR003" }
func (ruleCOR003) Severity() diagnostics.Severity { return diagnostics.SeverityWarning }
func (ruleCOR003) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
d, ok := raw.Stmt.GetNode().(*pg_query.Node_DeleteStmt)
if !ok {
continue
}
if d.DeleteStmt.WhereClause != nil {
continue
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "COR003",
Severity: diagnostics.SeverityWarning,
Message: fmt.Sprintf("DELETE FROM %q has no WHERE clause; this removes every row (use TRUNCATE if intentional)", relName(d.DeleteStmt.Relation)),
Line: line,
Col: col,
})
}
return out
}
+171
View File
@@ -0,0 +1,171 @@
package lint
import (
"fmt"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/hein/pgtidy/pkg/diagnostics"
"github.com/hein/pgtidy/pkg/pgast"
)
// MIG001 — CREATE INDEX without CONCURRENT.
// A non-concurrent index build holds ShareLock for its entire duration,
// blocking all writes on the table. In production migrations use
// CREATE INDEX CONCURRENTLY instead.
type ruleMIG001 struct{}
func (ruleMIG001) ID() string { return "MIG001" }
func (ruleMIG001) Severity() diagnostics.Severity { return diagnostics.SeverityWarning }
func (ruleMIG001) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
idx, ok := raw.Stmt.GetNode().(*pg_query.Node_IndexStmt)
if !ok {
continue
}
s := idx.IndexStmt
if s.Concurrent || s.Isconstraint {
continue
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "MIG001",
Severity: diagnostics.SeverityWarning,
Message: fmt.Sprintf("CREATE INDEX on %q without CONCURRENT blocks writes; use CREATE INDEX CONCURRENTLY", relName(s.Relation)),
Line: line,
Col: col,
})
}
return out
}
// MIG002 — ALTER TABLE ADD COLUMN with NOT NULL and no DEFAULT.
// Pre-PG11 this rewrites the whole table. Even on PG11+ the column may require
// a costly table scan to validate the NOT NULL constraint when no DEFAULT is
// supplied. Use a nullable column first, backfill, then add the constraint.
type ruleMIG002 struct{}
func (ruleMIG002) ID() string { return "MIG002" }
func (ruleMIG002) Severity() diagnostics.Severity { return diagnostics.SeverityWarning }
func (ruleMIG002) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
alt, ok := raw.Stmt.GetNode().(*pg_query.Node_AlterTableStmt)
if !ok {
continue
}
tbl := alt.AlterTableStmt
for _, cmdNode := range tbl.Cmds {
cmd, ok := cmdNode.GetNode().(*pg_query.Node_AlterTableCmd)
if !ok {
continue
}
ac := cmd.AlterTableCmd
if ac.Subtype != pg_query.AlterTableType_AT_AddColumn {
continue
}
col, ok := ac.Def.GetNode().(*pg_query.Node_ColumnDef)
if !ok {
continue
}
if hasNotNullNoDefault(col.ColumnDef.Constraints) {
line, col2 := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "MIG002",
Severity: diagnostics.SeverityWarning,
Message: fmt.Sprintf("ALTER TABLE %q ADD COLUMN with NOT NULL and no DEFAULT may rewrite the table; add nullable, backfill, then constrain", relName(tbl.Relation)),
Line: line,
Col: col2,
})
}
}
}
return out
}
// MIG003 — ALTER TABLE ADD CONSTRAINT (FK or CHECK) without NOT VALID.
// Without NOT VALID, PostgreSQL validates all existing rows immediately,
// holding locks that block concurrent writes. Use NOT VALID + a separate
// VALIDATE CONSTRAINT to spread the lock window.
type ruleMIG003 struct{}
func (ruleMIG003) ID() string { return "MIG003" }
func (ruleMIG003) Severity() diagnostics.Severity { return diagnostics.SeverityWarning }
func (ruleMIG003) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
alt, ok := raw.Stmt.GetNode().(*pg_query.Node_AlterTableStmt)
if !ok {
continue
}
tbl := alt.AlterTableStmt
for _, cmdNode := range tbl.Cmds {
cmd, ok := cmdNode.GetNode().(*pg_query.Node_AlterTableCmd)
if !ok {
continue
}
if cmd.AlterTableCmd.Subtype != pg_query.AlterTableType_AT_AddConstraint {
continue
}
con, ok := cmd.AlterTableCmd.Def.GetNode().(*pg_query.Node_Constraint)
if !ok {
continue
}
c := con.Constraint
if c.Contype != pg_query.ConstrType_CONSTR_FOREIGN && c.Contype != pg_query.ConstrType_CONSTR_CHECK {
continue
}
if c.SkipValidation {
continue // NOT VALID is present
}
kind := "FOREIGN KEY"
if c.Contype == pg_query.ConstrType_CONSTR_CHECK {
kind = "CHECK"
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "MIG003",
Severity: diagnostics.SeverityWarning,
Message: fmt.Sprintf("ALTER TABLE %q ADD %s CONSTRAINT without NOT VALID validates all rows immediately; use NOT VALID + VALIDATE CONSTRAINT", relName(tbl.Relation), kind),
Line: line,
Col: col,
})
}
}
return out
}
// hasNotNullNoDefault returns true when the constraint list has CONSTR_NOTNULL
// but no CONSTR_DEFAULT.
func hasNotNullNoDefault(constraints []*pg_query.Node) bool {
hasNN := false
hasDef := false
for _, cn := range constraints {
c, ok := cn.GetNode().(*pg_query.Node_Constraint)
if !ok {
continue
}
switch c.Constraint.Contype {
case pg_query.ConstrType_CONSTR_NOTNULL:
hasNN = true
case pg_query.ConstrType_CONSTR_DEFAULT:
hasDef = true
}
}
return hasNN && !hasDef
}
// relName returns "schema.rel" or "rel" from a RangeVar.
func relName(rv *pg_query.RangeVar) string {
if rv == nil {
return "?"
}
if rv.Schemaname != "" {
return rv.Schemaname + "." + rv.Relname
}
return rv.Relname
}
+135
View File
@@ -0,0 +1,135 @@
package lint
import (
"fmt"
"regexp"
"strings"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/hein/pgtidy/pkg/diagnostics"
"github.com/hein/pgtidy/pkg/pgast"
)
// reSnakeCase matches valid snake_case identifiers: lowercase letters, digits,
// and underscores, starting with a letter or underscore.
var reSnakeCase = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`)
func isSnakeCase(s string) bool {
return reSnakeCase.MatchString(s)
}
// NAM001 — table names must be snake_case.
type ruleNAM001 struct{}
func (ruleNAM001) ID() string { return "NAM001" }
func (ruleNAM001) Severity() diagnostics.Severity { return diagnostics.SeverityHint }
func (ruleNAM001) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
cs, ok := raw.Stmt.GetNode().(*pg_query.Node_CreateStmt)
if !ok {
continue
}
name := cs.CreateStmt.Relation.Relname
if name == "" || isSnakeCase(name) {
continue
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "NAM001",
Severity: diagnostics.SeverityHint,
Message: fmt.Sprintf("table name %q is not snake_case (expected %q)", name, toSnakeCase(name)),
Line: line,
Col: col,
})
}
return out
}
// NAM002 — column names in CREATE TABLE must be snake_case.
type ruleNAM002 struct{}
func (ruleNAM002) ID() string { return "NAM002" }
func (ruleNAM002) Severity() diagnostics.Severity { return diagnostics.SeverityHint }
func (ruleNAM002) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
cs, ok := raw.Stmt.GetNode().(*pg_query.Node_CreateStmt)
if !ok {
continue
}
for _, elt := range cs.CreateStmt.TableElts {
cd, ok := elt.GetNode().(*pg_query.Node_ColumnDef)
if !ok {
continue
}
name := cd.ColumnDef.Colname
if name == "" || isSnakeCase(name) {
continue
}
line, col := pgast.LocationToLineCol(src, int(cd.ColumnDef.Location))
out = append(out, diagnostics.Diagnostic{
RuleID: "NAM002",
Severity: diagnostics.SeverityHint,
Message: fmt.Sprintf("column name %q is not snake_case (expected %q)", name, toSnakeCase(name)),
Line: line,
Col: col,
})
}
}
return out
}
// NAM003 — function/procedure names must be snake_case.
type ruleNAM003 struct{}
func (ruleNAM003) ID() string { return "NAM003" }
func (ruleNAM003) Severity() diagnostics.Severity { return diagnostics.SeverityHint }
func (ruleNAM003) Check(stmts []*pg_query.RawStmt, src string) []diagnostics.Diagnostic {
var out []diagnostics.Diagnostic
for _, raw := range stmts {
cf, ok := raw.Stmt.GetNode().(*pg_query.Node_CreateFunctionStmt)
if !ok {
continue
}
// Funcname is a list of String nodes: [schema, funcname] or [funcname].
name := lastStringNode(cf.CreateFunctionStmt.Funcname)
if name == "" || isSnakeCase(name) {
continue
}
line, col := pgast.LocationToLineCol(src, pgast.FirstTokenOffset(src, int(raw.StmtLocation)))
out = append(out, diagnostics.Diagnostic{
RuleID: "NAM003",
Severity: diagnostics.SeverityHint,
Message: fmt.Sprintf("function name %q is not snake_case (expected %q)", name, toSnakeCase(name)),
Line: line,
Col: col,
})
}
return out
}
// lastStringNode returns the string value of the last node in a name-list
// (schema-qualified names like [schema, name] → name).
func lastStringNode(nodes []*pg_query.Node) string {
for i := len(nodes) - 1; i >= 0; i-- {
sv, ok := nodes[i].GetNode().(*pg_query.Node_String_)
if ok {
return sv.String_.Sval
}
}
return ""
}
// toSnakeCase converts a CamelCase or mixed-case identifier to snake_case as a
// suggested fix shown in the diagnostic message.
var reWordBoundary = regexp.MustCompile(`([a-z0-9])([A-Z])`)
func toSnakeCase(s string) string {
s = reWordBoundary.ReplaceAllString(s, "${1}_${2}")
return strings.ToLower(s)
}