feat(cmd/sqlc): Add the vet subcommand (#2344)

This is a proposal to add a new subcommand that runs queries through a set of lint rules.
Rules

Rules are defined in the configuration file. They consist of a name, message, and an expression. If the expression evaluates to true, an error is reported. These expressions are evaluated using cel-go.

Each expression has access to a query object, which is defined as the following struct:

---------

Co-authored-by: Kyle Conroy <kyle@sqlc.dev>

Author: kyleconroy

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/cubicdaiya/gonp v1.0.4
 	github.com/davecgh/go-spew v1.1.1
 	github.com/go-sql-driver/mysql v1.7.1
+	github.com/google/cel-go v0.16.0
 	github.com/google/go-cmp v0.5.9
 	github.com/jackc/pgconn v1.14.0
 	github.com/jackc/pgx/v4 v4.18.1
@@ -24,6 +25,8 @@ require (
 	gopkg.in/yaml.v3 v3.0.1
 )
 
+require github.com/stoewer/go-strcase v1.2.0 // indirect
+
 require (
 	github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect

go.sum

@@ -36,6 +36,8 @@ github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg=
 github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
+github.com/google/cel-go v0.16.0 h1:DG9YQ8nFCFXAs/FDDwBxmL1tpKNrdlGUM9U3537bX/Y=
+github.com/google/cel-go v0.16.0/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -151,6 +153,8 @@ github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I=
 github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
+github.com/stoewer/go-strcase v1.2.0 h1:Z2iHWqGXH00XYgqDmNgQbIBxf3wrNq0F3feEy0ainaU=
+github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=

internal/cmd/cmd.go

@@ -44,6 +44,7 @@ func Do(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) int
 	rootCmd.AddCommand(initCmd)
 	rootCmd.AddCommand(versionCmd)
 	rootCmd.AddCommand(uploadCmd)
+	rootCmd.AddCommand(NewCmdVet())
 
 	rootCmd.SetArgs(args)
 	rootCmd.SetIn(stdin)

internal/cmd/vet.go

@@ -0,0 +1,189 @@
+package cmd
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"runtime/trace"
+	"strings"
+
+	"github.com/google/cel-go/cel"
+	"github.com/spf13/cobra"
+
+	"github.com/kyleconroy/sqlc/internal/config"
+	"github.com/kyleconroy/sqlc/internal/debug"
+	"github.com/kyleconroy/sqlc/internal/opts"
+	"github.com/kyleconroy/sqlc/internal/plugin"
+)
+
+var ErrFailedChecks = errors.New("failed checks")
+
+func NewCmdVet() *cobra.Command {
+	return &cobra.Command{
+		Use:   "vet",
+		Short: "Vet examines queries",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			defer trace.StartRegion(cmd.Context(), "vet").End()
+			stderr := cmd.ErrOrStderr()
+			dir, name := getConfigPath(stderr, cmd.Flag("file"))
+			if err := Vet(cmd.Context(), ParseEnv(cmd), dir, name, stderr); err != nil {
+				if !errors.Is(err, ErrFailedChecks) {
+					fmt.Fprintf(stderr, "%s\n", err)
+				}
+				os.Exit(1)
+			}
+			return nil
+		},
+	}
+}
+
+func Vet(ctx context.Context, e Env, dir, filename string, stderr io.Writer) error {
+	configPath, conf, err := readConfig(stderr, dir, filename)
+	if err != nil {
+		return err
+	}
+
+	base := filepath.Base(configPath)
+	if err := config.Validate(conf); err != nil {
+		fmt.Fprintf(stderr, "error validating %s: %s\n", base, err)
+		return err
+	}
+
+	if err := e.Validate(conf); err != nil {
+		fmt.Fprintf(stderr, "error validating %s: %s\n", base, err)
+		return err
+	}
+
+	env, err := cel.NewEnv(
+		cel.StdLib(),
+		cel.Types(
+			&plugin.VetConfig{},
+			&plugin.VetQuery{},
+		),
+		cel.Variable("query",
+			cel.ObjectType("plugin.VetQuery"),
+		),
+		cel.Variable("config",
+			cel.ObjectType("plugin.VetConfig"),
+		),
+	)
+	if err != nil {
+		return fmt.Errorf("new env; %s", err)
+	}
+
+	checks := map[string]cel.Program{}
+	msgs := map[string]string{}
+
+	for _, c := range conf.Rules {
+		if c.Name == "" {
+			return fmt.Errorf("checks require a name")
+		}
+		if _, found := checks[c.Name]; found {
+			return fmt.Errorf("type-check error: a check with the name '%s' already exists", c.Name)
+		}
+		if c.Rule == "" {
+			return fmt.Errorf("type-check error: %s is empty", c.Name)
+		}
+		ast, issues := env.Compile(c.Rule)
+		if issues != nil && issues.Err() != nil {
+			return fmt.Errorf("type-check error: %s %s", c.Name, issues.Err())
+		}
+		prg, err := env.Program(ast)
+		if err != nil {
+			return fmt.Errorf("program construction error: %s %s", c.Name, err)
+		}
+		checks[c.Name] = prg
+		msgs[c.Name] = c.Msg
+	}
+
+	errored := true
+	for _, sql := range conf.SQL {
+		combo := config.Combine(*conf, sql)
+
+		// TODO: This feels like a hack that will bite us later
+		joined := make([]string, 0, len(sql.Schema))
+		for _, s := range sql.Schema {
+			joined = append(joined, filepath.Join(dir, s))
+		}
+		sql.Schema = joined
+
+		joined = make([]string, 0, len(sql.Queries))
+		for _, q := range sql.Queries {
+			joined = append(joined, filepath.Join(dir, q))
+		}
+		sql.Queries = joined
+
+		var name string
+		parseOpts := opts.Parser{
+			Debug: debug.Debug,
+		}
+
+		result, failed := parse(ctx, name, dir, sql, combo, parseOpts, stderr)
+		if failed {
+			return nil
+		}
+		req := codeGenRequest(result, combo)
+		cfg := vetConfig(req)
+		for _, query := range req.Queries {
+			q := vetQuery(query)
+			for _, name := range sql.Rules {
+				prg, ok := checks[name]
+				if !ok {
+					return fmt.Errorf("type-check error: a check with the name '%s' does not exist", name)
+				}
+				out, _, err := prg.Eval(map[string]any{
+					"query":  q,
+					"config": cfg,
+				})
+				if err != nil {
+					return err
+				}
+				tripped, ok := out.Value().(bool)
+				if !ok {
+					return fmt.Errorf("expression returned non-bool: %s", err)
+				}
+				if tripped {
+					// TODO: Get line numbers in the output
+					msg := msgs[name]
+					if msg == "" {
+						fmt.Fprintf(stderr, query.Filename+": %s: %s\n", q.Name, name, msg)
+					} else {
+						fmt.Fprintf(stderr, query.Filename+": %s: %s: %s\n", q.Name, name, msg)
+					}
+					errored = true
+				}
+			}
+		}
+	}
+	if errored {
+		return ErrFailedChecks
+	}
+	return nil
+}
+
+func vetConfig(req *plugin.CodeGenRequest) *plugin.VetConfig {
+	return &plugin.VetConfig{
+		Version: req.Settings.Version,
+		Engine:  req.Settings.Engine,
+		Schema:  req.Settings.Schema,
+		Queries: req.Settings.Queries,
+	}
+}
+
+func vetQuery(q *plugin.Query) *plugin.VetQuery {
+	var params []*plugin.VetParameter
+	for _, p := range q.Params {
+		params = append(params, &plugin.VetParameter{
+			Number: p.Number,
+		})
+	}
+	return &plugin.VetQuery{
+		Sql:    q.Text,
+		Name:   q.Name,
+		Cmd:    strings.TrimPrefix(":", q.Cmd),
+		Params: params,
+	}
+}

