Skip to content

Commit 97c992c

Browse files
committed
Expose NearestBlockAncestor, add byte-identical pin test, fill pyramid
Addresses the two remaining self-review concerns and aligns the package's tests with the test-pyramid rule that every production function ships its dedicated unit test by name. Concern #2 — drop the nearestBlockAncestor duplicate: - Expose NearestBlockAncestor from pkg/markdown/flavor so external rewriters (and the rule adapter) share the helper instead of duplicating it. Replace the rule's private copy in fix.go with flavor.NearestBlockAncestor. - Add it to the contract test, the markdown-library stable surface list, and a dedicated TestNearestBlockAncestorPublic test. Concern #3 — byte-identical pin test: - pkg/markdown/flavor/detect_pin_test.go adds a corpus-driven table test (pinCorpus) that maps each input to the exact Finding stream (feature + 1-based line + 1-based column, in document order). Plan 185 acceptance criterion "Table tests pin this" is now an explicit gate; any subtle reorder, drop, or shift in MDS034 diagnostics will break the test with a side-by-side diff. Test-pyramid alignment: - TestNearestBlockAncestor (subtests for the skip-non-block and orphan branches) plus TestNearestBlockAncestorPublic for the exported wrapper. - TestIsGitHubAlertPublic exercises both branches of IsGitHubAlert (alert blockquote / heading-first-child). - TestLineColPublic pins the documented 1-based semantics of the exported LineCol wrapper. - TestDualFindings covers the dualFindings helper I extracted from Detect in the previous commit, asserting both the keep-filter and the still-emits-other-features path. Coverage in pkg/markdown/flavor stays at 100%; mdsmith check and golangci-lint are clean. https://claude.ai/code/session_0144ZKUS2Zrg7xBft54qyoti
1 parent a7fed9e commit 97c992c

6 files changed

Lines changed: 271 additions & 33 deletions

File tree

