Extended the @sqlc-vet-disable flag to support the skipping of prov…

…ided 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.
sqlc-dev · kyleconroy · Dec 2, 2024 · Sep 28, 2024 · Sep 28, 2024 · Sep 28, 2024
commit d8a81106031a3ac5d8c1d84aa5b3b8cf94ddeb70
diff --git a/internal/cmd/vet.go b/internal/cmd/vet.go
@@ -11,6 +11,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime/trace"
+	"slices"
 	"strings"
 	"time"
 
@@ -538,11 +539,23 @@ 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].Metadata.Flags[QueryFlagSqlcVetDisable] {
-			if debug.Active {
-				log.Printf("Skipping vet rules for query: %s\n", query.Name)
+		md := result.Queries[i].Metadata
+		if md.Flags[QueryFlagSqlcVetDisable] {
+			// If the vet disable flag is specified without any rules listed, all rules are ignored.
+			if len(md.RuleSkiplist) == 0 {
+				if debug.Active {
+					log.Printf("Skipping all vet rules for query: %s\n", query.Name)
+				}
+				continue
+			}
+
+			// Rules which are listed to be disabled but not declared in the config file are rejected.
+			for r := range md.RuleSkiplist {
+				if !slices.Contains(s.Rules, r) {
+					fmt.Fprintf(c.Stderr, "%s: %s: rule-check error: rule %q does not exist in the config file\n", query.Filename, query.Name, r)
+					errored = true
+				}
 			}
-			continue
 		}
 
 		evalMap := map[string]any{
@@ -551,74 +564,81 @@ func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
 		}
 
 		for _, name := range s.Rules {
-			rule, ok := c.Rules[name]
-			if !ok {
-				return fmt.Errorf("type-check error: a rule with the name '%s' does not exist", name)
-			}
-
-			if rule.NeedsPrepare {
-				if prep == nil {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: database connection required\n", query.Filename, query.Name, name)
-					errored = true
-					continue
+			if _, skip := md.RuleSkiplist[name]; skip {
+				if debug.Active {
+					log.Printf("Skipping vet rule %q for query: %s\n", name, query.Name)
 				}
-				prepName := fmt.Sprintf("sqlc_vet_%d_%d", time.Now().Unix(), i)
-				if err := prep.Prepare(ctx, prepName, query.Text); err != nil {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: %s\n", query.Filename, query.Name, name, err)
-					errored = true
-					continue
+			} else {
+				rule, ok := c.Rules[name]
+				if !ok {
+					return fmt.Errorf("type-check error: a rule with the name '%s' does not exist", name)
 				}
-			}
 
-			// short-circuit for "sqlc/db-prepare" rule which doesn't have a CEL program
-			if rule.Program == nil {
-				continue
-			}
-
-			// Get explain output for this query if we need it
-			_, pgsqlOK := evalMap["postgresql"]
-			_, mysqlOK := evalMap["mysql"]
-			if rule.NeedsExplain && !(pgsqlOK || mysqlOK) {
-				if expl == nil {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: database connection required\n", query.Filename, query.Name, name)
-					errored = true
-					continue
+				if rule.NeedsPrepare {
+					if prep == nil {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: database connection required\n", query.Filename, query.Name, name)
+						errored = true
+						continue
+					}
+					prepName := fmt.Sprintf("sqlc_vet_%d_%d", time.Now().Unix(), i)
+					if err := prep.Prepare(ctx, prepName, query.Text); err != nil {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s: error preparing query: %s\n", query.Filename, query.Name, name, err)
+						errored = true
+						continue
+					}
 				}
-				engineOutput, err := expl.Explain(ctx, query.Text, query.Params...)
-				if err != nil {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: %s\n", query.Filename, query.Name, name, err)
-					errored = true
+
+				// short-circuit for "sqlc/db-prepare" rule which doesn't have a CEL program
+				if rule.Program == nil {
 					continue
 				}
 
-				evalMap["postgresql"] = engineOutput.PostgreSQL
-				evalMap["mysql"] = engineOutput.MySQL
-			}
+				// Get explain output for this query if we need it
+				_, pgsqlOK := evalMap["postgresql"]
+				_, mysqlOK := evalMap["mysql"]
+				if rule.NeedsExplain && !(pgsqlOK || mysqlOK) {
+					if expl == nil {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: database connection required\n", query.Filename, query.Name, name)
+						errored = true
+						continue
+					}
+					engineOutput, err := expl.Explain(ctx, query.Text, query.Params...)
+					if err != nil {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s: error explaining query: %s\n", query.Filename, query.Name, name, err)
+						errored = true
+						continue
+					}
+
+					evalMap["postgresql"] = engineOutput.PostgreSQL
+					evalMap["mysql"] = engineOutput.MySQL
+				}
 
-			if debug.Debug.DumpVetEnv {
-				fmt.Printf("vars for rule '%s' evaluating against query '%s':\n", name, query.Name)
-				debug.DumpAsJSON(evalMap)
-			}
+				if debug.Debug.DumpVetEnv {
+					fmt.Printf("vars for rule '%s' evaluating against query '%s':\n", name, query.Name)
+					debug.DumpAsJSON(evalMap)
+				}
 
-			out, _, err := (*rule.Program).Eval(evalMap)
-			if err != nil {
-				return err
-			}
-			tripped, ok := out.Value().(bool)
-			if !ok {
-				return fmt.Errorf("expression returned non-bool value: %v", out.Value())
-			}
-			if tripped {
-				// TODO: Get line numbers in the output
-				if rule.Message == "" {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s\n", query.Filename, query.Name, name)
-				} else {
-					fmt.Fprintf(c.Stderr, "%s: %s: %s: %s\n", query.Filename, query.Name, name, rule.Message)
+				out, _, err := (*rule.Program).Eval(evalMap)
+				if err != nil {
+					return err
+				}
+				tripped, ok := out.Value().(bool)
+				if !ok {
+					return fmt.Errorf("expression returned non-bool value: %v", out.Value())
+				}
+				if tripped {
+					// TODO: Get line numbers in the output
+					if rule.Message == "" {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s\n", query.Filename, query.Name, name)
+					} else {
+						fmt.Fprintf(c.Stderr, "%s: %s: %s: %s\n", query.Filename, query.Name, name, rule.Message)
+					}
+					errored = true
 				}
-				errored = true
 			}
 		}
 	}
+
 	if errored {
 		return ErrFailedChecks
 	}

diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go
@@ -65,7 +65,7 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query,
 		return nil, err
 	}
 
-	md.Params, md.Flags, err = metadata.ParseParamsAndFlags(cleanedComments)
+	md.Params, md.Flags, md.RuleSkiplist, err = metadata.ParseParamsAndFlags(cleanedComments)
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go
@@ -18,6 +18,10 @@ type Metadata struct {
 	Params   map[string]string
 	Flags    map[string]bool
 
+	// RuleSkiplist contains the names of rules to disable vetting for.
+	// If the map is empty, but the disable vet flag is specified, then all rules are ignored.
+	RuleSkiplist map[string]struct{}
+
 	Filename string
 }
 
@@ -113,9 +117,10 @@ func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string
 	return "", "", nil
 }
 
-func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool, error) {
+func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool, map[string]struct{}, error) {
 	params := make(map[string]string)
 	flags := make(map[string]bool)
+	ruleSkiplist := make(map[string]struct{})
 
 	for _, line := range comments {
 		s := bufio.NewScanner(strings.NewReader(line))
@@ -138,14 +143,27 @@ func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool,
 				rest = append(rest, paramToken)
 			}
 			params[name] = strings.Join(rest, " ")
+
+		case "@sqlc-vet-disable":
+			flags[token] = true
+
+			// Vet rules can all be disabled in the same line or split across lines .i.e.
+			// /* @sqlc-vet-disable sqlc/db-prepare delete-without-where */
+			// is equivalent to:
+			// /* @sqlc-vet-disable sqlc/db-prepare */
+			// /* @sqlc-vet-disable delete-without-where */
+			for s.Scan() {
+				ruleSkiplist[s.Text()] = struct{}{}
+			}
+
 		default:
 			flags[token] = true
 		}
 
 		if s.Err() != nil {
-			return params, flags, s.Err()
+			return params, flags, ruleSkiplist, s.Err()
 		}
 	}
 
-	return params, flags, nil
+	return params, flags, ruleSkiplist, nil
 }