Skip to content

Commit 3f3e6f0

Browse files
jedudenclaude
andauthored
Render summary front-matter through .RenderString (#408)
* Render summary front-matter as inline Markdown in lead and index single.html and list.html emitted the summary via {{ . }}, so a front-matter value like "Use \`<?catalog?>\` ..." rendered with literal backticks instead of <code> tags. feature-grid.html already ran the same field through .RenderString — only the docs and list templates lagged. Switch both to RenderString with display=inline (so no nested <p> forms inside the wrapping element). Add a verify-website-links probe asserting at least one rendered <p class="lead"> contains a <code> tag; goodSite carries the fixture and a new failure test pins regression detection. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Add template-source check for summary front-matter rendering The verify-website-links probe catches the bug at rendered-HTML stage, but only after a Hugo build. This Go test walks website/layouts/*.html directly and fails the moment a template references .Params.summary without going through .RenderString (or being a plain `if .Params.summary` predicate). baseof.html is exempt — its meta-description fallback intentionally renders plain text because meta tags don't accept HTML. Verified the check fires on the exact bug pattern by temporarily restoring `{{ with .Params.summary }}<p>{{ . }}</p>{{ end }}` in single.html: the test failed with the file:line of the violating expression. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Add lead-with-code fixture to verify-website-links happy-path test cmd/mdsmith-release/main_test.go has its own minimal Hugo-output fixture for TestRunVerifyWebsiteLinksHappyPath, parallel to goodSite() in internal/release/verifylinks_test.go. The new "summary front-matter renders inline markdown as <code>" probe requires at least one rendered <p class="lead"> with a <code> tag, so the e2e test tree needs the same fixture goodSite already carries. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Wrap directory-creation slice to satisfy lll (120 col) Adding the lead-fixture directory pushed the inline literal past the 120-column limit golangci-lint enforces. Pull the slice into a named local. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Address copilot review: loosen lead-probe regex; whole-file template scan Two threads on PR #408: 1. verifylinks.go probe regex was too tight: required <p class=lead> to have no extra attrs/classes and <code> to be the first nested element. Broadened to allow extra <p> attrs, additional classes alongside `lead` (quoted or unquoted), and arbitrary inline tags before <code>, while still anchoring to the same <p> block via `(?:[^<]|<[^/]|</[^p]|</p[^>])*` so code in a sibling <p> does not satisfy the probe. Added two tests: - AcceptsLooseHTMLShapes covers extra classes, unquoted class, attrs-before-class, and a leading <a> tag before <code>. - RejectsCodeOutsideLead pins the anchor: code in a sibling <p> still fails. 2. template_summary_test was per-line, so a multi-line `{{ with\n .Params.summary }}` would slip through. Switched to whole-file scan (FindAllStringIndex on the full content, line number from byte offset). Added DetectsMultiLineWith to pin the new behavior. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * docs: add summary front-matter rendering section to website-config Document the two safety checks added in this PR — the template-source Go test and the rendered-HTML probe — under a new section in docs/development/website-config.md. Names what data must satisfy what condition (the three allowed action shapes, the exemption for baseof.html, the anchor of <code> to the lead <p>). https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Drop rendered-HTML lead probe; rely on template-source test Reviewer flagged the verify-website-links lead probe as content-dependent (PR #408 review): if every docs summary ever drops its code spans, the probe fails even though the templates are correct. The template-source Go test in internal/release/template_summary_test.go already enforces the .RenderString invariant at authoring time and does not depend on what authors write in summaries, so it's the single source of truth. Removes: - The "summary front-matter renders inline markdown as <code>" probe and its 2-paragraph header comment. - The lead-with-code fixture from goodSite() and the AcceptsUnquotedHref test in internal/release. - The lead fixture from TestRunVerifyWebsiteLinksHappyPath in cmd/mdsmith-release (back to 4-dir inline literal). - FailsOnLiteralBackticksInLead, LeadProbe_AcceptsLooseHTMLShapes, and LeadProbe_RejectsCodeOutsideLead — all probe-specific tests. - The "Rendered-HTML probe" section in docs/development/ website-config.md. Also rewords the template_summary_test.go header and assert message to drop the ephemeral branch identifier (flagged by the same review). The comment now describes the regression self-contained, not by branch name. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Tighten template-source check: summary must be a RenderString argument Reviewer flagged that the previous regex (`\.RenderString\b`) treated any action containing both `.RenderString` and `.Params.summary` as safe — even if the two only co-occurred (e.g. `{{ if eq .Params.summary .RenderString }}` would pass). Replace with a regex that requires a method-call relationship: either RenderString first and summary after (positional argument), or summary first piped (one or more `|`) into RenderString. The positional form matches every current template (`.RenderString (dict ...) .Params.summary`); the piped form covers `.Params.summary | .RenderString`-style alternatives. Added TestSummaryFrontMatterCheck_RequiresRenderStringArgument with 8 sub-cases pinning both safe forms (positional, piped) and the rejected patterns (bare output, with rebind, co-occurrence in a comparison without pipe). https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Rewrite summary-rendering check with AST-aware tokenizer Addresses 15 issues from the multi-angle code review. The previous regex scanner had real correctness gaps and ergonomic limitations. Bugs the old scanner had: 1. Brace inside string literals (e.g. `{{ printf "{%s}" .Params.summary }}`) silently skipped the action — the naive `{{[^{}]*}}` regex stopped at the inner `{`. 2. Positional renderSummary regex matched when `.Params.summary` was nested in a non-RenderString call: `{{ .RenderString (printf "%s" .Params.summary) }}` falsely passed. 3. Piped renderSummary regex matched any later `.RenderString` mention: `{{ .Params.summary | print "x" .Page.RenderString }}` falsely passed because `.RenderString` appears after the pipe. 4. ifPredicate anchored to exact `if .Params.summary`, so compound forms (`if and .Params.summary $cond`) and `else if` were flagged as violations even though they only check presence. 5. Variable assignment `{{ $s := .Params.summary }}` was flagged indiscriminately; subfield access `{{ if .Params.summary.X }}` was flagged because the predicate regex required exact end. 6. Hugo comments mentioning the field (`{{/* .Params.summary */}}`) were treated as live references. The new scanner: - Tokenizes actions with quote-aware lexing (handles double-quoted and backtick strings; `{` and `}` inside strings no longer split actions) — fixes #1. - Strips `{{/* ... */}}` comments before scanning (preserves line numbers via newline padding) — fixes #6. - Classifies each action by leading keyword. `if` / `else if` / `range` are presence predicates regardless of compound form or subfield access — fixes #4 and the subfield case. `with` / `else with` are flagged as rebinding the dot. - Pipeline analysis: splits on `|` at paren-depth 0, walks each stage. `.Params.summary` is safe only when it is a top-level positional argument to `.RenderString` or the head of a pipeline whose terminal stage is `.RenderString`. Nested references inside parens are flagged — fixes #2 and #3. Other fixes in this commit: - `baseof.html` exemption is now by relative path (`_default/baseof.html`), not basename. Hugo idiomatically supports per-type baseof overrides; basename-only would silently exempt all of them. - The walker collects I/O errors into a side slice and continues, so a transient ReadFile failure on one file no longer masks every violation in the remaining files. - `layoutsPath` deleted; the walk calls the existing `repoRoot(t)` helper from messaging_test.go. - Three duplicated regex blocks collapsed into package-level vars and a single `scanSummaryViolations` helper. The multi-line test no longer round-trips through a tempfile. - baseof.html's meta description pipes the summary through `$.RenderString (dict "display" "inline") . | plainify` so backticks become `<code>` HTML and then plain text. SEO snippets ship clean prose instead of literal Markdown punctuation. - The pipe-form alternation (YAGNI in the previous regex) is generalized by the pipeline walker; no current template uses the piped form but the scanner handles it correctly when one does. Verified by: 19-case `TestClassifyAction_TableDriven` covering every safe and unsafe shape, plus three scenario tests (`TestFindActions_BalancedStrings`, `TestScanSummaryViolations_CommentsIgnored`, `TestScanSummaryViolations_MultiLineWith`, `TestScanSummaryViolations_BraceInString`). The real-layout walk (`TestSummaryFrontMatterRenderedThroughRenderString`) passes against the current `website/layouts/` tree, and the rendered meta description on the progressive-disclosure page now shows "Use <?catalog?> ..." rather than "Use \`<?catalog?>\` ...". docs/development/website-config.md updated to enumerate the safe and forbidden forms and explain the baseof.html meta-description plainify path. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Replace hand-rolled tokenizer with text/template/parse-based AST walker The previous regex/tokenizer scanner had real correctness gaps surfaced by the second code review (15 findings). Most traced back to one root cause: parsing Hugo templates with hand-rolled string scanning is fundamentally brittle. text/template/parse with parse.SkipFuncCheck parses every layout in website/layouts/ cleanly (verified across all 24 .html files), so this commit replaces ~250 lines of tokenizer with a ~150-line AST walker. The hand-rolled findActions, scanOneAction, skipDoubleQuoted, skipBacktickQuoted, splitPipeStages, firstToken, summaryHits, computeDepths, summaryRefRe, summaryCommentRe, and summaryAssignRe are all gone. Bugs the AST walker fixes: 1. Case sensitivity (e.g. `.params.summary`): now matched via strings.EqualFold on FieldNode.Ident. 2. Brace-in-string actions (`{{ printf "{%s}" .Params.summary }}`): the parser handles string literals natively; the action is an ActionNode whose pipe references .Params.summary. 3. RenderString nested in another call: the AST walker traces sub- PipeNode args, so `{{ .RenderString (printf "wrapper: %s" .Params.summary) }}` is correctly safe (summary feeds the sub-pipe whose output goes to RenderString) and `{{ printf "%v %v" (.RenderString "foo") .Params.summary }}` is correctly forbidden (.RenderString is a value, not a call). 4. Piped to wrong function: pipeOutputsSummaryViaRenderString walks stages and tracks whether summary's value flows into a .RenderString call, returning false for `{{ .Params.summary | print "x" .Page.RenderString }}` where .RenderString is just a value passed to print. 5. Qualified receivers: cmdIsRenderString now recognises FieldNode (.RenderString, .Page.RenderString), ChainNode (chained access), and VariableNode ($.RenderString — text/template parses the dollar-context form as a VariableNode with Ident=["$", "RenderString"], a detail confirmed by AST dump). 6. Subfield access in pipelines: fieldIsSummary matches any FieldNode whose Ident starts with [Params, summary], so `{{ .Params.summary.HTML | .RenderString }}` is safe. 7. CRLF inside multi-line actions: the lexer handles whitespace; no hand-rolled boundary set to maintain. 8. Comments mentioning the field: ParseComments is not set, so comments are dropped at parse time and never visited. 9. Variable assignment bypass: pipeAssignsSummary inspects PipeNode.Decl across IfNode/WithNode/RangeNode/ActionNode contexts, so `{{ if $s := .Params.summary }}` is flagged. 10. Range over string: RangeNode whose Pipe references .Params.summary is forbidden (range rebinds . to each rune). 11. Post-render filters: pipelineOutputsSummaryViaRenderString walks stages forward and allows any sequence after .RenderString, so `{{ ... | .RenderString | plainify }}` and `{{ $.RenderString (dict) .Params.summary | plainify }}` are safe — exactly the form baseof.html now uses. 12. baseof.html exemption removed. The meta-description fallback used `{{ with .Params.summary }}{{ . }}` which the scanner cannot follow (the dot is opaque). Switched to an `if`/ `else if` chain where each branch references its source explicitly. Now baseof.html is scanned natively — no path-based exception. All four sources (.Description, .Params.summary, .Params.description, .Site.Params.description) flow through the same $.RenderString | plainify projection, so any Markdown in any source ships as clean plain text. 13. classifyAction-called-twice eliminated (the walker visits each node once and accumulates violations). 14. computeDepths over-allocation eliminated (no depth array; the AST distinguishes nested calls structurally). 15. Test surface compressed: the 19-row TestClassifyAction table is replaced by a 28-row TestScanSummaryViolations table that runs full templates end-to-end through the same code path the main test uses, plus three scenario tests (multi-line with, CRLF, unterminated parse error). The findActions/balanced- strings/comments tests vanish — those properties belong to the parser now. Verified: all 28 table cases pass, hugo --minify rebuilds the site cleanly, the rendered meta description on the progressive-disclosure page reads "Use <?catalog?> ..." (plain text, no backticks), mdsmith-release verify-website-links exits 0, `mdsmith check .` passes on all 373 files. The docs/development/website-config.md page is updated to enumerate the new safe and forbidden forms and explain the four-branch meta-description projection. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Surface filepath.Rel errors in the layouts walk instead of silently dropping them Reviewer flagged on PR #408 that `rel, _ := filepath.Rel(...)` swallowed the error path. If the relative-path computation ever fails (e.g. an unexpected mount, symlink target outside layoutsDir), the violation output would lose file context and the test would still pass. Handle the error the same way the surrounding walker handles other I/O: record it in ioErrors and skip the file. The post-walk assertion fails the test if anything landed in ioErrors, so the diagnostic is always actionable. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Walk every parse tree, not just the root; detect qualified field access CRITICAL FIX: scanSummaryViolations was only walking tree.Root, which for Hugo layouts that use `{{ define "main" }}...{{ end }}` blocks (every page-rendering layout: index.html, _default/list.html, _default/single.html, rule/single.html) contains only the whitespace between defines — none of the actual rendering logic. The body of each define lives in a separate tree in the treeSet. The walker never visited it, so the test was silently passing without scanning any of the layouts that matter. Verified by injecting `{{ with .Params.summary }}<p class="lead">...` into list.html line 25 (a direct rebind, the exact regression the test is supposed to catch). Before fix: test passed despite the violation. After fix: test correctly fails with `_default/list.html:25: with .Params.summary rebinds the dot...`. The fix iterates every tree in treeSet so define-block bodies are walked. The wrapping tree (treeSet[path]) is also visited but contains only text fragments outside the defines, so no duplicate violations. Added regression test TestScanSummaryViolations_DefineBlock to pin the behavior. Two other gaps closed while here: 1. VariableNode handling. `$.Params.summary` (the dollar-context value reference) parses as a VariableNode with Ident `["$", "Params", "summary"]` — argReferencesSummary did not recognise VariableNode and so silently skipped any value-reference form (e.g. `{{ $.Params.summary }}` would have shipped raw output without being flagged). 2. Qualified field access. `.Page.Params.summary` parses as a FieldNode with Ident `["Page", "Params", "summary"]`. The previous fieldIsSummary required Ident[0] == "Params" exactly and so missed Page-qualified accesses. Generalised to identsReferenceSummary, which finds the `Params` → `summary` adjacency anywhere in the chain. Added TestScanSummaryViolations_QualifiedFieldAccess with four sub- cases covering both forms safe and unsafe. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Handle TemplateNode: `{{ template "x" .Params.summary }}` is a rebinding The previous code-review surfaced that `{{ template "name" pipe }}` and its `{{ block "name" pipe }}` shorthand pass the pipe value as the sub-template's dot. The sub-template's body sees `.` as the bound value — the same rebinding `with` does, but across a tree boundary. The walker did not visit TemplateNode at all, so an invocation like `{{ template "summary-box" .Params.summary }}` was silently safe. baseof.html uses `{{ block "main" . }}{{ end }}` (passing the page, not summary) so today no template is affected, but the gap is real. Added the case and a regression test covering three shapes (summary in template pipe → flagged, unrelated value → safe, block with summary → flagged). https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Add scanned-file counter and pin multi-line violation line number Two follow-ups from the code review: 1. Test could pass vacuously if `website/layouts/` ever disappears or the walker is misconfigured. Counter checks at least 5 .html files were scanned (the four page-rendering layouts plus baseof.html). 2. text/template/parse sets WithNode.Pos to the start of the pipe (the `.Params.summary` operand), not the `{{` opener. For the multi-line fixture `<p>\n{{ with\n .Params.summary }}\n...`, the reported line is 3 (the operand), not 2 (the `{{`). This is more diagnostic — readers see exactly which value is the problem. Pin the assertion at line 3 with a comment explaining the choice. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Sort treeSet keys; recurse into ChainNode receiver and sub-pipe Decls Three follow-ups from the sweep round of the code review: 1. Map iteration over treeSet was non-deterministic. A template with multiple `{{ define ... }}` blocks whose violations land in different defines produces a violations slice in shuffled order across runs, hurting triage. Sort the keys before walking. 2. argReferencesSummary's ChainNode case checked the trailing Field chain via chainIsSummary but never descended into the parenthesised receiver. `(.Params.summary).Foo` parses to ChainNode{Field:["Foo"], Node:PipeNode{...summary...}} and was silently safe. Recurse into n.Node. 3. pipeAssignsSummary only inspected the outer pipe's Decl, so `{{ .RenderString (dict) ($s := .Params.summary) }}` slid past the var-assignment guard — the bound `$s` is a name that escapes the per-action scan, the very pattern the rule forbids. Walk sub-pipelines too. Added two regression tests: TestScanSummaryViolations_ChainReceiver and TestScanSummaryViolations_SubPipeVarAssign. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Move scanner from internal/release/_test.go to internal/templatecheck Scanner code was hiding in a _test.go file in the release package — a frontend-template contract owned by release tooling, where authors editing website/layouts/ wouldn't think to look. Move it to a new package and reduce the release-side file to its integration role. New package internal/templatecheck: - Exports Scan(path, content) ([]Violation, error) - Exports Violation - All helpers (walker, walk, check*, *ReferencesSummary, cmdIsRenderString, identsReferenceSummary, pipeAssignsSummary, pipeOutputsSummaryViaRenderString) are package-private - Unit tests (table-driven + scenarios) live alongside the code internal/release/template_summary_test.go shrinks to one function: TestSummaryFrontMatterRenderedThroughRenderString, which walks website/layouts/ and calls templatecheck.Scan on each file. The release package still owns "did the website pass the contract"; the contract definition lives where contributors can find it. docs/development/website-config.md updated to point at both files — the classifier package and the integration test — and the "extend the AST classifier" pointer now refers to the new package. No behavior change. All 40+ unit tests in templatecheck pass; the release integration test still walks every .html file under website/layouts/ and the scanned-file counter remains. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Cover the templatecheck defensive branches to fix the codecov/patch gate The new package shipped at 90.4% coverage because nil-pipe and empty-Ident guards in the unexported helpers had no test driving them — naturally unreachable through real Hugo AST input but required to lift the patch number to project baseline (99.54%). Added TestHelpers_DefensiveBranches with 12 white-box subtests that synthesise the edge cases directly: nil PipeNode passed to each predicate, empty CommandNode.Args, FieldNode/ChainNode/ VariableNode with zero-length Ident/Field, unknown node type into argReferencesSummary, lineOf with a position past content end, walk(nil) at both interface and typed-nil levels. Also covered three real-but-untouched paths: ChainNode whose trailing field chain itself carries `Params.summary`, the recurse into ChainNode.Node receiver for `(.Params.summary).Foo`, and `.X | someFunc .Params.summary | .RenderString` where the value arrives via a middle stage's Args[1:]. Result: package now at 100.0% line coverage. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 * Address review angles A, D, Sweep: chain boundary, var-assign through render, docs Multiple findings from the angles: A1 — ChainNode boundary case (`(.Params).summary`): the `Params`/`summary` adjacency straddles the receiver/Field boundary. Neither half alone has the pair. Added tailIdents to flatten a chain receiver into one Ident slice and check for the pair across the boundary. Caught: bare ChainNode, nested ChainNode `((.A).Params).summary`, and function-call receiver via fallback recurse. A2 — `{{ $s := .RenderString .Params.summary }}` was being flagged as variable assignment of the raw summary, but the RHS routes summary through RenderString so the bound name holds template.HTML (rendered Markdown). Hugo emits template.HTML without re-escaping, so a later `{{ $s }}` ships rendered output. pipeAssignsSummary now skips the flag when the RHS already outputs via RenderString. Sweep #1 — Forbidden-forms list in docs was missing the TemplateNode case (`{{ template "name" .Params.summary }}` and the `{{ block }}` shorthand). Added. Sweep #2 — Vacuous-pass guard was `scanned >= 5` but the tree has 24 .html files. Raised to 20. Sweep #3 — `assert.Empty(t, formatted)` had no diagnostic message; sibling `ioErrors` line did. Added. Sweep #4 / D1 — Commented the deliberate case-sensitivity asymmetry: identsReferenceSummary uses EqualFold (Hugo Params map is case-insensitive); cmdIsRenderString uses `==` (Go method names are case-sensitive). Without the comment a maintainer "normalising" one would silently break the rule. Sweep #7 / D6 — Commented the inner `if n == nil` guard inside the *parse.ListNode case. The typed-nil ElseList of an if-without-else bypasses the outer guard; removing the inner check would re-introduce a panic. tailIdents — pruned the unreachable VariableNode case (no template syntax produces a ChainNode whose receiver is a bare VariableNode; `$s.Field` parses as VariableNode with the field appended to its own Ident). Per CLAUDE.md, no defensive branch without a red/green driver. Coverage: templatecheck package now at 100.0% of statements. https://claude.ai/code/session_01Ly1xEwP5pfsLrtTCTKBQE3 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent df4fcfa commit 3f3e6f0

7 files changed

Lines changed: 969 additions & 7 deletions

File tree

docs/development/website-config.md

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,98 @@ Tracked by `mdsmith-release sync-messaging` from the
9999
[`docs/brand/messaging.md`](../brand/messaging.md). Hand-edits
100100
to this field are reverted on the next sync. To change the
101101
text, edit the source file and run the sync.
102+
103+
## Summary front-matter rendering
104+
105+
Each docs page carries a `summary` front-matter field.
106+
The field holds inline Markdown. Templates render it
107+
through Hugo's `.RenderString` so backticks become
108+
`<code>` and `[text](url)` becomes `<a>`.
109+
110+
The classifier lives in
111+
[`internal/templatecheck`][tpl-check]. It exports
112+
`Scan(path, content)`. The function parses each
113+
template with Go's `text/template/parse` package.
114+
`SkipFuncCheck` mode is set so undefined Hugo
115+
helpers do not error. The walker then visits the AST
116+
and classifies each `.Params.summary` reference by
117+
node context.
118+
119+
The integration test in
120+
[`internal/release/template_summary_test.go`][tpl-test]
121+
walks `website/layouts/**/*.html` and calls
122+
`templatecheck.Scan` on each file. No regex
123+
tokenising. No exemption list. Comments, string
124+
literals, and CRLF line endings are handled by the
125+
parser.
126+
127+
Safe forms:
128+
129+
- A presence predicate — `{{ if .Params.summary }}`,
130+
the negated form, compound shapes
131+
(`{{ if and .Params.summary .X }}`,
132+
`{{ if or .Params.summary .Other }}`), the `else if`
133+
variant, subfield access
134+
(`{{ if .Params.summary.HTML }}`), or any other
135+
comparison that does not produce output.
136+
- A `.RenderString` call with the summary as a
137+
positional argument:
138+
`{{ .RenderString (dict "display" "inline") .Params.summary }}`.
139+
Qualified receivers (`.Page.RenderString`,
140+
`$.RenderString`) are recognised.
141+
- A pipeline that passes the summary through
142+
`.RenderString` and then any number of post-render
143+
filters: `{{ .Params.summary | .RenderString }}`,
144+
`{{ .Params.summary | strings.TrimSpace | .RenderString }}`,
145+
`{{ $.RenderString (dict "display" "inline") .Params.summary | plainify }}`.
146+
Once the value has rendered, downstream stages such
147+
as `plainify` or `safeHTML` are fine.
148+
- A sub-pipeline argument whose output feeds
149+
`.RenderString`:
150+
`{{ .RenderString (dict) (printf "wrapper: %s" .Params.summary) }}`.
151+
152+
Forbidden forms:
153+
154+
- `{{ with .Params.summary }}` and
155+
`{{ else with .Params.summary }}` — these rebind
156+
`.` to the summary string and the body typically
157+
emits the value raw.
158+
- `{{ range .Params.summary }}` — ranging over a
159+
string iterates rune-by-rune and emits each code
160+
point as an integer.
161+
- `{{ template "name" .Params.summary }}` and
162+
`{{ block "name" .Params.summary }}` — these pass
163+
the summary as the sub-template's dot. The
164+
sub-template lives in a separate parse tree; the
165+
scanner cannot follow the rebinding across the
166+
boundary.
167+
- The bare `{{ .Params.summary }}` action.
168+
- Variable assignment in any context — `{{ $s := .Params.summary }}`,
169+
`{{ if $s := .Params.summary }}`,
170+
`{{ range $i, $v := .Params.summary }}`. The bound
171+
name escapes the per-action check.
172+
- `.Params.summary` referenced in a value-emitting
173+
action whose pipe does not reach `.RenderString`:
174+
`{{ printf "%s" .Params.summary }}`,
175+
`{{ .Params.summary | print "x" .Page.RenderString }}`
176+
(the second example references `.RenderString` as
177+
a value passed to `print`, not as a method call).
178+
179+
`baseof.html` reuses the same projection. Each
180+
branch of its meta-description chain runs the
181+
source value through `$.RenderString` then
182+
`plainify`. The sources are `.Description`,
183+
`.Params.summary`, `.Params.description`, and
184+
`.Site.Params.description`. Meta content cannot
185+
carry HTML. Backticks become `<code>` then plain
186+
text. SEO snippets see clean prose.
187+
188+
The rule applies only to `.Params.summary` today. If
189+
other front-matter scalars (e.g. `lead`, `eyebrow`,
190+
`tagline`) gain inline-Markdown content in the future,
191+
extend the AST classifier in
192+
[`internal/templatecheck`][tpl-check] to accept the
193+
new field name set.
194+
195+
[tpl-check]: ../../internal/templatecheck/templatecheck.go
196+
[tpl-test]: ../../internal/release/template_summary_test.go
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package release
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/jeduden/mdsmith/internal/templatecheck"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// TestSummaryFrontMatterRenderedThroughRenderString walks every
15+
// `.html` file under `website/layouts/` and asserts none uses
16+
// `.Params.summary` in a forbidden context. The classifier lives
17+
// in `internal/templatecheck`; this is the integration test that
18+
// applies it to the real layouts. No template is exempt —
19+
// baseof.html's meta-description fallback uses
20+
// `{{ if .Params.summary }}{{ $.RenderString ... .Params.summary | plainify }}`
21+
// (no `with`-rebinding) so the scanner verifies it natively.
22+
//
23+
// See docs/development/website-config.md for the safe/forbidden
24+
// shape enumeration and `internal/templatecheck/templatecheck.go`
25+
// for the scanner implementation.
26+
func TestSummaryFrontMatterRenderedThroughRenderString(t *testing.T) {
27+
layoutsDir := filepath.Join(repoRoot(t), "website", "layouts")
28+
29+
var violations []templatecheck.Violation
30+
var ioErrors []string
31+
scanned := 0
32+
require.NoError(t, filepath.Walk(layoutsDir, func(path string, info os.FileInfo, err error) error {
33+
if err != nil {
34+
ioErrors = append(ioErrors, fmt.Sprintf("walk %s: %v", path, err))
35+
return nil
36+
}
37+
if info.IsDir() || filepath.Ext(path) != ".html" {
38+
return nil
39+
}
40+
rel, relErr := filepath.Rel(layoutsDir, path)
41+
if relErr != nil {
42+
ioErrors = append(ioErrors, fmt.Sprintf("rel %s: %v", path, relErr))
43+
return nil
44+
}
45+
data, readErr := os.ReadFile(path)
46+
if readErr != nil {
47+
ioErrors = append(ioErrors, fmt.Sprintf("read %s: %v", path, readErr))
48+
return nil
49+
}
50+
got, scanErr := templatecheck.Scan(rel, string(data))
51+
if scanErr != nil {
52+
ioErrors = append(ioErrors, fmt.Sprintf("scan %s: %v", path, scanErr))
53+
return nil
54+
}
55+
violations = append(violations, got...)
56+
scanned++
57+
return nil
58+
}))
59+
60+
formatted := make([]string, 0, len(violations))
61+
for _, v := range violations {
62+
formatted = append(formatted, fmt.Sprintf("%s:%d: %s", v.Path, v.Line, v.Why))
63+
}
64+
assert.Empty(t, formatted, "summary front-matter rendering violations")
65+
assert.Empty(t, ioErrors, "filesystem errors during scan")
66+
// Guard against the test passing vacuously if website/layouts/
67+
// ever disappears or the walker is misconfigured. The tree
68+
// currently holds 24 .html files (_default/, partials/,
69+
// shortcodes/, _markup/, rule/, index.html); set the floor at
70+
// 20 to catch a catastrophic regression while leaving headroom
71+
// for legitimate template cleanup.
72+
assert.GreaterOrEqual(t, scanned, 20, "expected to scan at least 20 .html files; got %d", scanned)
73+
}

0 commit comments

Comments
 (0)