Skip to content

Commit ba74889

Browse files
committed
Close the F5 silent-corruption path in taskCheckBoxEdits
The previous round's nil/empty-Lines guards did NOT prevent the silent-corruption case the F5 finding pointed at: NearestBlockAncestor SKIPS ancestors with empty Lines() and keeps walking up, so a TaskCheckBox under a Paragraph-with-empty-Lines under a ListItem-with-populated-Lines yields block=ListItem and start = bullet-position, not '['-position. The guards both pass; start+3 deletes three bytes from the wrong block. Add an explicit `f.Source[start] != '['` check that declines the edit when the byte at Lines.At(0).Start is not the bracket the task-list parser's invariant promises, plus a bounds check on start+3 against len(f.Source) for the truly-short-source case. Three new red/green tests pin the guard: - non-bracket start (paragraph with arbitrary Lines) - nil block ancestor (orphan TaskCheckBox) - bracket runs past EOF (2-byte source) https://claude.ai/code/session_0144ZKUS2Zrg7xBft54qyoti
1 parent ddc1f1d commit ba74889

2 files changed

Lines changed: 69 additions & 9 deletions

File tree

internal/rules/markdownflavor/fix.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,14 @@ func delimiterPairEdits(n ast.Node, markerLen int) []markdown.Edit {
145145
// taskCheckBoxEdits removes the "[X]" run plus a single trailing
146146
// space when present. Per the plan, the bullet itself is preserved.
147147
//
148-
// Relies on the goldmark task-list parser's invariant: it always
149-
// wraps a TaskCheckBox in a TextBlock whose first Lines() segment
150-
// starts at the '['. NearestBlockAncestor returns that TextBlock,
151-
// so block.Lines().At(0).Start indexes the bracket. If a hand-built
152-
// AST violates the invariant — TextBlock has empty Lines so
153-
// NearestBlockAncestor returns the enclosing ListItem instead —
154-
// start would point at the bullet ('- ') and start+3 would silently
155-
// delete the wrong bytes. The fix declines via the block==nil /
156-
// empty-Lines guards rather than producing corrupt output.
148+
// Relies on the goldmark task-list parser's invariant: it wraps a
149+
// TaskCheckBox in a TextBlock whose first Lines() segment starts at
150+
// the '['. The explicit `source[start] == '['` guard rejects the
151+
// case where a hand-built AST violates the invariant —
152+
// NearestBlockAncestor skips ancestors with empty Lines() so it can
153+
// return the enclosing ListItem (whose Lines start at the bullet,
154+
// not at '[') — and decline rather than silently delete three
155+
// arbitrary bytes from the wrong block.
157156
func taskCheckBoxEdits(f *lint.File, n *extast.TaskCheckBox) []markdown.Edit {
158157
block := flavor.NearestBlockAncestor(n)
159158
if block == nil {
@@ -164,7 +163,13 @@ func taskCheckBoxEdits(f *lint.File, n *extast.TaskCheckBox) []markdown.Edit {
164163
return nil
165164
}
166165
start := lines.At(0).Start
166+
if start < 0 || start >= len(f.Source) || f.Source[start] != '[' {
167+
return nil
168+
}
167169
end := start + 3
170+
if end > len(f.Source) {
171+
return nil
172+
}
168173
if end < len(f.Source) && f.Source[end] == ' ' {
169174
end++
170175
}

internal/rules/markdownflavor/fix_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import (
55

66
"github.com/stretchr/testify/assert"
77
"github.com/stretchr/testify/require"
8+
"github.com/yuin/goldmark/ast"
9+
extast "github.com/yuin/goldmark/extension/ast"
10+
"github.com/yuin/goldmark/text"
811

12+
"github.com/jeduden/mdsmith/internal/lint"
913
"github.com/jeduden/mdsmith/pkg/markdown/flavor/ext"
1014
)
1115

@@ -174,6 +178,57 @@ func TestRuleDualNodeEditsSupportedFeaturesReturnNil(t *testing.T) {
174178
assert.Nil(t, r.dualNodeEdits(nil, &ext.SubscriptNode{}))
175179
}
176180

181+
// TestTaskCheckBoxEditsDeclinesOnNonBracketStart hand-constructs an
182+
// AST that breaks the goldmark task-list invariant — a TaskCheckBox
183+
// whose NearestBlockAncestor returns a block starting at a non-'['
184+
// byte (here a bare paragraph). The guard must decline rather than
185+
// silently delete the wrong three bytes; this is the F5 case the
186+
// per-node Lines-empty check missed.
187+
func TestTaskCheckBoxEditsDeclinesOnNonBracketStart(t *testing.T) {
188+
// Source where offset 0 is 'p', not '['; NearestBlockAncestor's
189+
// returned block.Lines().At(0).Start will point at 'p'.
190+
src := []byte("plain paragraph text\n")
191+
f, err := lint.NewFile("t.md", src)
192+
require.NoError(t, err)
193+
194+
para := ast.NewParagraph()
195+
para.Lines().Append(text.NewSegment(0, len(src)-1))
196+
cb := extast.NewTaskCheckBox(true)
197+
para.AppendChild(para, cb)
198+
199+
got := taskCheckBoxEdits(f, cb)
200+
assert.Nil(t, got,
201+
"taskCheckBoxEdits must decline when Lines.At(0) is not at '['")
202+
}
203+
204+
// TestTaskCheckBoxEditsDeclinesOnNilBlock exercises the block==nil
205+
// guard with a TaskCheckBox that has no parent at all.
206+
func TestTaskCheckBoxEditsDeclinesOnNilBlock(t *testing.T) {
207+
src := []byte("- [ ] task\n")
208+
f, err := lint.NewFile("t.md", src)
209+
require.NoError(t, err)
210+
orphan := extast.NewTaskCheckBox(true)
211+
assert.Nil(t, taskCheckBoxEdits(f, orphan))
212+
}
213+
214+
// TestTaskCheckBoxEditsDeclinesWhenBracketRunsPastEOF guards the
215+
// end > len(source) branch: a TaskCheckBox at offset len-1 (last
216+
// byte is '[' with no body after it) cannot produce a 3-byte edit.
217+
func TestTaskCheckBoxEditsDeclinesWhenBracketRunsPastEOF(t *testing.T) {
218+
// Two bytes: "[\n" — start points at '['; start+3 exceeds the
219+
// 2-byte body. The fix must decline rather than build an edit
220+
// whose End is past EOF (which markdown.Splice would panic on).
221+
src := []byte("[\n")
222+
f, err := lint.NewFile("t.md", src)
223+
require.NoError(t, err)
224+
para := ast.NewParagraph()
225+
para.Lines().Append(text.NewSegment(0, len(src)))
226+
cb := extast.NewTaskCheckBox(true)
227+
para.AppendChild(para, cb)
228+
229+
assert.Nil(t, taskCheckBoxEdits(f, cb))
230+
}
231+
177232
// Splice's single-pass behaviour (adjacent edits, pure insertion,
178233
// replacement bytes) is exercised in pkg/markdown's TestSplice; the
179234
// rule layer just feeds edits into markdown.Splice.

0 commit comments

Comments
 (0)