docs/development/markdown-library.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ The stable surface:
233233
`HeadingIDExtra` shapes, `Detect`, the four
234234
`NewParser*` / `NewPooledParser*`
235235
constructors, and the small rewriter helpers
236-
(`FindHeadingID`, `IsGitHubAlert`, `LineCol`).
236+
(`FindHeadingID`, `IsGitHubAlert`, `LineCol`,
237+
`NearestBlockAncestor`).
237238
- Sub-package `pkg/markdown/flavor/ext`: the
238239
five custom extension Extender singletons
239240
(`Superscript`, `Subscript`, `MathBlock`,

internal/rules/markdownflavor/fix.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func delimiterPairEdits(n ast.Node, markerLen int) []edit {
147147
// The dual parser places every TaskCheckBox at the start of a
148148
// TextBlock so block.Lines().At(0).Start always points at '['.
149149
func taskCheckBoxEdits(f *lint.File, n *extast.TaskCheckBox) []edit {
150-
block := nearestBlockAncestor(n)
150+
block := flavor.NearestBlockAncestor(n)
151151
start := block.Lines().At(0).Start
152152
end := start + 3
153153
if end < len(f.Source) && f.Source[end] == ' ' {
@@ -156,21 +156,6 @@ func taskCheckBoxEdits(f *lint.File, n *extast.TaskCheckBox) []edit {
156156
return []edit{{start: start, end: end}}
157157
}
158158

159-
// nearestBlockAncestor walks up from n and returns the first block-
160-
// typed ancestor with non-empty Lines(). Mirrors the helper in
161-
// pkg/markdown/flavor; duplicated here to keep that helper internal.
162-
func nearestBlockAncestor(n ast.Node) ast.Node {
163-
for p := n.Parent(); p != nil; p = p.Parent() {
164-
if p.Type() != ast.TypeBlock {
165-
continue
166-
}
167-
if lines := p.Lines(); lines != nil && lines.Len() > 0 {
168-
return p
169-
}
170-
}
171-
return nil
172-
}
173-
174159
// wrapBareURL wraps a bare URL in angle brackets so the renderer
175160
// treats it as a CommonMark autolink. The detector reports a precise
176161
// span via fin.Start / fin.End.

pkg/markdown/flavor/contract_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,14 @@ func TestContract_ParserConstructors(t *testing.T) {
120120
}
121121

122122
// TestContract_Rewriters covers the small surface needed by external
123-
// rewriters: FindHeadingID, IsGitHubAlert, LineCol.
123+
// rewriters: FindHeadingID, IsGitHubAlert, LineCol, NearestBlockAncestor.
124124
func TestContract_Rewriters(t *testing.T) {
125125
source := []byte("# h\n")
126126
h := ast.NewHeading(1)
127127
_, _ = flavor.FindHeadingID(source, h)
128128
_ = flavor.IsGitHubAlert(ast.NewBlockquote(), source)
129129
_, _ = flavor.LineCol(source, 0)
130+
_ = flavor.NearestBlockAncestor(h)
130131
}
131132

132133
// TestContract_ExtensionExtenders pins the five custom extension

pkg/markdown/flavor/detect.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,15 @@ func LineCol(source []byte, offset int) (line, col int) {
483483
return lineCol(source, offset)
484484
}
485485

486+
// NearestBlockAncestor walks up from n and returns the first block-
487+
// typed ancestor with a non-empty Lines() segment, or nil when none
488+
// exists. Exposed for rewriters that hold an inline AST node and
489+
// need the block context that owns its source position (e.g. to
490+
// anchor a line-level edit on the containing paragraph).
491+
func NearestBlockAncestor(n ast.Node) ast.Node {
492+
return nearestBlockAncestor(n)
493+
}
494+
486495
// findHeadingID locates the trailing "{#id}" attribute block that the
487496
// goldmark attribute parser consumed. The Heading node's Lines segment
488497
// only covers the inner text, so we scan the raw line in source from

pkg/markdown/flavor/detect_edge_test.go

Lines changed: 111 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,25 +121,121 @@ func TestNodeByteRangeClampsNegativeStart(t *testing.T) {
121121
assert.Equal(t, 0, end)
122122
}
123123

124-
// TestNearestBlockAncestorSkipsNonBlockAncestors exercises the
125-
// "parent is not a block" branch in nearestBlockAncestor: when we
126-
// walk through an inline ancestor on the way up, the helper skips
127-
// it and keeps climbing.
128-
func TestNearestBlockAncestorSkipsNonBlockAncestors(t *testing.T) {
129-
// Build: Paragraph (block, has Lines) → Emphasis (inline) →
130-
// FootnoteLink (inline). Walking up from the FootnoteLink must
131-
// skip Emphasis and return the Paragraph.
124+
// TestNearestBlockAncestor exercises the "parent is not a block"
125+
// branch in nearestBlockAncestor: when the walk encounters an
126+
// inline ancestor on the way up, the helper skips it and keeps
127+
// climbing. Also covers the nil-parent return when an orphan node
128+
// has no ancestor at all.
129+
func TestNearestBlockAncestor(t *testing.T) {
130+
t.Run("skips non-block ancestors", func(t *testing.T) {
131+
// Paragraph (block, has Lines) → Emphasis (inline) →
132+
// FootnoteLink (inline). Walking up from the FootnoteLink must
133+
// skip Emphasis and return the Paragraph.
134+
p := ast.NewParagraph()
135+
// Append a line so findingFromBlock can resolve a position
136+
// later (not needed here, but keeps the block well-formed).
137+
p.Lines().Append(text.NewSegment(0, 1))
138+
em := ast.NewEmphasis(1)
139+
link := extast.NewFootnoteLink(1)
140+
p.AppendChild(p, em)
141+
em.AppendChild(em, link)
142+
assert.Same(t, ast.Node(p), nearestBlockAncestor(link))
143+
})
144+
145+
t.Run("returns nil for orphan node", func(t *testing.T) {
146+
assert.Nil(t, nearestBlockAncestor(extast.NewFootnoteLink(1)))
147+
})
148+
}
149+
150+
// TestNearestBlockAncestorPublic is the dedicated unit test for the
151+
// public NearestBlockAncestor wrapper. The wrapper delegates to the
152+
// unexported helper, so this confirms the surface forwards without
153+
// re-implementing the walk.
154+
func TestNearestBlockAncestorPublic(t *testing.T) {
132155
p := ast.NewParagraph()
133-
// Append a line so findingFromBlock can resolve a position
134-
// later (not needed here, but keeps the block well-formed).
135156
p.Lines().Append(text.NewSegment(0, 1))
136-
em := ast.NewEmphasis(1)
137157
link := extast.NewFootnoteLink(1)
138-
p.AppendChild(p, em)
139-
em.AppendChild(em, link)
158+
p.AppendChild(p, link)
159+
assert.Same(t, ast.Node(p), NearestBlockAncestor(link))
160+
assert.Nil(t, NearestBlockAncestor(extast.NewFootnoteLink(1)))
161+
}
162+
163+
// TestIsGitHubAlertPublic exercises the public IsGitHubAlert wrapper
164+
// on both branches of the underlying isGitHubAlert helper: a
165+
// well-formed alert blockquote returns true; a blockquote whose
166+
// first child is not a paragraph returns false.
167+
func TestIsGitHubAlertPublic(t *testing.T) {
168+
t.Run("recognises alert blockquote", func(t *testing.T) {
169+
src := []byte("> [!NOTE]\n> body\n")
170+
root := mkDoc(t, string(src))
171+
var bq *ast.Blockquote
172+
_ = ast.Walk(root.AST, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
173+
if !entering {
174+
return ast.WalkContinue, nil
175+
}
176+
if b, ok := n.(*ast.Blockquote); ok {
177+
bq = b
178+
return ast.WalkStop, nil
179+
}
180+
return ast.WalkContinue, nil
181+
})
182+
require.NotNil(t, bq, "expected the CommonMark parse to produce a *ast.Blockquote")
183+
assert.True(t, IsGitHubAlert(bq, src))
184+
})
140185

141-
got := nearestBlockAncestor(link)
142-
assert.Same(t, ast.Node(p), got)
186+
t.Run("rejects non-paragraph first child", func(t *testing.T) {
187+
// A blockquote whose first child is a heading short-circuits
188+
// the type assertion inside isGitHubAlert.
189+
src := []byte("> # heading\n")
190+
root := mkDoc(t, string(src))
191+
var bq *ast.Blockquote
192+
_ = ast.Walk(root.AST, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
193+
if !entering {
194+
return ast.WalkContinue, nil
195+
}
196+
if b, ok := n.(*ast.Blockquote); ok {
197+
bq = b
198+
return ast.WalkStop, nil
199+
}
200+
return ast.WalkContinue, nil
201+
})
202+
require.NotNil(t, bq)
203+
assert.False(t, IsGitHubAlert(bq, src))
204+
})
205+
}
206+
207+
// TestLineColPublic is the dedicated unit test for the public
208+
// LineCol wrapper. The contract test exercises the call shape; this
209+
// test pins the documented "1-based" semantics on real input.
210+
func TestLineColPublic(t *testing.T) {
211+
src := []byte("hello\nworld\n")
212+
line, col := LineCol(src, 6) // start of "world"
213+
assert.Equal(t, 2, line)
214+
assert.Equal(t, 1, col)
215+
}
216+
217+
// TestDualFindings exercises the pooled-parser helper extracted from
218+
// Detect. The accept predicate must filter findings at the helper's
219+
// own seam — a keep callback that rejects FeatureTables must not
220+
// emit a Tables finding even though the dual AST contains a Table.
221+
func TestDualFindings(t *testing.T) {
222+
src := []byte("| a | b |\n| - | - |\n| 1 | 2 |\n\n~~old~~\n")
223+
rejectTables := func(feat Feature) bool {
224+
return feat != FeatureTables
225+
}
226+
got := dualFindings(src, rejectTables)
227+
for _, f := range got {
228+
assert.NotEqual(t, FeatureTables, f.Feature,
229+
"keep predicate must suppress FeatureTables findings")
230+
}
231+
// Strikethrough is still kept, so the helper still does real work.
232+
found := false
233+
for _, f := range got {
234+
if f.Feature == FeatureStrikethrough {
235+
found = true
236+
}
237+
}
238+
assert.True(t, found, "expected at least one Strikethrough finding")
143239
}
144240

145241
// TestFindHeadingIDHandlesMissingLines exercises the
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package flavor
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
10+
"github.com/jeduden/mdsmith/pkg/markdown"
11+
)
12+
13+
// pinCase is one row in the byte-identical Detect corpus.
14+
type pinCase struct {
15+
name string
16+
src string
17+
want []findingShape
18+
}
19+
20+
// pinCorpus is the representative input corpus mapped to its
21+
// byte-exact Finding stream (feature, line, column for every emit,
22+
// in document order). Any code path that subtly reorders, drops,
23+
// duplicates, or shifts a finding will break TestDetectByteIdenticalPin
24+
// — that is the plan-185 byte-stable guarantee against MDS034
25+
// diagnostic drift. Start and End byte anchors are not pinned here:
26+
// they are exercised individually by the per-feature TestDetect*
27+
// tests and have less stable cross-feature semantics (some are
28+
// line-start anchors, some are zero-length).
29+
var pinCorpus = []pinCase{
30+
{
31+
name: "plain commonmark emits nothing",
32+
src: "# Heading\n\nA paragraph.\n\n- one\n- two\n",
33+
want: nil,
34+
},
35+
{
36+
name: "tables and bare URLs sort by start",
37+
src: "See https://example.com.\n\n" +
38+
"| a | b |\n| - | - |\n| 1 | 2 |\n",
39+
want: []findingShape{
40+
{FeatureBareURLAutolinks, 1, 5},
41+
{FeatureTables, 3, 1},
42+
},
43+
},
44+
{
45+
name: "every feature in one document",
46+
src: "# Title {#top}\n\n" +
47+
"- [ ] task\n\n" +
48+
"| a | b |\n| - | - |\n| 1 | 2 |\n\n" +
49+
"~~old~~ https://example.com\n\n" +
50+
"text^sup^ and H~2~O\n\n" +
51+
"$x+1$ inline\n\n" +
52+
"$$\nblock\n$$\n\n" +
53+
"*[API]: Application Programming Interface\n\n" +
54+
"Use API here.[^1]\n\n" +
55+
"[^1]: footnote body\n\n" +
56+
"term\n: definition\n\n" +
57+
"> [!NOTE]\n> Something.\n",
58+
want: []findingShape{
59+
{FeatureHeadingIDs, 1, 9},
60+
{FeatureTaskLists, 3, 3},
61+
{FeatureTables, 5, 1},
62+
{FeatureStrikethrough, 9, 1},
63+
{FeatureBareURLAutolinks, 9, 9},
64+
{FeatureSuperscript, 11, 5},
65+
{FeatureSubscript, 11, 16},
66+
{FeatureMathInline, 13, 1},
67+
{FeatureMathBlock, 15, 1},
68+
{FeatureAbbreviations, 19, 1},
69+
{FeatureFootnotes, 21, 1},
70+
{FeatureAbbreviations, 21, 5},
71+
{FeatureFootnotes, 23, 1},
72+
{FeatureDefinitionLists, 25, 1},
73+
{FeatureGitHubAlerts, 28, 1},
74+
},
75+
},
76+
{
77+
name: "github alert variants in order",
78+
src: "> [!NOTE]\n> n.\n\n" +
79+
"> [!TIP]\n> t.\n\n" +
80+
"> [!WARNING]\n> w.\n",
81+
want: []findingShape{
82+
{FeatureGitHubAlerts, 1, 1},
83+
{FeatureGitHubAlerts, 4, 1},
84+
{FeatureGitHubAlerts, 7, 1},
85+
},
86+
},
87+
}
88+
89+
// TestDetectByteIdenticalPin runs Detect over pinCorpus and asserts
90+
// the exact Finding stream for every entry.
91+
func TestDetectByteIdenticalPin(t *testing.T) {
92+
for _, tc := range pinCorpus {
93+
t.Run(tc.name, func(t *testing.T) {
94+
doc := markdown.Parse([]byte(tc.src))
95+
got := Detect(doc, nil)
96+
var gotShapes []findingShape
97+
for _, f := range got {
98+
gotShapes = append(gotShapes,
99+
findingShape{f.Feature, f.Line, f.Column})
100+
}
101+
assert.Equalf(t, tc.want, gotShapes,
102+
"finding stream drift; want vs got:\n%s\n",
103+
diffShapes(tc.want, gotShapes))
104+
})
105+
}
106+
}
107+
108+
// findingShape is the byte-identical pin tuple: the three Finding
109+
// fields whose stability is the documented MDS034 contract.
110+
type findingShape struct {
111+
Feature Feature
112+
Line int
113+
Column int
114+
}
115+
116+
// diffShapes renders a side-by-side report of expected vs actual
117+
// findings to make a drift-test failure self-diagnostic. assert's
118+
// default diff prints opaque struct dumps; this format mirrors the
119+
// stats-line style the rest of the test suite uses.
120+
func diffShapes(want, got []findingShape) string {
121+
var sb strings.Builder
122+
max := len(want)
123+
if len(got) > max {
124+
max = len(got)
125+
}
126+
sb.WriteString("idx | want feature line:col | got feature line:col\n")
127+
for i := 0; i < max; i++ {
128+
var w, g string
129+
if i < len(want) {
130+
w = fmt.Sprintf("%s %d:%d", want[i].Feature.Name(), want[i].Line, want[i].Column)
131+
} else {
132+
w = "(none)"
133+
}
134+
if i < len(got) {
135+
g = fmt.Sprintf("%s %d:%d", got[i].Feature.Name(), got[i].Line, got[i].Column)
136+
} else {
137+
g = "(none)"
138+
}
139+
marker := " "
140+
if w != g {
141+
marker = "*"
142+
}
143+
fmt.Fprintf(&sb, "%s%3d | %-30s | %s\n", marker, i, w, g)
144+
}
145+
return sb.String()
146+
}

0 commit comments

Comments
 (0)