internal/config/config.go

@@ -62,6 +62,7 @@ type Config struct {
 	SQL     []SQL    `json:"sql" yaml:"sql"`
 	Gen     Gen      `json:"overrides,omitempty" yaml:"overrides"`
 	Plugins []Plugin `json:"plugins" yaml:"plugins"`
+	Rules   []Rule   `json:"rules" yaml:"rules"`
 }
 
 type Project struct {
@@ -85,6 +86,12 @@ type Plugin struct {
 	} `json:"wasm" yaml:"wasm"`
 }
 
+type Rule struct {
+	Name string `json:"name" yaml:"name"`
+	Rule string `json:"rule" yaml:"rule"`
+	Msg  string `json:"message" yaml:"message"`
+}
+
 type Gen struct {
 	Go *GenGo `json:"go,omitempty" yaml:"go"`
 }
@@ -102,6 +109,7 @@ type SQL struct {
 	StrictOrderBy        *bool     `json:"strict_order_by" yaml:"strict_order_by"`
 	Gen                  SQLGen    `json:"gen" yaml:"gen"`
 	Codegen              []Codegen `json:"codegen" yaml:"codegen"`
+	Rules                []string  `json:"rules" yaml:"rules"`
 }
 
 // TODO: Figure out a better name for this

internal/endtoend/endtoend_test.go

