fmt: preserve ') VALUES (' seams and !=/<> operator spellings

Two formatting fixes reported from running sqlc fmt on a real project:

The boundary before a bare VALUES list was decided by comparing the
source lines of the neighbouring nodes (the last column and the first
value), which always differ when both paren lists are multi-line — so
an authored ') VALUES (' seam broke apart. The seam now gets its own
group, and AttachComments reads the author's actual choice out of the
source by scanning back from the first value across '(' and the VALUES
keyword.

The parsers normalized inequality spellings: PostgreSQL's grammar turns
!= into <>, and the MySQL converter mapped opcode.NE to != regardless
of what was written. The format parsers now read the author's spelling
back out of the source — PostgreSQL via a new NewFormatParser that
rewrites A_Expr names at the operator's recorded location, MySQL via
the same source-scan approach SQLite already used. The compiler's
parsers keep the canonical names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4UpYmwPdxzePtsRXoL1H8

Author: claude

internal/cmd/fmt.go

@@ -45,7 +45,9 @@ type queryFormatter interface {
 func newQueryFormatter(engine config.Engine) queryFormatter {
 	switch engine {
 	case config.EnginePostgreSQL:
-		return postgresql.NewParser()
+		// The format parser preserves the author's operator spellings
+		// (`!=` vs `<>`) that the grammar would otherwise normalize.
+		return postgresql.NewFormatParser()
 	case config.EngineSQLite:
 		return sqlite.NewParser()
 	case config.EngineMySQL:

internal/endtoend/testdata/fmt/mysql/query.sql

@@ -70,3 +70,6 @@ WHERE p.bio IS NOT NULL;
 SHOW WARNINGS;
 
 CREATE TABLE scores (points decimal(10, 5), views bigint unsigned NOT NULL);
+
+-- name: SpelledOperators :many
+SELECT id FROM authors WHERE name != ? AND bio <> ?;

internal/endtoend/testdata/fmt/mysql/stdout.txt

@@ -31,37 +31,30 @@
  FROM authors
  -- soft-deleted rows are filtered
  WHERE bio IS NOT NULL
-@@ -21,8 +25,9 @@
+@@ -21,7 +25,8 @@
  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
+ WHERE id <> ? AND name <> @user_name; # session variable stays
 
  -- name: CastUnsigned :one
- SELECT CAST(id AS UNSIGNED) FROM authors LIMIT 1;
-@@ -34,14 +39,11 @@
+@@ -34,11 +39,7 @@
  SELECT id FROM authors LIMIT 1;
 
  -- name: CreateAuthor :execresult
-+INSERT INTO authors (name, bio)
-+VALUES (?, ?);
++INSERT INTO authors (name, bio) VALUES (?, ?);
 -insert into authors (
 -  name, bio
 -) values (
 -  ?, ?
 -);
 
  -- name: CasePreserved :many
-+SELECT ID, Name FROM Authors WHERE Name != '' ORDER BY Name;
--SELECT ID, Name FROM Authors WHERE Name <> '' ORDER BY Name;
- 
- -- name: LiteralsSurvive :one
- SELECT true AS t, false AS f, 1.50 AS score, CASE WHEN name = '' THEN NULL ELSE name END AS n FROM authors LIMIT 1;
-@@ -62,7 +64,8 @@
+ SELECT ID, Name FROM Authors WHERE Name <> '' ORDER BY Name;
+@@ -62,7 +63,8 @@
  SELECT /*+ MAX_EXECUTION_TIME(1000) */ id FROM authors LIMIT 1;
 
  -- name: UpdateWithJoin :exec

internal/endtoend/testdata/fmt/postgresql/query.sql

@@ -29,5 +29,23 @@ SELECT id, name, bio, created_at FROM authors WHERE name LIKE $1 AND bio IS NOT
 -- name: AtParamsGlued :many
 SELECT name FROM authors WHERE name = @slug AND @filter::bool;
 
+-- name: CreateAuthorLong :one
+INSERT INTO authors (
+  name,
+  bio
+) VALUES (
+  $1,
+  $2
+)
+RETURNING *;
+
+-- name: CreateAuthorBrokenValues :one
+INSERT INTO authors (name, bio)
+VALUES ($1, $2)
+RETURNING *;
+
+-- name: SpelledOperators :many
+SELECT id FROM authors WHERE name != $1 AND bio <> $2;
+
 -- name: DeleteAuthor :exec
 DELETE FROM authors WHERE id = @id

internal/endtoend/testdata/fmt/postgresql/stdout.txt

@@ -20,18 +20,17 @@
  ORDER BY name;
 
  -- name: CreateAuthor :one
-@@ -13,8 +15,5 @@
+@@ -13,8 +15,4 @@
 -INSERT INTO authors (
 -  name, bio
 -) VALUES (
 -  $1, $2
 -)
-+INSERT INTO authors (name, bio)
-+VALUES ($1, $2)
++INSERT INTO authors (name, bio) VALUES ($1, $2)
  RETURNING *;
 
  -- name: PickyQuery :many
-@@ -21,5 +20,6 @@
+@@ -21,5 +19,6 @@
 -SELECT id, -- the primary key
 -       name
 +SELECT
@@ -40,8 +39,8 @@
  FROM authors
  WHERE id > $1;
 
-@@ -30,4 +30,4 @@
- SELECT name FROM authors WHERE name = @slug AND @filter::bool;
+@@ -48,4 +47,4 @@
+ SELECT id FROM authors WHERE name != $1 AND bio <> $2;
 
  -- name: DeleteAuthor :exec
 -DELETE FROM authors WHERE id = @id

internal/engine/dolphin/convert.go

@@ -21,6 +21,31 @@ type cc struct {
 	// lowercasing them for case-insensitive catalog matching; the format
 	// parser sets it (see NewFormatParser).
 	preserveCase bool
+	// src is the statement's source text, set alongside preserveCase: the
+	// format parser reads spellings the tree does not keep (`!=` vs `<>`)
+	// back out of it.
+	src string
+}
+
+// operatorSpelling reads the operator token that sits immediately before
+// the operand at byte offset rpos, reporting which of the candidate
+// spellings the author wrote. Source that does not end in a candidate right
+// there (a comment against the operand, no recorded position) returns
+// ok=false and the caller keeps the canonical name.
+func (c *cc) operatorSpelling(rpos int, candidates ...string) (string, bool) {
+	if c.src == "" || rpos <= 0 || rpos > len(c.src) {
+		return "", false
+	}
+	end := rpos
+	for end > 0 && (c.src[end-1] == ' ' || c.src[end-1] == '\t' || c.src[end-1] == '\r' || c.src[end-1] == '\n') {
+		end--
+	}
+	for _, cand := range candidates {
+		if end >= len(cand) && c.src[end-len(cand):end] == cand {
+			return cand, true
+		}
+	}
+	return "", false
 }
 
 func todo(n pcast.Node) *ast.TODO {
@@ -224,11 +249,20 @@ func (c *cc) convertBinaryOperationExpr(n *pcast.BinaryOperationExpr) ast.Node {
 			Location: n.OriginTextPosition(),
 		}
 	} else {
+		name := opToName(n.Op)
+		if n.Op == opcode.NE && c.src != "" {
+			// MySQL spells inequality two ways (!= and <>) and the tree
+			// keeps only the opcode, so the format parser reads the
+			// author's choice back out of the source.
+			if op, ok := c.operatorSpelling(n.R.OriginTextPosition(), "!=", "<>"); ok {
+				name = op
+			}
+		}
 		return &ast.A_Expr{
 			// TODO: Set kind
 			Name: &ast.List{
 				Items: []ast.Node{
-					&ast.String{Str: opToName(n.Op)},
+					&ast.String{Str: name},
 				},
 			},
 			Lexpr:    c.convert(n.L),

internal/engine/dolphin/parse.go

@@ -97,6 +97,11 @@ func (p *Parser) ParseFile(r io.Reader) (*ast.File, error) {
 	searchFrom := 0
 	for i := range stmtNodes {
 		converter := &cc{preserveCase: p.preserveCase}
+		if p.preserveCase {
+			// The format parser also preserves operator spellings the tree
+			// does not keep; the compiler's parser stays canonical.
+			converter.src = src
+		}
 		// 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.

internal/engine/postgresql/parse.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/sqlc-dev/sqlc/internal/source"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"
+	"github.com/sqlc-dev/sqlc/internal/sql/astutils"
 	"github.com/sqlc-dev/sqlc/internal/sql/sqlerr"
 )
 
@@ -144,7 +145,38 @@ func NewParser() *Parser {
 	return &Parser{}
 }
 
+// NewFormatParser returns the parser sqlc fmt uses. It differs from the
+// compiler's parser in one way: where the grammar normalizes an operator the
+// author may spell two ways (`!=` parses as `<>`), the spelling is read back
+// out of the source so formatting preserves it. The compiler keeps the
+// normalized name, which is the one the catalog knows.
+func NewFormatParser() *Parser {
+	return &Parser{preserveSpelling: true}
+}
+
 type Parser struct {
+	preserveSpelling bool
+}
+
+// restoreOperatorSpelling rewrites operators the grammar normalized back to
+// the spelling the author wrote. The scanner turns `!=` into `<>` before the
+// AST exists; an A_Expr's location points at the operator token, so the
+// source says which spelling to print.
+func restoreOperatorSpelling(n ast.Node, src string) {
+	astutils.Walk(astutils.VisitorFunc(func(node ast.Node) {
+		expr, ok := node.(*ast.A_Expr)
+		if !ok || expr.Name == nil || len(expr.Name.Items) != 1 {
+			return
+		}
+		s, ok := expr.Name.Items[0].(*ast.String)
+		if !ok || s.Str != "<>" {
+			return
+		}
+		loc := expr.Location
+		if loc >= 0 && loc+2 <= len(src) && src[loc:loc+2] == "!=" {
+			expr.Name = &ast.List{Items: []ast.Node{&ast.String{Str: "!="}}}
+		}
+	}), n)
 }
 
 var errSkip = errors.New("skip stmt")
@@ -208,6 +240,9 @@ func (p *Parser) ParseFile(r io.Reader) (*ast.File, error) {
 		if n == nil {
 			return nil, fmt.Errorf("unexpected nil node")
 		}
+		if p.preserveSpelling {
+			restoreOperatorSpelling(n, contents)
+		}
 		stmts = append(stmts, ast.Statement{
 			Raw: &ast.RawStmt{
 				Stmt:         n,

internal/sql/ast/comment.go

@@ -2,6 +2,7 @@ package ast
 
 import (
 	"sort"
+	"strings"
 
 	"github.com/sqlc-dev/sqlc/internal/sql/format"
 )
@@ -201,13 +202,70 @@ func AttachComments(raw *RawStmt, d format.Dialect, comments []Comment, src stri
 				break
 			}
 		}
-		if prevPos >= 0 && nextPos >= 0 && lineOf(prevPos) != lineOf(nextPos) {
+		if prevPos < 0 || nextPos < 0 {
+			continue
+		}
+		// The boundary before a bare VALUES list is a seam between two paren
+		// lists, so the neighbouring nodes' lines cannot decide it: with both
+		// lists broken they always differ, even when the author glued
+		// `) VALUES (`. Read the author's choice at the keyword itself.
+		if sel, ok := a.node.(*SelectStmt); ok && items(sel.ValuesLists) {
+			if broken, ok := valuesSeamBroken(src, nextPos); ok {
+				if broken {
+					table.breaks[a.node] = true
+				}
+				continue
+			}
+		}
+		if lineOf(prevPos) != lineOf(nextPos) {
 			table.breaks[a.node] = true
 		}
 	}
 	return table
 }
 
+// valuesSeamBroken reports whether the author broke the line before a bare
+// VALUES keyword. nextPos is the position of the first printed node inside
+// the list; scanning backward from it crosses `(` and then the keyword, and
+// the whitespace in front of the keyword holds the answer. Source that does
+// not scan that way (an extra paren, a comment against the keyword) returns
+// ok=false and the caller falls back to the line heuristic.
+func valuesSeamBroken(src string, nextPos int) (broken bool, ok bool) {
+	if nextPos <= 0 || nextPos > len(src) {
+		return false, false
+	}
+	isSpace := func(c byte) bool {
+		return c == ' ' || c == '\t' || c == '\r' || c == '\n'
+	}
+	i := nextPos - 1
+	for i >= 0 && isSpace(src[i]) {
+		i--
+	}
+	if i < 0 || src[i] != '(' {
+		return false, false
+	}
+	i--
+	for i >= 0 && isSpace(src[i]) {
+		i--
+	}
+	const keyword = "VALUES"
+	if i+1 < len(keyword) || !strings.EqualFold(src[i+1-len(keyword):i+1], keyword) {
+		return false, false
+	}
+	i -= len(keyword)
+	// A keyword, not the tail of an identifier.
+	if i >= 0 && !isSpace(src[i]) && src[i] != ')' {
+		return false, false
+	}
+	for i >= 0 && isSpace(src[i]) {
+		if src[i] == '\n' {
+			broken = true
+		}
+		i--
+	}
+	return broken, true
+}
+
 type anchor struct {
 	node Node
 	pos  int

internal/sql/ast/insert_stmt.go

@@ -61,8 +61,20 @@ func (n *InsertStmt) Format(buf *TrackedBuffer, d format.Dialect) {
 	if n.DefaultValues {
 		buf.WriteString(" DEFAULT VALUES")
 	} else if set(n.SelectStmt) {
-		buf.beforeClause(n.SelectStmt, d)
-		buf.Line()
+		if sel, ok := n.SelectStmt.(*SelectStmt); ok && items(sel.ValuesLists) {
+			// The seam before a bare VALUES list keeps the author's own
+			// choice — `) VALUES (` glued or VALUES on its own line — so it
+			// gets a group of its own: a break inside either paren list must
+			// not decide it. AttachComments reads the choice out of the
+			// source (see valuesSeamBroken) and marks the boundary.
+			buf.Group()
+			buf.beforeClause(n.SelectStmt, d)
+			buf.Line()
+			buf.EndGroup()
+		} else {
+			buf.beforeClause(n.SelectStmt, d)
+			buf.Line()
+		}
 		buf.astFormat(n.SelectStmt, d)
 	}