Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/sqlc: Add the vet subcommand #2344

Merged
merged 6 commits into from
Jun 27, 2023
Merged

cmd/sqlc: Add the vet subcommand #2344

merged 6 commits into from
Jun 27, 2023

Conversation

kyleconroy
Copy link
Collaborator

@kyleconroy kyleconroy commented Jun 20, 2023

vet

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:

message Config
{
  string version = 1;
  string engine = 2 ;
  repeated string schema = 3;
  repeated string queries = 4;
}

message Query
{
  // SQL body
  string sql = 1;
  // Name of the query
  string name = 2; 
  // One of :many, :one, :exec, etc.
  string cmd = 3;
  // Query parameters, if any
  repeated Parameter params = 4;
}


message Parameter
{
  int32 number = 1;
}

This struct would be expanded in the future to include more query information. We may also add information from a running database, such as the result from EXPLAIN.

Example

While these examples are simplistic, they give you an idea on what types of rules you can write.

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"

@kyleconroy kyleconroy marked this pull request as ready for review June 20, 2023 21:32
@@ -24,6 +25,8 @@ require (
gopkg.in/yaml.v3 v3.0.1
)

require github.com/stoewer/go-strcase v1.2.0 // indirect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have ended up in the longer list of indirect dependencies below right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure honestly. I'll make sure to run go mod tidy as that will put it in the right spot.

msgs := map[string]string{}

for _, c := range conf.Rules {
if c.Name == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider putting some of this basic validation inside of a shared config parse/validation area? Not sure if config.Validate() would be the right spot, or somewhere else, or maybe this idea just isn't correct if you think each subcommand should be responsible for validating its own "section" of the config (if it has one)?

@kyleconroy kyleconroy merged commit 3e0fca0 into main Jun 27, 2023
@kyleconroy kyleconroy deleted the kyle/sqlc-vet branch June 27, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants