fix(format): keep statement separators and canonical macro bodies

Assisted-by: GLM 5.3 Flash

format/format.go

@@ -254,11 +254,12 @@ func renderPreproc(line []token.Token) string {
 		line[2].Kind == token.String {
 		return "#include " + line[2].Text
 	}
-	parts := make([]string, 0, len(line)-1)
+	// The body of a directive, a macro definition included, is an ordinary
-	for _, t := range line[1:] {
+	// token run: rendering it through renderOps applies the same punctuation
-		parts = append(parts, t.Text)
+	// rules as everywhere else, so a macro body keeps its canonical spelling
-	}
+	// ($v, (a, b), the ';' separators between statements) instead of being
-	return "#" + strings.Join(parts, " ")
+	// spread with a space between every token.
+	return "#" + renderOps(line[1:])
 }
 
 // renderOps re-spaces a run of operand tokens into canonical form.  It never
@@ -348,6 +349,13 @@ func spaceBetween(prev, cur token.Token) bool {
 		return false
 	case token.Comma:
 		return false
+	case token.Semicolon:
+		// A ';' is a statement separator on the assembly path, not an
+		// operand: dropping it would fuse two statements into a line the
+		// assembler rejects, so it must survive as punctuation.  It glues
+		// to the statement it ends and the next statement takes one space,
+		// matching the toolchain's listing style.
+		return false
 	case token.Star, token.Plus, token.Minus, token.Slash, token.Pipe:
 		return false
 	case token.LShift, token.RShift, token.Arrow, token.At:
@@ -376,7 +384,9 @@ func spaceBetween(prev, cur token.Token) bool {
 		return false
 	case token.LAngle, token.RAngle:
 		return false
-	case token.Comma:
+	case token.Comma, token.Semicolon:
+		// The statement after a ';' separator takes its own space, exactly
+		// like the operand after a comma.
 		return true
 	}
 	return true

format/format_test.go

@@ -5,9 +5,11 @@ package format
 
 import (
 	"os"
+	"slices"
 	"strings"
 	"testing"
 
+	"sourcedock.dev/petrbalvin/gasm-devkit/ast"
 	"sourcedock.dev/petrbalvin/gasm-devkit/lexer"
 	"sourcedock.dev/petrbalvin/gasm-devkit/parser"
 	"sourcedock.dev/petrbalvin/gasm-devkit/token"
@@ -298,6 +300,185 @@ func TestCRLFInputIsNormalisedToLF(t *testing.T) {
 	}
 }
 
+// TestSemicolonSeparators pins the treatment of ';' statement separators.
+// The separator is load-bearing on the assembly path, where the parser reads
+// semicolon-separated statements: a formatter that drops it fuses two
+// statements into a line the assembler rejects, which is data corruption.
+// Each row pins the canonical spelling, one space after the ';', tight
+// before it, the way the toolchain's own sources and listings write it.
+func TestSemicolonSeparators(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{
+			name: "between instructions, tight",
+			in:   "TEXT ·f(SB), $0\nBYTE $0x48;BYTE $0xc7\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tBYTE $0x48; BYTE $0xc7\n\tRET\n",
+		},
+		{
+			name: "between instructions, spaced",
+			in:   "TEXT ·f(SB), $0\nBYTE $0x48 ; BYTE $0xc7\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tBYTE $0x48; BYTE $0xc7\n\tRET\n",
+		},
+		{
+			name: "after a label",
+			in:   "TEXT ·f(SB), $0\nlabel: BYTE $1; BYTE $2\nRET\n",
+			want: "TEXT ·f(SB), $0\nlabel:\n\tBYTE $1; BYTE $2\n\tRET\n",
+		},
+		{
+			// The continuation-spliced macro shape of the runtime sources:
+			// the lexer makes one logical line of the backslash continuations.
+			name: "inside a macro body, continued",
+			in:   "#define MOVLTOREG(v, off) \\\n\tMOVL $v, AX; \\\n\tMOVL AX, ret+off(FP)\n",
+			want: "#define MOVLTOREG(v, off) MOVL $v, AX; MOVL AX, ret+off(FP)\n",
+		},
+		{
+			name: "inside a macro body, one line",
+			in:   "#define PEAS BYTE $0x0a; BYTE $0x0b\n",
+			want: "#define PEAS BYTE $0x0a; BYTE $0x0b\n",
+		},
+		{
+			name: "several separators in one line",
+			in:   "TEXT ·f(SB), $0\nBYTE $1; BYTE $2; BYTE $3\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tBYTE $1; BYTE $2; BYTE $3\n\tRET\n",
+		},
+		{
+			name: "two separators back to back",
+			in:   "TEXT ·f(SB), $0\nBYTE $1;; BYTE $2\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tBYTE $1;; BYTE $2\n\tRET\n",
+		},
+		{
+			name: "inside a line comment, untouched",
+			in:   "TEXT ·f(SB), $0\n// keep; the; separators\nBYTE $1\nRET\n",
+			want: "TEXT ·f(SB), $0\n\t// keep; the; separators\n\tBYTE $1\n\tRET\n",
+		},
+		{
+			name: "after a statement, before a comment",
+			in:   "TEXT ·f(SB), $0\nMOVQ AX, BX; // tail\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tMOVQ AX, BX; // tail\n\tRET\n",
+		},
+		{
+			name: "last character on a line",
+			in:   "TEXT ·f(SB), $0\nBYTE $1;\nRET\n",
+			want: "TEXT ·f(SB), $0\n\tBYTE $1;\n\tRET\n",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := Source(tc.in)
+			if got != tc.want {
+				t.Fatalf("formatting mismatch:\n--- got ---\n%q\n--- want ---\n%q", got, tc.want)
+			}
+			if again := Source(got); again != got {
+				t.Fatalf("not idempotent:\n%q", again)
+			}
+			if in, out := strings.Count(tc.in, ";"), strings.Count(got, ";"); in != out {
+				t.Fatalf("semicolon count changed: %d -> %d\n%s", in, out, got)
+			}
+			if _, errs := parser.Parse("in.s", got); len(errs) > 0 {
+				t.Fatalf("formatted output no longer parses: %v", errs)
+			}
+		})
+	}
+}
+
+// TestSemicolonStatementRoundTrip proves the formatter's contract on the
+// path where ';' separates statements: parse the source the way the
+// assembler does, format it, re-parse the formatted text and compare the
+// statement sequence.  Raw operand texts are token-joined, so they are
+// insensitive to the whitespace a format pass chooses, and the comparison
+// can only fail when a token is lost: dropping a ';' fuses two statements
+// into one, exactly the corruption the released formatter committed.
+func TestSemicolonStatementRoundTrip(t *testing.T) {
+	src := "#define MOVLTOREG(v, off) \\\n" +
+		"\tMOVL $v, AX; \\\n" +
+		"\tMOVL AX, ret+off(FP)\n" +
+		"\n" +
+		"TEXT ·f(SB), NOSPLIT, $0\n" +
+		"BYTE $0x48; BYTE $0xc7\n" +
+		"first: BYTE $1; BYTE $2\n" +
+		"MOVLTOREG($42, 0)\n" +
+		"RET\n"
+
+	before, errs := parser.ParseWithOptions("in.s", src, parser.Options{Expand: true})
+	if len(errs) > 0 {
+		t.Fatalf("source does not parse: %v", errs)
+	}
+	formatted := Source(src)
+	after, errs := parser.ParseWithOptions("in.s", formatted, parser.Options{Expand: true})
+	if len(errs) > 0 {
+		t.Fatalf("formatted source does not parse: %v", errs)
+	}
+
+	want, got := stmtSignature(before), stmtSignature(after)
+	if !slices.Equal(got, want) {
+		t.Fatalf("statement sequence changed:\n--- before ---\n%q\n--- after ---\n%q", want, got)
+	}
+	if again := Source(formatted); again != formatted {
+		t.Fatalf("not idempotent:\n%q", again)
+	}
+
+	// The two BYTE statements on the first line must stay two: one fused
+	// statement here is the exact defect this package once shipped.
+	var bytes []string
+	for _, stmt := range stmtSignature(after) {
+		if rest, ok := strings.CutPrefix(stmt, "instr BYTE "); ok {
+			bytes = append(bytes, rest)
+		}
+	}
+	if want := []string{"$ 0x48", "$ 0xc7", "$ 1", "$ 2"}; !slices.Equal(bytes, want) {
+		t.Fatalf("BYTE statements after expansion = %q, want %q", bytes, want)
+	}
+}
+
+// stmtSignature flattens a parsed file into one string per declaration and
+// statement, in source order.  Every component is token-derived, so the
+// signature is stable across format passes and moves only when a token is
+// lost or gained.
+func stmtSignature(f *ast.File) []string {
+	var out []string
+	for _, d := range f.Decls {
+		switch d := d.(type) {
+		case *ast.Text:
+			out = append(out, "text "+d.Name.Raw)
+			for _, s := range d.Body {
+				out = append(out, stmtText(s))
+			}
+		case *ast.Globl:
+			out = append(out, "globl "+d.Name.Raw)
+		case *ast.Data:
+			out = append(out, "data "+d.Name.Raw)
+		case *ast.Include:
+			out = append(out, "include "+d.Header.Text)
+		case *ast.Preproc:
+			out = append(out, "preproc "+d.Raw)
+		}
+	}
+	for _, s := range f.Orphans {
+		out = append(out, stmtText(s))
+	}
+	return out
+}
+
+// stmtText renders one statement for stmtSignature.
+func stmtText(s ast.Stmt) string {
+	switch s := s.(type) {
+	case *ast.Label:
+		return "label " + s.Name.Text
+	case *ast.Instr:
+		parts := make([]string, 0, len(s.Operands)+1)
+		parts = append(parts, s.Mnemonic.Text)
+		for _, op := range s.Operands {
+			parts = append(parts, op.Raw)
+		}
+		return "instr " + strings.Join(parts, " ")
+	default:
+		return "stmt"
+	}
+}
+
 // lexOperands lexes a single operand string and drops the EOF token.
 func lexOperands(s string) []token.Token {
 	toks := lexer.Tokenize(s)