diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml new file mode 100644 index 0000000..adc0eb1 --- /dev/null +++ b/.github/workflows/build.yaml @@ -0,0 +1,61 @@ +name: Build + +on: + push: + branches: ["*"] # Any branch + tags: ["v*.*.*"] # Release tags + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: "1.24" + + - name: Install dependencies + run: | + go mod download + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.0.1 + + - name: Lint + run: golangci-lint run + + - name: Run tests + run: | + go test ./... + + test-action: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Docker + uses: docker/setup-buildx-action@v2 + + - name: Run SQLite schema lint action + uses: ./ + with: + schema-file: 'test_schemas/success.sql' + + - name: Run SQLite schema lint action with invalid schema + uses: ./ + with: + schema-file: 'test_schemas/failure-no-strict.sql' + continue-on-error: true + + - name: Check for expected failure + run: | + if [ $? -eq 0 ]; then + echo "The action succeeded unexpectedly." + exit 1 + else + echo "The action failed as expected." + fi \ No newline at end of file diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..5ef8789 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,87 @@ +version: "2" + +linters: + default: none + enable: + # Defaults + - errcheck + - govet + - ineffassign + - staticcheck + - unused + + # Extras + - depguard + - errorlint + - godox + - lll + - nolintlint + - sqlclosecheck + - whitespace + - wrapcheck + + settings: + depguard: + rules: + main: + deny: + - pkg: io/ioutil + desc: replace with the matching functions from `io` or `os` packages + - pkg: github.com/pkg/errors + desc: Should be replaced by standard lib errors package + errcheck: + # report about not checking of errors in type assertions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: true + errorlint: + errorf: true # Ensure Errorf only uses %w (not %v or %s etc) for errors + asserts: true # Require errors.As instead of type-asserting + comparison: true # Require errors.Is instead of equality-checking + godox: + # report any comments starting with keywords, this is useful for TODO or FIXME comments that + # might be left in the code accidentally and should be resolved before merging + keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting + - XXX + govet: + enable-all: true + disable: + - fieldalignment + lll: + line-length: 140 + tab-width: 4 + nolintlint: + require-explanation: true + require-specific: true + allow-unused: false + staticcheck: + go: "1.24" + checks: + - all + - -ST1000 # Re-enable this once we have docstrings + - -ST1001 # Dot imports are good sometimes (e.g., in test packages) + - -ST1003 # snake_case is better for non-exported symbols + - -ST1013 # HTTP status codes are shorter and more readable than names + exclusions: + generated: lax # Don't lint generated files + paths: + +formatters: + enable: + - gci + - gofmt + settings: + gci: + sections: + - standard + - default + - localmodule + gofmt: + simplify: true + exclusions: + generated: lax + paths: + +issues: + max-same-issues: 0 + max-issues-per-linter: 0 + uniq-by-line: false diff --git a/Dockerfile b/Dockerfile index d8f1d14..bce873c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,9 +1,17 @@ +from golang:alpine as builder + +run apk add sqlite-dev build-base + +copy . /code +workdir /code + +env CGO_ENABLED=1 +run go build -ldflags="-w -s -linkmode=external -extldflags=-static" -o sqlite_lint ./cmd/main.go + +# --- + from alpine:3.20 -run apk add sqlite +COPY --from=builder /code/sqlite_lint / -copy entrypoint.sh / -copy lints.sql / -run chmod +x /entrypoint.sh - -entrypoint ["/entrypoint.sh"] +entrypoint ["/sqlite_lint"] diff --git a/action.yml b/action.yml index fbcf803..2078759 100644 --- a/action.yml +++ b/action.yml @@ -3,7 +3,7 @@ name: SQLite schema lint description: Enforce constraints on SQLite schemas inputs: schema-file: - description: SQL schema file that will create + description: SQL schema file to lint required: true runs: using: 'docker' diff --git a/cmd/main.go b/cmd/main.go new file mode 100644 index 0000000..da6f7f5 --- /dev/null +++ b/cmd/main.go @@ -0,0 +1,60 @@ +package main + +import ( + "fmt" + "os" + + _ "github.com/mattn/go-sqlite3" + + "sqlite_lint/pkg/checks" +) + +const ( + GREEN = "\033[0;32m" + RED = "\033[0;31m" + RESET = "\033[0m" +) + +func main() { + // Check if a filepath argument is provided + if len(os.Args) < 2 { + fmt.Println("Please provide a filepath as the first argument.") + os.Exit(1) + } + + // Get the filepath from the first argument + filepath := os.Args[1] + + fmt.Printf("-----------------\nLinting %s\n", filepath) + + // Open the SQLite database + db, err := checks.OpenSchema(filepath) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + is_failure := false + // Execute each check against the database + for _, check := range checks.Checks { + results, err := check.Execute(db) + if err != nil { + panic(err) + } + + // If there are results, print them as lint errors + if len(results) > 0 { + is_failure = true + fmt.Printf(RED+"Check '%s' failed:\n"+RESET, check.Name) + for _, result := range results { + fmt.Printf(RED+"- %s: %s.%s\n"+RESET, result.ErrorMsg, result.TableName, result.ColumnName) + } + fmt.Printf(RED+"Explanation: %s\n\n"+RESET, check.Explanation) + } + } + if is_failure { + fmt.Println(RED + "Errors found" + RESET) + } else { + fmt.Println(GREEN + "Success" + RESET) + } +} diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..cfabb7b --- /dev/null +++ b/go.mod @@ -0,0 +1,8 @@ +module sqlite_lint + +go 1.24.0 + +require ( + github.com/jmoiron/sqlx v1.4.0 + github.com/mattn/go-sqlite3 v1.14.28 +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..48ad178 --- /dev/null +++ b/go.sum @@ -0,0 +1,11 @@ +filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= +filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= +github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpvNJ1Y= +github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg= +github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o= +github.com/jmoiron/sqlx v1.4.0/go.mod h1:ZrZ7UsYB/weZdl2Bxg6jCRO9c3YHl8r3ahlKmRT4JLY= +github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= +github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= +github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= +github.com/mattn/go-sqlite3 v1.14.28 h1:ThEiQrnbtumT+QMknw63Befp/ce/nUPgBPMlRFEum7A= +github.com/mattn/go-sqlite3 v1.14.28/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= diff --git a/pkg/checks/check.go b/pkg/checks/check.go new file mode 100644 index 0000000..1b2dd2d --- /dev/null +++ b/pkg/checks/check.go @@ -0,0 +1,128 @@ +package checks + +import ( + "fmt" + + "github.com/jmoiron/sqlx" +) + +// Check represents a database check with a name, SQL statement, and an explanation. +type Check struct { + Name string + Sql string + Explanation string +} + +// 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"` +} + +// Execute runs the SQL statement of the Check against the provided database and returns the +// resulting rows as a slice of CheckResult using sqlx. +func (c *Check) Execute(db *sqlx.DB) ([]CheckResult, error) { + var results []CheckResult + // return results, nil + // println(c.Sql) + if err := db.Select(&results, c.Sql); err != nil { + return nil, fmt.Errorf("failed to execute check '%s': %w", c.Name, err) + } + return results, nil +} + +var Checks = map[string]Check{ + "require_not_null": { + Name: "require_not_null", + Sql: ` + select 'Column should should be "not null"' as error_msg, + table_name, + column_name + from columns + where columns."notnull" = 0 + and fk_target_column is null + and is_primary_key = 0 -- primary keys are automatically not-null, but aren't listed as such in pragma_table_info + `, + 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.)", + }, + // { + // Name: "require_default_values", + // Sql: ` + // select 'Column should have a default value' as error_msg, + // table_name, + // column_name + // from columns + // where dflt_value is null + // and fk_target_column is null + // and is_primary_key = 0; + // `, + // Explanation: "All columns should have a default value specified, unless they are foreign keys or primary keys.", + // }, + "require_strict": { + Name: "require_strict", + Sql: ` + select 'Table should be marked "strict"' as error_msg, + name as table_name, + '' as column_name + from tables + where strict = 0; + `, + 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", + }, + "forbid_int_type": { + Name: "forbid_int_type", + Sql: ` + select 'Column should use "integer" type instead of "int"' as error_msg, + table_name, + column_name + from columns + where column_type like 'int'; + `, + Explanation: "All columns should use `integer` type instead of `int`.", + }, + "require_explicit_primary_key": { + Name: "require_explicit_primary_key", + Sql: ` + select 'Table should declare an explicit primary key' as error_msg, + tables.name as table_name, + '' as column_name + from tables + where not exists (select 1 from pragma_table_info(tables.name) where pk != 0); + `, + Explanation: "All tables must have a primary key. If it's rowid, it has to be named explicitly.", + }, + "require_indexes_for_foreign_keys": { + Name: "require_indexes_for_foreign_keys", + Sql: ` + with index_info as ( + select tables.name as table_name, + columns.name as column_name + from tables + join pragma_index_list(tables.name) as indexes + join pragma_index_info(indexes.name) as columns + + union + + select table_name, + column_name + from columns + where column_name = 'rowid' + and is_primary_key != 0 -- 'pk' is either 0, or the 1-based index of the column within the primary key + ), foreign_keys as ( + select * from columns where fk_target_column is not null + ) + select 'Foreign keys should point to indexed columns' as error_msg, + foreign_keys.table_name as table_name, + foreign_keys.column_name as column_name + from foreign_keys + left join index_info on foreign_keys.fk_target_table = index_info.table_name + and foreign_keys.fk_target_column = index_info.column_name + where index_info.column_name is null; + `, + Explanation: "Columns referenced by foreign keys must have indexes.", + }, +} diff --git a/pkg/checks/check_test.go b/pkg/checks/check_test.go new file mode 100644 index 0000000..eeff602 --- /dev/null +++ b/pkg/checks/check_test.go @@ -0,0 +1,71 @@ +package checks_test + +import ( + "slices" + "testing" + + _ "github.com/mattn/go-sqlite3" + + "sqlite_lint/pkg/checks" +) + +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 { + db, err := checks.OpenSchema(test_case.sqlFile) + if err != nil { + t.Fatalf("failed to open database: %v", err) + } + + for _, check := range checks.Checks { + results, err := check.Execute(db) + if err != nil { + t.Errorf("failed to execute check '%s': %v", check.Name, err) + } + + 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", check.Name, test_case.sqlFile) + } + } + } + } +} + +func TestSuccessCase(t *testing.T) { + db, err := checks.OpenSchema("../../test_schemas/success.sql") + if err != nil { + t.Fatalf("failed to open database: %v", err) + } + + for _, check := range checks.Checks { + results, err := check.Execute(db) + if err != nil { + t.Errorf("failed to execute check '%s': %v", check.Name, err) + } + if len(results) > 0 { + t.Errorf("Should have passed, but didn't: %s", "test_schemas/success.sql") + } + } +} diff --git a/pkg/checks/target.go b/pkg/checks/target.go new file mode 100644 index 0000000..e3ed31c --- /dev/null +++ b/pkg/checks/target.go @@ -0,0 +1,52 @@ +package checks + +import ( + "fmt" + "os" + + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" +) + +// OpenSchema opens a SQLite database in memory, executes the schema against it, and adds some views +func OpenSchema(filepath string) (*sqlx.DB, error) { + // Open a SQLite database in memory + db, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + return nil, fmt.Errorf("failed to open in-memory database: %w", err) + } + + // Read the SQL file + sqlBytes, err := os.ReadFile(filepath) + if err != nil { + return nil, fmt.Errorf("failed to read SQL file: %w", err) + } + + // Execute the SQL statements + db.MustExec(string(sqlBytes)) + + // Execute the SQL statements for creating views + db.MustExec(` + create view tables as + select l.* + from sqlite_schema s + left join pragma_table_list l on s.name = l.name + where s.type = 'table'; + + + create view columns as + select tables.name as table_name, + table_info.name as column_name, + table_info.type as column_type, + "notnull", + dflt_value, + pk as is_primary_key, + fk."table" as fk_target_table, + fk."to" as fk_target_column + from tables + join pragma_table_info(tables.name) as table_info + left join pragma_foreign_key_list(tables.name) as fk on fk."from" = column_name; + `) + + return db, nil +}