Skip to content

Commit 467abb7

Browse files
jedudenclaude
andcommitted
Make fix-pass probe actually exercise Fix
Address PR #215 review (threads on test lines 258 and 309): the fixPassProbeRule was returning a diagnostic on Check call #1, which is the pre-fix engine.CheckRules pass — applyFixPasses then saw an empty diagnostic list on call #2 and never invoked Fix. So the \"validates hydration during applyFixPasses\" assertion was actually just validating hydration during pre/post-fix CheckRules, which the other tests in this file already cover. Rework: - Track Check and Fix snapshots separately. - Trigger the diagnostic on Check call #2 (the applyFixPasses pass) so Fix actually fires, then assert exactly 1 Fix snapshot. - Pin exact phase counts: 3 Check calls, 1 Fix call. - Assert the Fix call's lint.File also has the per-file context hydrated, not just the Check calls — that's the real regression guard for catalog/include rules whose Fix paths consult these fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2fcb428 commit 467abb7

1 file changed

Lines changed: 70 additions & 45 deletions

File tree

internal/fix/fix_generated_ranges_test.go

Lines changed: 70 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -228,32 +228,47 @@ func TestFix_FilesHaveGitignoreFuncWithRootDir(t *testing.T) {
228228
}
229229
}
230230

231-
// fixPassProbeRule is a fixable rule that records, on each Check
232-
// invocation inside applyFixPasses, whether the parsedFile carries
233-
// the per-file context (MaxInputBytes / StripFrontMatter /
234-
// GitignoreFunc / GeneratedRanges) that the engine.Runner sets. Its
235-
// Fix returns the source unchanged so applyFixPasses stabilizes
236-
// after one iteration.
231+
// fileSnapshot records the per-file context fields that should be
232+
// hydrated identically across pre-fix Check, fix-pass Check, fix-pass
233+
// Fix, and post-fix Check. Captured on every invocation so the test
234+
// can assert hydration in each phase independently.
235+
type fileSnapshot struct {
236+
maxBytes int64
237+
stripFM bool
238+
hasGI bool
239+
hasRange bool
240+
}
241+
242+
func snapshotOf(f *lint.File) fileSnapshot {
243+
return fileSnapshot{
244+
maxBytes: f.MaxInputBytes,
245+
stripFM: f.StripFrontMatter,
246+
hasGI: f.GetGitignore() != nil,
247+
hasRange: len(f.GeneratedRanges) > 0,
248+
}
249+
}
250+
251+
// fixPassProbeRule is a fixable rule that records every Check and
252+
// Fix invocation it receives. Check returns a diagnostic on its
253+
// second call so that applyFixPasses (which runs Check after the
254+
// pre-fix engine.CheckRules has already run Check once) sees a
255+
// non-empty diagnostic list and is forced to call Fix. Fix returns
256+
// the source unchanged so applyFixPasses stabilizes after one
257+
// iteration.
237258
type fixPassProbeRule struct {
238-
id string
239-
name string
240-
maxBytes []int64
241-
stripFM []bool
242-
hasGI []bool
243-
hasRange []bool
259+
id string
260+
name string
261+
checkSnaps []fileSnapshot
262+
fixSnaps []fileSnapshot
263+
triggerOnNth int // 1-based: which Check call returns a diag
244264
}
245265

