From 8b422ada11f169ce64e350a8dd159b897aac3a13 Mon Sep 17 00:00:00 2001 From: wispem-wantex Date: Sat, 23 Aug 2025 18:54:00 -0700 Subject: [PATCH] Add sqlite-lint to 'pkg/schema/lint' --- pkg/schema/lint/checks.go | 136 ++++++++++++++++++ pkg/schema/lint/checks_test.go | 66 +++++++++ .../failure-has-foreign-key-no-index.sql | 12 ++ .../lint/test_schemas/failure-has-ints.sql | 13 ++ .../lint/test_schemas/failure-has-nulls.sql | 13 ++ .../lint/test_schemas/failure-no-strict.sql | 13 ++ .../lint/test_schemas/failure-total.sql | 28 ++++ pkg/schema/lint/test_schemas/success.sql | 18 +++ 8 files changed, 299 insertions(+) create mode 100644 pkg/schema/lint/checks.go create mode 100644 pkg/schema/lint/checks_test.go create mode 100644 pkg/schema/lint/test_schemas/failure-has-foreign-key-no-index.sql create mode 100644 pkg/schema/lint/test_schemas/failure-has-ints.sql create mode 100644 pkg/schema/lint/test_schemas/failure-has-nulls.sql create mode 100644 pkg/schema/lint/test_schemas/failure-no-strict.sql create mode 100644 pkg/schema/lint/test_schemas/failure-total.sql create mode 100644 pkg/schema/lint/test_schemas/success.sql diff --git a/pkg/schema/lint/checks.go b/pkg/schema/lint/checks.go new file mode 100644 index 0000000..ca43f24 --- /dev/null +++ b/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 + }, + }, +} diff --git a/pkg/schema/lint/checks_test.go b/pkg/schema/lint/checks_test.go new file mode 100644 index 0000000..76192fa --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/schema/lint/test_schemas/failure-has-foreign-key-no-index.sql b/pkg/schema/lint/test_schemas/failure-has-foreign-key-no-index.sql new file mode 100644 index 0000000..86a19b3 --- /dev/null +++ b/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; diff --git a/pkg/schema/lint/test_schemas/failure-has-ints.sql b/pkg/schema/lint/test_schemas/failure-has-ints.sql new file mode 100644 index 0000000..009dd24 --- /dev/null +++ b/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; diff --git a/pkg/schema/lint/test_schemas/failure-has-nulls.sql b/pkg/schema/lint/test_schemas/failure-has-nulls.sql new file mode 100644 index 0000000..6ab9e0a --- /dev/null +++ b/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; diff --git a/pkg/schema/lint/test_schemas/failure-no-strict.sql b/pkg/schema/lint/test_schemas/failure-no-strict.sql new file mode 100644 index 0000000..c45a92f --- /dev/null +++ b/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) +); diff --git a/pkg/schema/lint/test_schemas/failure-total.sql b/pkg/schema/lint/test_schemas/failure-total.sql new file mode 100644 index 0000000..eb2e4c6 --- /dev/null +++ b/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) +); diff --git a/pkg/schema/lint/test_schemas/success.sql b/pkg/schema/lint/test_schemas/success.sql new file mode 100644 index 0000000..e41809b --- /dev/null +++ b/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;