caddyconfig: unify Caddyfile lexer/parser/formatter (token-based formatter)

[francislavoie]

What / why

Unifies the Caddyfile lexer/parser and the standalone formatter. The formatter was a separate rune-by-rune state machine (formatter.go) that re-implemented heredoc/quote/backtick/escape/comment/brace handling independently of the lexer. This replaces it with a token-based formatter driven by the existing lexer, so there is a single source of truth for Caddyfile syntax.

The lexer previously dropped whitespace and comments and transformed token text, so it couldn't drive formatting. It now has an opt-in format mode that captures what a formatter needs.

No breaking changes to the parse path

The parser/Tokenize/Dispenser path is byte-for-byte unchanged. All new token state is populated only in format mode. New public surface only:

• Lex(input, filename, LexOptions{Comments, Raw}) — Tokenize delegates to it with the zero value
• Token.Raw(), Token.IsComment()
• FormatWithOptions(input, FormatOptions{...}), and FormatImports(filename, opts) ([]FormattedFile, error)

Highlights

• Format mode lexer: verbatim raw-source capture per token, comment tokens, inter-token separator kind (glued / space / line-continuation), and structural-brace splitting.
• Format rewrite: same signature/behavior; renders layout from the token stream instead of scanning runes. The old implementation was retained as a test-only differential oracle during development and deleted at the end.
• Follow-imports: caddy fmt --imports formats the target Caddyfile plus every file reachable via import (recursive, cycle-safe, cross-file snippet aware, env-substituted globs). Rejected with - (stdin) since there's no base directory.
• Idiomatic improvements over the old formatter (each covered by tests): comments now stay on their brace line (} # end, site { # note); empty blocks canonicalize to the expanded form; the max-one-blank-line rule applies uniformly after comment lines; a token glued after } breaks to its own line instead of emitting a stray tab. Two latent correctness bugs are fixed for free (a lone \r no longer changes config meaning; a backtick token after { is no longer mangled).
• WrapUnbracedSite (a FormatOptions flag, default off, not wired to the CLI): wraps a single unbraced site block in braces.
• Documented divergences from the old formatter, all on invalid/pathological input (which the parser rejects anyway): interior/glued braces are left literal rather than force-split; heredoc close follows lexer semantics; the <-eats-following-space quirk is dropped; the 10-level indentation cap is dropped.

Testing

• All existing formatter test cases ported.
• New table tests for the improvements, divergences, comments/continuations, imports, and braced-wrap.
• Native fuzz targets for the invariants: idempotency (valid input) and never-panics (all input), plus semantic-preservation (parsing the formatted output yields the same token structure). Fuzzing surfaced and fixed two real bugs during development.
• go build ./..., go vet, and the full caddyconfig/... + cmd/... suites pass.

Assistance Disclosure

This change was implemented end-to-end with AI assistance. Claude Code (Anthropic, Opus 4.8) produced the design spec, the implementation plan, and the code/tests, executed task-by-task via a subagent-driven workflow with per-task and whole-branch code review. I (the human author) directed the effort: I set the goals and constraints, made the idiomatic-Caddyfile formatting decisions and signed off on each behavioral divergence from the old formatter, reviewed the spec/plan, and steered scope. Adversarial AI review and fuzzing were used to find defects (two real bugs were caught and fixed this way). I have vetted the contribution for correctness.

🤖 Generated with Claude Code

[mohammed90]

Disclosure: This review was produced by an AI agent (GitHub Copilot, Claude Fable 5), operated and posted at the request of its human user, who has read and verified it. Findings were verified by building and running the code, not just reading it.

Verification performed

• ✅ go build ./..., go test -race -short on caddyconfig/..., cmd/..., caddytest/... — all pass
• ✅ golangci-lint run on changed packages — 0 issues
• ✅ All 4 formatter fuzzers (FuzzFormatIdempotent, FuzzFormatNoPanic, FuzzFormatSemanticPreserve, FuzzWrapUnbracedSiteIdempotent) ran 20s each — no failures
• ✅ Differential check: master vs. this branch produce byte-identical output on a realistic multi-site Caddyfile
• ✅ Parse path unaffected: Tokenize delegates to Lex with zero options; format-mode state is unreachable from the parser

