Add sqlite-lint to 'pkg/schema/lint'

pkg/schema/lint/checks.go

@@ -0,0 +1,136 @@
+package lint
+
+import (
+	"git.offline-twitter.com/offline-labs/gas-stack/pkg/schema"
+)
+
+type Check struct {
+	Name        string
+	Explanation string
+	Execute     func(schema.Schema) []CheckResult
+}
+
+// CheckResult represents a row in the query result with error message, table name, and column name.
+type CheckResult struct {
+	ErrorMsg   string `db:"error_msg"`
+	TableName  string `db:"table_name"`
+	ColumnName string `db:"column_name"`
+}
+
+var Checks = []Check{
+	{
+		Name: "require_not_null",
+		Explanation: "All columns should be marked as `not null` unless they are foreign keys.  (Primary keys are\n" +
+			"automatically not-null, and don't need to be specified.)",
+		Execute: func(s schema.Schema) (ret []CheckResult) {
+			for tablename := range s.Tables {
+				for _, column := range s.Tables[tablename].Columns {
+					if !column.IsNotNull && !column.IsForeignKey && !column.IsPrimaryKey {
+						ret = append(ret, CheckResult{
+							ErrorMsg:   "Column should be \"not null\"",
+							TableName:  tablename,
+							ColumnName: column.Name,
+						})
+					}
+				}
+			}
+			return
+		},
+	},
+	{
+		Name: "require_strict",
+		Explanation: "All tables should be marked as `strict` (must specify column types; types must be int,\n" +
+			"integer, real, text, blob or any).  This disallows all 'date' and 'time' column types.\n" +
+			"See more: https://www.sqlite.org/stricttables.html",
+		Execute: func(s schema.Schema) (ret []CheckResult) {
+			for tablename := range s.Tables {
+				if !s.Tables[tablename].IsStrict {
+					ret = append(ret, CheckResult{
+						ErrorMsg:   "Table should be marked \"strict\"",
+						TableName:  tablename,
+						ColumnName: "",
+					})
+				}
+			}
+			return
+		},
+	},
+	{
+		Name:        "forbid_int_type",
+		Explanation: "All columns should use `integer` type instead of `int`.",
+		Execute: func(s schema.Schema) (ret []CheckResult) {
+			for tablename := range s.Tables {
+				for _, column := range s.Tables[tablename].Columns {
+					if column.Type == "int" {
+						ret = append(ret, CheckResult{
+							ErrorMsg:   "Column should use \"integer\" type instead of \"int\"",
+							TableName:  tablename,
+							ColumnName: column.Name,
+						})
+					}
+				}
+			}
+			return
+		},
+	},
+	{
+		Name:        "require_explicit_primary_key",
+		Explanation: "All tables must have a primary key.  If it's rowid, it has to be named explicitly.",
+		Execute: func(s schema.Schema) (ret []CheckResult) {
+		tableloop:
+			for tablename := range s.Tables {
+				for _, column := range s.Tables[tablename].Columns {
+					if column.IsPrimaryKey {
+						continue tableloop
+					}
+				}
+				ret = append(ret, CheckResult{
+					ErrorMsg:   "Table should declare an explicit primary key",
+					TableName:  tablename,
+					ColumnName: "",
+				})
+			}
+			return
+		},
+	},
+	{
+		Name:        "require_indexes_for_foreign_keys",
+		Explanation: "Columns referenced by foreign keys must have indexes.",
+		Execute: func(s schema.Schema) (ret []CheckResult) {
+			for tablename := range s.Tables {
+			fk_loop:
+				for _, column := range s.Tables[tablename].Columns {
+					if !column.IsForeignKey {
+						continue
+					}
+
+					// Check if target column is a primary key
+					for _, target_col := range s.Tables[column.ForeignKeyTargetTable].Columns {
+						if target_col.Name == column.ForeignKeyTargetColumn && target_col.IsPrimaryKey {
+							continue fk_loop
+						}
+					}
+
+					// Check if target column is at the beginning of any index
+					for _, idx := range s.Indexes {
+						if idx.TableName != column.ForeignKeyTargetTable {
+							continue
+						}
+						for _, idx_col_name := range idx.Columns {
+							if column.ForeignKeyTargetColumn == idx_col_name {
+								continue fk_loop
+							}
+							break // Only look at 1st column
+						}
+					}
+					ret = append(ret, CheckResult{
+						ErrorMsg:   "Foreign keys should point to indexed columns",
+						TableName:  tablename,
+						ColumnName: column.Name,
+					})
+				}
+			}
+			return
+		},
+	},
+}

pkg/schema/lint/checks_test.go

