From 5b7e37fb69f62317a4a127f6e748ca6c8cd7ef58 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 1 Oct 2026 08:27:42 +0200 Subject: [PATCH] fix(11.1-07): find src= fences in the goldmark AST and refuse nested ones - collectFences walks every fenced code block the renderer parses - a nested src= fence is refused and is not extracted, synced or captioned - checkSnippets runs on the same parse as the other checkers - Sync rewrites drifted top-level fences and keeps the frontmatter Deviation: TestSyncRewritesDrift's drifted src= fence sat inside a list item, which is now refused. The drift case is a two-space top-level fence so Sync still rewrites it (problem line 11). --- internal/docsite/docsite.go | 13 ++-- internal/docsite/docsite_test.go | 6 +- internal/docsite/fences.go | 110 +++++++++++++++++++++++++++++++ internal/docsite/load.go | 5 -- internal/docsite/render.go | 10 ++- internal/docsite/snippet.go | 70 ++++++++++++++------ 6 files changed, 182 insertions(+), 32 deletions(-) create mode 100644 internal/docsite/fences.go diff --git a/internal/docsite/docsite.go b/internal/docsite/docsite.go index 3f7b659..873b15f 100644 --- a/internal/docsite/docsite.go +++ b/internal/docsite/docsite.go @@ -233,18 +233,23 @@ func writeOutputs(out string, files map[string][]byte) error { } // checkContent runs the accuracy checkers over every page and the root -// README.md: module identifiers, links and anchors, command names, -// consuming-application names in the sources, and the fence, callout and -// heading policy. +// README.md: src= snippets, module identifiers, links and anchors, +// command names, consuming-application names in the sources, and the +// fence, callout and heading policy. func (s *site) checkContent() ([]Problem, error) { docs, err := s.parseDocs() if err != nil { return nil, err } - idx, problems, err := buildIdentIndex(s.opts.Root) + problems, err := s.checkSnippets(docs) if err != nil { return nil, err } + idx, more, err := buildIdentIndex(s.opts.Root) + if err != nil { + return nil, err + } + problems = append(problems, more...) problems = append(problems, s.checkIdentifiers(idx, docs)...) problems = append(problems, s.checkLinks(docs)...) cp, err := s.checkCommands(docs) diff --git a/internal/docsite/docsite_test.go b/internal/docsite/docsite_test.go index a25244d..11d6cfa 100644 --- a/internal/docsite/docsite_test.go +++ b/internal/docsite/docsite_test.go @@ -325,7 +325,9 @@ func TestSnippetConfinement(t *testing.T) { } func TestSyncRewritesDrift(t *testing.T) { - doc := page("Start", "setup", 10, "Intro.\n\n- In a list:\n\n ```go src=pkg/lib.go#Greeting\n func stale() {}\n ```\n\n"+ + // Two leading spaces stay a top-level fence. A src= fence inside a + // list item is refused, so this drift case is not one. + doc := page("Start", "setup", 10, "Intro.\n\n ```go src=pkg/lib.go#Greeting\n func stale() {}\n ```\n\n"+ "```yaml src=config/app.yaml#db\ndb:\n host: localhost\n```\n\nOutro.\n") root := snippetTree(t, map[string]string{"docs/setup/start.md": doc}) path := filepath.Join(root, "docs/setup/start.md") @@ -334,7 +336,7 @@ func TestSyncRewritesDrift(t *testing.T) { if err != nil { t.Fatal(err) } - want := "docs/setup/start.md:13: snippet: body differs from pkg/lib.go#Greeting (run: summer docs:sync)" + want := "docs/setup/start.md:11: snippet: body differs from pkg/lib.go#Greeting (run: summer docs:sync)" if got := problemLines(problems); !slices.Equal(got, []string{want}) { t.Fatalf("problems = %q, want [%q]", got, want) } diff --git a/internal/docsite/fences.go b/internal/docsite/fences.go new file mode 100644 index 0000000..c898221 --- /dev/null +++ b/internal/docsite/fences.go @@ -0,0 +1,110 @@ +package docsite + +import ( + "bytes" + "fmt" + "strings" + + gast "github.com/yuin/goldmark/ast" +) + +// fenceLine is one source line of a fenced code block. line is the +// 0-based index in the Markdown body. text is the line with the +// trailing newline removed; goldmark has already dropped container +// markers such as a blockquote's "> ". +type fenceLine struct { + line int + text string +} + +// mdFence is one fenced code block taken from the goldmark AST the +// renderer uses, so a fence goldmark renders is visible to the checkers. +type mdFence struct { + info string // info string; empty when the fence has none + lang string // first field of info + line int // 0-based body line of the opening fence; -1 when unknown + topLevel bool // parent is the document + code []fenceLine + // top is the line-scanned fence, set only for a top-level fence + // that has an info string. Sync rewrites those; nested fences are + // refused instead. + top fence +} + +// collectFences walks every fenced code block in doc. doc must be a +// document parsed with newMarkdown, so callouts, blockquotes and lists +// are the same nodes the renderer sees. A top-level fence whose opening +// line openFence rejects is an error: the AST and the line view disagree +// and that must not pass silently. +func collectFences(doc gast.Node, body []byte) ([]mdFence, error) { + rawLines := strings.Split(string(body), "\n") + var out []mdFence + err := gast.Walk(doc, func(n gast.Node, entering bool) (gast.WalkStatus, error) { + fc, ok := n.(*gast.FencedCodeBlock) + if !entering || !ok { + return gast.WalkContinue, nil + } + mf, err := mdFenceFrom(fc, body, rawLines) + if err != nil { + return gast.WalkStop, err + } + out = append(out, mf) + return gast.WalkContinue, nil + }) + if err != nil { + return nil, err + } + return out, nil +} + +// mdFenceFrom builds one mdFence. The opening line is the info segment's +// line, or the line before the first body line when the fence has no info. +func mdFenceFrom(fc *gast.FencedCodeBlock, body []byte, rawLines []string) (mdFence, error) { + var mf mdFence + if fc.Info != nil { + mf.info = string(fc.Info.Segment.Value(body)) + mf.line = bytes.Count(body[:fc.Info.Segment.Start], []byte("\n")) + } else if fc.Lines().Len() > 0 { + mf.line = bytes.Count(body[:fc.Lines().At(0).Start], []byte("\n")) - 1 + } else { + mf.line = -1 + } + if field, _, ok := strings.Cut(mf.info, " "); ok { + mf.lang = field + } else { + mf.lang = mf.info + } + parent := fc.Parent() + mf.topLevel = parent != nil && parent.Kind() == gast.KindDocument + lines := fc.Lines() + for i := 0; i < lines.Len(); i++ { + seg := lines.At(i) + text := bytes.TrimRight(seg.Value(body), "\n") + mf.code = append(mf.code, fenceLine{ + line: bytes.Count(body[:seg.Start], []byte("\n")), + text: string(text), + }) + } + if !mf.topLevel || mf.info == "" { + return mf, nil + } + if mf.line < 0 || mf.line >= len(rawLines) { + return mdFence{}, fmt.Errorf("docsite: fence at body line %d: opening line is outside the source", mf.line+1) + } + opened, ok := openFence(rawLines[mf.line]) + if !ok { + return mdFence{}, fmt.Errorf("docsite: fence at body line %d: AST and source line disagree", mf.line+1) + } + opened.open = mf.line + closeLine := mf.line + 1 + if n := len(mf.code); n > 0 { + closeLine = mf.code[n-1].line + 1 + } + if closeLine >= 0 && closeLine < len(rawLines) && closesFence(rawLines[closeLine], opened) { + opened.close = closeLine + } else { + opened.close = -1 + } + mf.top = opened + return mf, nil +} diff --git a/internal/docsite/load.go b/internal/docsite/load.go index 968d1a9..00244a2 100644 --- a/internal/docsite/load.go +++ b/internal/docsite/load.go @@ -179,11 +179,6 @@ func assemble(opts Options) (*site, []Problem, error) { if err != nil || s == nil { return nil, problems, err } - sp, err := s.checkSnippets() - if err != nil { - return nil, nil, err - } - problems = append(problems, sp...) cp, err := s.checkContent() if err != nil { return nil, nil, err diff --git a/internal/docsite/render.go b/internal/docsite/render.go index 28acf43..61ed2b9 100644 --- a/internal/docsite/render.go +++ b/internal/docsite/render.go @@ -59,12 +59,20 @@ func (fenceAnnotator) Transform(doc *ast.Document, reader text.Reader, pc parser if !entering || !ok || fc.Info == nil { return ast.WalkContinue, nil } + // Captions only on the fences checkSnippets compares: top-level + // src= fences of docs/ pages. A nested or module README fence is + // not verified, so it must not claim a source. + verified := pctx != nil && pctx.page != nil && pctx.page.Module == "" && + fc.Parent() != nil && fc.Parent().Kind() == ast.KindDocument + if !verified { + return ast.WalkContinue, nil + } ref, ok := ParseSrc(string(fc.Info.Segment.Value(src))) if !ok { return ast.WalkContinue, nil } fc.SetAttributeString("data-src", []byte(ref.String())) - if pctx != nil && pctx.site.cfg.SourceURL != "" { + if pctx.site.cfg.SourceURL != "" { fc.SetAttributeString("data-href", []byte(strings.ReplaceAll(pctx.site.cfg.SourceURL, "{path}", ref.Path))) } return ast.WalkSkipChildren, nil diff --git a/internal/docsite/snippet.go b/internal/docsite/snippet.go index da383f8..0c2aad3 100644 --- a/internal/docsite/snippet.go +++ b/internal/docsite/snippet.go @@ -1,6 +1,7 @@ package docsite import ( + "bytes" "errors" "fmt" "go/ast" @@ -446,21 +447,31 @@ type drift struct { want string } -// checkFences verifies every src= fence in lines. lineBase is the 1-based -// file line of lines[0]; file is the display path for problems. -func checkFences(root, file string, lines []string, lineBase int) ([]drift, []Problem, error) { +// nestedSrcMessage is the problem for a src= fence that is not a +// top-level block. docs:sync cannot rewrite a line that still carries +// its container marker, so the fence is refused instead of extracted. +const nestedSrcMessage = "src= code block must be a top-level block of the page, not inside a callout, blockquote or list item" + +// checkFences verifies every src= fence in fences. lines are the body +// lines the fences index into. lineBase is the 1-based file line of +// lines[0]; file is the display path for problems. +func checkFences(root, file string, lines []string, lineBase int, fences []mdFence) ([]drift, []Problem, error) { var drifts []drift var problems []Problem - for _, f := range scanFences(lines) { + for _, f := range fences { ref, ok := ParseSrc(f.info) if !ok { continue } - line := lineBase + f.open + line := lineBase + f.line fail := func(msg string) { problems = append(problems, Problem{File: file, Line: line, Rule: "snippet", Message: msg}) } - if f.close < 0 { + if !f.topLevel { + fail(ref.String() + ": " + nestedSrcMessage) + continue + } + if f.top.close < 0 { fail("code block has no closing fence") continue } @@ -476,9 +487,9 @@ func checkFences(root, file string, lines []string, lineBase int) ([]drift, []Pr case err != nil: return nil, nil, err } - if fenceBody(lines, f) != strings.TrimRight(want, "\n") { + if fenceBody(lines, f.top) != strings.TrimRight(want, "\n") { fail(fmt.Sprintf("body differs from %s (run: summer docs:sync)", ref)) - drifts = append(drifts, drift{f: f, want: want}) + drifts = append(drifts, drift{f: f.top, want: want}) } } return drifts, problems, nil @@ -502,14 +513,20 @@ func unindent(l string, n int) string { return l[i:] } -// checkSnippets verifies the src= fences of every docs page. -func (s *site) checkSnippets() ([]Problem, error) { +// checkSnippets verifies the src= fences of every docs page. Module +// READMEs are skipped here; a later check refuses src= on those pages +// because their fences are rendered as written. +func (s *site) checkSnippets(docs []parsedDoc) ([]Problem, error) { var problems []Problem - for _, p := range s.pages { - if p.Module != "" { + for _, d := range docs { + if d.page == nil || d.page.Module != "" { continue } - _, ps, err := checkFences(s.opts.Root, p.Source, strings.Split(string(p.Body), "\n"), p.BodyLine) + fences, err := collectFences(d.doc, d.body) + if err != nil { + return nil, err + } + _, ps, err := checkFences(s.opts.Root, d.file, strings.Split(string(d.body), "\n"), d.line, fences) if err != nil { return nil, err } @@ -544,8 +561,20 @@ func Sync(opts Options) (SyncResult, []Problem, error) { if err != nil { return SyncResult{}, nil, fmt.Errorf("docs:sync: read %s: %w", abs, err) } - lines := strings.Split(string(raw), "\n") - drifts, ps, err := checkFences(opts.Root, s.rel(abs), lines, 1) + body, bodyLine := raw, 1 + if _, b, bl, ok := splitFrontmatter(raw); ok { + body, bodyLine = b, bl + } + if !bytes.HasSuffix(raw, body) { + return SyncResult{}, nil, fmt.Errorf("docs:sync: %s: body is not a suffix of the file", abs) + } + doc := s.parseRaw(newMarkdown(), body) + fences, err := collectFences(doc, body) + if err != nil { + return SyncResult{}, nil, err + } + lines := strings.Split(string(body), "\n") + drifts, ps, err := checkFences(opts.Root, s.rel(abs), lines, bodyLine, fences) if err != nil { return SyncResult{}, nil, err } @@ -560,21 +589,22 @@ func Sync(opts Options) (SyncResult, []Problem, error) { for i := len(drifts) - 1; i >= 0; i-- { d := drifts[i] pad := strings.Repeat(" ", d.f.indent) - var body []string + var rewritten []string for _, l := range strings.Split(strings.TrimRight(d.want, "\n"), "\n") { if l == "" { - body = append(body, "") + rewritten = append(rewritten, "") } else { - body = append(body, pad+l) + rewritten = append(rewritten, pad+l) } } - lines = slices.Concat(lines[:d.f.open+1], body, lines[d.f.close:]) + lines = slices.Concat(lines[:d.f.open+1], rewritten, lines[d.f.close:]) } st, err := os.Stat(abs) if err != nil { return SyncResult{}, nil, fmt.Errorf("docs:sync: %w", err) } - rewrites = append(rewrites, rewrite{path: abs, mode: st.Mode().Perm(), data: []byte(strings.Join(lines, "\n"))}) + rewritten := append(append([]byte{}, raw[:len(raw)-len(body)]...), []byte(strings.Join(lines, "\n"))...) + rewrites = append(rewrites, rewrite{path: abs, mode: st.Mode().Perm(), data: rewritten}) result.Snippets += len(drifts) result.Files++ }