Skip to content

Commit 7193fc5

Browse files
committed
Extended the @sqlc-vet-disable flag to support the skipping of provided rules.
This is an update to the original behaviour which had an "all-or-nothing" behaviour, either running all the rules on a query or skipping them all. Specified rules which aren't declared in the sqlc config file get rejected.
1 parent df9413c commit 7193fc5

File tree

3 files changed

+101
-63
lines changed

3 files changed

+101
-63
lines changed

internal/cmd/vet.go

Lines changed: 79 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"os"
1212
"path/filepath"
1313
"runtime/trace"
14+
"slices"
1415
"strings"
1516
"time"
1617

@@ -538,11 +539,23 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
538539
req := codeGenRequest(result, combo)
539540
cfg := vetConfig(req)
540541
for i, query := range req.Queries {
541-
if result.Queries[i].Metadata.Flags[QueryFlagSqlcVetDisable] {
542-
if debug.Active {
543-
log.Printf("Skipping vet rules for query: %s\n", query.Name)
542+
md := result.Queries[i].Metadata
543+
if md.Flags[QueryFlagSqlcVetDisable] {
544+
// If the vet disable flag is specified without any rules listed, all rules are ignored.
545+
if len(md.RuleSkiplist) == 0 {
546+
if debug.Active {
547+
log.Printf("Skipping all vet rules for query: %s\n", query.Name)
548+
}
549+
continue
550+
}
551+
552+
// Rules which are listed to be disabled but not declared in the config file are rejected.
553+
for r := range md.RuleSkiplist {
554+
if !slices.Contains(s.Rules, r) {
555+
fmt.Fprintf(c.Stderr, "%s: %s: rule-check error: rule %q does not exist in the config file\n", query.Filename, query.Name, r)
556+
errored = true
557+
}
544558
}
545-
continue
546559
}
547560

548561
evalMap := map[string]any{
@@ -551,74 +564,81 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
551564
}
552565

553566
for _, name := range s.Rules {
554-
rule, ok := c.Rules[name]
555-
if !ok {
556-
return fmt.Errorf("type-check error: a rule with the name '%s' does not exist", name)
557-
}
558-
559-
if rule.NeedsPrepare {
560-
if prep == nil {
561-
fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: database connection required\n", query.Filename, query.Name, name)
562-
errored = true
563-
continue
567+
if _, skip := md.RuleSkiplist[name]; skip {
568+
if debug.Active {
569+
log.Printf("Skipping vet rule %q for query: %s\n", name, query.Name)
564570
}
565-
prepName := fmt.Sprintf("sqlc_vet_%d_%d", time.Now().Unix(), i)
566-
if err := prep.Prepare(ctx, prepName, query.Text); err != nil {
567-
fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: %s\n", query.Filename, query.Name, name, err)
568-
errored = true
569-
continue
571+
} else {
572+
rule, ok := c.Rules[name]
573+
if !ok {
574+
return fmt.Errorf("type-check error: a rule with the name '%s' does not exist", name)
570575
}
571-
}
572576

573-
// short-circuit for "sqlc/db-prepare" rule which doesn't have a CEL program
574-
if rule.Program == nil {
575-
continue
576-
}
577-
578-
// Get explain output for this query if we need it
579-
_, pgsqlOK := evalMap["postgresql"]
580-
_, mysqlOK := evalMap["mysql"]
581-
if rule.NeedsExplain && !(pgsqlOK || mysqlOK) {
582-
if expl == nil {
583-
fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: database connection required\n", query.Filename, query.Name, name)
584-
errored = true
585-
continue
577+
if rule.NeedsPrepare {
578+
if prep == nil {
579+
fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: database connection required\n", query.Filename, query.Name, name)
580+
errored = true
581+
continue
582+
}
583+
prepName := fmt.Sprintf("sqlc_vet_%d_%d", time.Now().Unix(), i)
584+
if err := prep.Prepare(ctx, prepName, query.Text); err != nil {
585+
fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: %s\n", query.Filename, query.Name, name, err)
586+
errored = true
587+
continue
588+
}
586589
}
587-
engineOutput, err := expl.Explain(ctx, query.Text, query.Params...)
588-
if err != nil {
589-
fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: %s\n", query.Filename, query.Name, name, err)
590-
errored = true
590+
591+
// short-circuit for "sqlc/db-prepare" rule which doesn't have a CEL program
592+
if rule.Program == nil {
591593
continue
592594
}
593595

594-
evalMap["postgresql"] = engineOutput.PostgreSQL
595-
evalMap["mysql"] = engineOutput.MySQL
596-
}
596+
// Get explain output for this query if we need it
597+
_, pgsqlOK := evalMap["postgresql"]
598+
_, mysqlOK := evalMap["mysql"]
599+
if rule.NeedsExplain && !(pgsqlOK || mysqlOK) {
600+
if expl == nil {
601+
fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: database connection required\n", query.Filename, query.Name, name)
602+
errored = true
603+
continue
604+
}
605+
engineOutput, err := expl.Explain(ctx, query.Text, query.Params...)
606+
if err != nil {
607+
fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: %s\n", query.Filename, query.Name, name, err)
608+
errored = true
609+
continue
610+
}
611+
612+
evalMap["postgresql"] = engineOutput.PostgreSQL
613+
evalMap["mysql"] = engineOutput.MySQL
614+
}
597615

598-
if debug.Debug.DumpVetEnv {
599-
fmt.Printf("vars for rule '%s' evaluating against query '%s':\n", name, query.Name)
600-
debug.DumpAsJSON(evalMap)
601-
}
616+
if debug.Debug.DumpVetEnv {
617+
fmt.Printf("vars for rule '%s' evaluating against query '%s':\n", name, query.Name)
618+
debug.DumpAsJSON(evalMap)
619+
}
602620

603-
out, _, err := (*rule.Program).Eval(evalMap)
604-
if err != nil {
605-
return err
606-
}
607-
tripped, ok := out.Value().(bool)
608-
if !ok {
609-
return fmt.Errorf("expression returned non-bool value: %v", out.Value())
610-
}
611-
if tripped {
612-
// TODO: Get line numbers in the output
613-
if rule.Message == "" {
614-
fmt.Fprintf(c.Stderr, "%s: %s: %s\n", query.Filename, query.Name, name)
615-
} else {
616-
fmt.Fprintf(c.Stderr, "%s: %s: %s: %s\n", query.Filename, query.Name, name, rule.Message)
621+
out, _, err := (*rule.Program).Eval(evalMap)
622+
if err != nil {
623+
return err
624+
}
625+
tripped, ok := out.Value().(bool)
626+
if !ok {
627+
return fmt.Errorf("expression returned non-bool value: %v", out.Value())
628+
}
629+
if tripped {
630+
// TODO: Get line numbers in the output
631+
if rule.Message == "" {
632+
fmt.Fprintf(c.Stderr, "%s: %s: %s\n", query.Filename, query.Name, name)
633+
} else {
634+
fmt.Fprintf(c.Stderr, "%s: %s: %s: %s\n", query.Filename, query.Name, name, rule.Message)
635+
}
636+
errored = true
617637
}
618-
errored = true
619638
}
620639
}
621640
}
641+
622642
if errored {
623643
return ErrFailedChecks
624644
}

internal/compiler/parse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query,
6565
return nil, err
6666
}
6767

68-
md.Params, md.Flags, err = metadata.ParseParamsAndFlags(cleanedComments)
68+
md.Params, md.Flags, md.RuleSkiplist, err = metadata.ParseParamsAndFlags(cleanedComments)
6969
if err != nil {
7070
return nil, err
7171
}

internal/metadata/meta.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ type Metadata struct {
1818
Params map[string]string
1919
Flags map[string]bool
2020

21+
// RuleSkiplist contains the names of rules to disable vetting for.
22+
// If the map is empty, but the disable vet flag is specified, then all rules are ignored.
23+
RuleSkiplist map[string]struct{}
24+
2125
Filename string
2226
}
2327

@@ -113,9 +117,10 @@ func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string
113117
return "", "", nil
114118
}
115119

116-
func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool, error) {
120+
func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool, map[string]struct{}, error) {
117121
params := make(map[string]string)
118122
flags := make(map[string]bool)
123+
ruleSkiplist := make(map[string]struct{})
119124

120125
for _, line := range comments {
121126
s := bufio.NewScanner(strings.NewReader(line))
@@ -138,14 +143,27 @@ func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool,
138143
rest = append(rest, paramToken)
139144
}
140145
params[name] = strings.Join(rest, " ")
146+
147+
case "@sqlc-vet-disable":
148+
flags[token] = true
149+
150+
// Vet rules can all be disabled in the same line or split across lines .i.e.
151+
// /* @sqlc-vet-disable sqlc/db-prepare delete-without-where */
152+
// is equivalent to:
153+
// /* @sqlc-vet-disable sqlc/db-prepare */
154+
// /* @sqlc-vet-disable delete-without-where */
155+
for s.Scan() {
156+
ruleSkiplist[s.Text()] = struct{}{}
157+
}
158+
141159
default:
142160
flags[token] = true
143161
}
144162

145163
if s.Err() != nil {
146-
return params, flags, s.Err()
164+
return params, flags, ruleSkiplist, s.Err()
147165
}
148166
}
149167

150-
return params, flags, nil
168+
return params, flags, ruleSkiplist, nil
151169
}

0 commit comments

Comments
 (0)