@@ -0,0 +1,66 @@
+package lint_test
+
+import (
+	"slices"
+	"testing"
+
+	_ "github.com/mattn/go-sqlite3"
+	"github.com/stretchr/testify/require"
+
+	"git.offline-twitter.com/offline-labs/gas-stack/pkg/schema"
+	"git.offline-twitter.com/offline-labs/gas-stack/pkg/schema/lint"
+)
+
+func TestFailureCases(t *testing.T) {
+	test_cases := []struct {
+		sqlFile          string
+		expectedFailures []string
+	}{
+		{"test_schemas/failure-has-foreign-key-no-index.sql", []string{"require_indexes_for_foreign_keys"}},
+		{"test_schemas/failure-has-ints.sql", []string{"forbid_int_type"}},
+		{"test_schemas/failure-has-nulls.sql", []string{"require_not_null"}},
+		{"test_schemas/failure-no-strict.sql", []string{"require_strict"}},
+		{"test_schemas/failure-total.sql", []string{
+			"require_not_null",
+			"require_explicit_primary_key",
+			"forbid_int_type",
+			"require_strict",
+			"require_indexes_for_foreign_keys",
+		}},
+	}
+
+	for _, test_case := range test_cases {
+		t.Run(test_case.sqlFile, func(t *testing.T) {
+			schema, err := schema.SchemaFromSQLFile(test_case.sqlFile)
+			require.NoError(t, err)
+
+			for _, check := range lint.Checks {
+				results := check.Execute(schema)
+
+				is_failure := len(results) > 0
+				is_failure_expected := slices.Contains(test_case.expectedFailures, check.Name)
+
+				if is_failure != is_failure_expected {
+					if is_failure_expected {
+						t.Errorf("Expected check '%s' to fail, but it passed: %s", check.Name, test_case.sqlFile)
+					} else {
+						t.Errorf("Expected check '%s' to pass, but it failed: %s (%q.%q)", check.Name, test_case.sqlFile, results[0].TableName, results[0].ColumnName)
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestSuccessCase(t *testing.T) {
+	file := "test_schemas/success.sql"
+	schema, err := schema.SchemaFromSQLFile(file)
+	require.NoError(t, err)
+
+	for _, check := range lint.Checks {
+		results := check.Execute(schema)
+		for _, r := range results {
+			t.Errorf("Unexpected error in file %q: %#v", file, r)
+		}
+	}
+}

pkg/schema/lint/test_schemas/failure-has-foreign-key-no-index.sql

@@ -0,0 +1,12 @@
+create table stuff (
+    rowid integer primary key,
+    data text not null,
+    amount integer not null
+) strict;
+
+create table stuff2 (
+    weird_pk integer primary key,
+    label text not null unique,
+    stuff_id integer references stuff(rowid),
+    alternative_stuff_id integer references stuff(amount)
+) strict;

pkg/schema/lint/test_schemas/failure-has-ints.sql

@@ -0,0 +1,13 @@
+create table stuff (
+    rowid integer primary key,
+    data text not null,
+    amount integer not null
+) strict;
+create index index_stuff_amount on stuff (amount);
+
+create table stuff2 (
+    weird_pk integer primary key,
+    label text not null unique,
+    stuff_id int references stuff(rowid),
+    alternative_stuff_id integer references stuff(amount)
+) strict;

pkg/schema/lint/test_schemas/failure-has-nulls.sql

@@ -0,0 +1,13 @@
+create table stuff (
+    rowid integer primary key,
+    data text not null,
+    amount integer
+) strict;
+create index index_stuff_amount on stuff (amount);
+
+create table stuff2 (
+    weird_pk integer primary key,
+    label text not null unique,
+    stuff_id integer references stuff(rowid),
+    alternative_stuff_id integer references stuff(amount)
+) strict;

pkg/schema/lint/test_schemas/failure-no-strict.sql

@@ -0,0 +1,13 @@
+create table stuff (
+    rowid integer primary key,
+    data text not null,
+    amount integer not null
+) strict;
+create index index_stuff_amount on stuff (amount);
+
+create table stuff2 (
+    weird_pk integer primary key,
+    label text not null unique,
+    stuff_id integer references stuff(rowid),
+    alternative_stuff_id integer references stuff(amount)
+);

pkg/schema/lint/test_schemas/failure-total.sql

@@ -0,0 +1,28 @@
+PRAGMA foreign_keys = on;
+
+
+create table implicit_rowid (
+    a integer
+);
+
+create table explicit_rowid_not_pk (
+	rowid integer
+);
+
+create table explicit_rowid (
+	rowid integer primary key
+);
+
+create table without_rowid (
+	a integer primary key
+) without rowid;
+
+create table multi_column (
+	a int,
+	b integer,
+	primary key(a, b)
+);
+
+create table foreign_key_missing_index (
+	a references implicit_rowid(a)
+);

pkg/schema/lint/test_schemas/success.sql

@@ -0,0 +1,18 @@
+create table stuff (
+    rowid integer primary key,
+    data text not null,
+    amount integer not null
+) strict;
+create index index_stuff_amount on stuff (amount);
+
+create table stuff2 (
+    weird_pk integer primary key,
+    label text not null unique,
+    stuff_id integer references stuff(rowid),
+    alternative_stuff_id integer references stuff(amount)
+) strict;
+
+create table stuff3 (
+    weird_pk3 integer primary key,
+    stuff2_id integer not null references stuff2(weird_pk)
+) strict;