diff --git a/docs/howto/fmt.md b/docs/howto/fmt.md index 5840d48acf..5501d1564c 100644 --- a/docs/howto/fmt.md +++ b/docs/howto/fmt.md @@ -8,7 +8,8 @@ `sqlc fmt` rewrites the query files referenced by your configuration file in a canonical format. Each query is parsed with the engine's parser and printed back from the syntax tree, so formatting never depends on how the query was -written — only on what it means. +written — only on what it means. PostgreSQL, MySQL and SQLite are supported; +query files for other engines are left unchanged. Like `gofmt`, the formatter does not impose a maximum line width. A statement written on a single line stays on a single line, and a statement the author diff --git a/go.mod b/go.mod index 311891d1f9..adc00663e9 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/sqlc-dev/darkwing v0.1.0 github.com/sqlc-dev/doubleclick v1.0.0 - github.com/sqlc-dev/marino v0.3.0 + github.com/sqlc-dev/marino v0.3.1 github.com/sqlc-dev/meyer v0.1.2 github.com/sqlc-dev/oliphant v0.2.0 github.com/sqlc-dev/teesql v1.1.0 diff --git a/go.sum b/go.sum index 6199ffde96..470a77bfd1 100644 --- a/go.sum +++ b/go.sum @@ -69,8 +69,8 @@ github.com/sqlc-dev/darkwing v0.1.0 h1:P5dtJebmCiy10e6DcnBQ2d7Mwuz636obuSAzTlr/1 github.com/sqlc-dev/darkwing v0.1.0/go.mod h1:kPb+99a6U+oVWtLhsuvkuDb/YRm8deK6x+W/9mX6uug= github.com/sqlc-dev/doubleclick v1.0.0 h1:2/OApfQ2eLgcfa/Fqs8WSMA6atH0G8j9hHbQIgMfAXI= github.com/sqlc-dev/doubleclick v1.0.0/go.mod h1:ODHRroSrk/rr5neRHlWMSRijqOak8YmNaO3VAZCNl5Y= -github.com/sqlc-dev/marino v0.3.0 h1:e9cinBXJFFa3yRpokYNNinWvkewgd6XVDgnlG0bOmTw= -github.com/sqlc-dev/marino v0.3.0/go.mod h1:mQxC2dgDE0DWHMb2B5jZNk7KToJuS6wnxnffBfYnq08= +github.com/sqlc-dev/marino v0.3.1 h1:5LkfxftC+drpX1NE6ULTSJlRQ3FzvaFgmXSik+s0Q1Y= +github.com/sqlc-dev/marino v0.3.1/go.mod h1:mQxC2dgDE0DWHMb2B5jZNk7KToJuS6wnxnffBfYnq08= github.com/sqlc-dev/meyer v0.1.2 h1:40Ng9Glnx7CTf3yOYV7jfvDIN7voIFjrStkIULS+uws= github.com/sqlc-dev/meyer v0.1.2/go.mod h1:pS4USCRf/SLjWtaMcnTo4YrEEFKBj8CyyqlxcVUJQH8= github.com/sqlc-dev/oliphant v0.2.0 h1:jJ/s2fh4Plj3U1HsdqiY/clw/IHb4FCNaHfqirwxcsI= diff --git a/internal/cmd/fmt.go b/internal/cmd/fmt.go index ffa5833e7f..0e3616eba0 100644 --- a/internal/cmd/fmt.go +++ b/internal/cmd/fmt.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/sqlc-dev/sqlc/internal/config" + "github.com/sqlc-dev/sqlc/internal/engine/dolphin" "github.com/sqlc-dev/sqlc/internal/engine/postgresql" "github.com/sqlc-dev/sqlc/internal/engine/sqlite" "github.com/sqlc-dev/sqlc/internal/sql/ast" @@ -38,15 +39,17 @@ type queryFormatter interface { } // newQueryFormatter returns the formatter for engines fmt supports — -// SQLite and PostgreSQL today. An engine joins by teaching its parser to -// surface comments (meyer's and oliphant's ParseFile are the templates) and -// adding its case here. +// SQLite, PostgreSQL and MySQL today. An engine joins by teaching its +// parser to surface comments (meyer's, oliphant's and marino's ParseFile +// are the templates) and adding its case here. func newQueryFormatter(engine config.Engine) queryFormatter { switch engine { case config.EnginePostgreSQL: return postgresql.NewParser() case config.EngineSQLite: return sqlite.NewParser() + case config.EngineMySQL: + return dolphin.NewParser() default: return nil } @@ -300,7 +303,7 @@ func splitTrailingComment(seg string) (string, string) { line, _, _ := strings.Cut(rest, "\n") trimmed := strings.TrimSpace(line) switch { - case strings.HasPrefix(trimmed, "--"): + case strings.HasPrefix(trimmed, "--"), strings.HasPrefix(trimmed, "#"): return trimmed, seg[k+len(line):] case strings.HasPrefix(trimmed, "/*") && strings.HasSuffix(trimmed, "*/") && strings.Count(trimmed, "*/") == 1: @@ -370,6 +373,11 @@ func isCommentLine(line string) bool { return true case strings.HasPrefix(line, "--"): return true + case strings.HasPrefix(line, "#"): + // MySQL's line-comment syntax; text that reaches these helpers sits + // outside statements, where a # line in a file the engine parsed + // can only be a comment. + return true case strings.HasPrefix(line, "/*") && strings.HasSuffix(line, "*/") && strings.Count(line, "*/") == 1: // A block comment contained on a single line. return true diff --git a/internal/endtoend/testdata/fmt/mysql/query.sql b/internal/endtoend/testdata/fmt/mysql/query.sql index fa565aeb94..3b39777bbf 100644 --- a/internal/endtoend/testdata/fmt/mysql/query.sql +++ b/internal/endtoend/testdata/fmt/mysql/query.sql @@ -7,6 +7,32 @@ where id = ? limit 1; SELECT id, name, bio FROM authors ORDER BY name DESC; +-- name: PickyQuery :many +SELECT id, -- the primary key + name, + -- computed downstream + bio +FROM authors +-- soft-deleted rows are filtered +WHERE bio IS NOT NULL + AND id > ?; + +-- name: InlineBlock :many +SELECT /* inline note */ id, name FROM authors ORDER BY name; + +-- name: CountSigils :one +SELECT count(*) FROM authors +WHERE id <> ? AND name <> @user_name; # session variable stays + +-- name: CastUnsigned :one +SELECT CAST(id AS UNSIGNED) FROM authors LIMIT 1; + +-- name: FirstTwin :one +SELECT id FROM authors LIMIT 1; + +-- name: SecondTwin :one +SELECT id FROM authors LIMIT 1; + -- name: CreateAuthor :execresult insert into authors ( name, bio diff --git a/internal/endtoend/testdata/fmt/mysql/stderr.txt b/internal/endtoend/testdata/fmt/mysql/stderr.txt deleted file mode 100644 index 69bf0841dd..0000000000 --- a/internal/endtoend/testdata/fmt/mysql/stderr.txt +++ /dev/null @@ -1 +0,0 @@ -sqlc fmt does not yet support the mysql engine; query files left unchanged diff --git a/internal/endtoend/testdata/fmt/mysql/stdout.txt b/internal/endtoend/testdata/fmt/mysql/stdout.txt index e69de29bb2..3912c73530 100644 --- a/internal/endtoend/testdata/fmt/mysql/stdout.txt +++ b/internal/endtoend/testdata/fmt/mysql/stdout.txt @@ -0,0 +1,56 @@ +--- a/query.sql ++++ b/query.sql +@@ -1,6 +1,8 @@ + -- name: GetAuthor :one ++SELECT id, name, bio ++FROM authors ++WHERE id = ? ++LIMIT 1; +-select id,name , bio from authors +-where id = ? limit 1; + + # hash comment + -- name: ListAuthors :many +@@ -7,4 +9,5 @@ ++SELECT id, name, bio ++FROM authors +-SELECT id, name, bio FROM authors + ORDER BY name DESC; + + -- name: PickyQuery :many +@@ -11,7 +14,8 @@ ++SELECT ++ id, -- the primary key ++ name, ++ -- computed downstream ++ bio +-SELECT id, -- the primary key +- name, +- -- computed downstream +- bio + FROM authors + -- soft-deleted rows are filtered + WHERE bio IS NOT NULL +@@ -21,8 +25,9 @@ + SELECT /* inline note */ id, name FROM authors ORDER BY name; + + -- name: CountSigils :one ++SELECT count(*) ++FROM authors ++WHERE id != ? AND name != @user_name; # session variable stays +-SELECT count(*) FROM authors +-WHERE id <> ? AND name <> @user_name; # session variable stays + + -- name: CastUnsigned :one + SELECT CAST(id AS UNSIGNED) FROM authors LIMIT 1; +@@ -34,8 +39,5 @@ + SELECT id FROM authors LIMIT 1; + + -- name: CreateAuthor :execresult ++INSERT INTO authors (name, bio) ++VALUES (?, ?); +-insert into authors ( +- name, bio +-) values ( +- ?, ? +-); diff --git a/internal/engine/dolphin/convert.go b/internal/engine/dolphin/convert.go index 13506ce6c3..e3b99e7cd1 100644 --- a/internal/engine/dolphin/convert.go +++ b/internal/engine/dolphin/convert.go @@ -201,6 +201,7 @@ func (c *cc) convertBinaryOperationExpr(n *pcast.BinaryOperationExpr) ast.Node { c.convert(n.R), }, }, + Location: n.OriginTextPosition(), } } else { return &ast.A_Expr{ @@ -210,8 +211,9 @@ func (c *cc) convertBinaryOperationExpr(n *pcast.BinaryOperationExpr) ast.Node { &ast.String{Str: opToName(n.Op)}, }, }, - Lexpr: c.convert(n.L), - Rexpr: c.convert(n.R), + Lexpr: c.convert(n.L), + Rexpr: c.convert(n.R), + Location: n.OriginTextPosition(), } } } @@ -320,7 +322,8 @@ func (c *cc) convertColumnNames(cols []*pcast.ColumnName) *ast.List { for i := range cols { name := identifier(cols[i].Name.String()) list.Items = append(list.Items, &ast.ResTarget{ - Name: &name, + Name: &name, + Location: cols[i].OriginTextPosition(), }) } return list @@ -1089,6 +1092,7 @@ func (c *cc) convertIsNullExpr(n *pcast.IsNullExpr) ast.Node { c.convert(n.Expr), }, }, + Location: n.OriginTextPosition(), } } @@ -1523,6 +1527,7 @@ func (c *cc) convertTableName(n *pcast.TableName) *ast.RangeVar { return &ast.RangeVar{ Schemaname: &schema, Relname: &rel, + Location: n.OriginTextPosition(), } } diff --git a/internal/engine/dolphin/parse.go b/internal/engine/dolphin/parse.go index 7277710c9c..5b3aa7bb76 100644 --- a/internal/engine/dolphin/parse.go +++ b/internal/engine/dolphin/parse.go @@ -2,6 +2,7 @@ package dolphin import ( "errors" + "fmt" "io" "regexp" "strconv" @@ -48,28 +49,59 @@ func normalizeErr(err error) error { } func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { + f, err := p.ParseFile(r) + if err != nil { + return nil, err + } + // The compiler skips statements sqlc has no node for; the formatter + // must see them, so the filter lives here, not in ParseFile. + var stmts []ast.Statement + for _, stmt := range f.Stmts { + if _, ok := stmt.Raw.Stmt.(*ast.TODO); ok { + continue + } + stmts = append(stmts, stmt) + } + return stmts, nil +} + +// ParseFile parses like Parse and also carries the file's comments, which +// marino's lexer records as it scans. +func (p *Parser) ParseFile(r io.Reader) (*ast.File, error) { blob, err := io.ReadAll(r) if err != nil { return nil, err } - stmtNodes, _, err := p.pingcap.Parse(string(blob), "", "") + src := string(blob) + stmtNodes, _, err := p.pingcap.Parse(src, "", "") if err != nil { return nil, normalizeErr(err) } var stmts []ast.Statement + // A statement's text spans from the end of the previous statement + // through its terminator, so it carries the comments written above it + // (that's where the "-- name:" annotation lives). Each text is a + // contiguous slice of src laid down after the one before it, so + // searching from the previous statement's end pins every text to its + // own occurrence even when two statements read the same. + searchFrom := 0 for i := range stmtNodes { converter := &cc{} + // A statement sqlc has no node for converts to a TODO and stays in + // the list: the formatter needs its extent to keep it as written, + // and Parse filters it out for the compiler. out := converter.convert(stmtNodes[i]) - if _, ok := out.(*ast.TODO); ok { - continue - } - // TODO: Attach the text directly to the ast.Statement node text := stmtNodes[i].Text() - loc := strings.Index(string(blob), text) + idx := strings.Index(src[searchFrom:], text) + if idx < 0 { + return nil, fmt.Errorf("could not locate statement %d in source", i) + } + loc := searchFrom + idx + searchFrom = loc + len(text) stmtLen := len(text) - if text[stmtLen-1] == ';' { + if stmtLen > 0 && text[stmtLen-1] == ';' { stmtLen -= 1 // Subtract one to remove semicolon } @@ -81,7 +113,33 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { }, }) } - return stmts, nil + + var comments []ast.Comment + for _, c := range p.pingcap.Comments() { + comments = append(comments, ast.Comment{ + Text: strings.TrimRight(src[c.Begin:c.End], " \t\r\n"), + Start: c.Begin, + End: c.End, + OwnLine: ownLine(src, c.Begin), + }) + } + return &ast.File{Stmts: stmts, Comments: comments}, nil +} + +// ownLine reports that only blank space sits between the preceding line +// break and pos. +func ownLine(src string, pos int) bool { + for j := pos - 1; j >= 0; j-- { + switch src[j] { + case '\n': + return true + case ' ', '\t', '\r': + continue + default: + return false + } + } + return true } // https://dev.mysql.com/doc/refman/8.0/en/comments.html