@@ -123,6 +123,8 @@ func TestReplay(t *testing.T) {
 				if err == nil {
 					cmpDirectory(t, path, output)
 				}
+			case "vet":
+				err = cmd.Vet(ctx, env, path, "", &stderr)
 			default:
 				t.Fatalf("unknown command")
 			}

internal/endtoend/testdata/vet_failures/exec.json

@@ -0,0 +1,3 @@
+{
+  "command": "vet"
+}

internal/endtoend/testdata/vet_failures/query.sql

@@ -0,0 +1,25 @@
+CREATE TABLE authors (
+          id   BIGSERIAL PRIMARY KEY,
+          name text      NOT NULL,
+          bio  text
+);
+
+-- name: GetAuthor :one
+SELECT * FROM authors
+WHERE id = $1 LIMIT 1;
+
+-- name: ListAuthors :many
+SELECT * FROM authors
+ORDER BY name;
+
+-- name: CreateAuthor :one
+INSERT INTO authors (
+          name, bio
+) VALUES (
+  $1, $2
+)
+RETURNING *;
+
+-- name: DeleteAuthor :exec
+DELETE FROM authors
+WHERE id = $1;

internal/endtoend/testdata/vet_failures/schema.sql

@@ -0,0 +1,5 @@
+CREATE TABLE authors (
+          id   BIGSERIAL PRIMARY KEY,
+          name text      NOT NULL,
+          bio  text
+);

internal/endtoend/testdata/vet_failures/sqlc.yaml

@@ -0,0 +1,31 @@
+version: 2
+sql:
+  - schema: "query.sql"
+    queries: "query.sql"
+    engine: "postgresql"
+    gen:
+      go:
+        package: "authors"
+        out: "db"
+    rules:
+      - no-pg
+      - no-delete
+      - only-one-param
+      - no-exec
+rules:
+  - name: no-pg
+    message: "invalid engine: postgresql"
+    rule: |
+      config.engine == "postgresql"
+  - name: no-delete
+    message: "don't use delete statements"
+    rule: |
+      query.sql.contains("DELETE")
+  - name: only-one-param
+    message: "too many parameters"
+    rule: |
+      query.params.size() > 1
+  - name: no-exec
+    message: "don't use exec"
+    rule: |
+      query.cmd == "exec"

internal/endtoend/testdata/vet_failures/stderr.txt

@@ -0,0 +1,6 @@
+query.sql: GetAuthor: no-pg: invalid engine: postgresql
+query.sql: ListAuthors: no-pg: invalid engine: postgresql
+query.sql: CreateAuthor: no-pg: invalid engine: postgresql
+query.sql: CreateAuthor: only-one-param: too many parameters
+query.sql: DeleteAuthor: no-pg: invalid engine: postgresql
+query.sql: DeleteAuthor: no-delete: don't use delete statements