Skip to content

Commit

Permalink
feat(vet): Introduce a query annotation to opt out of sqlc vet rules (#…
Browse files Browse the repository at this point in the history
…2474)

* feat(vet): Introduce a query annotation to opt out of sqlc vet rules

* Add proper logging, and a test

* Don't remove comments when parsing query annotation metadata
  • Loading branch information
andrewmbenton authored Jul 18, 2023
1 parent 4d764a2 commit dddfe4f
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 7 deletions.
53 changes: 49 additions & 4 deletions docs/howto/vet.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
Rules are defined in the `sqlc` [configuration](../reference/config) file. They consist
of a name, message, and a [Common Expression Language (CEL)](https://github.com/google/cel-spec)
expression. Expressions are evaluated using [cel-go](https://github.com/google/cel-go).
If an expression evaluates to `true`, an error is reported using the given message.
If an expression evaluates to `true`, `sqlc vet` will report an error using the given message.

Each expression has access to variables from your sqlc configuration and queries,
defined in the following struct:
## Defining lint rules

Each lint rule's CEL expression has access to variables from your sqlc configuration and queries,
defined in the following struct.

```proto
message Config
Expand Down Expand Up @@ -109,4 +111,47 @@ example](https://github.com/kyleconroy/sqlc/blob/main/examples/authors/sqlc.yaml

Please note that `sqlc` does not manage or migrate your database. Use your
migration tool of choice to create the necessary database tables and objects
before running `sqlc vet`.
before running `sqlc vet` with the `sqlc/db-prepare` rule.

## Running lint rules

When you add the name of a defined rule to the rules list
for a [sql package](https://docs.sqlc.dev/en/stable/reference/config.html#sql),
`sqlc vet` will evaluate that rule against every query in the package.

In the example below, two rules are defined but only one is enabled.

```yaml
version: 2
sql:
- schema: "query.sql"
queries: "query.sql"
engine: "postgresql"
gen:
go:
package: "authors"
out: "db"
rules:
- no-delete
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")
```

### Opting-out of lint rules

For any query, you can tell `sqlc vet` not to evaluate lint rules using the
`@sqlc-vet-disable` query annotation.

```sql
/* name: GetAuthor :one */
/* @sqlc-vet-disable */
SELECT * FROM authors
WHERE id = ? LIMIT 1;
```
9 changes: 8 additions & 1 deletion internal/cmd/vet.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"log"
"os"
"path/filepath"
"runtime/trace"
Expand All @@ -31,6 +32,7 @@ import (
var ErrFailedChecks = errors.New("failed checks")

const RuleDbPrepare = "sqlc/db-prepare"
const QueryFlagSqlcVetDisable = "@sqlc-vet-disable"

func NewCmdVet() *cobra.Command {
return &cobra.Command{
Expand Down Expand Up @@ -249,7 +251,6 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
return ErrFailedChecks
}

// TODO: Add MySQL support
var prep preparer
if s.Database != nil {
if c.NoDatabase {
Expand Down Expand Up @@ -299,6 +300,12 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
req := codeGenRequest(result, combo)
cfg := vetConfig(req)
for i, query := range req.Queries {
if result.Queries[i].Flags[QueryFlagSqlcVetDisable] {
if debug.Active {
log.Printf("Skipping vet rules for query: %s\n", query.Name)
}
continue
}
q := vetQuery(query)
for _, name := range s.Rules {
// Built-in rule
Expand Down
3 changes: 3 additions & 0 deletions internal/endtoend/testdata/vet_disable/exec.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"command": "vet"
}
6 changes: 6 additions & 0 deletions internal/endtoend/testdata/vet_disable/query.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- name: SkipVet :exec
-- @sqlc-vet-disable
SELECT true;

-- name: RunVet :exec
SELECT true;
15 changes: 15 additions & 0 deletions internal/endtoend/testdata/vet_disable/sqlc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
version: 2
sql:
- schema: "query.sql"
queries: "query.sql"
engine: "postgresql"
gen:
go:
package: "db"
out: "db"
rules:
- always-fail
rules:
- name: always-fail
message: "Fail"
rule: "true"
1 change: 1 addition & 0 deletions internal/endtoend/testdata/vet_disable/stderr.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
query.sql: RunVet: always-fail: Fail
3 changes: 1 addition & 2 deletions internal/engine/dolphin/convert.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package dolphin

import (
"fmt"
"log"
"strings"

Expand Down Expand Up @@ -144,7 +143,7 @@ func (c *cc) convertAlterTableStmt(n *pcast.AlterTableStmt) ast.Node {

default:
if debug.Active {
fmt.Printf("dolphin.convert: Unknown alter table cmd %v\n", spec.Tp)
log.Printf("dolphin.convert: Unknown alter table cmd %v\n", spec.Tp)
}
continue
}
Expand Down
1 change: 1 addition & 0 deletions internal/metadata/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func ParseQueryFlags(comments []string) (map[string]bool, error) {
if strings.HasPrefix(cleanLine, "@") {
flagName := strings.SplitN(cleanLine, " ", 2)[0]
flags[flagName] = true
continue
}
}
return flags, nil
Expand Down

0 comments on commit dddfe4f

Please sign in to comment.