246266
func (r *fixPassProbeRule) ID() string { return r.id }
247267
func (r *fixPassProbeRule) Name() string { return r.name }
248268
func (r *fixPassProbeRule) Category() string { return "test" }
249269
func (r *fixPassProbeRule) Check(f *lint.File) []lint.Diagnostic {
250-
r.maxBytes = append(r.maxBytes, f.MaxInputBytes)
251-
r.stripFM = append(r.stripFM, f.StripFrontMatter)
252-
r.hasGI = append(r.hasGI, f.GetGitignore() != nil)
253-
r.hasRange = append(r.hasRange, len(f.GeneratedRanges) > 0)
254-
// Return one diagnostic on the first call so applyFixPasses also
255-
// invokes Fix; subsequent calls return nil so the loop stabilizes.
256-
if len(r.maxBytes) == 1 {
270+
r.checkSnaps = append(r.checkSnaps, snapshotOf(f))
271+
if len(r.checkSnaps) == r.triggerOnNth {
257272
return []lint.Diagnostic{{
258273
File: f.Path, Line: 1, Column: 1,
259274
RuleID: r.id, RuleName: r.name,
@@ -262,17 +277,31 @@ func (r *fixPassProbeRule) Check(f *lint.File) []lint.Diagnostic {
262277
}
263278
return nil
264279
}
265-
func (r *fixPassProbeRule) Fix(f *lint.File) []byte { return f.Source }
280+
func (r *fixPassProbeRule) Fix(f *lint.File) []byte {
281+
r.fixSnaps = append(r.fixSnaps, snapshotOf(f))
282+
return f.Source
283+
}
266284

267285
var _ rule.FixableRule = (*fixPassProbeRule)(nil)
268286

269287
// TestFix_FixPassesHydrateLintFile verifies that the parsedFile used
270288
// inside applyFixPasses carries the same per-file context that the
271-
// pre-fix and post-fix CheckRules calls already use. Without this,
272-
// fixable rules that consult these fields during their own Check or
273-
// Fix (notably catalog: GetGitignore for glob filtering, include:
274-
// MaxInputBytes for secondary reads) silently produce different
275-
// post-fix bytes than `mdsmith check` would have validated against.
289+
// pre-fix and post-fix CheckRules calls already use, AND that the
290+
// hydration is present when applyFixPasses calls fr.Fix (not just
291+
// fr.Check). Without this, fixable rules that consult these fields
292+
// during their own Check or Fix (notably catalog: GetGitignore for
293+
// glob filtering, include: MaxInputBytes for secondary reads)
294+
// silently produce different post-fix bytes than `mdsmith check`
295+
// would have validated against.
296+
//
297+
// Phase layout (one fixable rule, Fix returns same source):
298+
// 1. pre-fix engine.CheckRules → Check call #1
299+
// 2. applyFixPasses pass 1 → Check call #2, then Fix call #1
300+
// (loop sees source unchanged → break)
301+
// 3. post-fix engine.CheckRules → Check call #3
302+
//
303+
// The probe is configured to return a diagnostic on Check call #2 so
304+
// that step 2's Fix actually fires.
276305
func TestFix_FixPassesHydrateLintFile(t *testing.T) {
277306
dir := t.TempDir()
278307
mdPath := filepath.Join(dir, "doc.md")
@@ -284,7 +313,7 @@ func TestFix_FixPassesHydrateLintFile(t *testing.T) {
284313

285314
const ruleName = "fix-pass-probe"
286315
const wantMaxBytes int64 = 8192
287-
probe := &fixPassProbeRule{id: "MDS996", name: ruleName}
316+
probe := &fixPassProbeRule{id: "MDS996", name: ruleName, triggerOnNth: 2}
288317

289318
cfg := &config.Config{
290319
Rules: map[string]config.RuleCfg{
@@ -302,26 +331,22 @@ func TestFix_FixPassesHydrateLintFile(t *testing.T) {
302331
result := fixer.Fix([]string{mdPath})
303332
require.Empty(t, result.Errors, "unexpected errors: %v", result.Errors)
304333

305-
// applyFixPasses calls Check at least once on parsedFile; that
306-
// parsedFile must carry the per-file context.
307-
require.NotEmpty(t, probe.maxBytes,
308-
"fixable rule was never invoked inside applyFixPasses")
309-
for i, got := range probe.maxBytes {
310-
assert.Equal(t, wantMaxBytes, got,
311-
"fix-pass call %d: MaxInputBytes not propagated to parsedFile", i)
312-
}
313-
for i, got := range probe.stripFM {
314-
assert.True(t, got,
315-
"fix-pass call %d: StripFrontMatter not propagated to parsedFile", i)
316-
}
317-
for i, got := range probe.hasGI {
318-
assert.True(t, got,
319-
"fix-pass call %d: GitignoreFunc not propagated to parsedFile", i)
320-
}
321-
for i, got := range probe.hasRange {
322-
assert.True(t, got,
323-
"fix-pass call %d: GeneratedRanges not populated on parsedFile (catalog body unprotected)", i)
334+
// Three Check calls (pre-fix, fix-pass, post-fix) and exactly
335+
// one Fix call (from inside applyFixPasses). If the fix-pass
336+
// Check had no diagnostic to trigger Fix, fixSnaps would be
337+
// empty — that is the regression this test guards against.
338+
require.Len(t, probe.checkSnaps, 3,
339+
"expected 3 Check calls (pre-fix + fix-pass + post-fix), got %d", len(probe.checkSnaps))
340+
require.Len(t, probe.fixSnaps, 1,
341+
"expected 1 Fix call from inside applyFixPasses, got %d", len(probe.fixSnaps))
342+
343+
want := fileSnapshot{maxBytes: wantMaxBytes, stripFM: true, hasGI: true, hasRange: true}
344+
phases := []string{"pre-fix Check", "fix-pass Check", "post-fix Check"}
345+
for i, got := range probe.checkSnaps {
346+
assert.Equal(t, want, got, "%s: per-file context not hydrated", phases[i])
324347
}
348+
assert.Equal(t, want, probe.fixSnaps[0],
349+
"fix-pass Fix: per-file context not hydrated (catalog/include rules would silently behave differently)")
325350
}
326351

327352
// fieldRecordingRule captures f.MaxInputBytes and f.StripFrontMatter

0 commit comments

Comments
 (0)