improves mysql query metadata parsing (#266)

Author: cmoog

internal/dinosql/parser.go

@@ -340,20 +340,35 @@ func validateQueryName(name string) error {
 		isLetter := unicode.IsLetter(c) || c == '_'
 		isDigit := unicode.IsDigit(c)
 		if i == 0 && !isLetter {
-			return fmt.Errorf("invalid query name: %q", name)
+			return fmt.Errorf("invalid query name %q", name)
 		} else if !(isLetter || isDigit) {
-			return fmt.Errorf("invalid query name: %q", name)
+			return fmt.Errorf("invalid query name %q", name)
 		}
 	}
 	return nil
 }
 
-func parseMetadata(t string) (string, string, error) {
+type CommentSyntax int
+
+const (
+	CommentSyntaxDash CommentSyntax = iota
+	CommentSyntaxStar               // Note: this is the only style supported by the MySQL sqlparser
+	CommentSyntaxHash
+)
+
+func ParseMetadata(t string, commentStyle CommentSyntax) (string, string, error) {
 	for _, line := range strings.Split(t, "\n") {
-		if !strings.HasPrefix(line, "-- name:") {
+		if commentStyle == CommentSyntaxDash && !strings.HasPrefix(line, "-- name:") {
+			continue
+		}
+		if commentStyle == CommentSyntaxStar && !strings.HasPrefix(line, "/* name:") {
 			continue
 		}
 		part := strings.Split(strings.TrimSpace(line), " ")
+
+		if commentStyle == CommentSyntaxStar {
+			part = part[:len(part)-1] // removes the trailing "*/" element
+		}
 		if len(part) == 2 {
 			return "", "", fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows']: %s", line)
 		}
@@ -428,7 +443,7 @@ func parseQuery(c core.Catalog, stmt nodes.Node, source string) (*Query, error)
 	if err := validateFuncCall(&c, raw); err != nil {
 		return nil, err
 	}
-	name, cmd, err := parseMetadata(strings.TrimSpace(rawSQL))
+	name, cmd, err := ParseMetadata(strings.TrimSpace(rawSQL), CommentSyntaxDash)
 	if err != nil {
 		return nil, err
 	}

internal/dinosql/parser_test.go

@@ -121,7 +121,7 @@ func TestParseMetadata(t *testing.T) {
 		`-- name: CreateFoo :one something`,
 		`-- name: `,
 	} {
-		if _, _, err := parseMetadata(query); err == nil {
+		if _, _, err := ParseMetadata(query, CommentSyntaxDash); err == nil {
 			t.Errorf("expected invalid metadata: %q", query)
 		}
 	}

internal/mysql/param_test.go

@@ -118,7 +118,7 @@ func TestInsertParamSearcher(t *testing.T) {
 
 	tests := []testCase{
 		testCase{
-			input: "INSERT INTO users (first_name, last_name) VALUES (?, ?)",
+			input: "/* name: InsertNewUser :exec */\nINSERT INTO users (first_name, last_name) VALUES (?, ?)",
 			output: []*Param{
 				&Param{
 					OriginalName: ":v1",

internal/mysql/parse.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"io/ioutil"
 	"path/filepath"
-	"strings"
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/kyleconroy/sqlc/internal/dinosql"
@@ -153,10 +152,14 @@ func (q *Query) parseNameAndCmd() error {
 		return fmt.Errorf("cannot parse name and cmd from null query")
 	}
 	_, comments := sqlparser.SplitMarginComments(q.SQL)
-	err := q.parseLeadingComment(comments.Leading)
+	name, cmd, err := dinosql.ParseMetadata(comments.Leading, dinosql.CommentSyntaxStar)
 	if err != nil {
-		return fmt.Errorf("failed to parse leading comment %w", err)
+		return err
+	} else if name == "" || cmd == "" {
+		return fmt.Errorf("failed to parse query leading comment")
 	}
+	q.Name = name
+	q.Cmd = cmd
 	return nil
 }
 
@@ -257,7 +260,7 @@ func parseFrom(from sqlparser.TableExprs, isLeftJoined bool) (FromTables, string
 			}
 			return right, leftMostTableName, nil
 		default:
-			return nil, "",  fmt.Errorf("failed to parse table expr: %v", spew.Sdump(v))
+			return nil, "", fmt.Errorf("failed to parse table expr: %v", spew.Sdump(v))
 		}
 	}
 	return tables, defaultTableName, nil
@@ -303,7 +306,10 @@ func parseUpdate(node *sqlparser.Update, query string, s *Schema, settings dinos
 		DefaultTableName: defaultTable,
 		SchemaLookup:     s,
 	}
-	parsedQuery.parseNameAndCmd()
+	err = parsedQuery.parseNameAndCmd()
+	if err != nil {
+		return nil, err
+	}
 
 	return &parsedQuery, nil
 }
@@ -356,7 +362,11 @@ func parseInsert(node *sqlparser.Insert, query string, s *Schema, settings dinos
 		DefaultTableName: tableName,
 		SchemaLookup:     s,
 	}
-	parsedQuery.parseNameAndCmd()
+
+	err := parsedQuery.parseNameAndCmd()
+	if err != nil {
+		return nil, err
+	}
 	return parsedQuery, nil
 }
 
@@ -390,34 +400,6 @@ func parseDelete(node *sqlparser.Delete, query string, s *Schema, settings dinos
 	return parsedQuery, nil
 }
 
-func (q *Query) parseLeadingComment(comment string) error {
-	for _, line := range strings.Split(comment, "\n") {
-		if !strings.HasPrefix(line, "/* name:") {
-			continue
-		}
-		part := strings.Split(strings.TrimSpace(line), " ")
-		if len(part) == 3 {
-			return fmt.Errorf("missing query type [':one', ':many', ':exec', ':execrows']: %s", line)
-		}
-		if len(part) != 5 {
-			return fmt.Errorf("invalid query comment: %s", line)
-		}
-		queryName := part[2]
-		queryType := strings.TrimSpace(part[3])
-		switch queryType {
-		case ":one", ":many", ":exec", ":execrows":
-		default:
-			return fmt.Errorf("invalid query type: %s", queryType)
-		}
-		// if err := validateQueryName(queryName); err != nil {
-		// 	return err
-		// }
-		q.Name = queryName
-		q.Cmd = queryType
-	}
-	return nil
-}
-
 func parseSelectAliasExpr(exprs sqlparser.SelectExprs, s *Schema, tableAliasMap FromTables, defaultTable string) ([]Column, error) {
 	cols := []Column{}
 	for _, col := range exprs {

internal/mysql/parse_test.go

@@ -6,7 +6,6 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/davecgh/go-spew/spew"
 	"github.com/google/go-cmp/cmp"
 	"github.com/kyleconroy/sqlc/internal/dinosql"
 	"vitess.io/vitess/go/vt/sqlparser"
@@ -44,7 +43,13 @@ func TestParseConfig(t *testing.T) {
 func TestGeneratePkg(t *testing.T) {
 	_, err := GeneratePkg(mockSettings.Packages[0].Name, filename, filename, mockSettings)
 	if err != nil {
-		t.Fatal(err)
+		if pErr, ok := err.(*dinosql.ParserErr); ok {
+			for _, fileErr := range pErr.Errs {
+				t.Errorf("%s:%d:%d: %s\n", fileErr.Filename, fileErr.Line, fileErr.Column, fileErr.Err)
+			}
+		} else {
+			t.Errorf("failed to generate pkg %s", err)
+		}
 	}
 }
 
@@ -303,33 +308,28 @@ func TestParseSelect(t *testing.T) {
 }
 
 func TestParseLeadingComment(t *testing.T) {
-	type expected struct {
-		name string
-		cmd  string
+	type output struct {
+		Name string
+		Cmd  string
 	}
-	type testCase struct {
+	tests := []struct {
 		input  string
-		output expected
-	}
-
-	tests := []testCase{
-		testCase{
+		output output
+	}{
+		{
 			input:  "/* name: GetPeopleByID :many */",
-			output: expected{name: "GetPeopleByID", cmd: ":many"},
+			output: output{Name: "GetPeopleByID", Cmd: ":many"},
 		},
 	}
 
 	for _, tCase := range tests {
-		qu := &Query{}
-		err := qu.parseLeadingComment(tCase.input)
+		name, cmd, err := dinosql.ParseMetadata(tCase.input, dinosql.CommentSyntaxStar)
+		result := output{name, cmd}
 		if err != nil {
-			t.Errorf("Failed to parse leading comment %v", err)
+			t.Errorf("failed to parse leading comment: %w", err)
+		} else if diff := cmp.Diff(tCase.output, result); diff != "" {
+			t.Errorf("unexpectd result of query metadata parse: %s", diff)
 		}
-		if qu.Name != tCase.output.name || qu.Cmd != tCase.output.cmd {
-			t.Errorf("Leading comment parser returned unexpcted result: %v\n:\n Expected: [%v]\nRecieved:[%v]\n",
-				err, spew.Sdump(tCase.output), spew.Sdump(qu))
-		}
-
 	}
 }
 

internal/mysql/test_data/queries.sql

@@ -10,8 +10,8 @@ CREATE TABLE teachers (
 /* name: GetTeachersByID :one */
 SELECT * FROM teachers WHERE id = ?;
 
--- name: GetSomeTeachers :one
+/* name: GetSomeTeachers :one */
 SELECT school_id, id FROM teachers WHERE school_lng > ? AND school_lat < ?;
 
--- name: TeachersByID :one
+/* name: TeachersByID :one */
 SELECT id, school_lat FROM teachers WHERE id = ? LIMIT 10;