From 4db5c66374eee141029c7e7af70b7a70aa05de3c Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 12 Oct 2023 16:04:57 -0700 Subject: [PATCH 01/12] feat(compiler): Parse query parameter metadata from comments --- internal/cmd/shim.go | 8 +- internal/cmd/vet.go | 2 +- internal/compiler/compile.go | 11 +-- internal/compiler/parse.go | 17 ++--- internal/compiler/query.go | 9 +-- .../invalid_queries_foo/pgx/v4/stderr.txt | 2 +- .../invalid_queries_foo/pgx/v5/stderr.txt | 2 +- .../invalid_queries_foo/stdlib/stderr.txt | 2 +- internal/metadata/meta.go | 74 +++++++++++++------ internal/metadata/meta_test.go | 52 ++++++++----- internal/source/code.go | 1 + 11 files changed, 110 insertions(+), 70 deletions(-) diff --git a/internal/cmd/shim.go b/internal/cmd/shim.go index 5f108868c7..3d06cc775e 100644 --- a/internal/cmd/shim.go +++ b/internal/cmd/shim.go @@ -181,13 +181,13 @@ func pluginQueries(r *compiler.Result) []*plugin.Query { } } out = append(out, &plugin.Query{ - Name: q.Name, - Cmd: q.Cmd, + Name: q.Metadata.Name, + Cmd: q.Metadata.Cmd, Text: q.SQL, - Comments: q.Comments, + Comments: q.Metadata.Comments, Columns: columns, Params: params, - Filename: q.Filename, + Filename: q.Metadata.Filename, InsertIntoTable: iit, }) } diff --git a/internal/cmd/vet.go b/internal/cmd/vet.go index 31aa3ec33c..4f79fc8e8b 100644 --- a/internal/cmd/vet.go +++ b/internal/cmd/vet.go @@ -545,7 +545,7 @@ 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 result.Queries[i].Metadata.Flags[QueryFlagSqlcVetDisable] { if debug.Active { log.Printf("Skipping vet rules for query: %s\n", query.Name) } diff --git a/internal/compiler/compile.go b/internal/compiler/compile.go index a6744fc6d2..61e21dd9e4 100644 --- a/internal/compiler/compile.go +++ b/internal/compiler/compile.go @@ -90,14 +90,15 @@ func (c *Compiler) parseQueries(o opts.Parser) (*Result, error) { merr.Add(filename, src, loc, err) continue } - if query.Name != "" { - if _, exists := set[query.Name]; exists { - merr.Add(filename, src, stmt.Raw.Pos(), fmt.Errorf("duplicate query name: %s", query.Name)) + queryName := query.Metadata.Name + if queryName != "" { + if _, exists := set[queryName]; exists { + merr.Add(filename, src, stmt.Raw.Pos(), fmt.Errorf("duplicate query name: %s", queryName)) continue } - set[query.Name] = struct{}{} + set[queryName] = struct{}{} } - query.Filename = filepath.Base(filename) + query.Metadata.Filename = filepath.Base(filename) if query != nil { q = append(q, query) } diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index 53e3043c7d..fef1dbdafb 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -43,11 +43,12 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, return nil, errors.New("missing semicolon at end of file") } - name, cmd, err := metadata.ParseQueryNameAndType(strings.TrimSpace(rawSQL), c.parser.CommentSyntax()) + md, err := metadata.ParseQueryMetadata(rawSQL, c.parser.CommentSyntax()) if err != nil { return nil, err } - if err := validate.Cmd(raw.Stmt, name, cmd); err != nil { + + if err := validate.Cmd(raw.Stmt, md.Name, md.Cmd); err != nil { return nil, err } @@ -85,22 +86,14 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, } } - trimmed, comments, err := source.StripComments(expanded) - if err != nil { - return nil, err - } - - flags, err := metadata.ParseQueryFlags(comments) + trimmed, _, err := source.StripComments(expanded) if err != nil { return nil, err } return &Query{ RawStmt: raw, - Cmd: cmd, - Comments: comments, - Name: name, - Flags: flags, + Metadata: md, Params: anlys.Parameters, Columns: anlys.Columns, SQL: trimmed, diff --git a/internal/compiler/query.go b/internal/compiler/query.go index 117cf44813..df580c197b 100644 --- a/internal/compiler/query.go +++ b/internal/compiler/query.go @@ -1,6 +1,7 @@ package compiler import ( + "github.com/sqlc-dev/sqlc/internal/metadata" "github.com/sqlc-dev/sqlc/internal/sql/ast" ) @@ -41,15 +42,9 @@ type Column struct { type Query struct { SQL string - Name string - Cmd string // TODO: Pick a better name. One of: one, many, exec, execrows, copyFrom - Flags map[string]bool + Metadata metadata.Metadata Columns []*Column Params []Parameter - Comments []string - - // XXX: Hack - Filename string // Needed for CopyFrom InsertIntoTable *ast.TableName diff --git a/internal/endtoend/testdata/invalid_queries_foo/pgx/v4/stderr.txt b/internal/endtoend/testdata/invalid_queries_foo/pgx/v4/stderr.txt index 6b0840fc37..06ec54327f 100644 --- a/internal/endtoend/testdata/invalid_queries_foo/pgx/v4/stderr.txt +++ b/internal/endtoend/testdata/invalid_queries_foo/pgx/v4/stderr.txt @@ -1,5 +1,5 @@ # package querytest -query.sql:1:1: invalid query comment: -- name: ListFoos +query.sql:1:1: missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: -- name: ListFoos query.sql:5:1: invalid query comment: -- name: ListFoos :one :many query.sql:8:1: invalid query type: :two query.sql:11:1: query "DeleteFoo" specifies parameter ":one" without containing a RETURNING clause diff --git a/internal/endtoend/testdata/invalid_queries_foo/pgx/v5/stderr.txt b/internal/endtoend/testdata/invalid_queries_foo/pgx/v5/stderr.txt index 6b0840fc37..06ec54327f 100644 --- a/internal/endtoend/testdata/invalid_queries_foo/pgx/v5/stderr.txt +++ b/internal/endtoend/testdata/invalid_queries_foo/pgx/v5/stderr.txt @@ -1,5 +1,5 @@ # package querytest -query.sql:1:1: invalid query comment: -- name: ListFoos +query.sql:1:1: missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: -- name: ListFoos query.sql:5:1: invalid query comment: -- name: ListFoos :one :many query.sql:8:1: invalid query type: :two query.sql:11:1: query "DeleteFoo" specifies parameter ":one" without containing a RETURNING clause diff --git a/internal/endtoend/testdata/invalid_queries_foo/stdlib/stderr.txt b/internal/endtoend/testdata/invalid_queries_foo/stdlib/stderr.txt index 6b0840fc37..06ec54327f 100644 --- a/internal/endtoend/testdata/invalid_queries_foo/stdlib/stderr.txt +++ b/internal/endtoend/testdata/invalid_queries_foo/stdlib/stderr.txt @@ -1,5 +1,5 @@ # package querytest -query.sql:1:1: invalid query comment: -- name: ListFoos +query.sql:1:1: missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: -- name: ListFoos query.sql:5:1: invalid query comment: -- name: ListFoos :one :many query.sql:8:1: invalid query type: :two query.sql:11:1: query "DeleteFoo" specifies parameter ":one" without containing a RETURNING clause diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 4176da1e2b..64dc715c1d 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -1,11 +1,22 @@ package metadata import ( + "bufio" "fmt" "strings" "unicode" ) +type Metadata struct { + Name string + Cmd string + Comments []string + Params map[string]string + Flags map[string]bool + + Filename string +} + type CommentSyntax struct { Dash bool Hash bool @@ -44,8 +55,12 @@ func validateQueryName(name string) error { return nil } -func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string, error) { - for _, line := range strings.Split(t, "\n") { +func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, error) { + md := Metadata{} + s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(rawSql))) + var lines, comments []string + for s.Scan() { + line := s.Text() var prefix string if strings.HasPrefix(line, "--") { if !commentStyle.Dash { @@ -66,9 +81,14 @@ func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string prefix = "#" } if prefix == "" { + lines = append(lines, line) continue } + rest := line[len(prefix):] + rest = strings.TrimSuffix(rest, "*/") + comments = append(comments, rest) + if !strings.HasPrefix(strings.TrimSpace(rest), "name") { continue } @@ -76,35 +96,41 @@ func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string continue } if !strings.HasPrefix(rest, " name: ") { - return "", "", fmt.Errorf("invalid metadata: %s", line) + return md, fmt.Errorf("invalid metadata: %s", line) } - part := strings.Split(strings.TrimSpace(line), " ") - if prefix == "/*" { - part = part[:len(part)-1] // removes the trailing "*/" element - } - if len(part) == 2 { - return "", "", fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: %s", line) + comments = comments[:len(comments)-1] // Remove tha name line from returned comments + + parts := strings.Split(strings.TrimSpace(rest), " ") + + if len(parts) == 2 { + return md, fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: %s", line) } - if len(part) != 4 { - return "", "", fmt.Errorf("invalid query comment: %s", line) + if len(parts) > 3 { + return md, fmt.Errorf("invalid query comment: %s", line) } - queryName := part[2] - queryType := strings.TrimSpace(part[3]) + queryName := parts[1] + queryType := parts[2] switch queryType { case CmdOne, CmdMany, CmdExec, CmdExecResult, CmdExecRows, CmdExecLastId, CmdCopyFrom, CmdBatchExec, CmdBatchMany, CmdBatchOne: default: - return "", "", fmt.Errorf("invalid query type: %s", queryType) + return md, fmt.Errorf("invalid query type: %s", queryType) } if err := validateQueryName(queryName); err != nil { - return "", "", err + return md, err } - return queryName, queryType, nil + md.Name = queryName + md.Cmd = queryType } - return "", "", nil + + md.Comments = comments + md.Params, md.Flags = parseParamsAndFlags(md.Comments) + + return md, s.Err() } -func ParseQueryFlags(comments []string) (map[string]bool, error) { +func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool) { + params := make(map[string]string) flags := make(map[string]bool) for _, line := range comments { cleanLine := strings.TrimPrefix(line, "--") @@ -113,10 +139,16 @@ func ParseQueryFlags(comments []string) (map[string]bool, error) { cleanLine = strings.TrimSuffix(cleanLine, "*/") cleanLine = strings.TrimSpace(cleanLine) if strings.HasPrefix(cleanLine, "@") { - flagName := strings.SplitN(cleanLine, " ", 2)[0] - flags[flagName] = true + parts := strings.SplitN(cleanLine, " ", 2) + name := parts[0] + switch name { + case "param": + params[name] = parts[1] + default: + flags[name] = true + } continue } } - return flags, nil + return params, flags } diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index cbfcb6fba6..625e642e9e 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -16,8 +16,11 @@ func TestParseQueryNameAndType(t *testing.T) { `--name: CreateFoo :two`, "-- name:CreateFoo", `--name:CreateFoo :two`, + `-- name: CreateFoo :two`, + `-- name: CreateFoo :two`, + `-- name: CreateFoo :two`, } { - if _, _, err := ParseQueryNameAndType(query, CommentSyntax{Dash: true}); err == nil { + if _, err := ParseQueryMetadata(query, CommentSyntax{Dash: true}); err == nil { t.Errorf("expected invalid metadata: %q", query) } } @@ -27,21 +30,26 @@ func TestParseQueryNameAndType(t *testing.T) { `-- name comment`, `--name comment`, } { - if _, _, err := ParseQueryNameAndType(query, CommentSyntax{Dash: true}); err != nil { + if _, err := ParseQueryMetadata(query, CommentSyntax{Dash: true}); err != nil { t.Errorf("expected valid comment: %q", query) } } - query := `-- name: CreateFoo :one` - queryName, queryType, err := ParseQueryNameAndType(query, CommentSyntax{Dash: true}) - if err != nil { - t.Errorf("expected valid metadata: %q", query) - } - if queryName != "CreateFoo" { - t.Errorf("incorrect queryName parsed: %q", query) - } - if queryType != CmdOne { - t.Errorf("incorrect queryType parsed: %q", query) + for query, cs := range map[string]CommentSyntax{ + `-- name: CreateFoo :one`: {Dash: true}, + `# name: CreateFoo :one`: {Hash: true}, + `/* name: CreateFoo :one */`: {SlashStar: true}, + } { + queryMetadata, err := ParseQueryMetadata(query, cs) + if err != nil { + t.Errorf("expected valid metadata: %q", query) + } + if queryMetadata.Name != "CreateFoo" { + t.Errorf("incorrect queryName parsed: %q", query) + } + if queryMetadata.Cmd != CmdOne { + t.Errorf("incorrect queryType parsed: %q", query) + } } } @@ -52,14 +60,24 @@ func TestParseQueryFlags(t *testing.T) { "-- name: CreateFoo :one", "-- @flag-foo", }, + { + "-- name: CreateFoo :one", + "-- @flag-foo @flag-bar", + }, + { + "-- name: GetFoos :many", + "-- @param @flag-bar UUID", + "-- @flag-foo", + }, } { - flags, err := ParseQueryFlags(comments) - if err != nil { - t.Errorf("expected query flags to parse, got error: %s", err) - } + _, flags := parseParamsAndFlags(comments) if !flags["@flag-foo"] { t.Errorf("expected flag not found") } + + if flags["@flag-bar"] { + t.Errorf("unexpected flag found") + } } -} \ No newline at end of file +} diff --git a/internal/source/code.go b/internal/source/code.go index f34e3e3684..e60d6c7783 100644 --- a/internal/source/code.go +++ b/internal/source/code.go @@ -90,6 +90,7 @@ func Mutate(raw string, a []Edit) (string, error) { return s, nil } +// TODO remove and roll into query parsing func StripComments(sql string) (string, []string, error) { s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(sql))) var lines, comments []string From 655c28f886d24f02acfbb3637bc8c58c5b72309b Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 12 Oct 2023 22:53:00 -0700 Subject: [PATCH 02/12] update comment stripping, fix a test --- internal/compiler/parse.go | 2 +- .../comment_syntax/mysql/go/query.sql.go | 1 - internal/metadata/meta.go | 3 +-- internal/source/code.go | 20 ++++++------------- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index fef1dbdafb..0d7f1c3e07 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -86,7 +86,7 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, } } - trimmed, _, err := source.StripComments(expanded) + trimmed, err := source.StripComments(expanded) if err != nil { return nil, err } diff --git a/internal/endtoend/testdata/comment_syntax/mysql/go/query.sql.go b/internal/endtoend/testdata/comment_syntax/mysql/go/query.sql.go index 7f4b916150..29909a8da2 100644 --- a/internal/endtoend/testdata/comment_syntax/mysql/go/query.sql.go +++ b/internal/endtoend/testdata/comment_syntax/mysql/go/query.sql.go @@ -22,7 +22,6 @@ func (q *Queries) DoubleDash(ctx context.Context) (sql.NullString, error) { } const hash = `-- name: Hash :one -# name: Hash :one SELECT bar FROM foo LIMIT 1 ` diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 64dc715c1d..16b268ebaa 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -58,7 +58,7 @@ func validateQueryName(name string) error { func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, error) { md := Metadata{} s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(rawSql))) - var lines, comments []string + var comments []string for s.Scan() { line := s.Text() var prefix string @@ -81,7 +81,6 @@ func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, er prefix = "#" } if prefix == "" { - lines = append(lines, line) continue } diff --git a/internal/source/code.go b/internal/source/code.go index e60d6c7783..927be880b0 100644 --- a/internal/source/code.go +++ b/internal/source/code.go @@ -90,29 +90,21 @@ func Mutate(raw string, a []Edit) (string, error) { return s, nil } -// TODO remove and roll into query parsing -func StripComments(sql string) (string, []string, error) { +func StripComments(sql string) (string, error) { s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(sql))) - var lines, comments []string + var lines []string for s.Scan() { t := s.Text() - if strings.HasPrefix(t, "-- name:") { - continue - } - if strings.HasPrefix(t, "/* name:") && strings.HasSuffix(t, "*/") { - continue - } if strings.HasPrefix(t, "--") { - comments = append(comments, strings.TrimPrefix(t, "--")) continue } if strings.HasPrefix(t, "/*") && strings.HasSuffix(t, "*/") { - t = strings.TrimPrefix(t, "/*") - t = strings.TrimSuffix(t, "*/") - comments = append(comments, t) + continue + } + if strings.HasPrefix(t, "#") { continue } lines = append(lines, t) } - return strings.Join(lines, "\n"), comments, s.Err() + return strings.Join(lines, "\n"), s.Err() } From 97b05713d70c0a4cff5461c76687ce7443d56c94 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 12 Oct 2023 23:09:43 -0700 Subject: [PATCH 03/12] parse `@param` name correctly --- internal/metadata/meta.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 16b268ebaa..8ddee2ffaf 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -141,8 +141,9 @@ func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool) parts := strings.SplitN(cleanLine, " ", 2) name := parts[0] switch name { - case "param": - params[name] = parts[1] + case "@param": + paramParts := strings.SplitN(parts[1], " ", 2) + params[paramParts[0]] = paramParts[1] default: flags[name] = true } From 767db2c1d9ff1b196949896759fea99a90754e3a Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Thu, 12 Oct 2023 23:26:19 -0700 Subject: [PATCH 04/12] add a test for metadata param parsing --- internal/metadata/meta_test.go | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index 625e642e9e..fb080f0de6 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -54,6 +54,42 @@ func TestParseQueryNameAndType(t *testing.T) { } +func TestParseQueryParams(t *testing.T) { + for _, comments := range [][]string{ + { + "-- name: CreateFoo :one", + "-- @param foo_id UUID", + }, + { + "-- name: CreateFoo :one", + "-- @param foo_id UUID", + "-- invalid", + }, + { + "-- name: CreateFoo :one", + "-- @invalid", + "-- @param foo_id UUID", + }, + { + "-- name: GetFoos :many", + "-- @param foo_id UUID", + "-- @param @invalid UUID", + }, + } { + params, _ := parseParamsAndFlags(comments) + + _, ok := params["foo_id"] + if !ok { + t.Errorf("expected param not found") + } + + _, ok = params["invalid"] + if ok { + t.Errorf("unexpected param found") + } + } +} + func TestParseQueryFlags(t *testing.T) { for _, comments := range [][]string{ { From 2fdb3fa6e70a27aec8adafea81f46bbd94f24298 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 10:25:49 -0700 Subject: [PATCH 05/12] use a word scanner for flag and param comment line parsing --- internal/metadata/meta.go | 57 +++++++++++++++++++++++----------- internal/metadata/meta_test.go | 32 +++++++++++++++++-- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 8ddee2ffaf..133dac9da1 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -123,32 +123,53 @@ func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, er } md.Comments = comments - md.Params, md.Flags = parseParamsAndFlags(md.Comments) + + var err error + md.Params, md.Flags, err = parseParamsAndFlags(md.Comments) + if err != nil { + return md, err + } return md, s.Err() } -func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool) { +func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool, error) { params := make(map[string]string) flags := make(map[string]bool) + for _, line := range comments { - cleanLine := strings.TrimPrefix(line, "--") - cleanLine = strings.TrimPrefix(cleanLine, "/*") - cleanLine = strings.TrimPrefix(cleanLine, "#") - cleanLine = strings.TrimSuffix(cleanLine, "*/") - cleanLine = strings.TrimSpace(cleanLine) - if strings.HasPrefix(cleanLine, "@") { - parts := strings.SplitN(cleanLine, " ", 2) - name := parts[0] - switch name { - case "@param": - paramParts := strings.SplitN(parts[1], " ", 2) - params[paramParts[0]] = paramParts[1] - default: - flags[name] = true - } + s := bufio.NewScanner(strings.NewReader(line)) + s.Split(bufio.ScanWords) + + s.Scan() // The first token is always the comment indicator, e.g. "--" + s.Scan() + token := s.Text() + + if !strings.HasPrefix(token, "@") { continue } + + switch token { + case "@param": + s.Scan() + name := s.Text() + var rest []string + for s.Scan() { + paramToken := s.Text() + if paramToken == "*/" { + break + } + rest = append(rest, paramToken) + } + params[name] = strings.Join(rest, " ") + default: + flags[token] = true + } + + if s.Err() != nil { + return params, flags, s.Err() + } } - return params, flags + + return params, flags, nil } diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index fb080f0de6..a446deeed4 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -60,6 +60,14 @@ func TestParseQueryParams(t *testing.T) { "-- name: CreateFoo :one", "-- @param foo_id UUID", }, + { + "/* name: CreateFoo :one */", + "/* @param foo_id UUID */", + }, + { + "# name: CreateFoo :one", + "# @param foo_id UUID", + }, { "-- name: CreateFoo :one", "-- @param foo_id UUID", @@ -76,13 +84,20 @@ func TestParseQueryParams(t *testing.T) { "-- @param @invalid UUID", }, } { - params, _ := parseParamsAndFlags(comments) + params, _, err := parseParamsAndFlags(comments) + if err != nil { + t.Error("expected comments to parse:", err) + } - _, ok := params["foo_id"] + pt, ok := params["foo_id"] if !ok { t.Errorf("expected param not found") } + if pt != "UUID" { + t.Error("unexpected param metadata:", pt) + } + _, ok = params["invalid"] if ok { t.Errorf("unexpected param found") @@ -96,6 +111,14 @@ func TestParseQueryFlags(t *testing.T) { "-- name: CreateFoo :one", "-- @flag-foo", }, + { + "/* name: CreateFoo :one */", + "/* @flag-foo */", + }, + { + "# name: CreateFoo :one", + "# @flag-foo", + }, { "-- name: CreateFoo :one", "-- @flag-foo @flag-bar", @@ -106,7 +129,10 @@ func TestParseQueryFlags(t *testing.T) { "-- @flag-foo", }, } { - _, flags := parseParamsAndFlags(comments) + _, flags, err := parseParamsAndFlags(comments) + if err != nil { + t.Error("expected comments to parse:", err) + } if !flags["@flag-foo"] { t.Errorf("expected flag not found") From a6c0d238fb074e12f4f085e447def00b714c441f Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 11:17:58 -0700 Subject: [PATCH 06/12] forgot that I partially cleaned comments already --- internal/metadata/meta.go | 1 - internal/metadata/meta_test.go | 53 ++++++++++++++++------------------ 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 133dac9da1..7e7d5a1977 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -141,7 +141,6 @@ func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool, s := bufio.NewScanner(strings.NewReader(line)) s.Split(bufio.ScanWords) - s.Scan() // The first token is always the comment indicator, e.g. "--" s.Scan() token := s.Text() diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index a446deeed4..a31d335081 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -57,31 +57,27 @@ func TestParseQueryNameAndType(t *testing.T) { func TestParseQueryParams(t *testing.T) { for _, comments := range [][]string{ { - "-- name: CreateFoo :one", - "-- @param foo_id UUID", + " name: CreateFoo :one", + " @param foo_id UUID", }, { - "/* name: CreateFoo :one */", - "/* @param foo_id UUID */", + " name: CreateFoo :one ", + " @param foo_id UUID ", }, { - "# name: CreateFoo :one", - "# @param foo_id UUID", + " name: CreateFoo :one", + " @param foo_id UUID", + " invalid", }, { - "-- name: CreateFoo :one", - "-- @param foo_id UUID", - "-- invalid", + " name: CreateFoo :one", + " @invalid", + " @param foo_id UUID", }, { - "-- name: CreateFoo :one", - "-- @invalid", - "-- @param foo_id UUID", - }, - { - "-- name: GetFoos :many", - "-- @param foo_id UUID", - "-- @param @invalid UUID", + " name: GetFoos :many ", + " @param foo_id UUID ", + " @param @invalid UUID ", }, } { params, _, err := parseParamsAndFlags(comments) @@ -108,25 +104,26 @@ func TestParseQueryParams(t *testing.T) { func TestParseQueryFlags(t *testing.T) { for _, comments := range [][]string{ { - "-- name: CreateFoo :one", - "-- @flag-foo", + " name: CreateFoo :one", + " @flag-foo", }, { - "/* name: CreateFoo :one */", - "/* @flag-foo */", + " name: CreateFoo :one ", + " @flag-foo ", }, { - "# name: CreateFoo :one", - "# @flag-foo", + " name: CreateFoo :one", + " @flag-foo @flag-bar", }, { - "-- name: CreateFoo :one", - "-- @flag-foo @flag-bar", + " name: GetFoos :many", + " @param @flag-bar UUID", + " @flag-foo", }, { - "-- name: GetFoos :many", - "-- @param @flag-bar UUID", - "-- @flag-foo", + " name: GetFoos :many", + " @flag-foo", + " @param @flag-bar UUID", }, } { _, flags, err := parseParamsAndFlags(comments) From f83ed9934a0d9cadecb16f5b49518238c6f65c19 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 12:09:36 -0700 Subject: [PATCH 07/12] slightly more useful metadata test output --- internal/metadata/meta_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index a31d335081..a76af7d6ed 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -45,10 +45,10 @@ func TestParseQueryNameAndType(t *testing.T) { t.Errorf("expected valid metadata: %q", query) } if queryMetadata.Name != "CreateFoo" { - t.Errorf("incorrect queryName parsed: %q", query) + t.Errorf("incorrect queryName parsed: (%q) %q", queryMetadata.Name, query) } if queryMetadata.Cmd != CmdOne { - t.Errorf("incorrect queryType parsed: %q", query) + t.Errorf("incorrect queryType parsed: (%q) %q", queryMetadata.Cmd, query) } } From 20ff7f142b2d1633fc7902a62b1f34cdc99766f5 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 12:46:53 -0700 Subject: [PATCH 08/12] drop unnecessary metadata test cases --- internal/metadata/meta_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index a76af7d6ed..ff208f28fa 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -16,9 +16,6 @@ func TestParseQueryNameAndType(t *testing.T) { `--name: CreateFoo :two`, "-- name:CreateFoo", `--name:CreateFoo :two`, - `-- name: CreateFoo :two`, - `-- name: CreateFoo :two`, - `-- name: CreateFoo :two`, } { if _, err := ParseQueryMetadata(query, CommentSyntax{Dash: true}); err == nil { t.Errorf("expected invalid metadata: %q", query) From 689832cb84c71dc8c1e4df4e76f365a6fa27d871 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 15:57:14 -0700 Subject: [PATCH 09/12] walk back changes, implement comment cleaning in source package --- internal/compiler/compile.go | 4 +- internal/compiler/parse.go | 17 +++++++- internal/engine/dolphin/parse.go | 6 +-- internal/engine/postgresql/parse.go | 6 +-- internal/engine/sqlite/parse.go | 6 +-- internal/metadata/meta.go | 65 ++++++++++------------------- internal/metadata/meta_test.go | 18 ++++---- internal/source/code.go | 61 +++++++++++++++++++++++++-- 8 files changed, 114 insertions(+), 69 deletions(-) diff --git a/internal/compiler/compile.go b/internal/compiler/compile.go index 61e21dd9e4..9f4a5170ef 100644 --- a/internal/compiler/compile.go +++ b/internal/compiler/compile.go @@ -8,10 +8,10 @@ import ( "path/filepath" "strings" - "github.com/sqlc-dev/sqlc/internal/metadata" "github.com/sqlc-dev/sqlc/internal/migrations" "github.com/sqlc-dev/sqlc/internal/multierr" "github.com/sqlc-dev/sqlc/internal/opts" + "github.com/sqlc-dev/sqlc/internal/source" "github.com/sqlc-dev/sqlc/internal/sql/ast" "github.com/sqlc-dev/sqlc/internal/sql/sqlerr" "github.com/sqlc-dev/sqlc/internal/sql/sqlpath" @@ -20,7 +20,7 @@ import ( // TODO: Rename this interface Engine type Parser interface { Parse(io.Reader) ([]ast.Statement, error) - CommentSyntax() metadata.CommentSyntax + CommentSyntax() source.CommentSyntax IsReservedKeyword(string) bool } diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index 0d7f1c3e07..dfbd96463a 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -43,7 +43,18 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, return nil, errors.New("missing semicolon at end of file") } - md, err := metadata.ParseQueryMetadata(rawSQL, c.parser.CommentSyntax()) + cleanedComments, err := source.CleanedComments(rawSQL, c.parser.CommentSyntax()) + if err != nil { + return nil, err + } + + md := metadata.Metadata{} + md.Name, md.Cmd, err = metadata.ParseQueryNameAndType(rawSQL, metadata.CommentSyntax(c.parser.CommentSyntax())) + if err != nil { + return nil, err + } + + md.Params, md.Flags, err = metadata.ParseParamsAndFlags(cleanedComments) if err != nil { return nil, err } @@ -86,11 +97,13 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, } } - trimmed, err := source.StripComments(expanded) + trimmed, comments, err := source.StripComments(expanded) if err != nil { return nil, err } + md.Comments = comments + return &Query{ RawStmt: raw, Metadata: md, diff --git a/internal/engine/dolphin/parse.go b/internal/engine/dolphin/parse.go index 676362c448..22d3a1d224 100644 --- a/internal/engine/dolphin/parse.go +++ b/internal/engine/dolphin/parse.go @@ -10,7 +10,7 @@ import ( "github.com/pingcap/tidb/parser" _ "github.com/pingcap/tidb/parser/test_driver" - "github.com/sqlc-dev/sqlc/internal/metadata" + "github.com/sqlc-dev/sqlc/internal/source" "github.com/sqlc-dev/sqlc/internal/sql/ast" "github.com/sqlc-dev/sqlc/internal/sql/sqlerr" ) @@ -86,8 +86,8 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { } // https://dev.mysql.com/doc/refman/8.0/en/comments.html -func (p *Parser) CommentSyntax() metadata.CommentSyntax { - return metadata.CommentSyntax{ +func (p *Parser) CommentSyntax() source.CommentSyntax { + return source.CommentSyntax{ Dash: true, SlashStar: true, Hash: true, diff --git a/internal/engine/postgresql/parse.go b/internal/engine/postgresql/parse.go index c1ac83381c..957a3073ae 100644 --- a/internal/engine/postgresql/parse.go +++ b/internal/engine/postgresql/parse.go @@ -12,7 +12,7 @@ import ( nodes "github.com/pganalyze/pg_query_go/v4" "github.com/pganalyze/pg_query_go/v4/parser" - "github.com/sqlc-dev/sqlc/internal/metadata" + "github.com/sqlc-dev/sqlc/internal/source" "github.com/sqlc-dev/sqlc/internal/sql/ast" "github.com/sqlc-dev/sqlc/internal/sql/sqlerr" ) @@ -199,8 +199,8 @@ func normalizeErr(err error) error { } // https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-COMMENTS -func (p *Parser) CommentSyntax() metadata.CommentSyntax { - return metadata.CommentSyntax{ +func (p *Parser) CommentSyntax() source.CommentSyntax { + return source.CommentSyntax{ Dash: true, SlashStar: true, } diff --git a/internal/engine/sqlite/parse.go b/internal/engine/sqlite/parse.go index bf0bacad9f..56005dd2ee 100644 --- a/internal/engine/sqlite/parse.go +++ b/internal/engine/sqlite/parse.go @@ -8,7 +8,7 @@ import ( "github.com/antlr/antlr4/runtime/Go/antlr/v4" "github.com/sqlc-dev/sqlc/internal/engine/sqlite/parser" - "github.com/sqlc-dev/sqlc/internal/metadata" + "github.com/sqlc-dev/sqlc/internal/source" "github.com/sqlc-dev/sqlc/internal/sql/ast" ) @@ -86,8 +86,8 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { return stmts, nil } -func (p *Parser) CommentSyntax() metadata.CommentSyntax { - return metadata.CommentSyntax{ +func (p *Parser) CommentSyntax() source.CommentSyntax { + return source.CommentSyntax{ Dash: true, Hash: false, SlashStar: true, diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 7e7d5a1977..56654a0a99 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" "unicode" + + "github.com/sqlc-dev/sqlc/internal/source" ) type Metadata struct { @@ -17,11 +19,7 @@ type Metadata struct { Filename string } -type CommentSyntax struct { - Dash bool - Hash bool - SlashStar bool -} +type CommentSyntax source.CommentSyntax const ( CmdExec = ":exec" @@ -55,12 +53,8 @@ func validateQueryName(name string) error { return nil } -func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, error) { - md := Metadata{} - s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(rawSql))) - var comments []string - for s.Scan() { - line := s.Text() +func ParseQueryNameAndType(t string, commentStyle CommentSyntax) (string, string, error) { + for _, line := range strings.Split(t, "\n") { var prefix string if strings.HasPrefix(line, "--") { if !commentStyle.Dash { @@ -83,11 +77,7 @@ func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, er if prefix == "" { continue } - rest := line[len(prefix):] - rest = strings.TrimSuffix(rest, "*/") - comments = append(comments, rest) - if !strings.HasPrefix(strings.TrimSpace(rest), "name") { continue } @@ -95,45 +85,35 @@ func ParseQueryMetadata(rawSql string, commentStyle CommentSyntax) (Metadata, er continue } if !strings.HasPrefix(rest, " name: ") { - return md, fmt.Errorf("invalid metadata: %s", line) + return "", "", fmt.Errorf("invalid metadata: %s", line) } - comments = comments[:len(comments)-1] // Remove tha name line from returned comments - - parts := strings.Split(strings.TrimSpace(rest), " ") - - if len(parts) == 2 { - return md, fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: %s", line) + part := strings.Split(strings.TrimSpace(line), " ") + if prefix == "/*" { + part = part[:len(part)-1] // removes the trailing "*/" element } - if len(parts) > 3 { - return md, fmt.Errorf("invalid query comment: %s", line) + if len(part) == 3 { + return "", "", fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows', ':execlastid', ':execresult', ':copyfrom', 'batchexec', 'batchmany', 'batchone']: %s", line) } - queryName := parts[1] - queryType := parts[2] + if len(part) != 4 { + return "", "", fmt.Errorf("invalid query comment: %s", line) + } + queryName := part[2] + queryType := strings.TrimSpace(part[3]) switch queryType { case CmdOne, CmdMany, CmdExec, CmdExecResult, CmdExecRows, CmdExecLastId, CmdCopyFrom, CmdBatchExec, CmdBatchMany, CmdBatchOne: default: - return md, fmt.Errorf("invalid query type: %s", queryType) + return "", "", fmt.Errorf("invalid query type: %s", queryType) } if err := validateQueryName(queryName); err != nil { - return md, err + return "", "", err } - md.Name = queryName - md.Cmd = queryType + return queryName, queryType, nil } - - md.Comments = comments - - var err error - md.Params, md.Flags, err = parseParamsAndFlags(md.Comments) - if err != nil { - return md, err - } - - return md, s.Err() + return "", "", nil } -func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool, error) { +func ParseParamsAndFlags(comments []string) (map[string]string, map[string]bool, error) { params := make(map[string]string) flags := make(map[string]bool) @@ -155,9 +135,6 @@ func parseParamsAndFlags(comments []string) (map[string]string, map[string]bool, var rest []string for s.Scan() { paramToken := s.Text() - if paramToken == "*/" { - break - } rest = append(rest, paramToken) } params[name] = strings.Join(rest, " ") diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index ff208f28fa..fad18597e2 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -17,7 +17,7 @@ func TestParseQueryNameAndType(t *testing.T) { "-- name:CreateFoo", `--name:CreateFoo :two`, } { - if _, err := ParseQueryMetadata(query, CommentSyntax{Dash: true}); err == nil { + if _, _, err := ParseQueryNameAndType(query, CommentSyntax{Dash: true}); err == nil { t.Errorf("expected invalid metadata: %q", query) } } @@ -27,7 +27,7 @@ func TestParseQueryNameAndType(t *testing.T) { `-- name comment`, `--name comment`, } { - if _, err := ParseQueryMetadata(query, CommentSyntax{Dash: true}); err != nil { + if _, _, err := ParseQueryNameAndType(query, CommentSyntax{Dash: true}); err != nil { t.Errorf("expected valid comment: %q", query) } } @@ -37,15 +37,15 @@ func TestParseQueryNameAndType(t *testing.T) { `# name: CreateFoo :one`: {Hash: true}, `/* name: CreateFoo :one */`: {SlashStar: true}, } { - queryMetadata, err := ParseQueryMetadata(query, cs) + queryName, queryCmd, err := ParseQueryNameAndType(query, cs) if err != nil { t.Errorf("expected valid metadata: %q", query) } - if queryMetadata.Name != "CreateFoo" { - t.Errorf("incorrect queryName parsed: (%q) %q", queryMetadata.Name, query) + if queryName != "CreateFoo" { + t.Errorf("incorrect queryName parsed: (%q) %q", queryName, query) } - if queryMetadata.Cmd != CmdOne { - t.Errorf("incorrect queryType parsed: (%q) %q", queryMetadata.Cmd, query) + if queryCmd != CmdOne { + t.Errorf("incorrect queryCmd parsed: (%q) %q", queryCmd, query) } } @@ -77,7 +77,7 @@ func TestParseQueryParams(t *testing.T) { " @param @invalid UUID ", }, } { - params, _, err := parseParamsAndFlags(comments) + params, _, err := ParseParamsAndFlags(comments) if err != nil { t.Error("expected comments to parse:", err) } @@ -123,7 +123,7 @@ func TestParseQueryFlags(t *testing.T) { " @param @flag-bar UUID", }, } { - _, flags, err := parseParamsAndFlags(comments) + _, flags, err := ParseParamsAndFlags(comments) if err != nil { t.Error("expected comments to parse:", err) } diff --git a/internal/source/code.go b/internal/source/code.go index 927be880b0..8b88a24136 100644 --- a/internal/source/code.go +++ b/internal/source/code.go @@ -15,6 +15,12 @@ type Edit struct { OldFunc func(string) int } +type CommentSyntax struct { + Dash bool + Hash bool + SlashStar bool +} + func LineNumber(source string, head int) (int, int) { // Calculate the true line and column number for a query, ignoring spaces var comment bool @@ -90,21 +96,70 @@ func Mutate(raw string, a []Edit) (string, error) { return s, nil } -func StripComments(sql string) (string, error) { +func StripComments(sql string) (string, []string, error) { s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(sql))) - var lines []string + var lines, comments []string for s.Scan() { t := s.Text() + if strings.HasPrefix(t, "-- name:") { + continue + } + if strings.HasPrefix(t, "/* name:") && strings.HasSuffix(t, "*/") { + continue + } + if strings.HasPrefix(t, "# name:") { + continue + } if strings.HasPrefix(t, "--") { + comments = append(comments, strings.TrimPrefix(t, "--")) continue } if strings.HasPrefix(t, "/*") && strings.HasSuffix(t, "*/") { + t = strings.TrimPrefix(t, "/*") + t = strings.TrimSuffix(t, "*/") + comments = append(comments, t) continue } if strings.HasPrefix(t, "#") { + comments = append(comments, strings.TrimPrefix(t, "#")) continue } lines = append(lines, t) } - return strings.Join(lines, "\n"), s.Err() + return strings.Join(lines, "\n"), comments, s.Err() +} + +func CleanedComments(rawSQL string, cs CommentSyntax) ([]string, error) { + s := bufio.NewScanner(strings.NewReader(strings.TrimSpace(rawSQL))) + var comments []string + for s.Scan() { + line := s.Text() + var prefix string + if strings.HasPrefix(line, "--") { + if !cs.Dash { + continue + } + prefix = "--" + } + if strings.HasPrefix(line, "/*") { + if !cs.SlashStar { + continue + } + prefix = "/*" + } + if strings.HasPrefix(line, "#") { + if !cs.Hash { + continue + } + prefix = "#" + } + if prefix == "" { + continue + } + + rest := line[len(prefix):] + rest = strings.TrimSuffix(rest, "*/") + comments = append(comments, rest) + } + return comments, s.Err() } From 49e8e362992cd9bdb58a38c2f4b23c8409abf603 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 16:04:23 -0700 Subject: [PATCH 10/12] cleanup --- internal/compiler/parse.go | 8 +++++--- internal/metadata/meta.go | 4 ++-- internal/metadata/meta_test.go | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index dfbd96463a..86d56efde8 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -43,13 +43,15 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, return nil, errors.New("missing semicolon at end of file") } - cleanedComments, err := source.CleanedComments(rawSQL, c.parser.CommentSyntax()) + md := metadata.Metadata{} + + md.Name, md.Cmd, err = metadata.ParseQueryNameAndType(rawSQL, metadata.CommentSyntax(c.parser.CommentSyntax())) if err != nil { return nil, err } - md := metadata.Metadata{} - md.Name, md.Cmd, err = metadata.ParseQueryNameAndType(rawSQL, metadata.CommentSyntax(c.parser.CommentSyntax())) + // TODO eventually can use this for name and type/cmd parsing too + cleanedComments, err := source.CleanedComments(rawSQL, c.parser.CommentSyntax()) if err != nil { return nil, err } diff --git a/internal/metadata/meta.go b/internal/metadata/meta.go index 56654a0a99..97ff36dbd2 100644 --- a/internal/metadata/meta.go +++ b/internal/metadata/meta.go @@ -9,6 +9,8 @@ import ( "github.com/sqlc-dev/sqlc/internal/source" ) +type CommentSyntax source.CommentSyntax + type Metadata struct { Name string Cmd string @@ -19,8 +21,6 @@ type Metadata struct { Filename string } -type CommentSyntax source.CommentSyntax - const ( CmdExec = ":exec" CmdExecResult = ":execresult" diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index fad18597e2..3b1cf313f7 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -79,7 +79,7 @@ func TestParseQueryParams(t *testing.T) { } { params, _, err := ParseParamsAndFlags(comments) if err != nil { - t.Error("expected comments to parse:", err) + t.Errorf("expected comments to parse, got err: %s", err) } pt, ok := params["foo_id"] @@ -125,7 +125,7 @@ func TestParseQueryFlags(t *testing.T) { } { _, flags, err := ParseParamsAndFlags(comments) if err != nil { - t.Error("expected comments to parse:", err) + t.Errorf("expected comments to parse, got err: %s", err) } if !flags["@flag-foo"] { From c1fce0ba32ae0252456e75fe1a0f76e15c5b1425 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 16:07:32 -0700 Subject: [PATCH 11/12] simplify diff --- internal/compiler/parse.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/internal/compiler/parse.go b/internal/compiler/parse.go index 86d56efde8..0b626e1081 100644 --- a/internal/compiler/parse.go +++ b/internal/compiler/parse.go @@ -43,13 +43,20 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, return nil, errors.New("missing semicolon at end of file") } - md := metadata.Metadata{} - - md.Name, md.Cmd, err = metadata.ParseQueryNameAndType(rawSQL, metadata.CommentSyntax(c.parser.CommentSyntax())) + name, cmd, err := metadata.ParseQueryNameAndType(rawSQL, metadata.CommentSyntax(c.parser.CommentSyntax())) if err != nil { return nil, err } + if err := validate.Cmd(raw.Stmt, name, cmd); err != nil { + return nil, err + } + + md := metadata.Metadata{ + Name: name, + Cmd: cmd, + } + // TODO eventually can use this for name and type/cmd parsing too cleanedComments, err := source.CleanedComments(rawSQL, c.parser.CommentSyntax()) if err != nil { @@ -61,10 +68,6 @@ func (c *Compiler) parseQuery(stmt ast.Node, src string, o opts.Parser) (*Query, return nil, err } - if err := validate.Cmd(raw.Stmt, md.Name, md.Cmd); err != nil { - return nil, err - } - var anlys *analysis if c.analyzer != nil { // TODO: Handle panics From 0269502d9d567d216ecb2d3435d5153e39c73e09 Mon Sep 17 00:00:00 2001 From: Andrew Benton Date: Fri, 13 Oct 2023 16:18:28 -0700 Subject: [PATCH 12/12] ensure at least one test case for parsing @whatever without a leading space --- internal/metadata/meta_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/metadata/meta_test.go b/internal/metadata/meta_test.go index 3b1cf313f7..3c2be6d6de 100644 --- a/internal/metadata/meta_test.go +++ b/internal/metadata/meta_test.go @@ -63,7 +63,7 @@ func TestParseQueryParams(t *testing.T) { }, { " name: CreateFoo :one", - " @param foo_id UUID", + "@param foo_id UUID", " invalid", }, { @@ -106,7 +106,7 @@ func TestParseQueryFlags(t *testing.T) { }, { " name: CreateFoo :one ", - " @flag-foo ", + "@flag-foo ", }, { " name: CreateFoo :one",