Strengths

• Defensive design is excellent. The FormatWithOptions backstop (re-lex the output, verify token texts match and the output is a fixed point, else fall back to the trimmed input) makes the idempotency and semantic-preservation invariants enforced at runtime, not just tested. Lex errors also fall back rather than panic.
• The legacy formatter was vendored as a differential oracle during development, then retired — good methodology — and divergences (D1–D4) / improvements (I1–I4) are explicitly classified and individually tested.
• resolveImportGlob extracted from parser.doImport correctly preserves the #867/#2096/#5295 behaviors and is shared with import discovery.
• Comments in the tricky parts (brace folding, continuation framing, heredoc-at-EOF) explain why, which this code badly needs.

Findings

1. --imports skips the FormattingDifference exit-code check (moderate). Plain caddy fmt returns ExitCodeFailedStartup when input is unformatted (commonly used as a CI gate). The new --imports branch in cmd/commandfuncs.go returns success unconditionally, so caddy fmt --diff --imports can't gate CI. Suggest applying the same check across all returned files.

2. Performance regression (~6.5×): 3.0 ms → 19.6 ms per Format of a 75 KB file (measured on this branch vs. master). Cause: format-mode lexing with raw capture, plus the backstop's second lex + second render. Absolute cost is fine for a CLI, but FormattingDifference runs Format on every Caddyfile adaptation. Not a blocker; a BenchmarkFormat would help track it.

3. Planning artifacts committed in-tree. docs/superpowers/plans/… and docs/superpowers/specs/… (~2,200 lines) look like AI-workflow planning docs. Caddy keeps documentation in the website repo; these should probably be dropped from the merge.

4. Minor behavior changes in doImport worth calling out in the PR description:

• Error strings changed casing/shape ("Failed to get absolute path…" → wrapped lowercase; the glob-limit error now goes through p.WrapErr).
• A glob matching only dotfiles previously proceeded silently; now the dotfile filter runs before the empty-match check, so it logs "No files matching import glob pattern". Arguably an improvement, but user-visible.

5. Small --imports nits: the diff path re-reads each file from disk (double read; races with concurrent edits) and hard-fails on read error, while FormatImports itself warns-and-skips — inconsistent failure policy. --overwrite also rewrites unchanged files (mtime churn).

Verdict

Solid, carefully engineered work. Finding 1 is the only one I'd consider merge-blocking; finding 3 needs a maintainer decision; the rest are nits/documentation.

[elee1766]

here is the review from my agent:

PR 7880 review

I am an AI coding agent (OpenCode, GPT-5.6 Sol). I reviewed this change with targeted parser/formatter reproductions and CodeRabbit CLI assistance. The human posting this review should verify and understand every finding before submission.

Reproduced findings

High: Format mode splits braces that are literal parser arguments

Locations: caddyconfig/caddyfile/lexer.go:104-106, caddyconfig/caddyfile/lexer.go:133-166

Lex runs splitStructuralBraces whenever either Comments or Raw is enabled. That changes token boundaries accepted by the normal parser.

This input parses successfully before formatting:

localhost
respond foo{

It formats to respond foo { and then fails parsing because the newly structural block is unclosed.

Likewise, this valid directive segment:

localhost
respond {} 200

formats to:

localhost
respond {
}

200

That changes one directive's arguments into a block followed by a new directive. The semantic backstop does not catch either case because it compares format-mode token streams rather than the normal parse-mode token stream.

High: A blank line after import absorbs global options into the import

Location: caddyconfig/caddyfile/formatter.go:570-576

The exception that preserves a standalone opening brace after a top-level import only applies when breaks < 2.

Input:

import foo.caddy

{
    debug
}

Actual output:

import foo.caddy {
	debug
}

This changes debug from a global option into an import block-mapping entry.

High: Import discovery ignores snippet declaration order

Locations: caddyconfig/caddyfile/format_imports.go:86-100, caddyconfig/caddyfile/format_imports.go:141-163

Discovery collects all snippet names and retroactively classifies every matching import as a snippet import. The parser only recognizes snippets already defined when it reaches an import.

Given a real file named foo, this configuration imports that file through the parser but omits it from FormatImports:

import foo

(foo) {
	respond ok
}

High: Recursive imports using {args[n]} fail under --imports

Locations: caddyconfig/caddyfile/format_imports.go:81-110, caddyconfig/caddyfile/format_imports.go:217-244

Discovery scans imported files without carrying the invocation arguments that the parser applies.

# Caddyfile
import selector.caddy child.caddy

# selector.caddy
import {args[0]}

Normal parsing resolves and imports child.caddy. FormatImports instead returns a glob-pattern error for {args[0]}.

High: Unbounded indentation permits quadratic memory amplification

Locations: caddyconfig/caddyfile/formatter.go:480-484, caddyconfig/caddyfile/formatter.go:637-650

Each opening block increases nesting, and every later line writes that many tabs. Linear-size deeply nested input therefore produces quadratic output. FormatWithOptions then lexes and renders the expanded output again for its fixed-point check. The previous formatter explicitly capped indentation at ten levels to avoid this resource-exhaustion class.

Medium: caddy fmt --imports returns success for unformatted files

Location: cmd/commandfuncs.go:734-758

The imports branch returns success after preview or diff output and bypasses the FormattingDifference check at cmd/commandfuncs.go:794-799.

This was reproduced with an unformatted file: ordinary caddy fmt returns a formatting failure, while caddy fmt --imports and caddy fmt --imports --diff return success. This breaks CI formatting checks. CodeRabbit independently reported the same issue as a major finding.

Additional correctness findings

High: Symlinked directory cycles bypass the import cycle guard

Locations: caddyconfig/caddyfile/format_imports.go:54-66, caddyconfig/caddyfile/format_imports.go:115-124

The visited set is keyed by caddy.FastAbs, which cleans lexical paths but does not resolve symlinks. If link points to the current directory and the root imports link/Caddyfile, discovery can visit link/Caddyfile, link/link/Caddyfile, and so on until path-length or resource failure. Importing a file both directly and through a symlink can also format the same physical file more than once.

High: WrapUnbracedSite can wrap an import as the site address

Locations: caddyconfig/caddyfile/formatter.go:123-155, caddyconfig/caddyfile/formatter.go:265-269

isSingleUnbracedSite treats the first line as an address list without accounting for leading imports.

import common.caddy
localhost
respond 200

With WrapUnbracedSite, this can become:

import common.caddy {
	localhost
	respond 200
}

The site is converted into import block-mapping content.

Medium: Environment expansion can create imports that discovery misses

Locations: caddyconfig/caddyfile/format_imports.go:81-110, caddyconfig/caddyfile/format_imports.go:153-168

The parser expands environment variables across the complete file before tokenization. Discovery tokenizes the original file and only expands the extracted import argument. For example, with LINE='import child.caddy', a line containing {$LINE} imports the child through the parser but not through FormatImports. Expansion can also alter quoting, comments, whitespace, or snippet classification.

Medium: Discovery follows imports inside unused snippets and unused blocks

Location: caddyconfig/caddyfile/format_imports.go:217-244

scanTokens treats every first-of-line import as active without modeling whether its containing snippet or substitution block is ever expanded. Consequently, --imports --overwrite may rewrite files referenced only by dead configuration.

(unused) {
	import unrelated.caddy
}

localhost {
	respond ok
}

The parser never executes this import unless the snippet is invoked, but discovery includes unrelated.caddy.

Medium: Quoted import directives are skipped even though the parser executes them

Location: caddyconfig/caddyfile/format_imports.go:237-241

Discovery requires tok.wasQuoted == 0, while the parser tests the token value without a quotedness restriction. A line such as "import" child.caddy is therefore followed by the parser but omitted by FormatImports.

Medium: Existing imported files that cannot be read are silently omitted

Locations: caddyconfig/caddyfile/format_imports.go:126-131, caddyconfig/caddyfile/format_imports.go:287-293

Read failures are logged and skipped, and the operation can still report success. The parser returns an error for an unreadable matched import. This is especially misleading for --imports --overwrite, which can claim success while leaving part of the active configuration unformatted.

Medium: Multi-file overwrite can leave a partially updated configuration set

Location: cmd/commandfuncs.go:725-731

Files are truncated and rewritten sequentially. If a later write fails, earlier files remain modified. The operation reports an error but leaves a mixed old/new configuration set. Atomic replacement per file and clearer partial-update handling would reduce this risk.

Medium: Imported paths are reopened without identity verification

Locations: caddyconfig/caddyfile/format_imports.go:126-137, caddyconfig/caddyfile/format_imports.go:287-299, cmd/commandfuncs.go:725-731

Discovery reads pathnames, formatting reopens them, and overwrite later opens the same names again. Replacing an imported path with a symlink between those operations can redirect formatted bytes to another target, particularly when formatting with elevated privileges in a directory writable by another user.

Medium: Heredoc formatting has quadratic body construction

Locations: caddyconfig/caddyfile/formatter.go:40, caddyconfig/caddyfile/formatter.go:88-90, caddyconfig/caddyfile/lexer.go:530-549

The formatter lexes heredocs multiple times, while finalizeHeredoc builds the body using repeated string concatenation. Large multi-line heredocs therefore repeatedly copy the accumulated body. This is a performance regression from the previous rune-stream formatter and should use a builder or equivalent linear construction.

Medium: WrapUnbracedSite rejects a normal site after a nested directive block

Locations: caddyconfig/caddyfile/formatter.go:185-204, caddyconfig/caddyfile/formatter.go:606-610

Returning from a directive block to nesting zero sets returnedToTop; the following directive is then interpreted as a second top-level group.

localhost
route {
	respond ok
}
file_server

With wrapping requested, this remains unwrapped instead of becoming one braced site.

Low: Valid escaped quotes disable formatting for the whole file

Location: caddyconfig/caddyfile/formatter.go:356-364

hasUnformattableToken rejects any unquoted raw token containing " or a backtick, including valid escaped delimiters such as respond foo\"bar. The formatter then returns the original trimmed input, allowing unrelated formatting differences to evade FormattingDifference.

Low: Exported Lex options unexpectedly change ordinary token boundaries

Locations: caddyconfig/caddyfile/lexer.go:64-105, caddyconfig/caddyfile/lexer.go:110-166

The API documents Comments as emitting comments and Raw as recording source bytes, but enabling either also splits structural braces. For example, Tokenize treats {} as one token while Lex(..., LexOptions{Raw: true}) returns separate { and } tokens. Brace normalization should be formatter-internal, explicitly configurable, or documented as part of a distinct formatting lexer API.

Low: FormatImports returns inconsistently normalized paths

Locations: caddyconfig/caddyfile/format_imports.go:271-298

The root path preserves the caller's spelling, while imported paths are generally absolute. A call with Caddyfile can therefore return Caddyfile followed by /absolute/path/to/import.caddy, producing mixed headers and an ambiguous public API contract.

Test coverage issue reported by CodeRabbit

Low: The overwrite test does not verify that the root file is rewritten

Location: cmd/commands_test.go:73-94

CodeRabbit noted that TestCmdFmtImportsOverwrite uses an already formatted root fixture and only asserts the imported file afterward. The root fixture should also be deliberately unformatted, then read back and asserted so the test covers both root and imported-file writes.

Verification performed

• go test ./caddyconfig/caddyfile ./cmd passes on the PR branch without the temporary reproduction tests.
• git diff --check origin/master...HEAD passes.
• Temporary tests reproduced the brace-splitting, import/global-options, snippet-order, recursive-argument, and exit-status defects, then were removed.
• CodeRabbit CLI independently identified the --imports exit-status defect and the missing root overwrite assertion. Its full scan did not complete before the ten-minute timeout.